Fix run-constructor option

[qian-hu]

Closes #335.
Addresses #66.

The current implementation requires using the end of the constructor's proof as the initial state for the test proof when the '--run-constructor' option is activated. However, an issue arises when this option is enabled while running a proof on a contract without a constructor - the initialization code fails to execute. In such cases, it disrupts the execution continuity as it lacks the end of the constructor's initial proof.

This PR proposes two potential solutions to address the abovementioned issue. The initial solution involves raising an error message to guide the user in providing a contract constructor. A more practical alternative suggests treating a contract without an explicit constructor as one with an empty constructor. This modification would also enable initializing the value assignments outside of functions in such cases, as in:

contract ConstructorTest is Test {
    bool flag = true;
}

The second approach is adopted to address issues of initializing variables.

[nwatson22]

Thanks for fixing this. I would suggest the second option. Right now the Contract.constructor field is set in Contract.__init__ only if there is an explicit contract member where type='constructor'. We probably want to populate this field for all contracts, maybe at the end of contract initialization if there isn't already one. It may be necessary to modify the Constructor class to be able to construct one not tied to a method, and it should still use the init_bytecode rather than the bin_runtime by the check in _method_to_cfg.

[PetarMax]

Would this PR address the issue of initialising non-constant variables with default values?

[palinatolmach]

@PetarMax I think it's supported and checked in ContructorTest:

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

import "forge-std/Test.sol";

contract ConstructorTest is Test {
    bool flag = true;

    function test_constructor() public {
        assert(flag);
    }

    function testFail_constructor() public {
        assert(!flag);
    }
}

Or do you mean something else?

[PetarMax]

@PetarMax I think it's supported and checked in ContructorTest:

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

import "forge-std/Test.sol";

contract ConstructorTest is Test {
    bool flag = true;

    function test_constructor() public {
        assert(flag);
    }

    function testFail_constructor() public {
        assert(!flag);
    }
}

Or do you mean something else?

Yes, that's what I meant - does it also work for analysing non-tests (i.e., functions in isolation)?

[palinatolmach]

I think it should, but not sure. @qian-hu could you please check if this works when running a proof for a function that doesn't start with test or testFail?

[qian-hu]

@PetarMax, yes, it works for analyzing non-tests. The --run-constructor option needs to be enabled to initialize variables outside of functions.

[palinatolmach]

Thanks @PetarMax and @qian-hu! It all looks good to me now; my only remaining request is to a non-test, now that it won't crash on SLOAD. We can't test it as a test-prefixed function since it won't fail, but @PetarMax suggested that we can execute something like

    function run_constructor() public {
        assert(flag);
    }

and assert that — as flag gets set to 1 — in the KCFG, there are no execution paths that end with the EVMC_REVERT status. That might be similar to how we perform foundry_show and then compare expected output in test_foundry_prove.

[palinatolmach]

I approved this PR to unblock, but left several suggestions on tests.

[palinatolmach]

@qian-hu the PR looks great, many thanks for addressing all the comments!