Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Incomplete Patch #18714 #18978

Open
1 task done
chen-robert opened this issue Jan 8, 2024 · 1 comment
Open
1 task done

[Bug]: Incomplete Patch #18714 #18978

chen-robert opened this issue Jan 8, 2024 · 1 comment
Labels

Comments

@chen-robert
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

What happened?

We were investigating patch #18714.

Some comments:

  1. The goal is to reset GasMeter instead of BlockGasMeter. So shouldn't we be using withGasMeter instead of withBlockGasMeter here?
  2. It’s probably better to move the reset code into the for loop here. This helps guarantee that GasMeter for each tx are independent and don't affect each other. The same goes for CheckTx, the GasMeter could be reset here.

Apart from the recommendations above. Given that the bug is found on v0.47.6, I think it makes sense to backport the fix to v0.47.x? Or are those branches no longer maintained?

Additionally, the main branch was never affected by non-decodable transactions, since the check here catches the error early and returns a result with GasUsed set to 0. Nonetheless, resetting still helps guard against other future errors within runTx from contaminating gas usage for subsequent txs.

Cosmos SDK Version

0.47

How to reproduce?

No response

@alexanderbez
Copy link
Contributor

Note, this should be patched for v0.50.x (Eden) only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 📋 Backlog
Development

No branches or pull requests

2 participants