Forge resets storage var to 0 after self-destruct (before tx ends)

[0xA5DF]

Component

Forge

Have you ensured that all of these are up to date?

• [X] Foundry
• [X] Foundryup

What version of Foundry are you on?

forge 0.2.0 (92f8951 2022-08-08T00:03:54.483346829Z)

What command(s) is the bug in?

forge test

Operating System

Linux

Describe the bug

The selfdestruct opcode shouldn't destroy the contract till the tx ends, that means that if in the same tx we call the contract again it should function with no difference.

When running forge test on a contract that self-destroys (via delegate call) and then reads storage var x, the value of that var is zero instead of 5 (it shows both in the event emitted and in the return value).

When running the same code on Rinkeby testnet, it indeed emits the value 5 (here's the tx ).

Contract code (contracts/Destruct.sol):

//SPDX-License-Identifier: Unlicense
pragma solidity ^0.8.0;

contract Destruct{
    function destroyMe(address payable add) public{
        selfdestruct(add);
    }
}

contract Proxy{
    uint x = 5;
    Destruct d = new Destruct();
    event Log(uint x);

    function runDestoryMe(address payable add) public returns(uint){
        bytes memory data = abi.encodeWithSelector(Destruct.destroyMe.selector, add);
        address(d).delegatecall(data);
        emit Log(x);
        return x;
    }
}

Test code:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";

import "../contracts/Destruct.sol";

contract ContractTest is Test {
    function setUp() public {}

    function testExample() public {
        Proxy p = new Proxy();
        uint x = p.runDestoryMe(payable(address(this)));
        assertEq(x, 5);
    }
}

Command output

$ forge test -vvv                            
warning: Unknown section [default] found in foundry.toml. This notation for profiles has been deprecated and may result in the profile not being registered in future versions. Please use [profile.default] instead or run `forge config --fix`.
[⠰] Compiling...
No files changed, compilation skipped

Running 1 test for test/Contract.t.sol:ContractTest
[FAIL. Reason: Undefined.] testExample() (gas: 262851)
Logs:
  Error: a == b not satisfied [uint]
    Expected: 5
      Actual: 0

Traces:
  [262851] ContractTest::testExample() 
    ├─ [205839] → new Proxy@"0xce71…c246"
    │   ├─ [34887] → new Destruct@"0x037f…dd8f"
    │   │   └─ ← 174 bytes of code
    │   └─ ← 472 bytes of code
    ├─ [9582] Proxy::runDestoryMe(ContractTest: [0xb4c79dab8f259c7aee6e5b2aa729821864227e84]) 
    │   ├─ [5257] Destruct::destroyMe(ContractTest: [0xb4c79dab8f259c7aee6e5b2aa729821864227e84]) [delegatecall]
    │   │   └─ ← ()
    │   ├─ emit Log(x: 0)
    │   └─ ← 0
    ├─ emit log(: "Error: a == b not satisfied [uint]")
    ├─ emit log_named_uint(key: "  Expected", val: 5)
    ├─ emit log_named_uint(key: "    Actual", val: 0)
    ├─ [0] VM::store(VM: [0x7109709ecfa91a80626ff3989d68f67f5b1dd12d], 0x6661696c65640000000000000000000000000000000000000000000000000000, 0x0000000000000000000000000000000000000000000000000000000000000001) 
    │   └─ ← ()
    └─ ← ()

Test result: FAILED. 0 passed; 1 failed; finished in 378.13µs

Failing tests:
Encountered 1 failing test in test/Contract.t.sol:ContractTest
[FAIL. Reason: Undefined.] testExample() (gas: 262851)

Encountered a total of 1 failing tests, 0 tests succeeded

[onbjerg]

This directly contradicts what has been observed in https://github.com/foundry-rs/foundry/issues/1543 so I'm not entirely sure what the behavior is

[0xA5DF]

I tested the other bug, and they both exists on my machine. It seems like forge is resetting the storage, but not deleting the code of the contract.

Here's the modified test code (also made x public in the contract code):

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";
import "forge-std/console.sol";

import "../contracts/Destruct.sol";

contract ContractTest is Test {
    function setUp() public {}

    function testExample() public {
        Proxy p = new Proxy();
        uint x = p.runDestoryMe(payable(address(this)));
        uint32 pSize = getSize(address(p));
        console.log("Proxy size is: ", pSize);
        x = p.x();
        console.log("p.x is: ",x);
        assertEq(x, 5);
    }

    function getSize(address c) public view returns (uint32) {
        uint32 size;
        assembly {
            size := extcodesize(c)
        }
        return size;
    }
}

And here's the output:

$ forge test -vv
warning: Unknown section [default] found in foundry.toml. This notation for profiles has been deprecated and may result in the profile not being registered in future versions. Please use [profile.default] instead or run `forge config --fix`.
[⠰] Compiling...
No files changed, compilation skipped

Running 1 test for test/Contract.t.sol:ContractTest
[FAIL. Reason: Undefined.] testExample() (gas: 270484)
Logs:
  Proxy size is:  487
  p.x is:  0
  Error: a == b not satisfied [uint]
    Expected: 5
      Actual: 0

Test result: FAILED. 0 passed; 1 failed; finished in 671.49µs

Failing tests:
Encountered 1 failing test in test/Contract.t.sol:ContractTest
[FAIL. Reason: Undefined.] testExample() (gas: 270484)

Encountered a total of 1 failing tests, 0 tests succeeded

[0xA5DF]

P.S. The code also works fine, I've added the following function to the contract and it just returns the same value

    function add(uint y) public returns(uint) {
        return x+y;
    }

This test will output added is: 30:

        Proxy p = new Proxy();
        uint x = p.runDestoryMe(payable(address(this)));
        uint32 pSize = getSize(address(p));
        console.log("Proxy size is: ", pSize);
        uint added = p.add(30);
        console.log("added is: ",added);
        x = p.x();
        console.log("p.x is: ",x);

[onbjerg]

@rakita Storage seems to be reset before the transaction ends, but not the account code. Is this the intended behavior of selfdestruct according to spec?

[rakita]

Account code needs to be reset with storage.

[0xA5DF]

They both should reset, but only once tx completes, beforehand it should still operate and have no change to storage.

Ethereum yellow paper definition for selfdestruct:

Halt execution and register account for later deletion

And on the 'Transaction Execution' section:

The final state, σ', is reached after deleting all accounts that either appear in the self-destruct set or are touched and empty.

Also, as said above, I've tested this on the Rinkeby test chain, and it shows that the storage hasn't changed between the selfdestruct to the end of the tx.

[rakita]

(before tx ends) i missed this. I am not 100% sure as selfdestruct logic is not that great, but I think you are right. I have one failing eth/tests (recently added) with selfdestruct that is probably related: https://github.com/bluealloy/revm/blob/0ae75553776071288d56852372c09bff7a6d268d/bins/revme/src/statetest/runner.rs#L96

[rakita]

@0xA5DF thank you for pointing this out!

I rewrote that part of revm to simplify it and correct the wrong behaviour, in the first implementation I overthought things. It should be correct now. https://github.com/bluealloy/revm/commit/6d04a7f9856e0c05c3787ce57944380bfe1dc705

@onbjerg integration should only be in the renaming of subroutine to journaled_state and you probably will need to integrate this return of ExecutionResult: https://github.com/bluealloy/revm/commit/0ae75553776071288d56852372c09bff7a6d268d I will not push a new release as I want to add some additional changes as in db returning Result<Option<.. that we talked about.

[mattsse]

closed via #2877