This repository has been archived by the owner on Apr 4, 2024. It is now read-only.
fix: don't revert gas refund logic when transaction reverted #751
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yihuang
requested review from
fedekunze,
jolube and
khoslaventures
as code owners
yihuang
changed the title
yihuang
changed the title
yihuang
force-pushed
the
fix-gas-refund
branch
from
0f7b548
to
9c87cb6
Compare
Codecov Report
@@ Coverage Diff @@
## main #751 +/- ##
==========================================
+ Coverage 55.84% 55.86% +0.01%
==========================================
Files 64 64
Lines 5857 5859 +2
==========================================
+ Hits 3271 3273 +2
Misses 2386 2386
Partials 200 200
|
yihuang
commented
Nov 16, 2021
yihuang
commented
Nov 16, 2021
fedekunze
suggested changes
Nov 16, 2021
x/evm/types/compiled_contract.go
Outdated
Comment on lines
71
to
72
| // CompiledContract2 support json format of newer solc | ||
| type CompiledContract2 struct { |
x/evm/handler_test.go
Outdated
| return crypto.CreateAddress(suite.from, nonce) | ||
| } | ||
|
|
||
| func (suite *EvmTestSuite) TestGasWhenReverted() { |
There was a problem hiding this comment.
can we refactor this test? it's not maintainable
There was a problem hiding this comment.
I realize we don't need a special contract to reproduce the contract revert, so I changed it to the existing erc20 contract, is it good now?
Apply suggestions from code review changelog
yihuang
force-pushed
the
fix-gas-refund
branch
from
08bbde4
to
948b39c
Compare
fedekunze
approved these changes
Nov 16, 2021
13 tasks
13 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Problem: When there are hooks and the transaction reverted, gas refund logic is reverted too.
Solution: Don't revert the gas refund logic
For contributor use:
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerFor admin use:
WIP,R4R,docs, etc)