From 83513e66ddb59221ccb82d197856c3aeb14fffd3 Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Wed, 7 Jan 2026 17:51:24 +0900 Subject: [PATCH] add suggested changes from gary Co-authored-by: Gary Rong --- core/state/statedb.go | 37 ++++++++++++----------------- core/state/statedb_hooked.go | 45 ++++++++++++++++-------------------- core/vm/instructions.go | 25 ++++++++++++-------- core/vm/interface.go | 14 +++++------ 4 files changed, 58 insertions(+), 63 deletions(-) diff --git a/core/state/statedb.go b/core/state/statedb.go index 73e3a7b756..de5c875c7e 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -425,13 +425,6 @@ func (s *StateDB) HasSelfDestructed(addr common.Address) bool { return false } -// ContractExistedBeforeCurTx returns true if a contract exists and was not created -// in the current transaction. -func (s *StateDB) ContractExistedBeforeCurTx(addr common.Address) bool { - obj := s.getStateObject(addr) - return obj != nil && !obj.newContract -} - /* * SETTERS */ @@ -516,21 +509,13 @@ func (s *StateDB) SetStorage(addr common.Address, storage map[common.Hash]common } // 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 SelfDestruct. -func (s *StateDB) SelfDestruct(addr common.Address) uint256.Int { +func (s *StateDB) SelfDestruct(addr common.Address) { stateObject := s.getStateObject(addr) - var prevBalance uint256.Int if stateObject == nil { - return prevBalance - } - prevBalance = *(stateObject.Balance()) - // Regardless of whether it is already destructed or not, we do have to - // journal the balance-change, if we set it to zero here. - if !stateObject.Balance().IsZero() { - stateObject.SetBalance(new(uint256.Int)) + return } // If it is already marked as self-destructed, we do not need to add it // for journalling a second time. @@ -538,18 +523,16 @@ func (s *StateDB) SelfDestruct(addr common.Address) uint256.Int { s.journal.destruct(addr) stateObject.markSelfdestructed() } - return prevBalance } -func (s *StateDB) SelfDestruct6780(addr common.Address) (uint256.Int, bool) { +func (s *StateDB) SelfDestruct6780(addr common.Address) { stateObject := s.getStateObject(addr) if stateObject == nil { - return uint256.Int{}, false + return } if stateObject.newContract { - return s.SelfDestruct(addr), true + s.SelfDestruct(addr) } - return *(stateObject.Balance()), false } // SetTransientState sets transient storage for a given account. It @@ -677,6 +660,16 @@ func (s *StateDB) CreateContract(addr common.Address) { } } +// IsNewContract reports whether the contract at the given address was deployed +// during the current transaction. +func (s *StateDB) IsNewContract(addr common.Address) bool { + obj := s.getStateObject(addr) + if obj == nil { + return false + } + return obj.newContract +} + // Copy creates a deep, independent copy of the state. // Snapshots of the copied state cannot be applied to the copy. func (s *StateDB) Copy() *StateDB { diff --git a/core/state/statedb_hooked.go b/core/state/statedb_hooked.go index 3cff4648bb..b5dcef6eef 100644 --- a/core/state/statedb_hooked.go +++ b/core/state/statedb_hooked.go @@ -52,6 +52,10 @@ func (s *hookedStateDB) CreateContract(addr common.Address) { s.inner.CreateContract(addr) } +func (s *hookedStateDB) IsNewContract(addr common.Address) bool { + return s.inner.IsNewContract(addr) +} + func (s *hookedStateDB) GetBalance(addr common.Address) *uint256.Int { return s.inner.GetBalance(addr) } @@ -211,22 +215,12 @@ func (s *hookedStateDB) SetState(address common.Address, key common.Hash, value return prev } -func (s *hookedStateDB) SelfDestruct(address common.Address) uint256.Int { - prev := s.inner.SelfDestruct(address) - - if s.hooks.OnBalanceChange != nil && !prev.IsZero() { - s.hooks.OnBalanceChange(address, prev.ToBig(), new(big.Int), tracing.BalanceDecreaseSelfdestruct) - } - - return prev +func (s *hookedStateDB) SelfDestruct(address common.Address) { + s.inner.SelfDestruct(address) } -func (s *hookedStateDB) SelfDestruct6780(address common.Address) (uint256.Int, bool) { - return s.inner.SelfDestruct6780(address) -} - -func (s *hookedStateDB) ContractExistedBeforeCurTx(address common.Address) bool { - return s.inner.ContractExistedBeforeCurTx(address) +func (s *hookedStateDB) SelfDestruct6780(address common.Address) { + s.inner.SelfDestruct6780(address) } func (s *hookedStateDB) AddLog(log *types.Log) { @@ -238,8 +232,11 @@ func (s *hookedStateDB) AddLog(log *types.Log) { } func (s *hookedStateDB) Finalise(deleteEmptyObjects bool) { - defer s.inner.Finalise(deleteEmptyObjects) - if s.hooks.OnBalanceChange != nil || s.hooks.OnNonceChangeV2 != nil || s.hooks.OnNonceChange != nil || s.hooks.OnCodeChangeV2 != nil || s.hooks.OnCodeChange != nil { + callHooks := s.hooks.OnBalanceChange != nil || + s.hooks.OnNonceChangeV2 != nil || s.hooks.OnNonceChange != nil || + s.hooks.OnCodeChangeV2 != nil || s.hooks.OnCodeChange != nil + + if callHooks { for addr := range s.inner.journal.dirties { obj := s.inner.stateObjects[addr] if obj != nil && obj.selfDestructed { @@ -250,25 +247,23 @@ func (s *hookedStateDB) Finalise(deleteEmptyObjects bool) { } } if s.hooks.OnNonceChangeV2 != nil { - prevNonce := obj.Nonce() - s.hooks.OnNonceChangeV2(addr, prevNonce, 0, tracing.NonceChangeSelfdestruct) + s.hooks.OnNonceChangeV2(addr, obj.Nonce(), 0, tracing.NonceChangeSelfdestruct) } else if s.hooks.OnNonceChange != nil { - prevNonce := obj.Nonce() - s.hooks.OnNonceChange(addr, prevNonce, 0) + s.hooks.OnNonceChange(addr, obj.Nonce(), 0) } - prevCodeHash := s.inner.GetCodeHash(addr) - prevCode := s.inner.GetCode(addr) - // if an initcode invokes selfdestruct, do not emit a code change. + // If an initcode invokes selfdestruct, do not emit a code change. + prevCodeHash := s.inner.GetCodeHash(addr) if prevCodeHash == types.EmptyCodeHash { continue } if s.hooks.OnCodeChangeV2 != nil { - s.hooks.OnCodeChangeV2(addr, prevCodeHash, prevCode, types.EmptyCodeHash, nil, tracing.CodeChangeSelfDestruct) + s.hooks.OnCodeChangeV2(addr, prevCodeHash, s.inner.GetCode(addr), types.EmptyCodeHash, nil, tracing.CodeChangeSelfDestruct) } else if s.hooks.OnCodeChange != nil { - s.hooks.OnCodeChange(addr, prevCodeHash, prevCode, types.EmptyCodeHash, nil) + s.hooks.OnCodeChange(addr, prevCodeHash, s.inner.GetCode(addr), types.EmptyCodeHash, nil) } } } } + s.inner.Finalise(deleteEmptyObjects) } diff --git a/core/vm/instructions.go b/core/vm/instructions.go index fa774bc57d..60fd70d90d 100644 --- a/core/vm/instructions.go +++ b/core/vm/instructions.go @@ -879,10 +879,15 @@ func opSelfdestruct(pc *uint64, evm *EVM, scope *ScopeContext) ([]byte, error) { beneficiary := scope.Stack.pop() balance := evm.StateDB.GetBalance(scope.Contract.Address()) - if common.BytesToAddress(beneficiary.Bytes()) != scope.Contract.Address() { + // The funds are burned immediately if the beneficiary is the caller itself, + // in this case, the beneficiary's balance is not increased. + if beneficiary.Bytes20() != scope.Contract.Address() { evm.StateDB.AddBalance(beneficiary.Bytes20(), balance, tracing.BalanceIncreaseSelfdestruct) } + // Clear any leftover funds for the account being destructed. + evm.StateDB.SubBalance(scope.Contract.Address(), balance, tracing.BalanceDecreaseSelfdestruct) evm.StateDB.SelfDestruct(scope.Contract.Address()) + if tracer := evm.Config.Tracer; tracer != nil { if tracer.OnEnter != nil { tracer.OnEnter(evm.depth, byte(SELFDESTRUCT), scope.Contract.Address(), beneficiary.Bytes20(), []byte{}, 0, balance.ToBig()) @@ -897,21 +902,23 @@ func opSelfdestruct(pc *uint64, evm *EVM, scope *ScopeContext) ([]byte, error) { func opSelfdestruct6780(pc *uint64, evm *EVM, scope *ScopeContext) ([]byte, error) { beneficiary := scope.Stack.pop() balance := evm.StateDB.GetBalance(scope.Contract.Address()) - createdInTx := !evm.StateDB.ContractExistedBeforeCurTx(scope.Contract.Address()) - targetIsSelf := scope.Contract.Address() == common.BytesToAddress(beneficiary.Bytes()) - if createdInTx { - // if the contract is not preexisting, the balance is immediately burned on selfdestruct-to-self - evm.StateDB.SubBalance(scope.Contract.Address(), balance, tracing.BalanceDecreaseSelfdestruct) - if !targetIsSelf { + if evm.StateDB.IsNewContract(scope.Contract.Address()) { + // If the contract is not preexisting, the balance is immediately burned + // on selfdestruct-to-self. In this case, the beneficiary's balance is + // not increased. + if scope.Contract.Address() != beneficiary.Bytes20() { evm.StateDB.AddBalance(beneficiary.Bytes20(), balance, tracing.BalanceIncreaseSelfdestruct) } - } else if !targetIsSelf { - // if the contract is preexisting, the balance isn't burned on selfdestruct-to-self + evm.StateDB.SubBalance(scope.Contract.Address(), balance, tracing.BalanceDecreaseSelfdestruct) + } else { + // If the contract is preexisting, send all Ether in the account to the + // target. Note the balance isn't burned on selfdestruct-to-self. evm.StateDB.SubBalance(scope.Contract.Address(), balance, tracing.BalanceDecreaseSelfdestruct) evm.StateDB.AddBalance(beneficiary.Bytes20(), balance, tracing.BalanceIncreaseSelfdestruct) } evm.StateDB.SelfDestruct6780(scope.Contract.Address()) + if tracer := evm.Config.Tracer; tracer != nil { if tracer.OnEnter != nil { tracer.OnEnter(evm.depth, byte(SELFDESTRUCT), scope.Contract.Address(), beneficiary.Bytes20(), []byte{}, 0, balance.ToBig()) diff --git a/core/vm/interface.go b/core/vm/interface.go index 1c9f031917..fbc6529d18 100644 --- a/core/vm/interface.go +++ b/core/vm/interface.go @@ -57,22 +57,22 @@ type StateDB interface { GetTransientState(addr common.Address, key common.Hash) common.Hash SetTransientState(addr common.Address, key, value common.Hash) - SelfDestruct(common.Address) uint256.Int + SelfDestruct(common.Address) HasSelfDestructed(common.Address) bool // SelfDestruct6780 is post-EIP6780 selfdestruct, which means that it's a // send-all-to-beneficiary, unless the contract was created in this same // transaction, in which case it will be destructed. - // This method returns the prior balance, along with a boolean which is - // true iff the object was indeed destructed. - SelfDestruct6780(common.Address) (uint256.Int, bool) + SelfDestruct6780(common.Address) // Exist reports whether the given account exists in state. // Notably this also returns true for self-destructed accounts within the current transaction. Exist(common.Address) bool - // ExistBeforeCurTx returns true if a contract exists and was not created - // in the current transaction. - ContractExistedBeforeCurTx(addr common.Address) bool + + // IsNewContract reports whether the contract at the given address was deployed + // during the current transaction. + IsNewContract(addr common.Address) bool + // Empty returns whether the given account is empty. Empty // is defined according to EIP161 (balance = nonce = code = 0). Empty(common.Address) bool