LFVM: SelfDestruct gas and refund calculation (#733)

* add gas selfdestruct test

* remove selfdestruct gas functions. integrate them into operation func

* improve constants comments and remove unused gas constants

* make functions for self destruction costs and tests them

* add test for all selfdestruct dynamic costs

* changes requested in review. add and use getAccessCost from #730

* fix typo and rephrase revision check

* fix comment phrasing

go/interpreter/lfvm/gas.go

@@ -23,25 +23,10 @@ const (
 	ColdSloadCostEIP2929         tosca.Gas = 2100 // Cost of cold SLOAD after EIP 2929
 	ColdAccountAccessCostEIP2929 tosca.Gas = 2600 // Cost of cold account access after EIP 2929
 
-	// CreateBySelfdestructGas is used when the refunded account is one that does
-	// not exist. This logic is similar to call.
-	// Introduced in Tangerine Whistle (Eip 150)
-	CreateBySelfdestructGas tosca.Gas = 25000
-
-	SelfdestructGasEIP150             tosca.Gas = 5000  // Gas cost of SELFDESTRUCT post EIP-150
-	SelfdestructRefundGas             tosca.Gas = 24000 // Refunded following a selfdestruct operation.
 	SloadGasEIP2200                   tosca.Gas = 800   // Cost of SLOAD after EIP 2200 (part of Istanbul)
 	SstoreClearsScheduleRefundEIP2200 tosca.Gas = 15000 // Once per SSTORE operation for clearing an originally existing storage slot
 
-	// SstoreClearsScheduleRefundEIP3529 is the refund for clearing a storage slot after EIP-3529.
-	// In EIP-2200: SstoreResetGas was 5000.
-	// In EIP-2929: SstoreResetGas was changed to '5000 - COLD_SLOAD_COST'.
-	// In EIP-3529: SSTORE_CLEARS_SCHEDULE is defined as SSTORE_RESET_GAS + ACCESS_LIST_STORAGE_KEY_COST
-	// Which becomes: 5000 - 2100 + 1900 = 4800
-	SstoreClearsScheduleRefundEIP3529 tosca.Gas = 4800
-
 	SstoreResetGasEIP2200      tosca.Gas = 5000  // Once per SSTORE operation from clean non-zero to something else
-	SstoreSentryGasEIP2200     tosca.Gas = 2300  // Minimum gas required to be present for an SSTORE call, not consumed
 	SstoreSetGasEIP2200        tosca.Gas = 20000 // Once per SSTORE operation from clean zero to non-zero
 	WarmStorageReadCostEIP2929 tosca.Gas = 100   // Cost of reading warm storage after EIP 2929
 
@@ -74,8 +59,6 @@ func getBerlinGasPriceInternal(op OpCode) tosca.Gas {
 		gp = 0
 	case DELEGATECALL:
 		gp = 0
-	case SELFDESTRUCT:
-		gp = 5000
 	}
 	return gp
 }
@@ -239,7 +222,7 @@ func getStaticGasPriceInternal(op OpCode) tosca.Gas {
 	case DELEGATECALL:
 		return 700
 	case SELFDESTRUCT:
-		return 0 // should be 5000 according to evm.code
+		return 5000
 	}
 
 	if op.isSuperInstruction() {
@@ -342,43 +325,3 @@ func gasEip2929AccountCheck(c *context, address tosca.Address) error {
 	}
 	return nil
 }
-
-func gasSelfdestruct(c *context) tosca.Gas {
-	gas := SelfdestructGasEIP150
-	var address = tosca.Address(c.stack.peekN(0).Bytes20())
-
-	// if beneficiary needs to be created
-	if !c.context.AccountExists(address) && c.context.GetBalance(c.params.Recipient) != (tosca.Value{}) {
-		gas += CreateBySelfdestructGas
-	}
-	//lint:ignore SA1019 deprecated functions to be migrated in #616
-	if !c.context.HasSelfDestructed(c.params.Recipient) {
-		c.refund += SelfdestructRefundGas
-	}
-	return gas
-}
-
-func gasSelfdestructEIP2929(c *context) tosca.Gas {
-	var (
-		gas     tosca.Gas
-		address = tosca.Address(c.stack.peekN(0).Bytes20())
-	)
-	//lint:ignore SA1019 deprecated functions to be migrated in #616
-	if !c.context.IsAddressInAccessList(address) {
-		// If the caller cannot afford the cost, this change will be rolled back
-		c.context.AccessAccount(address)
-		gas = ColdAccountAccessCostEIP2929
-	}
-	// if empty and transfers value
-	if !c.context.AccountExists(address) && c.context.GetBalance(c.params.Recipient) != (tosca.Value{}) {
-		gas += CreateBySelfdestructGas
-	}
-	// do this only for Berlin and not after London fork
-	if c.isAtLeast(tosca.R09_Berlin) && !c.isAtLeast(tosca.R10_London) {
-		//lint:ignore SA1019 deprecated functions to be migrated in #616
-		if !c.context.HasSelfDestructed(c.params.Recipient) {
-			c.refund += SelfdestructRefundGas
-		}
-	}
-	return gas
-}

go/interpreter/lfvm/instructions.go

@@ -723,19 +723,46 @@ func opSelfdestruct(c *context) {
 		return
 	}
 
-	gasfunc := gasSelfdestruct
+	beneficiary := tosca.Address(c.stack.pop().Bytes20())
+	// Selfdestruct gas cost defined in EIP-105 (see https://eips.ethereum.org/EIPS/eip-150)
+	cost := tosca.Gas(0)
 	if c.isAtLeast(tosca.R09_Berlin) {
-		gasfunc = gasSelfdestructEIP2929
+		// as https://eips.ethereum.org/EIPS/eip-2929#selfdestruct-changes says,
+		// selfdestruct does not charge for warm access
+		if accessStatus := c.context.AccessAccount(beneficiary); accessStatus != tosca.WarmAccess {
+			cost += getAccessCost(accessStatus)
+		}
 	}
+	cost += selfDestructNewAccountCost(c.context.AccountExists(beneficiary),
+		c.context.GetBalance(c.params.Recipient))
 	// even death is not for free
-	if !c.useGas(gasfunc(c)) {
+	if !c.useGas(cost) {
 		return
 	}
-	beneficiary := tosca.Address(c.stack.pop().Bytes20())
-	c.context.SelfDestruct(c.params.Recipient, beneficiary)
+
+	destructed := c.context.SelfDestruct(c.params.Recipient, beneficiary)
+	c.refund += selfDestructRefund(destructed, c.params.Revision)
 	c.status = statusSelfDestructed
 }
 
+func selfDestructNewAccountCost(accountExists bool, balance tosca.Value) tosca.Gas {
+	if !accountExists && balance != (tosca.Value{}) {
+		// cost of creating an account defined in eip-150 (see https://eips.ethereum.org/EIPS/eip-150)
+		// CreateBySelfdestructGas is used when the refunded account is one that does
+		// not exist. This logic is similar to call.
+		return 25_000
+	}
+	return 0
+}
+
+func selfDestructRefund(destructed bool, revision tosca.Revision) tosca.Gas {
+	// Since London and after there is no more refund (see https://eips.ethereum.org/EIPS/eip-3529)
+	if destructed && revision < tosca.R10_London {
+		return 24_000
+	}
+	return 0
+}
+
 func opChainId(c *context) {
 	id := c.params.ChainID
 	c.stack.pushUndefined().SetBytes32(id[:])

go/interpreter/lfvm/instructions_test.go

@@ -958,3 +958,123 @@ func TestCall_ChargesForAccessAfterBerlin(t *testing.T) {
 		}
 	}
 }
+func TestSelfDestruct_StaticModeReportsError(t *testing.T) {
+	ctxt := context{
+		params: tosca.Parameters{
+			Static: true,
+		},
+	}
+	opSelfdestruct(&ctxt)
+	if ctxt.status != statusError {
+		t.Errorf("unexpected status after call, wanted ERROR, got %v", ctxt.status)
+	}
+}
+
+func TestSelfDestruct_Refund(t *testing.T) {
+	tests := map[string]struct {
+		destructed bool
+		revision   tosca.Revision
+		refund     tosca.Gas
+	}{
+		"istanbul": {
+			revision: tosca.R07_Istanbul,
+		},
+		"berlin-first-destructed": {
+			destructed: true,
+			revision:   tosca.R09_Berlin,
+			refund:     24_000,
+		},
+		"berlin-not-first-destructed": {
+			revision: tosca.R09_Berlin,
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			refund := selfDestructRefund(test.destructed, test.revision)
+			if refund != test.refund {
+				t.Errorf("unexpected refund, wanted %d, got %d", test.refund, refund)
+			}
+		})
+	}
+}
+
+func TestSelfDestruct_NewAccountCost(t *testing.T) {
+
+	tests := map[string]struct {
+		beneficiaryExists bool
+		balance           tosca.Value
+		cost              tosca.Gas
+	}{
+		"account exists no balance": {
+			beneficiaryExists: true,
+			balance:           tosca.Value{},
+			cost:              0,
+		},
+		"account exists with balance": {
+			beneficiaryExists: true,
+			balance:           tosca.Value{1},
+			cost:              0,
+		},
+		"new account without balance": {
+			beneficiaryExists: false,
+			balance:           tosca.Value{},
+			cost:              0,
+		},
+		"new account with balance": {
+			beneficiaryExists: false,
+			balance:           tosca.Value{1},
+			cost:              25_000,
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			cost := selfDestructNewAccountCost(test.beneficiaryExists, test.balance)
+			if cost != test.cost {
+				t.Errorf("unexpected gas, wanted %d, got %d", test.cost, cost)
+			}
+		})
+	}
+}
+
+func TestSelfDestruct_ExistingAccountToNewBeneficiary(t *testing.T) {
+	// This tests produces the combination of context calls/results for the maximum dynamic gas cost possible.
+
+	beneficiaryAddress := tosca.Address{1}
+	selfAddress := tosca.Address{2}
+	// added to gas to ensure operation is not simply setting gas to zero.
+	gasDelta := tosca.Gas(1)
+
+	ctrl := gomock.NewController(t)
+	runContext := tosca.NewMockRunContext(ctrl)
+	runContext.EXPECT().AccessAccount(beneficiaryAddress).Return(tosca.ColdAccess)
+	runContext.EXPECT().AccountExists(beneficiaryAddress).Return(false)
+	runContext.EXPECT().GetBalance(selfAddress).Return(tosca.Value{1})
+	runContext.EXPECT().SelfDestruct(selfAddress, beneficiaryAddress).Return(true)
+
+	ctxt := context{
+		params: tosca.Parameters{
+			BlockParameters: tosca.BlockParameters{
+				Revision: tosca.R13_Cancun,
+			},
+			Recipient: selfAddress,
+		},
+		status:  statusRunning,
+		stack:   NewStack(),
+		memory:  NewMemory(),
+		context: runContext,
+		// 25_000 for new account, 2_600 for beneficiary access
+		gas: 27_600 + gasDelta,
+	}
+	ctxt.stack.push(new(uint256.Int).SetBytes(beneficiaryAddress[:]))
+
+	opSelfdestruct(&ctxt)
+
+	if ctxt.status != statusSelfDestructed {
+		t.Errorf("unexpected status, wanted %v, got %v", statusRunning, ctxt.status)
+	}
+	if ctxt.gas != gasDelta {
+		t.Errorf("unexpected remaining gas, wanted %v, got %d", gasDelta, ctxt.gas)
+	}
+}

go/tosca/world_state.go

@@ -34,6 +34,8 @@ type WorldState interface {
 	GetStorage(Address, Key) Word
 	SetStorage(Address, Key, Word) StorageStatus
 
+	// SelfDestruct destroys addr and transfers its balance to beneficiary.
+	// Returns true if the given account is destructed for the first time in the ongoing transaction, false otherwise.
 	SelfDestruct(addr Address, beneficiary Address) bool
 }