feat: support state overrides in eth_call #337
Conversation
| func (b *Backend) DoCall( | ||
| args evmtypes.TransactionArgs, blockNr rpctypes.BlockNumber, | ||
| args evmtypes.TransactionArgs, blockNr rpctypes.BlockNumber, overrides *json.RawMessage, | ||
| ) (*evmtypes.MsgEthereumTxResponse, error) { |
There was a problem hiding this comment.
nit: formatting of the arguments - can we make them vertical?
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| for key, value := range *account.StateDiff { | ||
| db.SetState(addr, key, value) | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map Warning
| @@ -474,7 +474,7 @@ func (s *TestSuite) TestDoCall() { | |||
| s.SetupTest() // reset test and queries | |||
| tc.registerMock() | |||
|
|
|||
| msgEthTx, err := s.backend.DoCall(tc.callArgs, tc.blockNum) | |||
There was a problem hiding this comment.
It would be good to add a simple test case to check whether the expected result is returned when the state overrides input is set.
| @@ -788,6 +788,7 @@ func (s *KeeperTestSuite) TestApplyMessageWithConfig() { | |||
| true, | |||
| config, | |||
| txConfig, | |||
| nil, | |||
There was a problem hiding this comment.
Similar to the case in test_call_tx.go, it would be good to add a test case for when the state overrides input is set.
| @@ -55,6 +60,42 @@ type RPCTransaction struct { | |||
| // StateOverride is the collection of overridden accounts. | |||
| type StateOverride map[common.Address]OverrideAccount | |||
|
|
|||
| // Apply overrides the fields of specified accounts into the given state. | |||
| func (diff *StateOverride) Apply(db *statedb.StateDB) error { | |||
There was a problem hiding this comment.
As of go-ethereum v1.15.10, the StateOverride.Apply method includes logic for handling precompile addresses.
From a compatibility standpoint, it may be best to either align the logic to match exactly, or alternatively, directly import the override package from Geth and use its Apply method. In the latter case, the StateDB.Finalise method would likely need to be implemented as a noop function.
| @@ -477,6 +477,22 @@ func (s *StateDB) SetState(addr common.Address, key, value common.Hash) common.H | |||
| return common.Hash{} | |||
| } | |||
|
|
|||
| // SetBalance sets the balance of account associated with addr to amount. | |||
| func (s *StateDB) SetBalance(addr common.Address, amount *uint256.Int) { | |||
There was a problem hiding this comment.
I think this interface is aligned to go-ethereum v1.11.x.
Currently, cosmos/evm is compatibiile to go-ethereum v1.15.x.
You can check added arg, reason tracing.BalanceChangeReason. (ref)
| @@ -263,6 +273,12 @@ func (s *stateObject) SetState(key common.Hash, value common.Hash) common.Hash { | |||
| return prev | |||
| } | |||
|
|
|||
| func (s *stateObject) SetStorage(storage Storage) { | |||
There was a problem hiding this comment.
I think Comment is needed because it can be confusing considering stateObject.SetStorage does not exist in go-ethereum (v1.15.10)
I suspect the reason this method was added is, it is difficult to use stateObject.StateState as it is. Because SetState sets value to dirtyStorage, and GetCommitedState method cannot get overrided state. So it seems that you alse added stateObject.overrideStorage and let GetCommittedState get state from overrideStorage.
I'm not sure my suspection is correct but anyway, it will helpful for devs to manange code if background explanation of this method is added.
Description
upstream feature from crypto-org-chain/ethermint#369, for test info
Closes: #336
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
mainbranchReviewers Checklist
All items are required.
Please add a note if the item is not applicable
and please add your handle next to the items reviewed
if you only reviewed selected items.
I have...
Unreleasedsection inCHANGELOG.md