From 7a489623ac09158f98c3a839cff88a14db3f039d Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Tue, 10 Jan 2023 08:24:30 -0500 Subject: [PATCH] core/state: remove notion of fake storage (#24916) This PR removes the notion of fakeStorage from the state objects, and instead, for any state modifications that are needed, it simply makes the changes. --- core/state/state_object.go | 32 ------- core/state/statedb.go | 10 ++- eth/tracers/api_test.go | 170 ++++++++++++++++++++++++++++++++++++- internal/ethapi/api.go | 4 + 4 files changed, 181 insertions(+), 35 deletions(-) diff --git a/core/state/state_object.go b/core/state/state_object.go index bea8f17be..6bd6f1832 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -84,7 +84,6 @@ type stateObject struct { originStorage Storage // Storage cache of original entries to dedup rewrites, reset for every transaction pendingStorage Storage // Storage entries that need to be flushed to disk, at the end of an entire block dirtyStorage Storage // Storage entries that have been modified in the current transaction execution - fakeStorage Storage // Fake storage which constructed by caller for debugging purpose. // Cache flags. // When an object is marked suicided it will be delete from the trie @@ -173,10 +172,6 @@ func (s *stateObject) getTrie(db Database) (Trie, error) { // GetState retrieves a value from the account storage trie. func (s *stateObject) GetState(db Database, key common.Hash) common.Hash { - // If the fake storage is set, only lookup the state here(in the debugging mode) - if s.fakeStorage != nil { - return s.fakeStorage[key] - } // If we have a dirty value for this state entry, return it value, dirty := s.dirtyStorage[key] if dirty { @@ -188,10 +183,6 @@ func (s *stateObject) GetState(db Database, key common.Hash) common.Hash { // GetCommittedState retrieves a value from the committed account storage trie. func (s *stateObject) GetCommittedState(db Database, key common.Hash) common.Hash { - // If the fake storage is set, only lookup the state here(in the debugging mode) - if s.fakeStorage != nil { - return s.fakeStorage[key] - } // If we have a pending write or clean cached, return that if value, pending := s.pendingStorage[key]; pending { return value @@ -251,11 +242,6 @@ func (s *stateObject) GetCommittedState(db Database, key common.Hash) common.Has // SetState updates a value in account storage. func (s *stateObject) SetState(db Database, key, value common.Hash) { - // If the fake storage is set, put the temporary state update here. - if s.fakeStorage != nil { - s.fakeStorage[key] = value - return - } // If the new value is the same as old, don't set prev := s.GetState(db, key) if prev == value { @@ -270,24 +256,6 @@ func (s *stateObject) SetState(db Database, key, value common.Hash) { s.setState(key, value) } -// SetStorage replaces the entire state storage with the given one. -// -// After this function is called, all original state will be ignored and state -// lookup only happens in the fake state storage. -// -// Note this function should only be used for debugging purpose. -func (s *stateObject) SetStorage(storage map[common.Hash]common.Hash) { - // Allocate fake storage if it's nil. - if s.fakeStorage == nil { - s.fakeStorage = make(Storage) - } - for key, value := range storage { - s.fakeStorage[key] = value - } - // Don't bother journal since this function should only be used for - // debugging and the `fake` storage won't be committed to database. -} - func (s *stateObject) setState(key, value common.Hash) { s.dirtyStorage[key] = value } diff --git a/core/state/statedb.go b/core/state/statedb.go index fb2459ffc..ebbf64a01 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -442,9 +442,15 @@ func (s *StateDB) SetState(addr common.Address, key, value common.Hash) { // SetStorage replaces the entire storage for the specified account with given // storage. This function should only be used for debugging. 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 + // to a previous incarnation of the object. + s.stateObjectsDestruct[addr] = struct{}{} stateObject := s.GetOrNewStateObject(addr) - if stateObject != nil { - stateObject.SetStorage(storage) + for k, v := range storage { + stateObject.SetState(s.db, k, v) } } diff --git a/eth/tracers/api_test.go b/eth/tracers/api_test.go index 334f23efd..29ec80868 100644 --- a/eth/tracers/api_test.go +++ b/eth/tracers/api_test.go @@ -454,12 +454,21 @@ func TestTracingWithOverrides(t *testing.T) { t.Parallel() // Initialize test accounts accounts := newAccounts(3) + storageAccount := common.Address{0x13, 37} genesis := &core.Genesis{ Config: params.TestChainConfig, Alloc: core.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)}, + // An account with existing storage + storageAccount: { + Balance: new(big.Int), + Storage: map[common.Hash]common.Hash{ + common.HexToHash("0x03"): common.HexToHash("0x33"), + common.HexToHash("0x04"): common.HexToHash("0x44"), + }, + }, }, } genBlocks := 10 @@ -579,6 +588,164 @@ func TestTracingWithOverrides(t *testing.T) { }, want: `{"gas":72666,"failed":false,"returnValue":"000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"}`, }, + /* + pragma solidity =0.8.12; + + contract Test { + uint private x; + + function test2() external { + x = 1337; + revert(); + } + + function test() external returns (uint) { + x = 1; + try this.test2() {} catch (bytes memory) {} + return x; + } + } + */ + { // First with only code override, not storage override + blockNumber: rpc.LatestBlockNumber, + call: ethapi.TransactionArgs{ + From: &randomAccounts[0].addr, + To: &randomAccounts[2].addr, + Data: newRPCBytes(common.Hex2Bytes("f8a8fd6d")), // + }, + config: &TraceCallConfig{ + StateOverrides: ðapi.StateOverride{ + randomAccounts[2].addr: ethapi.OverrideAccount{ + Code: newRPCBytes(common.Hex2Bytes("6080604052348015600f57600080fd5b506004361060325760003560e01c806366e41cb7146037578063f8a8fd6d14603f575b600080fd5b603d6057565b005b60456062565b60405190815260200160405180910390f35b610539600090815580fd5b60006001600081905550306001600160a01b03166366e41cb76040518163ffffffff1660e01b8152600401600060405180830381600087803b15801560a657600080fd5b505af192505050801560b6575060015b60e9573d80801560e1576040519150601f19603f3d011682016040523d82523d6000602084013e60e6565b606091505b50505b506000549056fea26469706673582212205ce45de745a5308f713cb2f448589177ba5a442d1a2eff945afaa8915961b4d064736f6c634300080c0033")), + }, + }, + }, + want: `{"gas":44100,"failed":false,"returnValue":"0000000000000000000000000000000000000000000000000000000000000001"}`, + }, + { // Same again, this time with storage override + blockNumber: rpc.LatestBlockNumber, + call: ethapi.TransactionArgs{ + From: &randomAccounts[0].addr, + To: &randomAccounts[2].addr, + Data: newRPCBytes(common.Hex2Bytes("f8a8fd6d")), // + }, + config: &TraceCallConfig{ + StateOverrides: ðapi.StateOverride{ + randomAccounts[2].addr: ethapi.OverrideAccount{ + Code: newRPCBytes(common.Hex2Bytes("6080604052348015600f57600080fd5b506004361060325760003560e01c806366e41cb7146037578063f8a8fd6d14603f575b600080fd5b603d6057565b005b60456062565b60405190815260200160405180910390f35b610539600090815580fd5b60006001600081905550306001600160a01b03166366e41cb76040518163ffffffff1660e01b8152600401600060405180830381600087803b15801560a657600080fd5b505af192505050801560b6575060015b60e9573d80801560e1576040519150601f19603f3d011682016040523d82523d6000602084013e60e6565b606091505b50505b506000549056fea26469706673582212205ce45de745a5308f713cb2f448589177ba5a442d1a2eff945afaa8915961b4d064736f6c634300080c0033")), + State: newStates([]common.Hash{{}}, []common.Hash{{}}), + }, + }, + }, + //want: `{"gas":46900,"failed":false,"returnValue":"0000000000000000000000000000000000000000000000000000000000000539"}`, + want: `{"gas":44100,"failed":false,"returnValue":"0000000000000000000000000000000000000000000000000000000000000001"}`, + }, + { // No state override + blockNumber: rpc.LatestBlockNumber, + call: ethapi.TransactionArgs{ + From: &randomAccounts[0].addr, + To: &storageAccount, + Data: newRPCBytes(common.Hex2Bytes("f8a8fd6d")), // + }, + config: &TraceCallConfig{ + StateOverrides: ðapi.StateOverride{ + storageAccount: ethapi.OverrideAccount{ + Code: newRPCBytes([]byte{ + // SLOAD(3) + SLOAD(4) (which is 0x77) + byte(vm.PUSH1), 0x04, + byte(vm.SLOAD), + byte(vm.PUSH1), 0x03, + byte(vm.SLOAD), + byte(vm.ADD), + // 0x77 -> MSTORE(0) + byte(vm.PUSH1), 0x00, + byte(vm.MSTORE), + // RETURN (0, 32) + byte(vm.PUSH1), 32, + byte(vm.PUSH1), 00, + byte(vm.RETURN), + }), + }, + }, + }, + want: `{"gas":25288,"failed":false,"returnValue":"0000000000000000000000000000000000000000000000000000000000000077"}`, + }, + { // Full state override + // The original storage is + // 3: 0x33 + // 4: 0x44 + // With a full override, where we set 3:0x11, the slot 4 should be + // removed. So SLOT(3)+SLOT(4) should be 0x11. + blockNumber: rpc.LatestBlockNumber, + call: ethapi.TransactionArgs{ + From: &randomAccounts[0].addr, + To: &storageAccount, + Data: newRPCBytes(common.Hex2Bytes("f8a8fd6d")), // + }, + config: &TraceCallConfig{ + StateOverrides: ðapi.StateOverride{ + storageAccount: ethapi.OverrideAccount{ + Code: newRPCBytes([]byte{ + // SLOAD(3) + SLOAD(4) (which is now 0x11 + 0x00) + byte(vm.PUSH1), 0x04, + byte(vm.SLOAD), + byte(vm.PUSH1), 0x03, + byte(vm.SLOAD), + byte(vm.ADD), + // 0x11 -> MSTORE(0) + byte(vm.PUSH1), 0x00, + byte(vm.MSTORE), + // RETURN (0, 32) + byte(vm.PUSH1), 32, + byte(vm.PUSH1), 00, + byte(vm.RETURN), + }), + State: newStates( + []common.Hash{common.HexToHash("0x03")}, + []common.Hash{common.HexToHash("0x11")}), + }, + }, + }, + want: `{"gas":25288,"failed":false,"returnValue":"0000000000000000000000000000000000000000000000000000000000000011"}`, + }, + { // Partial state override + // The original storage is + // 3: 0x33 + // 4: 0x44 + // With a partial override, where we set 3:0x11, the slot 4 as before. + // So SLOT(3)+SLOT(4) should be 0x55. + blockNumber: rpc.LatestBlockNumber, + call: ethapi.TransactionArgs{ + From: &randomAccounts[0].addr, + To: &storageAccount, + Data: newRPCBytes(common.Hex2Bytes("f8a8fd6d")), // + }, + config: &TraceCallConfig{ + StateOverrides: ðapi.StateOverride{ + storageAccount: ethapi.OverrideAccount{ + Code: newRPCBytes([]byte{ + // SLOAD(3) + SLOAD(4) (which is now 0x11 + 0x44) + byte(vm.PUSH1), 0x04, + byte(vm.SLOAD), + byte(vm.PUSH1), 0x03, + byte(vm.SLOAD), + byte(vm.ADD), + // 0x55 -> MSTORE(0) + byte(vm.PUSH1), 0x00, + byte(vm.MSTORE), + // RETURN (0, 32) + byte(vm.PUSH1), 32, + byte(vm.PUSH1), 00, + byte(vm.RETURN), + }), + StateDiff: &map[common.Hash]common.Hash{ + common.HexToHash("0x03"): common.HexToHash("0x11"), + }, + }, + }, + }, + want: `{"gas":25288,"failed":false,"returnValue":"0000000000000000000000000000000000000000000000000000000000000055"}`, + }, } for i, tc := range testSuite { result, err := api.TraceCall(context.Background(), tc.call, rpc.BlockNumberOrHash{BlockNumber: &tc.blockNumber}, tc.config) @@ -605,7 +772,8 @@ func TestTracingWithOverrides(t *testing.T) { json.Unmarshal(resBytes, &have) json.Unmarshal([]byte(tc.want), &want) if !reflect.DeepEqual(have, want) { - t.Errorf("test %d, result mismatch, have\n%v\n, want\n%v\n", i, string(resBytes), want) + t.Logf("result: %v\n", string(resBytes)) + t.Errorf("test %d, result mismatch, have\n%v\n, want\n%v\n", i, have, want) } } } diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 58be2f8bf..b6efba079 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -909,6 +909,10 @@ func (diff *StateOverride) Apply(state *state.StateDB) error { } } } + // Now finalize the changes. Finalize is normally performed between transactions. + // By using finalize, the overrides are semantically behaving as + // if they were created in a transaction just before the tracing occur. + state.Finalise(false) return nil }