feat: EIP-161 clear empty touched accounts

x/evm/keeper/integration_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/evmos/ethermint/types"
+	"github.com/evmos/ethermint/x/evm/statedb"
 	"github.com/evmos/ethermint/x/evm/testutil"
 	"github.com/stretchr/testify/suite"
 )
@@ -102,6 +103,173 @@ func (suite *IntegrationTestSuite) TestEIP161_CreateNonce() {
 	)
 }
 
+func (suite *IntegrationTestSuite) TestEIP161_TouchEmptyDeletes() {
+	// From EIP-161 point D:
+	// At the end of the transaction, any account touched by the execution of
+	// that transaction which is now empty SHALL instead become non-existent
+	// (i.e. deleted).
+
+	// Where:
+	// An account is considered to be touched when it is involved in any
+	// potentially state-changing operation. This includes, but is not limited
+	// to, being the recipient of a transfer of zero value.
+
+	// An account changes state when:
+	// 1) it is the target or refund of a SUICIDE operation for zero or more
+	// value;
+	// 2) it is the source or destination of a CALL operation or message-call
+	// transaction transferring zero or more value;
+	// 3) it is the source or creation of a CREATE operation or
+	// contract-creation transaction endowing zero or more value;
+
+	// Not relevant:
+	// 4) as the block author (“miner”) it is the recipient of block-rewards or
+	// transaction-fees of zero or more value.
+
+	// Some cases aren't relevant when clearing state. e.g. as an EOA
+	// sending a tx, I will never be empty as my nonce increments.
+	// Account state changes with a >0 value will also *not* clear the account
+	// as it will be non-empty with a >0 balance.
+
+	var contractAddr common.Address
+	targetAddr := common.Address{10}
+
+	tests := []struct {
+		name                string
+		malleate            func()
+		wantContractDeleted bool
+		wantAccountDeleted  bool
+	}{
+		{
+			"not touched",
+			func() {
+				_, rsp, err := suite.CallContract(
+					testutil.EIP161TestContract,
+					contractAddr,
+					common.Big0,
+					"selfDestructTo",
+					common.Address{20}, // unrelated account
+				)
+				suite.Require().NoError(err)
+				suite.Require().Empty(rsp.VmError)
+			},
+			true,
+			false,
+		},
+		{
+			"self destruct target - 0 value",
+			func() {
+				// beneficiary account with 0 value should be touched
+				_, rsp, err := suite.CallContract(
+					testutil.EIP161TestContract,
+					contractAddr,
+					common.Big0,
+					"selfDestructTo",
+					targetAddr,
+				)
+				suite.Require().NoError(err)
+				suite.Require().Empty(rsp.VmError)
+			},
+			true,
+			true,
+		},
+		{
+			"call target - 0 value",
+			func() {
+				// target account with 0 value should be touched
+				_, rsp, err := suite.CallContract(
+					testutil.EIP161TestContract,
+					contractAddr,
+					common.Big0,
+					"callAccount",
+					targetAddr,
+				)
+				suite.Require().NoError(err)
+				suite.Require().Empty(rsp.VmError)
+			},
+			false,
+			true,
+		},
+	}
+
+	for _, tt := range tests {
+		suite.Run(tt.name, func() {
+			// Setup test
+			// Deploy contract
+			contractAddr = suite.DeployContract(testutil.EIP161TestContract)
+
+			// Create empty account
+			acc := statedb.NewEmptyAccount()
+			err := suite.App.EvmKeeper.SetAccount(suite.Ctx, targetAddr, *acc)
+			suite.Require().NoError(err, "empty target should be created")
+
+			targetAcc := suite.App.EvmKeeper.GetAccount(suite.Ctx, targetAddr)
+			suite.Require().NotNil(targetAcc, "empty account should exist")
+			suite.Require().True(targetAcc.IsEmpty())
+
+			// Run test specific setup
+			tt.malleate()
+
+			// Check result
+			targetAcc = suite.App.EvmKeeper.GetAccount(suite.Ctx, targetAddr)
+			if tt.wantAccountDeleted {
+				suite.Require().Nil(
+					targetAcc,
+					"EIP-161: empty account should be deleted after being touched",
+				)
+			} else {
+				suite.Require().NotNil(
+					targetAcc,
+					"EIP-161: empty account should not be deleted if not touched",
+				)
+			}
+
+			contractAcc := suite.App.EvmKeeper.GetAccount(suite.Ctx, contractAddr)
+			if tt.wantContractDeleted {
+				suite.Require().Nil(
+					contractAcc,
+					"EIP-161: contract should be deleted after touching empty account",
+				)
+			} else {
+				suite.Require().NotNil(
+					contractAcc,
+					"EIP-161: contract should not be deleted if not touching empty account",
+				)
+			}
+		})
+	}
+}
+
+func (suite *IntegrationTestSuite) TestEIP161_CallDeletesEmpty() {
+	addr := suite.DeployContract(testutil.EIP161TestContract)
+
+	// Create an empty account in state
+	targetAddr := common.Address{10}
+	acc := statedb.NewEmptyAccount()
+
+	err := suite.App.EvmKeeper.SetAccount(suite.Ctx, targetAddr, *acc)
+	suite.Require().NoError(err, "target should be created")
+
+	targetAcc := suite.App.EvmKeeper.GetAccount(suite.Ctx, targetAddr)
+	suite.Require().True(targetAcc.IsEmpty())
+
+	_, rsp, err := suite.CallContract(
+		testutil.EIP161TestContract,
+		addr,
+		common.Big0, // no transfer
+		"callAccount",
+		targetAddr,
+	)
+	suite.Require().NoError(err)
+	suite.Require().Empty(rsp.VmError)
+
+	targetAcc = suite.App.EvmKeeper.GetAccount(suite.Ctx, targetAddr)
+	suite.Require().Nil(
+		targetAcc,
+		"EIP-161: empty account should be deleted after being touched",
+	)
+}
+
 func (suite *IntegrationTestSuite) TestEIP161_CallGas() {
 	addr := suite.DeployContract(testutil.EIP161TestContract)
 

x/evm/statedb/ctx.go

@@ -29,7 +29,7 @@ func NewSnapshotCtx(initialCtx sdk.Context) *SnapshotCommitCtx {
 	// Create an initial snapshot of the initialCtx so no state is written until
 	// Commit() is called. The ID is -1 but disregarded along with the
 	// StoreRevertKey indices as this is only to branch the ctx.
-	_ = sCtx.Snapshot(0, StoreRevertKey{0, 0, 0})
+	_ = sCtx.Snapshot(0, initialStoreRevertKey)
 
 	return sCtx
 }

x/evm/statedb/statedb.go

@@ -70,6 +70,11 @@ func (s *StateDB) Keeper() Keeper {
 	return s.keeper
 }
 
+// TouchAccount marks the account as touched.
+func (s *StateDB) TouchAccount(addr common.Address) {
+	s.ephemeralStore.SetTouched(addr)
+}
+
 // AddLog adds a log, called by evm.
 func (s *StateDB) AddLog(log *ethtypes.Log) {
 	log.TxHash = s.txConfig.TxHash
@@ -255,6 +260,8 @@ func (s *StateDB) ForEachStorage(addr common.Address, cb func(key, value common.
 
 // AddBalance adds amount to the account associated with addr.
 func (s *StateDB) AddBalance(addr common.Address, amount *big.Int) {
+	s.TouchAccount(addr)
+
 	// Only allow positive amounts.
 	// TODO: Geth apparently allows negative amounts, but can cause negative
 	// balance which is not allowed in bank keeper
@@ -272,6 +279,8 @@ func (s *StateDB) AddBalance(addr common.Address, amount *big.Int) {
 
 // SubBalance subtracts amount from the account associated with addr.
 func (s *StateDB) SubBalance(addr common.Address, amount *big.Int) {
+	s.TouchAccount(addr)
+
 	if amount.Sign() == 0 {
 		return
 	}
@@ -286,6 +295,8 @@ func (s *StateDB) SubBalance(addr common.Address, amount *big.Int) {
 
 // SetNonce sets the nonce of account.
 func (s *StateDB) SetNonce(addr common.Address, nonce uint64) {
+	s.TouchAccount(addr)
+
 	account := s.getOrNewAccount(addr)
 
 	account.Nonce = nonce
@@ -309,6 +320,8 @@ func (s *StateDB) SetCode(addr common.Address, code []byte) {
 
 // SetState sets the contract state.
 func (s *StateDB) SetState(addr common.Address, key, value common.Hash) {
+	s.TouchAccount(addr)
+
 	// We cannot attempt to skip noop changes by just checking committed state
 	// Example:
 	// 1. With committed state to 0x0
@@ -444,6 +457,20 @@ func (s *StateDB) Commit() error {
 		}
 	}
 
+	// EIP-161: Clear touched empty accounts, after execution of suicide list
+	touchedAddrs := s.ephemeralStore.GetAllTouched()
+	for _, addr := range touchedAddrs {
+		account := s.keeper.GetAccount(s.ctx.CurrentCtx(), addr)
+		// Ignore non-existent or non-empty accounts
+		if account == nil || !account.IsEmpty() {
+			continue
+		}
+
+		if err := s.keeper.DeleteAccount(s.ctx.CurrentCtx(), addr); err != nil {
+			return fmt.Errorf("failed to delete empty account: %w", err)
+		}
+	}
+
 	// Commit after account deletions
 	s.ctx.Commit()
 

x/evm/statedb/store.go

@@ -7,11 +7,20 @@ import (
 	ethtypes "github.com/ethereum/go-ethereum/core/types"
 )
 
+// initialStoreRevertKey is the initial state of the store.
+var initialStoreRevertKey = StoreRevertKey{
+	RefundIndex:           0,
+	SuicidedAccountsIndex: 0,
+	LogsIndex:             0,
+	TouchedAccountsIndex:  0,
+}
+
 // StoreRevertKey defines the required information to revert to a previous state.
 type StoreRevertKey struct {
 	RefundIndex           int
 	SuicidedAccountsIndex int
 	LogsIndex             int
+	TouchedAccountsIndex  int
 }
 
 // EphemeralStore provides in-memory state of the refund and suicided accounts
@@ -20,6 +29,7 @@ type EphemeralStore struct {
 	RefundStates          []uint64
 	SuicidedAccountStates []common.Address
 	Logs                  []*ethtypes.Log
+	TouchedAccounts       []common.Address
 }
 
 // NewEphemeralStore creates a new EphemeralStore.
@@ -28,6 +38,7 @@ func NewEphemeralStore() *EphemeralStore {
 		RefundStates:          nil,
 		SuicidedAccountStates: nil,
 		Logs:                  nil,
+		TouchedAccounts:       nil,
 	}
 }
 
@@ -37,6 +48,7 @@ func (es *EphemeralStore) GetRevertKey() StoreRevertKey {
 		RefundIndex:           len(es.RefundStates),
 		SuicidedAccountsIndex: len(es.SuicidedAccountStates),
 		LogsIndex:             len(es.Logs),
+		TouchedAccountsIndex:  len(es.TouchedAccounts),
 	}
 }
 
@@ -49,28 +61,36 @@ func (es *EphemeralStore) Revert(key StoreRevertKey) {
 	es.RefundStates = es.RefundStates[:key.RefundIndex]
 	es.SuicidedAccountStates = es.SuicidedAccountStates[:key.SuicidedAccountsIndex]
 	es.Logs = es.Logs[:key.LogsIndex]
+	es.TouchedAccounts = es.TouchedAccounts[:key.TouchedAccountsIndex]
 }
 
-func (es *EphemeralStore) ValidateRevertKey(key StoreRevertKey) error {
-	if key.RefundIndex > len(es.RefundStates) {
+func validateIndex(idx int, targetLen int, name string) error {
+	if idx > targetLen {
 		return fmt.Errorf(
-			"invalid RefundIndex, %d is greater than the length of the refund states (%d)",
-			key.RefundIndex, len(es.RefundStates),
+			"invalid %s index, %d is greater than the length of the %s (%d)",
+			name, idx, name, targetLen,
 		)
 	}
 
-	if key.SuicidedAccountsIndex > len(es.SuicidedAccountStates) {
-		return fmt.Errorf(
-			"invalid SuicidedAccountsIndex, %d is greater than the length of the suicided accounts (%d)",
-			key.SuicidedAccountsIndex, len(es.SuicidedAccountStates),
-		)
+	return nil
+}
+
+func (es *EphemeralStore) ValidateRevertKey(key StoreRevertKey) error {
+	validations := []struct {
+		idx    int
+		length int
+		name   string
+	}{
+		{key.RefundIndex, len(es.RefundStates), "Refund"},
+		{key.SuicidedAccountsIndex, len(es.SuicidedAccountStates), "SuicidedAccounts"},
+		{key.LogsIndex, len(es.Logs), "Logs"},
+		{key.TouchedAccountsIndex, len(es.TouchedAccounts), "TouchedAccounts"},
 	}
 
-	if key.LogsIndex > len(es.Logs) {
-		return fmt.Errorf(
-			"invalid LogsIndex, %d is greater than the length of the logs (%d)",
-			key.LogsIndex, len(es.Logs),
-		)
+	for _, v := range validations {
+		if err := validateIndex(v.idx, v.length, v.name); err != nil {
+			return err
+		}
 	}
 
 	return nil
@@ -147,3 +167,32 @@ func (es *EphemeralStore) GetAccountSuicided(addr common.Address) bool {
 func (es *EphemeralStore) GetAllSuicided() []common.Address {
 	return es.SuicidedAccountStates
 }
+
+// -----------------------------------------------------------------------------
+// Touched accounts
+
+// SetTouched sets the given account as touched.
+func (es *EphemeralStore) SetTouched(addr common.Address) {
+	// If already in the list, ignore
+	if es.IsTouched(addr) {
+		return
+	}
+
+	es.TouchedAccounts = append(es.TouchedAccounts, addr)
+}
+
+// IsTouched returns true if the given account is touched.
+func (es *EphemeralStore) IsTouched(addr common.Address) bool {
+	for _, a := range es.TouchedAccounts {
+		if a == addr {
+			return true
+		}
+	}
+
+	return false
+}
+
+// GetAllTouched returns all touched accounts.
+func (es *EphemeralStore) GetAllTouched() []common.Address {
+	return es.TouchedAccounts
+}