Merge pull request #6290 from onflow/ramtin/evm-patch-self-destruct

fvm/evm/emulator/state/base.go

@@ -300,8 +300,8 @@ func (v *BaseView) DeleteAccount(addr gethCommon.Address) error {
 	if err != nil {
 		return err
 	}
-	if acc == nil {
-		return fmt.Errorf("account doesn't exist to be deleted")
+	if acc == nil { // if account doesn't exist return
+		return nil
 	}
 
 	// 2. remove the code

fvm/evm/emulator/state/delta.go

@@ -109,8 +109,8 @@ func (d *DeltaView) Exist(addr gethCommon.Address) (bool, error) {
 
 // CreateAccount creates a new account for the given address
 //
-// if address already extists (even if destructed), carry over the balance
-// and reset the data from the orginal account.
+// if address already exists (even if destructed), carry over the balance
+// and reset the data from the original account.
 func (d *DeltaView) CreateAccount(addr gethCommon.Address) error {
 	// if is already created return
 	if d.IsCreated(addr) {
@@ -137,7 +137,7 @@ func (d *DeltaView) CreateAccount(addr gethCommon.Address) error {
 		d.nonces[addr] = 0
 		d.codes[addr] = nil
 		d.codeHashes[addr] = gethTypes.EmptyCodeHash
-		// carrying over the balance. (legacy behaviour of the Geth stateDB)
+		// carrying over the balance. (legacy behavior of the Geth stateDB)
 		d.balances[addr] = balance
 
 		// flag addr as recreated, this flag helps with postponing deletion of slabs
@@ -199,12 +199,7 @@ func (d *DeltaView) HasSelfDestructed(addr gethCommon.Address) (bool, *uint256.I
 //
 // if an account has been created in this transaction, it would return an error
 func (d *DeltaView) SelfDestruct(addr gethCommon.Address) error {
-	// if it has been recently created, calling self destruct is not a valid operation
-	if d.IsCreated(addr) {
-		return fmt.Errorf("invalid operation, can't selfdestruct an account that is just created")
-	}
-
-	// if it doesn't exist, return false
+	// if it doesn't exist, return
 	exists, err := d.Exist(addr)
 	if err != nil {
 		return err
@@ -213,6 +208,12 @@ func (d *DeltaView) SelfDestruct(addr gethCommon.Address) error {
 		return nil
 	}
 
+	// if already set to be self destructed, return
+	_, found := d.toBeDestructed[addr]
+	if found {
+		return nil
+	}
+
 	// flag the account for destruction and capture the balance
 	// before destruction
 	d.toBeDestructed[addr], err = d.GetBalance(addr)

fvm/evm/emulator/state/stateDB.go

@@ -107,15 +107,15 @@ func (db *StateDB) IsNewContract(addr gethCommon.Address) bool {
 // while this address exists for the rest of transaction,
 // the balance of this account is return zero after the SelfDestruct call.
 func (db *StateDB) SelfDestruct(addr gethCommon.Address) {
-	err := db.lastestView().SelfDestruct(addr)
-	db.handleError(err)
+	db.handleError(fmt.Errorf("legacy self destruct is not supported"))
 }
 
 // Selfdestruct6780 would only follow the self destruct steps if account is a new contract
 // either just created, or address had balance before but got a contract deployed to it (in this tx).
 func (db *StateDB) Selfdestruct6780(addr gethCommon.Address) {
 	if db.IsNewContract(addr) {
-		db.SelfDestruct(addr)
+		err := db.lastestView().SelfDestruct(addr)
+		db.handleError(err)
 	}
 }
 
@@ -390,8 +390,10 @@ func (db *StateDB) Commit(finalize bool) error {
 			}
 			deleted = true
 		}
-		// then create new ones
-		// an account might be in a single transaction be deleted and recreated
+		if deleted {
+			continue
+		}
+		// create new accounts
 		if db.IsCreated(addr) {
 			err = db.baseView.CreateAccount(
 				addr,
@@ -405,9 +407,6 @@ func (db *StateDB) Commit(finalize bool) error {
 			}
 			continue
 		}
-		if deleted {
-			continue
-		}
 		err = db.baseView.UpdateAccount(
 			addr,
 			db.GetBalance(addr),
@@ -435,6 +434,10 @@ func (db *StateDB) Commit(finalize bool) error {
 
 	// update slots
 	for _, sk := range sortedSlots {
+		// don't update slots if self destructed
+		if db.HasSelfDestructed(sk.Address) {
+			continue
+		}
 		err = db.baseView.UpdateSlot(
 			sk,
 			db.GetState(sk.Address, sk.Key),
@@ -458,9 +461,9 @@ func (db *StateDB) Finalize() error {
 	return wrapError(err)
 }
 
-// Prepare is a highlevel logic that sadly is considered to be part of the
+// Prepare is a high level logic that sadly is considered to be part of the
 // stateDB interface and not on the layers above.
-// based on parameters that are passed it updates accesslists
+// based on parameters that are passed it updates access-lists
 func (db *StateDB) Prepare(rules gethParams.Rules, sender, coinbase gethCommon.Address, dest *gethCommon.Address, precompiles []gethCommon.Address, txAccesses gethTypes.AccessList) {
 	if rules.IsBerlin {
 		db.AddAddressToAccessList(sender)
@@ -485,7 +488,7 @@ func (db *StateDB) Prepare(rules gethParams.Rules, sender, coinbase gethCommon.A
 }
 
 // Reset resets uncommitted changes and transient artifacts such as error, logs,
-// preimages, access lists, ...
+// pre-images, access lists, ...
 // The method is often called between execution of different transactions
 func (db *StateDB) Reset() {
 	db.views = []*DeltaView{NewDeltaView(db.baseView)}

fvm/evm/emulator/state/stateDB_test.go

@@ -343,4 +343,102 @@ func TestStateDB(t *testing.T) {
 		require.NotEqual(t, gethCommon.Hash{}, root)
 		require.NotEqual(t, gethTypes.EmptyRootHash, root)
 	})
+
+	t.Run("test Selfdestruct6780 functionality", func(t *testing.T) {
+		ledger := testutils.GetSimpleValueStore()
+		db, err := state.NewStateDB(ledger, rootAddr)
+		require.NoError(t, err)
+
+		// test 1 - an already existing contract
+		// fail for selfdestruction
+		addr1 := testutils.RandomCommonAddress(t)
+		balance1 := uint256.NewInt(100)
+		code1 := []byte("some code")
+		db.CreateAccount(addr1)
+		db.SetCode(addr1, code1)
+		db.AddBalance(addr1, balance1, tracing.BalanceChangeTransfer)
+		require.NoError(t, db.Error())
+		err = db.Commit(true)
+		require.NoError(t, err)
+		// renew db
+		db, err = state.NewStateDB(ledger, rootAddr)
+		require.NoError(t, err)
+		// call self destruct
+		db.Selfdestruct6780(addr1)
+		require.NoError(t, db.Error())
+		// noop is expected
+		require.Equal(t, balance1, db.GetBalance(addr1))
+		require.Equal(t, code1, db.GetCode(addr1))
+		require.NoError(t, db.Error())
+
+		// test 2 - account exist before with some balance
+		// but not a contract - selfdestruct should work
+		addr2 := testutils.RandomCommonAddress(t)
+		balance2 := uint256.NewInt(200)
+		db.CreateAccount(addr2)
+		db.AddBalance(addr2, balance2, tracing.BalanceChangeTransfer)
+		require.NoError(t, db.Error())
+		// commit and renew db
+		err = db.Commit(true)
+		require.NoError(t, err)
+		db, err = state.NewStateDB(ledger, rootAddr)
+		require.NoError(t, err)
+		// call self destruct should not work
+		db.Selfdestruct6780(addr2)
+		require.NoError(t, db.Error())
+		// still no impact
+		require.Equal(t, balance2, db.GetBalance(addr2))
+		require.Empty(t, db.GetCode(addr2))
+		require.NoError(t, db.Error())
+		// commit and renew db
+		err = db.Commit(true)
+		require.NoError(t, err)
+		db, err = state.NewStateDB(ledger, rootAddr)
+		require.NoError(t, err)
+		// set code and call contract creation
+		db.SetCode(addr2, code1)
+		db.CreateContract(addr2)
+		require.Equal(t, code1, db.GetCode(addr2))
+		// now calling selfdestruct should do the job
+		db.Selfdestruct6780(addr2)
+		require.NoError(t, db.Error())
+		err = db.Commit(true)
+		require.NoError(t, err)
+		db, err = state.NewStateDB(ledger, rootAddr)
+		require.NoError(t, err)
+		// now query
+		require.Equal(t, uint256.NewInt(0), db.GetBalance(addr2))
+		require.Empty(t, db.GetCode(addr2))
+		require.NoError(t, db.Error())
+
+		// test 3 - create account and call self destruct in a single operation
+		// selfdestruct should work
+		db, err = state.NewStateDB(ledger, rootAddr)
+		require.NoError(t, err)
+		addr3 := testutils.RandomCommonAddress(t)
+		balance3 := uint256.NewInt(300)
+		key := testutils.RandomCommonHash(t)
+		value := testutils.RandomCommonHash(t)
+		db.CreateAccount(addr3)
+		db.CreateContract(addr3)
+		db.SetCode(addr3, code1)
+		db.SetState(addr3, key, value)
+		db.AddBalance(addr3, balance3, tracing.BalanceChangeTransfer)
+		require.NoError(t, db.Error())
+		// call self destruct
+		db.Selfdestruct6780(addr3)
+		require.NoError(t, db.Error())
+		// commit changes
+		err = db.Commit(true)
+		require.NoError(t, err)
+		// renew db
+		db, err = state.NewStateDB(ledger, rootAddr)
+		require.NoError(t, err)
+		// account should not exist
+		require.False(t, db.Exist(addr3))
+		require.Equal(t, uint256.NewInt(0), db.GetBalance(addr3))
+		require.Empty(t, db.GetCode(addr3))
+		require.Equal(t, gethCommon.Hash{}, db.GetState(addr3, key))
+		require.NoError(t, db.Error())
+	})
 }