Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: branching over symbolic call addresses #349

Merged
merged 33 commits into from
Aug 22, 2024

Conversation

daejunpark
Copy link
Collaborator

@daejunpark daejunpark commented Aug 20, 2024

When a call target address may be aliased with multiple existing addresses, or may be non-existent, explicitly branching into multiple paths for each potential alias or non-existent address.

Although this change may increase the number of potential paths, it eliminates the time needed to solve must-alias conditions, which can take up to 1s each. Also, the path branching is unlikely to exponentially grow, unless there are many duplicated instances of contracts with similar interfaces, because most paths will immediately revert due to unimplemented selectors. In such cases, the time saved by reducing solver overhead outweighs the cost of increased path branching.

As a result, "unknown calls" no longer exist, and the related options, --uninterpreted-unknown-calls and --return-size-of-unknown-calls, are no longer needed and have been deprecated.

src/halmos/sevm.py Show resolved Hide resolved
src/halmos/sevm.py Show resolved Hide resolved
src/halmos/sevm.py Outdated Show resolved Hide resolved
src/halmos/sevm.py Show resolved Hide resolved
addr = alias_addr if alias_addr is not None else account_addr

account_code: Optional[Contract] = ex.code.get(addr, None)

codehash = (
f_extcodehash(addr)
EMPTY_KECCAK
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this correct?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's a weird nuance around dead/empty accounts for which this returns 0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

py-evm does this:

    if state.account_is_empty(account):
        computation.stack_push_bytes(constants.NULL_BYTE)
    else:
        computation.stack_push_bytes(state.get_code_hash(account))

with

    def account_is_empty(self, address: Address) -> bool:
        return (
            not self.account_has_code_or_nonce(address)
            and self.get_balance(address) == 0
        )

weird but this passes in foundry:

    function test_extcodehash_empty() external {
        address emptyCodeAddr = address(new Empty());
        assertEq(emptyCodeAddr.code.length, 0, "Empty contract should have no code");

        bytes32 codehash;
        bytes32 nextCodehash;
        assembly {
            codehash := extcodehash(emptyCodeAddr)
            nextCodehash := extcodehash(add(emptyCodeAddr, 1))
        }

        assertEq(codehash, keccak256(""), "Expected codehash of the empty string");
        assertEq(nextCodehash, 0, "Expected 0");
    }

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what? so, extcodehash of a contract account with empty code is different from that of a non-existing account?

so, here we should return 0, based on the current implementation of vms?

Copy link
Collaborator

@karmacoma-eth karmacoma-eth Aug 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what? so, extcodehash of a contract account with empty code is different from that of a non-existing account?

yes, https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1052.md#specification is a bit more explicit

The EXTCODEHASH of the account without code is c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470 what is the keccack256 hash of empty data.
The EXTCODEHASH of non-existent account is 0.
The EXTCODEHASH of a precompiled contract is either c5d246... or 0.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, here we should return 0, based on the current implementation of vms?

we need to have a more proper is_empty check for the account, which is also affected by the balance. This needs to pass:

    function testAccountWithOrWithoutBalance() public {
        address a = address(0x42);
        assertEq(extcodehash(a), bytes32(0), "empty account has extcodehash 0");

        vm.deal(a, 1 ether);
        assertEq(extcodehash(a), 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470, "account with balance has extcodehash keccak('')");
    }

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

full test:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.18;

import "forge-std/Test.sol";

contract Empty {
    constructor() {
        assembly {
            return(0, 0)
        }
    }
}

contract EmptyDeployerTest is Test {
    function extcodehash(address _addr) public view returns (bytes32) {
        bytes32 hash;
        assembly {
            hash := extcodehash(_addr)
        }
        return hash;
    }

    function testEmptyDeployer() public {
        address a = vm.computeCreateAddress(address(this), vm.getNonce(address(this)));
        assertEq(extcodehash(a), bytes32(0), "empty account has extcodehash 0");
        assertEq(vm.getNonce(a), 0);

        // do deploy
        new Empty();

        assertEq(a.code.length, 0);
        assertEq(extcodehash(a), keccak256(""));
        assertEq(vm.getNonce(a), 1);
    }


    function testAccountWithOrWithoutBalance() public {
        address a = address(0x42);
        assertEq(extcodehash(a), bytes32(0), "empty account has extcodehash 0");

        vm.deal(a, 1 ether);
        assertEq(extcodehash(a), keccak256(""), "account with balance has extcodehash keccak('')");
    }
}

in our case if we don't want to model nonces, we can probably differentiate between not being in code, vs being in code with empty bytecode

@karmacoma-eth
Copy link
Collaborator

tangential but related, do you think this test should pass?

    function check_unknown_initial_balance(address addr) public {
        assertEq(addr.balance, 0);
    }

@daejunpark
Copy link
Collaborator Author

tangential but related, do you think this test should pass?

    function check_unknown_initial_balance(address addr) public {
        assertEq(addr.balance, 0);
    }

eventually yes, but in a separate pr. this pr is only for handling code of symbolic addresses.

src/halmos/utils.py Show resolved Hide resolved
tests/regression/test/Extcodehash.t.sol Outdated Show resolved Hide resolved
tests/regression/test/Buffers.t.sol Show resolved Hide resolved
src/halmos/sevm.py Outdated Show resolved Hide resolved
src/halmos/sevm.py Show resolved Hide resolved
src/halmos/sevm.py Outdated Show resolved Hide resolved
src/halmos/sevm.py Show resolved Hide resolved
src/halmos/sevm.py Outdated Show resolved Hide resolved
addr = alias_addr if alias_addr is not None else account_addr

account_code: Optional[Contract] = ex.code.get(addr, None)

codehash = (
f_extcodehash(addr)
EMPTY_KECCAK
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's a weird nuance around dead/empty accounts for which this returns 0

addr = alias_addr if alias_addr is not None else account_addr

account_code: Optional[Contract] = ex.code.get(addr, None)

codehash = (
f_extcodehash(addr)
EMPTY_KECCAK
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

py-evm does this:

    if state.account_is_empty(account):
        computation.stack_push_bytes(constants.NULL_BYTE)
    else:
        computation.stack_push_bytes(state.get_code_hash(account))

with

    def account_is_empty(self, address: Address) -> bool:
        return (
            not self.account_has_code_or_nonce(address)
            and self.get_balance(address) == 0
        )

weird but this passes in foundry:

    function test_extcodehash_empty() external {
        address emptyCodeAddr = address(new Empty());
        assertEq(emptyCodeAddr.code.length, 0, "Empty contract should have no code");

        bytes32 codehash;
        bytes32 nextCodehash;
        assembly {
            codehash := extcodehash(emptyCodeAddr)
            nextCodehash := extcodehash(add(emptyCodeAddr, 1))
        }

        assertEq(codehash, keccak256(""), "Expected codehash of the empty string");
        assertEq(nextCodehash, 0, "Expected 0");
    }

@daejunpark daejunpark merged commit d541a0f into main Aug 22, 2024
57 checks passed
@daejunpark daejunpark deleted the feat/branch-symbolic-calls branch August 22, 2024 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants