bug: selfdestruct has no effect in test whereas a user expects it would

[sjkelleyjr]

Component

Forge

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

• Foundry
• Foundryup

What version of Foundry are you on?

forge 0.2.0 (bab38d6 2022-05-06T00:04:42.708656+00:00)

What command(s) is the bug in?

forge test

Operating System

macOS (M1)

Describe the bug

I'm not sure if this is actually a bug, or I just can't figure out how to achieve what I want.

I'm trying to test a very basic selfdestruct example, and it doesn't seem to be behaving as I expect:

SelfDestructor.sol:

pragma solidity 0.8.10;

contract SelfDestructor {
  function kill() external {
      selfdestruct(payable(msg.sender)); // send the funds to msg.sender just to test
  }
}

SelfDestructor.t.sol:

pragma solidity 0.8.10;

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

import "../src/SelfDestructor.sol";

contract SelfDestructorTest is Test {
    
    SelfDestructor killer;

    function setUp() public {
        killer = new SelfDestructor();
    }

    function testKill() public {
        killer.kill();

        vm.roll(block.number + 1);

        uint32 killerSize = getSize(address(killer));
        console.log(killerSize);

        killer.kill();

        vm.roll(block.number + 1);

        killerSize = getSize(address(killer));
        console.log(killerSize);
    }

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

Two things are confusing. Firstly, the size isn't changing between kill() invocations, and second, I can call the contract's kill() twice in the same test, both indicating that the selfdestruct isn't doing anything when it's called.

I've tried both with and without the vm.roll calls. What am I missing?

[onbjerg]

selfdestruct doesn't take effect until the call is over, which it isn't until the test is over.

[sjkelleyjr]

Good to know! Is there any way to get around this limitation? Can you make a test depend on the state of a previous test? Also, is there a list of things that behave this way, so I know to look out for them in the future?

(for anyone else who stumbles on this, I managed to learn this myself before @onbjerg's response, by putting the self destruct in the setUp(), so that's one way of getting around the limitation)

[onbjerg]

Good to know! Is there any way to get around this limitation?

Not currently... Is there any specific reason you need to test that the contract was self-destructed by checking the code size? I realize this may be an odd question.

I think there was another issue on here where OP asked for a cheatcode to perform the state transitions mid-test, but I think there would be too many side effects, and the technical difficulty of implementing it is pretty high for a small use case.

selfdestruct is (eventually, maybe, hopefully) going to be disabled at some point, so I wouldn't rely on it too much.

Can you make a test depend on the state of a previous test?

No, you can only use setUp

Also, is there a list of things that behave this way, so I know to look out for them in the future?

There isn't, I'm not sure yet whether or not there should be - some of these oddities are mostly "odd" because most people really haven't had the need to know. This selfdestruct behavior comes to mind, as well as a Solidity optimization regarding calls that might also be weird the first time you encounter it (but isn't necessarily true for all Solidity versions, so hard to document)

What do you think, does your use case warrant a cheatcode, or is there some other feature/adjustment you feel would accomodate your use case?

[sjkelleyjr]

Not currently... Is there any specific reason you need to test that the contract was self-destructed by checking the code size? I realize this may be an odd question.

It's not as odd a question as my use case 😅. I'm an auditor, and I like to test my exploits out with unit tests when possible. So in this case, I'm exploiting a contract and need to be able to selfdestruct it and assert things about the contracts that depend on the selfdestructed contract.

I think there was another issue on here where OP asked for a cheatcode to perform the state transitions mid-test, but I think there would be too many side effects, and the technical difficulty of implementing it is pretty high for a small use case.

I was looking for this cheatcode in the docs and didn't find one, so that's why I created the issue in case I missed it. I understand this is a niche use case though. This is why I was asking about the list of things that behave this way, so I could gauge whether I should be asking for a cheatcode or if this is unique to selfdestruct (also so I know what to look out for in the future, as you could imagine, this was a head scratcher).

What do you think, does your use case warrant a cheatcode, or is there some other feature/adjustment you feel would accomodate your use case?

I don't know enough about the roadmap or how others are using foundry to say. I can give you my use case and I have faith that the foundry team will make the right decision for the long term success of the project 🙂.

[brockelmore]

@onbjerg what if we add an inspector (or use existing inspector) and cheatcode and forge-std that:

1. records selfdestructed addresses (in the selfdestruct callback)
2. vm.destructed(addr) -> bool returns if a contract was selfdestructed
3. forge-std destroySelfDestructed(addr /* overload with another addr that would be eth recipient */) which calls vm.destructed(addr) and if so, etches code(0) and sends eth to overloaded recipient?

[onbjerg]

Unsure, I still think it is a bit niche, especially considering that selfdestruct is a candidate for removal. But if we do add it, having a forge-std helper makes sense.

[meetmangukiya]

Instead of committing state mid-execution, how about just clearing the code(set_code) whenever we detect a destruction from inspector?

[onbjerg]

We could, however that also breaks 1:1 EVM behavior which might not be desired.

[meetmangukiya]

Maybe a cheatcode that proxies finalize can do too?

[onbjerg]

#2654 contradicts this issue - did this behavior change recently?

[PaulRBerg]

did this behavior change recently?

No, it didn't. selfdestruct still doesn't remove the code of the destructed code until the test execution finishes.

[PaulRBerg]

Is there any specific reason you need to test that the contract was self-destructed by checking the code size? I realize this may be an odd question.

I am using delegatecall, and I want to check that the caller is destroyed, and not the callee.

This might be a bit overkill, since I'm technically testing compiler features, but delegate calling is a sensitive enough operation that warrants this kind of additional check.

[oveddan]

I'm running into something similar. I want to create a test that:

• creates a proxy contract with a deterministic address using create2
• deletes it then selfdestruct's it
• creates another proxy contract at same address using create2

Im not sure if it's a foundry issue or general limitation of creating contracts that way
this is what my test looks like:

pragma solidity ^0.8.13;

import "forge-std/Test.sol";
import {Clones} from "openzeppelin/proxy/Clones.sol";

contract ContractToClone {
    address public owner;

    function initialize(address _owner) external {
        if (owner != address(0)) revert("Already initialied");
        owner = _owner;
    }

    function goodbye() external {
        selfdestruct(payable(owner));
    }
}

/// @notice Meant to test out determistic cloning techniques
contract DetermistingCloningTest is Test {
    function test_cloneCanBeCreatedTwiceUsingSameAddressIfDeleted() external {
        ContractToClone cloneableContract = new ContractToClone();

        address owner = vm.addr(1);

        bytes32 salt = bytes32(uint256(5));

        address clonedAddress = Clones.cloneDeterministic(address(cloneableContract), salt);

        ContractToClone clonedContract = ContractToClone(clonedAddress);
        clonedContract.initialize(owner);

        assertEq(clonedContract.owner(), owner);

        // now delete it, then destroy it
        delete clonedAddress;
        clonedContract.goodbye();

        Clones.cloneDeterministic(address(cloneableContract), salt);
    }
}

The last line of the test results in an error: "[FAIL. Reason: ERC1167: create2 failed]"

For reference, here are the contents of the function Clone.cloneDeterministic:

    /**
     * @dev Deploys and returns the address of a clone that mimics the behaviour of `implementation`.
     *
     * This function uses the create2 opcode and a `salt` to deterministically deploy
     * the clone. Using the same `implementation` and `salt` multiple time will revert, since
     * the clones cannot be deployed twice at the same address.
     */
     
library Clones {
    function cloneDeterministic(address implementation, bytes32 salt) internal returns (address instance) {
        /// @solidity memory-safe-assembly
        assembly {
            // Cleans the upper 96 bits of the `implementation` word, then packs the first 3 bytes
            // of the `implementation` address with the bytecode before the address.
            mstore(0x00, or(shr(0xe8, shl(0x60, implementation)), 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000))
            // Packs the remaining 17 bytes of `implementation` with the bytecode after the address.
            mstore(0x20, or(shl(0x78, implementation), 0x5af43d82803e903d91602b57fd5bf3))
            instance := create2(0, 0x09, 0x37, salt)
        }
        require(instance != address(0), "ERC1167: create2 failed");
    }
}

[oveddan]

thanks to @onbjerg suggestion the behavior works as expectedly if selfdestruct is called in setUp:

/// @notice Meant to test out determistic cloning techniques
contract DetermistingCloningTest is Test {
    ContractToClone cloneableContract;
    address owner;
    bytes32 salt;

    function setUp() external {
        cloneableContract = new ContractToClone();
        owner = vm.addr(1);
        salt = bytes32(uint256(5));

        address clonedAddress = Clones.cloneDeterministic(address(cloneableContract), salt);
        ContractToClone clonedContract = ContractToClone(clonedAddress);

        // now destroy it
        clonedContract.goodbye();
    }

    function test_succeedsWhen_cloneCreatedInSetup() external {
        // we already created and self-destructed the cloned contract in setup.
        // we should be able to succeed now.
        address clonedAddress = Clones.cloneDeterministic(address(cloneableContract), salt);

        ContractToClone clonedContract = ContractToClone(clonedAddress);
        clonedContract.initialize(owner);

        assertEq(clonedContract.owner(), owner);
    }

    function test_revertsWhen_cloneCreatedInSameTest() external {
        // this test creates 2 cloned contracts at the same address, but destroys the first one
        // before creating the second one.  it fails because foundry hasn't cleared the transaction in the same
        // test.

        bytes32 saltForTest = bytes32(uint256(6));
        address clonedAddress = Clones.cloneDeterministic(address(cloneableContract), saltForTest);

        ContractToClone clonedContract = ContractToClone(clonedAddress);
        clonedContract.initialize(owner);

        assertEq(clonedContract.owner(), owner);

        // now delete it, then destroy it
        clonedContract.goodbye();

        // clone the contract again, using the same cloneable contract and salt - it should revert
        vm.expectRevert();
        Clones.cloneDeterministic(address(cloneableContract), saltForTest);
    }
}