diff --git a/core/vm/gas_table.go b/core/vm/gas_table.go index 9ef9d162c..7d8dd8b4b 100644 --- a/core/vm/gas_table.go +++ b/core/vm/gas_table.go @@ -555,26 +555,31 @@ func gasStaticCall(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memo } func gasSelfdestruct(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memorySize uint64) (*multigas.MultiGas, uint64, error) { - var gas uint64 + multiGas := multigas.ZeroGas() // EIP150 homestead gas reprice fork: if evm.chainRules.IsEIP150 { - gas = params.SelfdestructGasEIP150 + // Selfdestruct operation considered as storage access. + // See rationale in: https://github.com/OffchainLabs/nitro/blob/master/docs/decisions/0002-multi-dimensional-gas-metering.md + multiGas.SafeIncrement(multigas.ResourceKindStorageAccess, params.SelfdestructGasEIP150) var address = common.Address(stack.Back(0).Bytes20()) + // New account creation considered as storage growth. + // See rationale in: https://github.com/OffchainLabs/nitro/blob/master/docs/decisions/0002-multi-dimensional-gas-metering.md if evm.chainRules.IsEIP158 { // if empty and transfers value if evm.StateDB.Empty(address) && evm.StateDB.GetBalance(contract.Address()).Sign() != 0 { - gas += params.CreateBySelfdestructGas + multiGas.SafeIncrement(multigas.ResourceKindStorageGrowth, params.CreateBySelfdestructGas) } } else if !evm.StateDB.Exist(address) { - gas += params.CreateBySelfdestructGas + multiGas.SafeIncrement(multigas.ResourceKindStorageGrowth, params.CreateBySelfdestructGas) } } if !evm.StateDB.HasSelfDestructed(contract.Address()) { evm.StateDB.AddRefund(params.SelfdestructRefundGas) } - return multigas.ZeroGas(), gas, nil + singleGas, _ := multiGas.SingleGas() + return multiGas, singleGas, nil } func gasExtCall(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memorySize uint64) (*multigas.MultiGas, uint64, error) { diff --git a/core/vm/operations_acl.go b/core/vm/operations_acl.go index 461dd6730..c69ae04d7 100644 --- a/core/vm/operations_acl.go +++ b/core/vm/operations_acl.go @@ -251,22 +251,27 @@ var ( func makeSelfdestructGasFn(refundsEnabled bool) gasFunc { gasFunc := func(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memorySize uint64) (*multigas.MultiGas, uint64, error) { var ( - gas uint64 - address = common.Address(stack.peek().Bytes20()) + multiGas = multigas.ZeroGas() + address = common.Address(stack.peek().Bytes20()) ) if !evm.StateDB.AddressInAccessList(address) { // If the caller cannot afford the cost, this change will be rolled back evm.StateDB.AddAddressToAccessList(address) - gas = params.ColdAccountAccessCostEIP2929 + // Cold account access considered as storage access. + // See rationale in: https://github.com/OffchainLabs/nitro/blob/master/docs/decisions/0002-multi-dimensional-gas-metering.md + multiGas.SafeIncrement(multigas.ResourceKindStorageAccess, params.ColdAccountAccessCostEIP2929) } // if empty and transfers value if evm.StateDB.Empty(address) && evm.StateDB.GetBalance(contract.Address()).Sign() != 0 { - gas += params.CreateBySelfdestructGas + // New account creation considered as storage growth. + // See rationale in: https://github.com/OffchainLabs/nitro/blob/master/docs/decisions/0002-multi-dimensional-gas-metering.md + multiGas.SafeIncrement(multigas.ResourceKindStorageGrowth, params.CreateBySelfdestructGas) } if refundsEnabled && !evm.StateDB.HasSelfDestructed(contract.Address()) { evm.StateDB.AddRefund(params.SelfdestructRefundGas) } - return multigas.ZeroGas(), gas, nil + singleGas, _ := multiGas.SingleGas() + return multiGas, singleGas, nil } return gasFunc } diff --git a/core/vm/operations_gas_test.go b/core/vm/operations_gas_test.go index acf66ba9b..cf8754e87 100644 --- a/core/vm/operations_gas_test.go +++ b/core/vm/operations_gas_test.go @@ -1082,3 +1082,181 @@ func TestGasCreateEip3860(t *testing.T) { func TestGasCreate2Eip3860(t *testing.T) { testGasCreateFunc(t, gasCreate2Eip3860, true, true) } + +type GasSelfdestructFuncTestCase struct { + name string // descriptive name for the test case + isEIP150 bool // whether the test is for EIP-150 + isEIP158 bool // whether the test is for EIP-158 + beneficiaryExists bool // whether beneficiary account exists + slotInAccessList bool // whether the slot is in the access list + hasBeenDestructed bool // whether the contract has been destructed before + expectedMultiGas *multigas.MultiGas // expected multi gas after the operation + expectedRefund uint64 // expected refund after the operation, if any +} + +func testGasSelfdestructFuncWithCases(t *testing.T, config *params.ChainConfig, gasSelfdestructFunc gasFunc, testCases []GasSelfdestructFuncTestCase) { + t.Helper() + + contractGas := uint64(100000) + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + stateDb, _ := state.New(types.EmptyRootHash, state.NewDatabaseForTesting()) + blockCtx := BlockContext{ + BlockNumber: big.NewInt(0), // ensures block 0 is passed into Rules() + } + + evm := NewEVM(blockCtx, stateDb, config, Config{}) + + caller := common.Address{} + contractAddr := common.Address{1} + beneficiaryAddr := common.Address{2} + contract := NewContract(caller, contractAddr, new(uint256.Int), contractGas, nil) + + stack := newstack() + mem := NewMemory() + + // Set chain rules based on test case + evm.chainRules.IsEIP150 = tc.isEIP150 + evm.chainRules.IsEIP158 = tc.isEIP158 + + if tc.beneficiaryExists { + stateDb.CreateAccount(beneficiaryAddr) + } + + stateDb.CreateAccount(contractAddr) + if tc.hasBeenDestructed { + stateDb.SelfDestruct(contractAddr) + } else { + stateDb.SetBalance(contractAddr, uint256.NewInt(1), tracing.BalanceChangeUnspecified) + } + + stack.push(new(uint256.Int).SetBytes(beneficiaryAddr.Bytes())) + + multiGas, _, err := gasSelfdestructFunc(evm, contract, stack, mem, 0) + if err != nil { + t.Fatalf("Unexpected error for test case %s: %v", tc.name, err) + } + + if *multiGas != *tc.expectedMultiGas { + t.Errorf("Expected multi gas %d, got %d for test case: %s", + tc.expectedMultiGas, multiGas, tc.name) + } + + if stateDb.GetRefund() != tc.expectedRefund { + t.Errorf("Expected refund %d, got %d for test case: %s", + tc.expectedRefund, stateDb.GetRefund(), tc.name) + } + }) + } +} + +// Base SELFDESTRUCT gas function test +func TestGasSelfdestruct(t *testing.T) { + testCases := []GasSelfdestructFuncTestCase{ + { + name: "idle selfdestruct with refund", + expectedMultiGas: multigas.ZeroGas(), + expectedRefund: params.SelfdestructRefundGas, + }, + { + name: "idle selfdestruct without refund", + expectedMultiGas: multigas.ZeroGas(), + beneficiaryExists: true, + hasBeenDestructed: true, + }, + { + name: "selfdestruct exisit with EIP-150 with refund", + isEIP150: true, + expectedMultiGas: multigas.StorageAccessGas(params.SelfdestructGasEIP150), + beneficiaryExists: true, + expectedRefund: params.SelfdestructRefundGas, + }, + { + name: "selfdestruct not-exisit with EIP-150 and EIP-158 without refund", + isEIP150: true, + isEIP158: true, + expectedMultiGas: multigas.StorageAccessGas(params.SelfdestructGasEIP150), + hasBeenDestructed: true, + }, + { + name: "selfdestruct exisit with EIP-150 and EIP-158 with refund", + beneficiaryExists: false, + isEIP150: true, + isEIP158: true, + expectedMultiGas: multigas.StorageAccessGas(params.SelfdestructGasEIP150).Set(multigas.ResourceKindStorageGrowth, params.CreateBySelfdestructGas), + expectedRefund: params.SelfdestructRefundGas, + }, + } + + testGasSelfdestructFuncWithCases(t, params.TestChainConfig, gasSelfdestruct, testCases) +} + +// Modern (EIP-2929) SELFDESTRUCT gas function test with access list +func TestMakeSelfdestructGasFn(t *testing.T) { + testCases := []GasSelfdestructFuncTestCase{ + { + name: "selfdestruct - no access list - with refund", + expectedMultiGas: multigas.StorageAccessGas(params.ColdAccountAccessCostEIP2929).Set(multigas.ResourceKindStorageGrowth, params.CreateBySelfdestructGas), + expectedRefund: params.SelfdestructRefundGas, + }, + { + name: "has been destructed - no access list - no refund", + expectedMultiGas: multigas.StorageAccessGas(params.ColdAccountAccessCostEIP2929), + hasBeenDestructed: true, + }, + { + name: "selfdestruct - in access list - with refund", + expectedMultiGas: multigas.StorageAccessGas(params.ColdAccountAccessCostEIP2929).Set(multigas.ResourceKindStorageGrowth, params.CreateBySelfdestructGas), + expectedRefund: params.SelfdestructRefundGas, + slotInAccessList: true, + }, + { + name: "has been destructed - in access list - no refund", + expectedMultiGas: multigas.StorageAccessGas(params.ColdAccountAccessCostEIP2929), + hasBeenDestructed: true, + slotInAccessList: true, + }, + } + + testGasSelfdestructFuncWithCases(t, params.TestChainConfig, makeSelfdestructGasFn(true), testCases) +} + +// Statelessness mode (EIP-4762) SELFDESTRUCT gas function test +func TestGasSelfdestructEIP4762(t *testing.T) { + stateDb, _ := state.New(types.EmptyRootHash, state.NewDatabaseForTesting()) + evm := NewEVM(BlockContext{}, stateDb, params.TestChainConfig, Config{}) + + caller := common.Address{} + contractAddr := common.Address{1} + beneficiaryAddr := common.Address{2} + contractGas := uint64(100000) + contract := NewContract(caller, contractAddr, new(uint256.Int), contractGas, nil) + + stack := newstack() + mem := NewMemory() + + // Setup access list + accessList := state.NewAccessEvents(evm.StateDB.PointCache()) + accessListForExpected := state.NewAccessEvents(evm.StateDB.PointCache()) + evm.AccessEvents = accessList + + stateDb.CreateAccount(beneficiaryAddr) + + stateDb.CreateAccount(contractAddr) + stateDb.SetBalance(contractAddr, uint256.NewInt(0), tracing.BalanceChangeUnspecified) + + stack.push(new(uint256.Int).SetBytes(beneficiaryAddr.Bytes())) + + expectedStorageAccessGas := accessListForExpected.BasicDataGas(contractAddr, false) + accessListForExpected.BasicDataGas(beneficiaryAddr, false) + expectedMultiGas := multigas.StorageAccessGas(expectedStorageAccessGas) + + multiGas, _, err := gasSelfdestructEIP4762(evm, contract, stack, mem, 0) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + if *multiGas != *expectedMultiGas { + t.Errorf("Expected multi gas %d, got %d", expectedMultiGas, multiGas) + } +} diff --git a/core/vm/operations_verkle.go b/core/vm/operations_verkle.go index 302e04814..9ae6bbc42 100644 --- a/core/vm/operations_verkle.go +++ b/core/vm/operations_verkle.go @@ -137,7 +137,9 @@ func gasSelfdestructEIP4762(evm *EVM, contract *Contract, stack *Stack, mem *Mem statelessGas += evm.AccessEvents.BasicDataGas(beneficiaryAddr, true) } } - return multigas.ZeroGas(), statelessGas, nil + multiGas := multigas.StorageAccessGas(statelessGas) + singleGas, _ := multiGas.SingleGas() + return multiGas, singleGas, nil } func gasCodeCopyEip4762(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memorySize uint64) (*multigas.MultiGas, uint64, error) {