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.

Sign up for GitHub

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

Jump to bottom

Add nuances of .codehash usage to the documentation #14794

Open
ustas-eth opened this issue Jan 21, 2024 · 3 comments · May be fixed by #14793
Open

Add nuances of .codehash usage to the documentation #14794

ustas-eth opened this issue Jan 21, 2024 · 3 comments · May be fixed by #14793
Assignees
@mehtavishwa30
Labels
documentation 📖 low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited.

Comments

@ustas-eth
Copy link

Page

https://docs.soliditylang.org/en/v0.8.23/types.html#members-of-addresses

Abstract

addr.codehash output depends on the addr balance. It'll output 0 if the balance is 0 (and addr doesn't have any code, of course). Otherwise, if the balance is > 0, it'll output keccak256("").

The issue might be considered a bug because the existing documentation treats .codehash as an equivalent of keccak256(addr.code), which, as you can see below in the PoC, has a different output.

PoC:

pragma solidity 0.8.23;

contract Test1 {
    address subject = address(123);

    function getCodehash()
        external
        payable
        returns (
            bytes32 codeHashBefore,
            bytes32 codeHashAfter,
            bytes32 hashBefore,
            bytes32 hashAfter
        )
    {
        hashBefore = subject.codehash;
        codeHashBefore = keccak256(subject.code);

        payable(subject).transfer(1);

        hashAfter = subject.codehash;
        codeHashAfter = keccak256(subject.code);
    }
}

The function's output:

{
	"0": "bytes32: codeHashBefore 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470",
	"1": "bytes32: codeHashAfter 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470",
	"2": "bytes32: hashBefore 0x0000000000000000000000000000000000000000000000000000000000000000",
	"3": "bytes32: hashAfter 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470"
}

Also, see this report: https://code4rena.com/reports/2023-10-wildcat#h-02-codehash-check-in-factory-contracts-does-not-account-for-non-empty-addresses

Pull request

#14793
@ustas-eth ustas-eth linked a pull request Jan 21, 2024 that will close this issue
Open
@Rassska
Copy link

Rassska commented Jan 22, 2024

Agree, should be addressed. The recent contest showed that there is a slight misunderstanding: https://code4rena.com/reports/2023-10-wildcat#h-02-codehash-check-in-factory-contracts-does-not-account-for-non-empty-addresses

@github-actions GitHub Actions
Copy link

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Apr 21, 2024
@ustas-eth
Copy link
Author

The PR is waiting for an approval

@github-actions github-actions bot removed the stale The issue/PR was marked as stale because it has been open for too long. label Apr 22, 2024
@mehtavishwa30 mehtavishwa30 self-assigned this Apr 22, 2024
@mehtavishwa30 mehtavishwa30 added low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. labels Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mehtavishwa30 mehtavishwa30

Labels
documentation 📖 low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update documentation about codehash behaviour according to EIP-1052 and EIP-161 ustas-eth/solidity
3 participants
@mehtavishwa30 @Rassska @ustas-eth