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

Solidity hex"" should be represented as bytes, not bytes1 #39

Closed
plotchy opened this issue May 15, 2023 · 4 comments · Fixed by #32
Closed

Solidity hex"" should be represented as bytes, not bytes1 #39

plotchy opened this issue May 15, 2023 · 4 comments · Fixed by #32

Comments

@plotchy
Copy link
Collaborator

plotchy commented May 15, 2023

This has implications when using low level calls and trying to hit fallbacks or receives

contract A {

    function foo() public {
        address a = address(0);
        a.call(hex"");
        a.delegatecall(hex"");

        bytes memory data = hex"01234567";
    }
}

Notice that member access is looking for call(bytes1) and delegatecall(bytes1)
image

@plotchy
Copy link
Collaborator Author

plotchy commented May 15, 2023

Actually this gets weird. All are valid solidity. Would recommend bytes as default

contract A {

    function foo() public {
        bytes1 a = hex"";
        bytes2 b = hex"";
        bytes3 c = hex"";
        bytes4 d = hex"";
        bytes memory e = hex"";
    }
}

@brockelmore
Copy link
Contributor

brockelmore commented May 15, 2023

It is because we are trying to do type inference from a literal to a specified target type as defined by where the variable is being piped into (we have to do this basically), but it gets tricky for function calls because we have to deduce what function we are actually calling (i.e. disambiguate the function). Should be trivial for call and builtins to get this right though.

Thats why for things where its a simple assignment a la bytes1 a = .. we handle it correctly but for in-place operations its not so simple (func calls, bit operations, etc)

@plotchy
Copy link
Collaborator Author

plotchy commented May 15, 2023

Makes sense. Solidity won't compile with this. Hard for us as we're not trying to enforce valid solidity.

image

@brockelmore
Copy link
Contributor

brockelmore commented May 15, 2023

If we have parse errors on invalid solidity thats okay. and realistically we should be able to detect this and stop parsing

@brockelmore brockelmore linked a pull request May 15, 2023 that will close this issue
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 a pull request may close this issue.

2 participants