-
Notifications
You must be signed in to change notification settings - Fork 46
feat: post tx hooks #54
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
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
e2ab519
feat: evm hooks
Reecepbcups 9145b91
Merge pull request #53 from strangelove-ventures/sl/evm-tx-hooks
vladjdk 3f91385
lint fix
vladjdk 6124709
Merge branch 'main' into reece/post-tx-hooks
vladjdk 87f0803
hooks refactor (match ethermint)
vladjdk f8cb047
state transition test - evm cumulative gas spent > cosmos block gas l…
vladjdk 66fcc8e
Merge branch 'main' into reece/post-tx-hooks
vladjdk 675ce4d
lints
vladjdk 89c5058
note for gas in hooks
vladjdk 1d0e683
use errorsmod.wrap
vladjdk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
package keeper | ||
|
||
import ( | ||
"github.com/ethereum/go-ethereum/common" | ||
"github.com/ethereum/go-ethereum/core" | ||
ethtypes "github.com/ethereum/go-ethereum/core/types" | ||
|
||
"github.com/cosmos/evm/x/vm/types" | ||
|
||
errorsmod "cosmossdk.io/errors" | ||
|
||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
) | ||
|
||
// Event Hooks | ||
// These can be utilized to customize evm transaction processing. | ||
|
||
var _ types.EvmHooks = MultiEvmHooks{} | ||
|
||
// MultiEvmHooks combine multiple evm hooks, all hook functions are run in array sequence | ||
type MultiEvmHooks []types.EvmHooks | ||
|
||
// NewMultiEvmHooks combine multiple evm hooks | ||
func NewMultiEvmHooks(hooks ...types.EvmHooks) MultiEvmHooks { | ||
return hooks | ||
} | ||
|
||
// PostTxProcessing delegate the call to underlying hooks | ||
func (mh MultiEvmHooks) PostTxProcessing(ctx sdk.Context, sender common.Address, msg core.Message, receipt *ethtypes.Receipt) error { | ||
for i := range mh { | ||
if err := mh[i].PostTxProcessing(ctx, sender, msg, receipt); err != nil { | ||
return errorsmod.Wrapf(err, "EVM hook %T failed", mh[i]) | ||
} | ||
} | ||
return nil | ||
} |
This file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
package keeper_test | ||
|
||
import ( | ||
"errors" | ||
"math/big" | ||
|
||
"github.com/ethereum/go-ethereum/common" | ||
"github.com/ethereum/go-ethereum/core" | ||
ethtypes "github.com/ethereum/go-ethereum/core/types" | ||
|
||
"github.com/cosmos/evm/x/vm/keeper" | ||
"github.com/cosmos/evm/x/vm/statedb" | ||
"github.com/cosmos/evm/x/vm/types" | ||
|
||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
) | ||
|
||
// LogRecordHook records all the logs | ||
type LogRecordHook struct { | ||
Logs []*ethtypes.Log | ||
} | ||
|
||
func (dh *LogRecordHook) PostTxProcessing(ctx sdk.Context, sender common.Address, msg core.Message, receipt *ethtypes.Receipt) error { | ||
dh.Logs = receipt.Logs | ||
return nil | ||
} | ||
|
||
// FailureHook always fail | ||
type FailureHook struct{} | ||
|
||
func (dh FailureHook) PostTxProcessing(ctx sdk.Context, sender common.Address, msg core.Message, receipt *ethtypes.Receipt) error { | ||
return errors.New("post tx processing failed") | ||
} | ||
|
||
func (suite *KeeperTestSuite) TestEvmHooks() { | ||
testCases := []struct { | ||
msg string | ||
setupHook func() types.EvmHooks | ||
expFunc func(hook types.EvmHooks, result error) | ||
}{ | ||
{ | ||
"log collect hook", | ||
func() types.EvmHooks { | ||
return &LogRecordHook{} | ||
}, | ||
func(hook types.EvmHooks, result error) { | ||
suite.Require().NoError(result) | ||
suite.Require().Equal(1, len((hook.(*LogRecordHook).Logs))) | ||
}, | ||
}, | ||
{ | ||
"always fail hook", | ||
func() types.EvmHooks { | ||
return &FailureHook{} | ||
}, | ||
func(hook types.EvmHooks, result error) { | ||
suite.Require().Error(result) | ||
}, | ||
}, | ||
} | ||
|
||
for _, tc := range testCases { | ||
suite.SetupTest() | ||
hook := tc.setupHook() | ||
suite.network.App.EVMKeeper.SetHooks(keeper.NewMultiEvmHooks(hook)) | ||
|
||
k := suite.network.App.EVMKeeper | ||
ctx := suite.network.GetContext() | ||
txHash := common.BigToHash(big.NewInt(1)) | ||
vmdb := statedb.New(ctx, k, statedb.NewTxConfig( | ||
common.BytesToHash(ctx.HeaderHash()), | ||
txHash, | ||
0, | ||
0, | ||
)) | ||
|
||
vmdb.AddLog(ðtypes.Log{ | ||
Topics: []common.Hash{}, | ||
Address: suite.keyring.GetAddr(0), | ||
}) | ||
logs := vmdb.Logs() | ||
receipt := ðtypes.Receipt{ | ||
TxHash: txHash, | ||
Logs: logs, | ||
} | ||
result := k.PostTxProcessing(ctx, suite.keyring.GetAddr(0), ethtypes.Message{}, receipt) | ||
|
||
tc.expFunc(hook, result) | ||
} | ||
} |
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
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.
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.
Not too familiar with this code path at this point, but is this the expected behavior? Possibly we should be stopping execution if we've gone over the gas limit and returning a out of gas error?
Uh oh!
There was an error while loading. Please reload this page.
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.
I viewed some historical Ethermint updates, and it seems like this value from the receipt is only used with external tooling. I still haven't found a concrete example of when something like this could happen, but if you look at
evm/x/vm/keeper/state_transition.go
Lines 410 to 431 in 6124709
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.
I added a test where this case would be hit. This could probably happen in the real world where someone's transaction is at the end of the FIFO list and the gas limit exceeds the rest of the Cosmos block gas limit, but the consumed gas still remains under it. In this case, the EVM would refund the gas.
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.
Maybe a separate question, but since we're refunding any gas not consumed via
res.GasUsed
, does that mean the post tx hooks are essentially free to execute?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.
Yeah, added a comment above them to clarify this