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

cast sig does not behave as expected on some inputs #1521

Closed
2 tasks done
zk-tarts opened this issue May 6, 2022 · 8 comments
Closed
2 tasks done

cast sig does not behave as expected on some inputs #1521

zk-tarts opened this issue May 6, 2022 · 8 comments
Assignees
Labels
C-cast Command: cast T-bug Type: bug

Comments

@zk-tarts
Copy link
Contributor

zk-tarts commented May 6, 2022

Component

Cast

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

  • Foundry
  • Foundryup

What version of Foundry are you on?

nightly-8478c0841b00789c735df68ad111105fa9204f55

What command(s) is the bug in?

cast sig

Operating System

No response

Describe the bug

This should fail but does not

cast sig "foo(uint999)"
0x54511ad7

This should succeed

cast sig "foo(function() external)"
# panic with 'Illegal abi' message

The root of the problem is really over in ethers-rs. The parser does not handle function types in human readable ABI and does not make sure the bytes/int/uint sizes are correct, among other things. ref gakonst/ethers-rs#474

Some more examples

Arrays with unit sizes are techincally valid solidity

In solidity this expands out to uint[60].

foo(uint[1 minutes] memory)

Arrays have a maximum size in solidity

This gets parsed as correct but is not.

cast sig "foo(uint[99999999999999999])"
0xfece5cb9
@zk-tarts zk-tarts added the T-bug Type: bug label May 6, 2022
@zk-tarts zk-tarts changed the title cast sig does not behave as expected cast sig does not behave as expected on some inputs May 6, 2022
@zk-tarts
Copy link
Contributor Author

zk-tarts commented May 6, 2022

Should I have opened this issue in https://github.com/gakonst/ethers-rs/ ? It's really more of a problem there than it is a foundry bug.

@onbjerg
Copy link
Member

onbjerg commented May 6, 2022

This should succeed
cast sig "foo(function() external)"

I don't think it should? You cannot make these functions external, so they will never have a 4byte signature.

The others make sense to me.

There is another issue: Some Cast commands (cast call for example), we cannot use structs. AbiParser supports it, but it really just expands them into tuples. We should probably allow people to pass tuples (even though it is technically invalid?) to support people calling functions with structs (and functions that return structs)

Ref #670

It's really more of a problem there than it is a foundry bug.

Probably but I think having a tracking issue here also makes sense 🤷‍♂️

@onbjerg onbjerg added the C-cast Command: cast label May 6, 2022
@devanoneth
Copy link
Contributor

FYI this is also seems to be an issue with forge script. I cannot seem to pass a struct as the args for the run function (even after specifying my run function signature with -s).

For -s, I've tried:

run(ScriptTypes.Contracts)
run((address, address, address, address))
run(address, address, address, address)

All to no avail. Would appreciate any support on this!

@onbjerg onbjerg mentioned this issue Jun 20, 2022
2 tasks
@onbjerg
Copy link
Member

onbjerg commented Jun 24, 2022

@mattsse Assigning you to this since it's the same underlying issue as #2025 and you assigned yourself to that one

@mattsse
Copy link
Member

mattsse commented Jun 28, 2022

function types are currently not supported by ethabi, there's simply not a type variant for them

tracked here rust-ethereum/ethabi#273

@zk-tarts
Copy link
Contributor Author

If ethabi doesn't add function types then you could just treat them as bytes24 https://docs.soliditylang.org/en/latest/abi-spec.html#types

@mattsse
Copy link
Member

mattsse commented Jun 28, 2022

there are essentially two issues:

  • encoding/decoding
  • signature

which is handled here

https://github.com/rust-ethereum/ethabi/blob/451d029c69a1fce7c4d57cc117e985a31dc53627/ethabi/src/param_type/writer.rs#L25-L52

and would require another variant

@onbjerg
Copy link
Member

onbjerg commented Jul 1, 2022

Should be fixed in #2151, please let me know if it's not!

@onbjerg onbjerg closed this as completed Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cast Command: cast T-bug Type: bug
Projects
No open projects
Archived in project
Development

No branches or pull requests

4 participants