Apply the external fee payment to EVM gas refunds logic #1
Conversation
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the EVM gas refund logic by allowing external fee payments stored in the context to override the default refund denomination and amount.
- Introduces
ContextPaidFeesKeyto read paid fees from the context and adjust refund calculations. - Updates
RefundGasto override the refund denom and amount based on the percentage of gas used. - Adds new tests covering both standard refunds and refunds with context-based fees.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| x/vm/keeper/gas.go | Added context-based fee extraction and adjusted refund logic. |
| x/vm/keeper/gas_test.go | New test cases for gas refunds, including external fee scenarios. |
Comments suppressed due to low confidence (1)
x/vm/keeper/gas.go:46
- No test covers the case where the context value is present but not of type
sdk.Coins. Consider adding a test to ensure that non-coin context values fall back to the original denom and amount.
if val := ctx.Value(ContextPaidFeesKey{}); val != nil {
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| // Attempt to extract the paid coin from the context | ||
| // This is used when fee abstraction is applied into the fee payment | ||
| // If no value is found under the context, the original denom is used | ||
| if val := ctx.Value(ContextPaidFeesKey{}); val != nil { |
There was a problem hiding this comment.
No, the ContextPaidFeesKey{} needs to be applied to the CTX, like this:
… (#201) * feat(precompiles): add BalanceHandler to handle native balance change * refactor: remove parts of calling SetBalanceChangeEntries * chore: fix lint * chore(precompiles/distribution): remove unused helper function * chore(precompiles): modify comments * chore: restore modification to be applied later * chore: fix typo * chore: resolve conflict * chore: fix lint * test(precompiles/common) add unit test cases * chore: fix lint * fix(test): precompile test case that intermittently fails * refactor: move mock evm keeper to x/vm/types/mocks * chore: add KVStoreKeys() method to mock evmKeeper * refactoring balance handling * test(precompile/common): improve unit test for balance handler * refactor(precompiles): separate common logic * Revert "refactor(precompiles): separate common logic" This reverts commit 25b89f32b7553e1b38d80d8c789f3638f966e5d4. * Revert "Merge pull request #1 from zsystm/poc/precompiles-balance-handler" This reverts commit 46cd52739da67aae2395840ea6f3311cc7d90a5e, reversing changes made to b532fd547d56ce6b3d82562f0be5133da3ca6bb3. --------- Co-authored-by: zsystm <actor93kor@gmail.com> Co-authored-by: Vlad J <vladjdk@gmail.com>
Description
This applies the external fee payments to the gas refund logic:
ContextPaidFeesKeysdk.Coinsamount * leftoverGas / gasUsedType of change
Please delete options that are not relevant.
How Has This Been Tested?
All original tests are passing, since the code is safe when no data is found on the context.