From 859213809271d617bef1668c162b5b17cbd2acd8 Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Thu, 13 Jul 2023 11:42:07 +0200 Subject: [PATCH 1/7] core: remove unused return value from Suicide --- core/state/statedb.go | 5 ++--- core/vm/interface.go | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/core/state/statedb.go b/core/state/statedb.go index 71a863add701..887c696b539f 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -479,10 +479,10 @@ func (s *StateDB) SetStorage(addr common.Address, storage map[common.Hash]common // // The account's state object is still available until the state is committed, // getStateObject will return a non-nil account after Suicide. -func (s *StateDB) Suicide(addr common.Address) bool { +func (s *StateDB) Suicide(addr common.Address) { stateObject := s.getStateObject(addr) if stateObject == nil { - return false + return } s.journal.append(suicideChange{ account: &addr, @@ -491,7 +491,6 @@ func (s *StateDB) Suicide(addr common.Address) bool { }) stateObject.markSuicided() stateObject.data.Balance = new(big.Int) - return true } // SetTransientState sets transient storage for a given account. It diff --git a/core/vm/interface.go b/core/vm/interface.go index b83f78307eb7..d829cfe3aed2 100644 --- a/core/vm/interface.go +++ b/core/vm/interface.go @@ -51,7 +51,7 @@ type StateDB interface { GetTransientState(addr common.Address, key common.Hash) common.Hash SetTransientState(addr common.Address, key, value common.Hash) - Suicide(common.Address) bool + Suicide(common.Address) HasSuicided(common.Address) bool // Exist reports whether the given account exists in state. From 5d11677cf1653f7c59570ca962b6a6f7d447f7e9 Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Thu, 13 Jul 2023 11:59:55 +0200 Subject: [PATCH 2/7] core/vm, core/state: rename instances of 'suicide' to 'selfdestruct' for code consistency. remove unecessary check for nil stateObject in statedb.SelfDestruct --- core/state/journal.go | 10 +++++----- core/state/state_object.go | 14 +++++++------- core/state/statedb.go | 21 +++++++++------------ core/vm/gas_table.go | 2 +- core/vm/instructions.go | 2 +- core/vm/interface.go | 4 ++-- core/vm/operations_acl.go | 2 +- 7 files changed, 26 insertions(+), 29 deletions(-) diff --git a/core/state/journal.go b/core/state/journal.go index 95a218b3a349..f7ec26625b16 100644 --- a/core/state/journal.go +++ b/core/state/journal.go @@ -100,9 +100,9 @@ type ( prevAccountOrigin []byte prevStorageOrigin map[common.Hash][]byte } - suicideChange struct { + selfDestructChange struct { account *common.Address - prev bool // whether account had already suicided + prev bool // whether account had already self-destructed prevbalance *big.Int } @@ -184,15 +184,15 @@ func (ch resetObjectChange) dirtied() *common.Address { return ch.account } -func (ch suicideChange) revert(s *StateDB) { +func (ch selfDestructChange) revert(s *StateDB) { obj := s.getStateObject(*ch.account) if obj != nil { - obj.suicided = ch.prev + obj.selfDestructed = ch.prev obj.setBalance(ch.prevbalance) } } -func (ch suicideChange) dirtied() *common.Address { +func (ch selfDestructChange) dirtied() *common.Address { return ch.account } diff --git a/core/state/state_object.go b/core/state/state_object.go index 2529ec69290f..05f07b9e8671 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -78,12 +78,12 @@ type stateObject struct { // Cache flags. dirtyCode bool // true if the code was updated - // Flag whether the account was marked as suicided. The suicided account + // Flag whether the account was marked as self-destructed. The self-destructed account // is still accessible in the scope of same transaction. - suicided bool + selfDestructed bool - // Flag whether the account was marked as deleted. The suicided account - // or the account is considered as empty will be marked as deleted at + // Flag whether the account was marked as deleted. a self-destructed account + // or an account that is considered as empty will be marked as deleted at // the end of transaction and no longer accessible anymore. deleted bool } @@ -116,8 +116,8 @@ func (s *stateObject) EncodeRLP(w io.Writer) error { return rlp.Encode(w, &s.data) } -func (s *stateObject) markSuicided() { - s.suicided = true +func (s *stateObject) markSelfdestructed() { + s.selfDestructed = true } func (s *stateObject) touch() { @@ -446,7 +446,7 @@ func (s *stateObject) deepCopy(db *StateDB) *stateObject { obj.dirtyStorage = s.dirtyStorage.Copy() obj.originStorage = s.originStorage.Copy() obj.pendingStorage = s.pendingStorage.Copy() - obj.suicided = s.suicided + obj.selfDestructed = s.selfDestructed obj.dirtyCode = s.dirtyCode obj.deleted = s.deleted return obj diff --git a/core/state/statedb.go b/core/state/statedb.go index 887c696b539f..dbd988f821e0 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -271,7 +271,7 @@ func (s *StateDB) SubRefund(gas uint64) { } // Exist reports whether the given account address exists in the state. -// Notably this also returns true for suicided accounts. +// Notably this also returns true for self-destructed accounts. func (s *StateDB) Exist(addr common.Address) bool { return s.getStateObject(addr) != nil } @@ -397,10 +397,10 @@ func (s *StateDB) StorageTrie(addr common.Address) (Trie, error) { return cpy.getTrie(s.db) } -func (s *StateDB) HasSuicided(addr common.Address) bool { +func (s *StateDB) HasSelfDestructed(addr common.Address) bool { stateObject := s.getStateObject(addr) if stateObject != nil { - return stateObject.suicided + return stateObject.selfDestructed } return false } @@ -474,22 +474,19 @@ func (s *StateDB) SetStorage(addr common.Address, storage map[common.Hash]common } } -// Suicide marks the given account as suicided. +// SelfDestruct marks the given account as selfdestructed. // This clears the account balance. // // The account's state object is still available until the state is committed, // getStateObject will return a non-nil account after Suicide. -func (s *StateDB) Suicide(addr common.Address) { +func (s *StateDB) SelfDestruct(addr common.Address) { stateObject := s.getStateObject(addr) - if stateObject == nil { - return - } - s.journal.append(suicideChange{ + s.journal.append(selfDestructChange{ account: &addr, - prev: stateObject.suicided, + prev: stateObject.selfDestructed, prevbalance: new(big.Int).Set(stateObject.Balance()), }) - stateObject.markSuicided() + stateObject.markSelfdestructed() stateObject.data.Balance = new(big.Int) } @@ -891,7 +888,7 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool) { // Thus, we can safely ignore it here continue } - if obj.suicided || (deleteEmptyObjects && obj.empty()) { + if obj.selfDestructed || (deleteEmptyObjects && obj.empty()) { obj.deleted = true // We need to maintain account deletions explicitly (will remain diff --git a/core/vm/gas_table.go b/core/vm/gas_table.go index 79aba9d4ff50..5153c8b7a3de 100644 --- a/core/vm/gas_table.go +++ b/core/vm/gas_table.go @@ -472,7 +472,7 @@ func gasSelfdestruct(evm *EVM, contract *Contract, stack *Stack, mem *Memory, me } } - if !evm.StateDB.HasSuicided(contract.Address()) { + if !evm.StateDB.HasSelfDestructed(contract.Address()) { evm.StateDB.AddRefund(params.SelfdestructRefundGas) } return gas, nil diff --git a/core/vm/instructions.go b/core/vm/instructions.go index 505aef412775..d9d2a75e033e 100644 --- a/core/vm/instructions.go +++ b/core/vm/instructions.go @@ -821,7 +821,7 @@ func opSelfdestruct(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext beneficiary := scope.Stack.pop() balance := interpreter.evm.StateDB.GetBalance(scope.Contract.Address()) interpreter.evm.StateDB.AddBalance(beneficiary.Bytes20(), balance) - interpreter.evm.StateDB.Suicide(scope.Contract.Address()) + interpreter.evm.StateDB.SelfDestruct(scope.Contract.Address()) if tracer := interpreter.evm.Config.Tracer; tracer != nil { tracer.CaptureEnter(SELFDESTRUCT, scope.Contract.Address(), beneficiary.Bytes20(), []byte{}, 0, balance) tracer.CaptureExit([]byte{}, 0, nil) diff --git a/core/vm/interface.go b/core/vm/interface.go index d829cfe3aed2..c4f2dd362190 100644 --- a/core/vm/interface.go +++ b/core/vm/interface.go @@ -51,8 +51,8 @@ type StateDB interface { GetTransientState(addr common.Address, key common.Hash) common.Hash SetTransientState(addr common.Address, key, value common.Hash) - Suicide(common.Address) - HasSuicided(common.Address) bool + SelfDestruct(common.Address) + HasSelfDestructed(common.Address) bool // Exist reports whether the given account exists in state. // Notably this should also return true for suicided accounts. diff --git a/core/vm/operations_acl.go b/core/vm/operations_acl.go index 551e1f5f1188..04c6409ebd86 100644 --- a/core/vm/operations_acl.go +++ b/core/vm/operations_acl.go @@ -235,7 +235,7 @@ func makeSelfdestructGasFn(refundsEnabled bool) gasFunc { if evm.StateDB.Empty(address) && evm.StateDB.GetBalance(contract.Address()).Sign() != 0 { gas += params.CreateBySelfdestructGas } - if refundsEnabled && !evm.StateDB.HasSuicided(contract.Address()) { + if refundsEnabled && !evm.StateDB.HasSelfDestructed(contract.Address()) { evm.StateDB.AddRefund(params.SelfdestructRefundGas) } return gas, nil From 9de470d4f0e0c620bdef5e07b02e14f34235a2c3 Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Thu, 13 Jul 2023 12:13:06 +0200 Subject: [PATCH 3/7] replace more occurances --- accounts/abi/bind/backend.go | 2 +- cmd/evm/internal/t8ntool/execution.go | 2 +- core/state/state_test.go | 4 ++-- core/state/statedb.go | 2 +- core/state/statedb_fuzz_test.go | 4 ++-- core/state/statedb_test.go | 8 ++++---- core/vm/instructions.go | 2 +- core/vm/interface.go | 2 +- eth/tracers/native/call_flat.go | 6 +++--- tests/state_test_util.go | 4 ++-- 10 files changed, 18 insertions(+), 18 deletions(-) diff --git a/accounts/abi/bind/backend.go b/accounts/abi/bind/backend.go index c16990f395c4..34530a645518 100644 --- a/accounts/abi/bind/backend.go +++ b/accounts/abi/bind/backend.go @@ -29,7 +29,7 @@ import ( var ( // ErrNoCode is returned by call and transact operations for which the requested // recipient contract to operate on does not exist in the state db or does not - // have any code associated with it (i.e. suicided). + // have any code associated with it (i.e. self-destructed). ErrNoCode = errors.New("no contract code at given address") // ErrNoPendingState is raised when attempting to perform a pending state action diff --git a/cmd/evm/internal/t8ntool/execution.go b/cmd/evm/internal/t8ntool/execution.go index fb85794b76da..aef1a6f62ab5 100644 --- a/cmd/evm/internal/t8ntool/execution.go +++ b/cmd/evm/internal/t8ntool/execution.go @@ -239,7 +239,7 @@ func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig, if miningReward >= 0 { // Add mining reward. The mining reward may be `0`, which only makes a difference in the cases // where - // - the coinbase suicided, or + // - the coinbase selfdestructed, or // - there are only 'bad' transactions, which aren't executed. In those cases, // the coinbase gets no txfee, so isn't created, and thus needs to be touched var ( diff --git a/core/state/state_test.go b/core/state/state_test.go index b6c11728bee4..dc9df371d3c3 100644 --- a/core/state/state_test.go +++ b/core/state/state_test.go @@ -208,7 +208,7 @@ func TestSnapshot2(t *testing.T) { so0.SetBalance(big.NewInt(42)) so0.SetNonce(43) so0.SetCode(crypto.Keccak256Hash([]byte{'c', 'a', 'f', 'e'}), []byte{'c', 'a', 'f', 'e'}) - so0.suicided = false + so0.selfDestructed = false so0.deleted = false state.setStateObject(so0) @@ -220,7 +220,7 @@ func TestSnapshot2(t *testing.T) { so1.SetBalance(big.NewInt(52)) so1.SetNonce(53) so1.SetCode(crypto.Keccak256Hash([]byte{'c', 'a', 'f', 'e', '2'}), []byte{'c', 'a', 'f', 'e', '2'}) - so1.suicided = true + so1.selfDestructed = true so1.deleted = true state.setStateObject(so1) diff --git a/core/state/statedb.go b/core/state/statedb.go index dbd988f821e0..456d8b1d11be 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -478,7 +478,7 @@ func (s *StateDB) SetStorage(addr common.Address, storage map[common.Hash]common // This clears the account balance. // // The account's state object is still available until the state is committed, -// getStateObject will return a non-nil account after Suicide. +// getStateObject will return a non-nil account after SelfDestruct. func (s *StateDB) SelfDestruct(addr common.Address) { stateObject := s.getStateObject(addr) s.journal.append(selfDestructChange{ diff --git a/core/state/statedb_fuzz_test.go b/core/state/statedb_fuzz_test.go index d014b2b3cca2..4dca96efd39b 100644 --- a/core/state/statedb_fuzz_test.go +++ b/core/state/statedb_fuzz_test.go @@ -95,9 +95,9 @@ func newStateTestAction(addr common.Address, r *rand.Rand, index int) testAction }, }, { - name: "Suicide", + name: "Selfdestruct", fn: func(a testAction, s *StateDB) { - s.Suicide(addr) + s.SelfDestruct(addr) }, }, } diff --git a/core/state/statedb_test.go b/core/state/statedb_test.go index 8290a154ff37..6848b01cb117 100644 --- a/core/state/statedb_test.go +++ b/core/state/statedb_test.go @@ -301,9 +301,9 @@ func newTestAction(addr common.Address, r *rand.Rand) testAction { }, }, { - name: "Suicide", + name: "SelfDestruct", fn: func(a testAction, s *StateDB) { - s.Suicide(addr) + s.SelfDestruct(addr) }, }, { @@ -453,7 +453,7 @@ func (test *snapshotTest) checkEqual(state, checkstate *StateDB) error { } // Check basic accessor methods. checkeq("Exist", state.Exist(addr), checkstate.Exist(addr)) - checkeq("HasSuicided", state.HasSuicided(addr), checkstate.HasSuicided(addr)) + checkeq("HasSelfdestructed", state.HasSelfDestructed(addr), checkstate.HasSelfDestructed(addr)) checkeq("GetBalance", state.GetBalance(addr), checkstate.GetBalance(addr)) checkeq("GetNonce", state.GetNonce(addr), checkstate.GetNonce(addr)) checkeq("GetCode", state.GetCode(addr), checkstate.GetCode(addr)) @@ -727,7 +727,7 @@ func TestDeleteCreateRevert(t *testing.T) { state, _ = New(root, state.db, state.snaps) // Simulate self-destructing in one transaction, then create-reverting in another - state.Suicide(addr) + state.SelfDestruct(addr) state.Finalise(true) id := state.Snapshot() diff --git a/core/vm/instructions.go b/core/vm/instructions.go index d9d2a75e033e..4b6514ec8c32 100644 --- a/core/vm/instructions.go +++ b/core/vm/instructions.go @@ -408,7 +408,7 @@ func opExtCodeCopy(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) // emptyCodeHash. If the precompile account is not transferred any amount on a private or // customized chain, the return value will be zero. // -// 5. Caller tries to get the code hash for an account which is marked as suicided +// 5. Caller tries to get the code hash for an account which is marked as self-destructed // in the current transaction, the code hash of this account should be returned. // // 6. Caller tries to get the code hash for an account which is marked as deleted, this diff --git a/core/vm/interface.go b/core/vm/interface.go index c4f2dd362190..05fb480f968c 100644 --- a/core/vm/interface.go +++ b/core/vm/interface.go @@ -55,7 +55,7 @@ type StateDB interface { HasSelfDestructed(common.Address) bool // Exist reports whether the given account exists in state. - // Notably this should also return true for suicided accounts. + // Notably this should also return true for self-destructed accounts. Exist(common.Address) bool // Empty returns whether the given account is empty. Empty // is defined according to EIP161 (balance = nonce = code = 0). diff --git a/eth/tracers/native/call_flat.go b/eth/tracers/native/call_flat.go index 8612c23387d6..cb67b53cfd32 100644 --- a/eth/tracers/native/call_flat.go +++ b/eth/tracers/native/call_flat.go @@ -248,7 +248,7 @@ func flatFromNested(input *callFrame, traceAddress []int, convertErrs bool, ctx case vm.CREATE, vm.CREATE2: frame = newFlatCreate(input) case vm.SELFDESTRUCT: - frame = newFlatSuicide(input) + frame = newFlatSelfdestruct(input) case vm.CALL, vm.STATICCALL, vm.CALLCODE, vm.DELEGATECALL: frame = newFlatCall(input) default: @@ -330,9 +330,9 @@ func newFlatCall(input *callFrame) *flatCallFrame { } } -func newFlatSuicide(input *callFrame) *flatCallFrame { +func newFlatSelfdestruct(input *callFrame) *flatCallFrame { return &flatCallFrame{ - Type: "suicide", + Type: "selfdestruct", Action: flatCallAction{ SelfDestructed: &input.From, Balance: input.Value, diff --git a/tests/state_test_util.go b/tests/state_test_util.go index 667e951b6603..6956fe5b966d 100644 --- a/tests/state_test_util.go +++ b/tests/state_test_util.go @@ -197,7 +197,7 @@ func (t *StateTest) Run(subtest StateSubtest, vmconfig vm.Config, snapshotter bo } post := t.json.Post[subtest.Fork][subtest.Index] // N.B: We need to do this in a two-step process, because the first Commit takes care - // of suicides, and we need to touch the coinbase _after_ it has potentially suicided. + // of self-destructs, and we need to touch the coinbase _after_ it has potentially self-destructed. if root != common.Hash(post.Root) { return snaps, statedb, fmt.Errorf("post state root mismatch: got %x, want %x", root, post.Root) } @@ -275,7 +275,7 @@ func (t *StateTest) RunNoVerify(subtest StateSubtest, vmconfig vm.Config, snapsh } // Add 0-value mining reward. This only makes a difference in the cases // where - // - the coinbase suicided, or + // - the coinbase self-destructed, or // - there are only 'bad' transactions, which aren't executed. In those cases, // the coinbase gets no txfee, so isn't created, and thus needs to be touched statedb.AddBalance(block.Coinbase(), new(big.Int)) From 977c7bc496383a67cb73fab8a08cdd8097ff2dbd Mon Sep 17 00:00:00 2001 From: jwasinger Date: Thu, 13 Jul 2023 13:39:29 +0200 Subject: [PATCH 4/7] Update core/state/state_object.go Co-authored-by: Martin Holst Swende --- core/state/state_object.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/state/state_object.go b/core/state/state_object.go index 05f07b9e8671..ed5f2b294540 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -82,7 +82,7 @@ type stateObject struct { // is still accessible in the scope of same transaction. selfDestructed bool - // Flag whether the account was marked as deleted. a self-destructed account + // Flag whether the account was marked as deleted. A self-destructed account // or an account that is considered as empty will be marked as deleted at // the end of transaction and no longer accessible anymore. deleted bool From 17b35a3bee1cd84c56461934ae548cd0c0eef94b Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Thu, 13 Jul 2023 13:40:27 +0200 Subject: [PATCH 5/7] add nil check back in --- core/state/statedb.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/state/statedb.go b/core/state/statedb.go index 456d8b1d11be..4b17e70255e0 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -481,6 +481,9 @@ func (s *StateDB) SetStorage(addr common.Address, storage map[common.Hash]common // getStateObject will return a non-nil account after SelfDestruct. func (s *StateDB) SelfDestruct(addr common.Address) { stateObject := s.getStateObject(addr) + if stateObject == nil { + return + } s.journal.append(selfDestructChange{ account: &addr, prev: stateObject.selfDestructed, From 3048c104d986ff949c6e435bc582b9e5dff266ac Mon Sep 17 00:00:00 2001 From: jwasinger Date: Thu, 13 Jul 2023 15:35:17 +0200 Subject: [PATCH 6/7] Update cmd/evm/internal/t8ntool/execution.go Co-authored-by: lightclient <14004106+lightclient@users.noreply.github.com> --- cmd/evm/internal/t8ntool/execution.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/evm/internal/t8ntool/execution.go b/cmd/evm/internal/t8ntool/execution.go index aef1a6f62ab5..338427bd463a 100644 --- a/cmd/evm/internal/t8ntool/execution.go +++ b/cmd/evm/internal/t8ntool/execution.go @@ -239,7 +239,7 @@ func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig, if miningReward >= 0 { // Add mining reward. The mining reward may be `0`, which only makes a difference in the cases // where - // - the coinbase selfdestructed, or + // - the coinbase self-destructed, or // - there are only 'bad' transactions, which aren't executed. In those cases, // the coinbase gets no txfee, so isn't created, and thus needs to be touched var ( From d21ff65233969694ca259f85a51062cd5c51a701 Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Fri, 14 Jul 2023 14:10:59 +0200 Subject: [PATCH 7/7] revert change in tracer for backwards-compatibility --- eth/tracers/native/call_flat.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eth/tracers/native/call_flat.go b/eth/tracers/native/call_flat.go index cb67b53cfd32..266ab9900146 100644 --- a/eth/tracers/native/call_flat.go +++ b/eth/tracers/native/call_flat.go @@ -332,7 +332,7 @@ func newFlatCall(input *callFrame) *flatCallFrame { func newFlatSelfdestruct(input *callFrame) *flatCallFrame { return &flatCallFrame{ - Type: "selfdestruct", + Type: "suicide", Action: flatCallAction{ SelfDestructed: &input.From, Balance: input.Value,