add suggested changes from gary

Co-authored-by: Gary Rong <garyrong0905@gmail.com>

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
+		return
-	}
-	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))
 	}
 	// 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 {

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 {
+func (s *hookedStateDB) SelfDestruct(address common.Address) {
-	prev := s.inner.SelfDestruct(address)
+	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) SelfDestruct6780(address common.Address) (uint256.Int, bool) {
+func (s *hookedStateDB) SelfDestruct6780(address common.Address) {
-	return s.inner.SelfDestruct6780(address)
+	s.inner.SelfDestruct6780(address)
-}
-
-func (s *hookedStateDB) ContractExistedBeforeCurTx(address common.Address) bool {
-	return s.inner.ContractExistedBeforeCurTx(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)
+	callHooks := s.hooks.OnBalanceChange != nil ||
-	if s.hooks.OnBalanceChange != nil || s.hooks.OnNonceChangeV2 != nil || s.hooks.OnNonceChange != nil || s.hooks.OnCodeChangeV2 != nil || s.hooks.OnCodeChange != 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, obj.Nonce(), 0, tracing.NonceChangeSelfdestruct)
-					s.hooks.OnNonceChangeV2(addr, prevNonce, 0, tracing.NonceChangeSelfdestruct)
 				} else if s.hooks.OnNonceChange != nil {
-					prevNonce := obj.Nonce()
+					s.hooks.OnNonceChange(addr, obj.Nonce(), 0)
-					s.hooks.OnNonceChange(addr, prevNonce, 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)
 }

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 evm.StateDB.IsNewContract(scope.Contract.Address()) {
-		// if the contract is not preexisting, the balance is immediately burned on selfdestruct-to-self
+		// If the contract is not preexisting, the balance is immediately burned
-		evm.StateDB.SubBalance(scope.Contract.Address(), balance, tracing.BalanceDecreaseSelfdestruct)
+		// on selfdestruct-to-self. In this case, the beneficiary's balance is
-		if !targetIsSelf {
+		// not increased.
+		if scope.Contract.Address() != beneficiary.Bytes20() {
 			evm.StateDB.AddBalance(beneficiary.Bytes20(), balance, tracing.BalanceIncreaseSelfdestruct)
 		}
-	} else if !targetIsSelf {
+		evm.StateDB.SubBalance(scope.Contract.Address(), balance, tracing.BalanceDecreaseSelfdestruct)
-		// if the contract is preexisting, the balance isn't burned on selfdestruct-to-self
+	} 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())

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
+	SelfDestruct6780(common.Address)
-	// true iff the object was indeed destructed.
-	SelfDestruct6780(common.Address) (uint256.Int, bool)
 
 	// 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.
+	// IsNewContract reports whether the contract at the given address was deployed
-	ContractExistedBeforeCurTx(addr common.Address) bool
+	// 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