-
Notifications
You must be signed in to change notification settings - Fork 18
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
fix: allow for no bank balances in all simulations #137
Conversation
aljo242
commented
Aug 22, 2024
- Always use 0denom dummy fee value in simulation
- Add in calculated gas consumption amount to account for keeper codepaths that are not hit because of this (in sim only)
- add non-mocked bank keeper testing
- lint fixes due to version bump
96bd7d2
to
9447315
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Also implemeted testcase with our testsuite and it works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Only thing is it seems weird to hard code bank send gas usage here.
One question. In a slack there was a discussion about non consensus breaking patch, but seems like all the games with a gas meter are consensus breaking. |
6326c15
to
5811698
Compare
* init * recreate and fix bug * helper * ante tests * ok * fix via gas consume call during simulation * test * revert * lint fix * retract more * replace linter * set lint to what we use (cherry picked from commit f1f216e) # Conflicts: # x/feemarket/ante/expected_keepers.go # x/feemarket/ante/mocks/mock_bank_keeper.go # x/feemarket/ante/suite/suite.go # x/feemarket/post/fee_test.go