-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
feat: add RefundGas
function to GasMeter
#9403
Conversation
RefundGas
function to GasMeter
Codecov Report
@@ Coverage Diff @@
## master #9403 +/- ##
==========================================
+ Coverage 60.39% 60.41% +0.02%
==========================================
Files 589 589
Lines 37099 37113 +14
==========================================
+ Hits 22405 22423 +18
+ Misses 12715 12712 -3
+ Partials 1979 1978 -1
|
Can you please show a real world use-case? Both in the description as well as a godoc on the method. Not sure when this would be useful. I have done gas-limited calls to CosmWasm contracts via submessages, which is an interesting use-case and I would be happy if there was a primitive that would accomplish some of this. But Please check out https://github.com/CosmWasm/wasmd/blob/master/x/wasm/keeper/msg_dispatcher.go#L46-L71 Happy to see the code that would use this. |
@ethanfrey great question. So go-ethereum uses the This PR adds the functionality to subtract a given amount of gas from the gas pool to fully support this use case within Ethermint. |
Thanks for the explanation. Can you add a few lines of docs above the |
…nto fedekunze/refund-gas
@ethanfrey done |
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.
thanks for the update
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.
ACK. Okay to merge this into master at this point in the release process @AmauryM and @robert-zaremba ?
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.
Nothing's breaking in this PR, so I'm fine to merge this into master.
We're manually testing v0.43 on the regen side right now, I added a note to possibly test this regen-network/regen-ledger#352.
* feat: add RefundGas function to GasMeter * changelog * add comment about use case * Apply suggestions from code review Co-authored-by: Alessio Treglia <alessio@tendermint.com> Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> (cherry picked from commit 90edeb6) # Conflicts: # CHANGELOG.md
* feat: add RefundGas function to GasMeter * changelog * add comment about use case * Apply suggestions from code review Co-authored-by: Alessio Treglia <alessio@tendermint.com> Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> (cherry picked from commit 90edeb6) # Conflicts: # CHANGELOG.md
* feat: add `RefundGas` function to `GasMeter` (#9403) * feat: add RefundGas function to GasMeter * changelog * add comment about use case * Apply suggestions from code review Co-authored-by: Alessio Treglia <alessio@tendermint.com> Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> (cherry picked from commit 90edeb6) # Conflicts: # CHANGELOG.md * conflicts * fix * Update CHANGELOG.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Co-authored-by: Federico Kunze <federico.kunze94@gmail.com> Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>
Why is this change not merged into the 0.43 series? Our chain is currently running with 0.42.6 which required an update to include the GasMeter in our custom antehandler ... this is now broken during our testing on 0.43-beta1 because it was not ported over. |
Everything currently in master will be included in RC0. Once we cut RC0, we will create a separate release branch for v0.43. This change did not make it into the initial beta release and we have not provided an updated release since. Apologies for the wait but rest assured this will be included in RC0. |
@AmauryM how is this missing in 0.43? |
it's not in 0.43-beta1, but it's in 0.43-rc0 |
I'm wondering if there's some restrictions/limitations on how much gas one can refund? Empirically, I've found that when the entire gas consumed is refunded, the tx will timeout with // always times out
ctx.GasMeter().RefundGas(ctx.GasMeter().GasConsumed(), "test")
// succeeds
ctx.GasMeter().RefundGas(ctx.GasMeter().GasConsumed() / 10, "test") Any ideas why? EDIT: discovered that one can only safely refund the gas consumed in the handler execution itself. // beginning of handler
startGas := ctx.GasMeter().GasConsumed()
...
ctx.GasMeter().RefundGas(ctx.GasMeter().GasConsumed() - startGas, "test") |
* CLI: `query ibc-transfer escrow-address` (cosmos#9383) * Fix the liveliness test Docker error (cosmos#9396) (cosmos#9402) (cherry picked from commit 44a4138) Co-authored-by: Riccardo Montagnin <riccardo.montagnin@gmail.com> * docs: fix broken interfaces links (backport cosmos#9448) (cosmos#9478) * fix interfaces links (cosmos#9448) Co-authored-by: ryanchrypto <12519942+ryanchrypto@users.noreply.github.com> Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> (cherry picked from commit 37fc37d) # Conflicts: # docs/building-modules/messages-and-queries.md # docs/building-modules/module-interfaces.md # docs/building-modules/module-manager.md # docs/building-modules/query-services.md * resolve merge conflicts * revert rename codec Co-authored-by: Ryan Christoffersen <12519942+ryanchristo@users.noreply.github.com> Co-authored-by: ryanchrypto <12519942+ryanchrypto@users.noreply.github.com> * backport: fix ibc genesis export bug (cosmos#9401) * correctly set next identifier sequence in genesis export for clients/connections/channels * add changelog * fix linting (cosmos#9531) * fix: testnet cli command update genesis supply (backport cosmos#9497) (cosmos#9513) * fix: testnet cli command update genesis supply (cosmos#9497) <!-- The default pull request template is for types feat, fix, or refactor. For other templates, add one of the following parameters to the url: - template=docs.md - template=other.md --> ## Description closes: cosmos#9372 <!-- Add a description of the changes that this PR introduces and the files that are the most critical to review. --> ### This PR makes the `testnet` command update the bank genesis supply. When using the `testnet` cli command, it creates nodes and balances, but does **not** update the supply. When using this in conjunction with `add-genesis-account` which **does** update the supply, it creates an invalid genesis file. This PR updates the testnet command to properly set the supply. * feat: add header hash to `Context` (backport cosmos#9390) (cosmos#9395) * feat: add header hash to `Context` (cosmos#9390) * baseapp, types: add header hash to * changelog (cherry picked from commit 151d6c5) # Conflicts: # CHANGELOG.md * Fix conflicts Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Co-authored-by: Amaury M <1293565+amaurym@users.noreply.github.com> * fix: x/gov deposits querier (Initial Deposit) (backport cosmos#9288) (cosmos#9453) * fix: x/gov deposits querier (Initial Deposit) (cosmos#9288) * copied from old PR * fix errors * add test * Update x/gov/client/utils/query.go Co-authored-by: Robert Zaremba <robert@zaremba.ch> * fix tests * fix failing test * add test * update test * fix tests * fix deposit query * fix test * update tests * add more tests * address lint error * address lint error * review changes Co-authored-by: Robert Zaremba <robert@zaremba.ch> (cherry picked from commit 66ee994) # Conflicts: # CHANGELOG.md # x/gov/client/cli/query.go # x/gov/client/testutil/cli_test.go # x/gov/client/utils/query.go * resolve conflicts Co-authored-by: MD Aleem <72057206+aleem1314@users.noreply.github.com> Co-authored-by: aleem1314 <aleem@vitwit.com> * feat: add `RefundGas` function to `GasMeter` (backport cosmos#9403) (cosmos#9444) * feat: add `RefundGas` function to `GasMeter` (cosmos#9403) * feat: add RefundGas function to GasMeter * changelog * add comment about use case * Apply suggestions from code review Co-authored-by: Alessio Treglia <alessio@tendermint.com> Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> (cherry picked from commit 90edeb6) # Conflicts: # CHANGELOG.md * conflicts * fix * Update CHANGELOG.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Co-authored-by: Federico Kunze <federico.kunze94@gmail.com> Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * fix: Bank module init genesis optimization (backport cosmos#9428) (cosmos#9440) * fix: Bank module init genesis optimization (cosmos#9428) * optimize the bank module genesis initialization * remove k.setBalances & k.clearBalances and update changelog * fix lint Co-authored-by: Aaron Craelius <aaron@regen.network> (cherry picked from commit 2ae7875) # Conflicts: # CHANGELOG.md # x/bank/keeper/genesis.go # x/bank/keeper/send.go * fix conflicts * Update CHANGELOG.md Co-authored-by: yys <sw.yunsuk@gmail.com> Co-authored-by: Amaury M <1293565+amaurym@users.noreply.github.com> * fix: update simapp to use correct default broadcast mode (backport cosmos#9408) (cosmos#9527) * fix: update simapp to use correct default broadcast mode (cosmos#9408) (cherry picked from commit 80330ec) * Add changelog Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> Co-authored-by: Amaury M <1293565+amaurym@users.noreply.github.com> * Backport: IBC query header/node-state fixes (cosmos#9385) * fix ibc query header/node-state cmds * changelog Co-authored-by: Amaury M <1293565+amaurym@users.noreply.github.com> * build(deps): tendermint version (backport cosmos#9541) (cosmos#9542) * build(deps): tendermint version (cosmos#9541) * bump tendermint version * go mod tidy (cherry picked from commit e4673ad) * add changelog entry * Update CHANGELOG.md * Update CHANGELOG.md Co-authored-by: Marko <marbar3778@yahoo.com> Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> * feat: add cosmos-sdk Version (backport cosmos#9429) (cosmos#9543) * feat: add cosmos-sdk Version (cosmos#9429) <!-- < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < < ☺ v ✰ Thanks for creating a PR! ✰ v Before smashing the submit button please review the checkboxes. v If a checkbox is n/a - please still include it but + a little note why ☺ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --> ## Description Add CosmosSDKVersion to nodeInfo. closes: cosmos#9420 (cherry picked from commit 105ad99) # Conflicts: # CHANGELOG.md # CONTRIBUTING.md # docs/core/proto-docs.md * resolve conflicts * resolve conflicts Co-authored-by: Marko <marbar3778@yahoo.com> * fix: set header hash every block (backport cosmos#9552) (cosmos#9555) * fix: set header hash every block (cosmos#9552) ## Description - Sets the header hash on every block (ref cosmos#9390). Previously was only set during initialization for `deliverState`. - Closes cosmos#9514 <!-- Add a description of the changes that this PR introduces and the files that are the most critical to review. --> * chore: Update release notes and Changelog for 0.42.6 (cosmos#9544) * Update release notes * Clean up changelog Co-authored-by: Ethan Buchman <ethan@coinculture.info> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Riccardo Montagnin <riccardo.montagnin@gmail.com> Co-authored-by: Ryan Christoffersen <12519942+ryanchristo@users.noreply.github.com> Co-authored-by: ryanchrypto <12519942+ryanchrypto@users.noreply.github.com> Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> Co-authored-by: Marko <marbar3778@yahoo.com> Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Co-authored-by: Amaury M <1293565+amaurym@users.noreply.github.com> Co-authored-by: MD Aleem <72057206+aleem1314@users.noreply.github.com> Co-authored-by: aleem1314 <aleem@vitwit.com> Co-authored-by: Federico Kunze <federico.kunze94@gmail.com> Co-authored-by: yys <sw.yunsuk@gmail.com> Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
Adds support for refunding gas directly to the gas meter. Otherwise, you need to create a new
GasMeter
with the same gas limit and consume the desired amount:Note: this PR doesn't affect how fees are managed