From fe74b098b423284d979b5abca3d4e590f84a0dc9 Mon Sep 17 00:00:00 2001 From: rjl493456442 Date: Tue, 23 Jul 2024 20:54:35 +0800 Subject: [PATCH] core/state: fix SetStorage override behavior (#30185) This pull request fixes the broken feature where the entire storage set is overridden. Originally, the storage set override was achieved by marking the associated account as deleted, preventing access to the storage slot on disk. However, since #29520, this flag is also checked when accessing the account, rendering the account unreachable. A fix has been applied in this pull request, which re-creates a new state object with all account metadata inherited. --- core/state/statedb.go | 30 ++++++++++++++++++------------ internal/ethapi/api_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/core/state/statedb.go b/core/state/statedb.go index 2ea0662ce0..08dbc44538 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -854,22 +854,28 @@ func (s *StateDB) SetState(addr common.Address, key, value common.Hash) { // storage. This function should only be used for debugging and the mutations // must be discarded afterwards. func (s *StateDB) SetStorage(addr common.Address, storage map[common.Hash]common.Hash) { - // SetStorage needs to wipe existing storage. We achieve this by pretending - // that the account self-destructed earlier in this block, by flagging - // it in stateObjectsDestruct. The effect of doing so is that storage lookups - // will not hit disk, since it is assumed that the disk-data is belonging + // SetStorage needs to wipe the existing storage. We achieve this by marking + // the account as self-destructed in this block. The effect is that storage + // lookups will not hit the disk, as it is assumed that the disk data belongs // to a previous incarnation of the object. // - // TODO(rjl493456442) this function should only be supported by 'unwritable' - // state and all mutations made should all be discarded afterwards. - if _, ok := s.stateObjectsDestruct[addr]; !ok { - s.stateObjectsDestruct[addr] = nil + // TODO (rjl493456442): This function should only be supported by 'unwritable' + // state, and all mutations made should be discarded afterward. + obj := s.getStateObject(addr) + if obj != nil { + if _, ok := s.stateObjectsDestruct[addr]; !ok { + s.stateObjectsDestruct[addr] = obj + } } - - stateObject := s.getOrNewStateObject(addr) - + newObj := s.createObject(addr) for k, v := range storage { - stateObject.SetState(k, v) + newObj.SetState(k, v) + } + // Inherit the metadata of original object if it was existent + if obj != nil { + newObj.SetCode(common.BytesToHash(obj.CodeHash()), obj.code) + newObj.SetNonce(obj.Nonce()) + newObj.SetBalance(obj.Balance(), tracing.BalanceChangeUnspecified) } } diff --git a/internal/ethapi/api_test.go b/internal/ethapi/api_test.go index 07e5761951..a6d620b330 100644 --- a/internal/ethapi/api_test.go +++ b/internal/ethapi/api_test.go @@ -855,15 +855,24 @@ func TestEstimateGas(t *testing.T) { func TestCall(t *testing.T) { t.Parallel() + // Initialize test accounts var ( accounts = newAccounts(3) + dad = common.HexToAddress("0x0000000000000000000000000000000000000dad") genesis = &core.Genesis{ Config: params.MergedTestChainConfig, Alloc: types.GenesisAlloc{ accounts[0].addr: {Balance: big.NewInt(params.Ether)}, accounts[1].addr: {Balance: big.NewInt(params.Ether)}, accounts[2].addr: {Balance: big.NewInt(params.Ether)}, + dad: { + Balance: big.NewInt(params.Ether), + Nonce: 1, + Storage: map[common.Hash]common.Hash{ + common.Hash{}: common.HexToHash("0x0000000000000000000000000000000000000000000000000000000000000001"), + }, + }, }, } genBlocks = 10 @@ -1024,6 +1033,32 @@ func TestCall(t *testing.T) { // }, // want: "0x0122000000000000000000000000000000000000000000000000000000000000", // }, + // Clear the entire storage set + { + blockNumber: rpc.LatestBlockNumber, + call: TransactionArgs{ + From: &accounts[1].addr, + // Yul: + // object "Test" { + // code { + // let dad := 0x0000000000000000000000000000000000000dad + // if eq(balance(dad), 0) { + // revert(0, 0) + // } + // let slot := sload(0) + // mstore(0, slot) + // return(0, 32) + // } + // } + Input: hex2Bytes("610dad6000813103600f57600080fd5b6000548060005260206000f3"), + }, + overrides: StateOverride{ + dad: OverrideAccount{ + State: &map[common.Hash]common.Hash{}, + }, + }, + want: "0x0000000000000000000000000000000000000000000000000000000000000000", + }, } for i, tc := range testSuite { result, err := api.Call(context.Background(), tc.call, &rpc.BlockNumberOrHash{BlockNumber: &tc.blockNumber}, &tc.overrides, &tc.blockOverrides)