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

docs(cast): document abi-encode / abi-decode with nested structs that contain arrays #1286

Open
zerosnacks opened this issue Sep 12, 2024 · 10 comments
Labels
A-cast Area: cast good first issue Good for newcomers T-enhancement Type: enhance existing docs
Milestone

Comments

@zerosnacks
Copy link
Member

zerosnacks commented Sep 12, 2024

Sections

cast abi-encode
cast abi-decode

Describe the bug

Document how to correctly pass arguments to cast abi-encode when it has multiple parameters in a nested struct with one of them being an array.

Context: https://gist.github.com/pcaversaccio/0ef8fb8034594e012a4903dfa992369e

From: #1286 (comment)

They're 2 different encodings, cast abi-encode encodes for function arguments, not a single struct.

Essentially you're comparing the encoding of (cast abi-encode):

function f( (address,uint256,bytes)[] arg1, address arg2, bytes32 arg3 )

to (expected):

function g( ((address,uint256,bytes)[],address,bytes32) arg1 )

(which is equivalent to the abi.encode(arg1))

You're looking for the second one, so the correct command is achieved by wrapping the struct fields in another pair of parentheses:

cast abi-encode "f(((address,uint256,bytes)[],address,bytes32))" "([(0xD7f9f54194C633F36CCD5F3da84ad4a1c38cB2cB,0,0x79ba5097),(0xD7f9f54194C633F36CCD5F3da84ad4a1c38cB2cB,0,0x79ba5097)],0x8f7a9912416e8AdC4D9c21FAe1415D3318A11897,0x646563656e7472616c697a6174696f6e206973206e6f74206f7074696f6e616c)"

The examples in OP:

$ cast abi-encode "UpgradeProposal((uint256)[],uint256)" "[(1)]" "123"
0x0000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000007b00000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000001

$ cast abi-encode "UpgradeProposal(((uint256)[],uint256))" "([(1)],123)"
0x00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000007b00000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000001

Note the extra 0x00..20 at the start

@zerosnacks zerosnacks added the T-to-fix Type: issue in docs label Sep 12, 2024
@zerosnacks zerosnacks changed the title bug(cast): abi-encode with array args does not work as expected bug(cast): abi-encode with arrays in args does not work as expected Sep 12, 2024
@pcaversaccio
Copy link
Contributor

Fwiw, the issue is related to how cast abi-encode works with nested structs that also contain arrays (see my above linked gist). Also, when I try to decode the correct payload it fails:

cast abi-decode -i "UpgradeProposal((address,uint256,bytes)[],address,bytes32)" 0x00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000060000000000000000000000000defd1edee3e8c5965216bd59c866f7f5307c9b29646563656e7472616c697a6174696f6e206973206e6f74206f7074696f6e616c00000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000012000000000000000000000000000000000000000000000000000000000000001c00000000000000000000000000000000000000000000000000000000000000260000000000000000000000000d7f9f54194c633f36ccd5f3da84ad4a1c38cb2cb00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000479ba509700000000000000000000000000000000000000000000000000000000000000000000000000000000303a465b659cbb0ab36ee643ea362c509eeb521300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000479ba509700000000000000000000000000000000000000000000000000000000000000000000000000000000c2ee6b6af7d616f6e27ce7f4a451aedc2b0f5f5c00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000479ba5097000000000000000000000000000000000000000000000000000000000000000000000000000000005d8ba173dc6c3c90c8f7c04c9288bef5fdbad06e00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000479ba509700000000000000000000000000000000000000000000000000000000

IMG_20240912_135330_456.jpg

@zerosnacks zerosnacks changed the title bug(cast): abi-encode with arrays in args does not work as expected bug(cast): abi-encode with nested structs that contain arrays does not work as expected Sep 12, 2024
@zerosnacks zerosnacks changed the title bug(cast): abi-encode with nested structs that contain arrays does not work as expected bug(cast): abi-encode / abi-decode with nested structs that contain arrays does not work as expected Sep 12, 2024
@DaniPopes
Copy link
Member

The syntax is correct for the first block, not the second

@pcaversaccio
Copy link
Contributor

The syntax is correct for the first block, not the second

can you elaborate? Not sure I can follow.

@DaniPopes
Copy link
Member

In OP "Basic example" is correct, each parameter corresponds to a CLI argument; the "versus (does not work)" block is not correct

@pcaversaccio
Copy link
Contributor

pcaversaccio commented Sep 12, 2024

In OP "Basic example" is correct, each parameter corresponds to a CLI argument; the "versus (does not work)" block is not correct

correct - as background this example is coming from me since I literally played around with this for an hour yesterday. The syntax ([(1)],123) would make much more sense as it's a nested struct that I represent here.

Also, more importantly, please check my bash script and the encoding of the complex struct. It's not correct even with the given syntax:

cast abi-encode "UpgradeProposal((address,uint256,bytes)[],address,bytes32)" "[(0xD7f9f54194C633F36CCD5F3da84ad4a1c38cB2cB,0,0x79ba5097),(0xD7f9f54194C633F36CCD5F3da84ad4a1c38cB2cB,0,0x79ba5097)]" "0x8f7a9912416e8AdC4D9c21FAe1415D3318A11897" "0x646563656e7472616c697a6174696f6e206973206e6f74206f7074696f6e616c"

The above command will return a wrong encoding. If you use the correct encoding result from the Solidity script in my test you will see it.

@DaniPopes
Copy link
Member

DaniPopes commented Sep 12, 2024

They're 2 different encodings, cast abi-encode encodes for function arguments, not a single struct.

Essentially you're comparing the encoding of (cast abi-encode):

function f( (address,uint256,bytes)[] arg1, address arg2, bytes32 arg3 )

to (expected):

function g( ((address,uint256,bytes)[],address,bytes32) arg1 )

(which is equivalent to the abi.encode(arg1))

You're looking for the second one, so the correct command is achieved by wrapping the struct fields in another pair of parentheses:

cast abi-encode "f(((address,uint256,bytes)[],address,bytes32))" "([(0xD7f9f54194C633F36CCD5F3da84ad4a1c38cB2cB,0,0x79ba5097),(0xD7f9f54194C633F36CCD5F3da84ad4a1c38cB2cB,0,0x79ba5097)],0x8f7a9912416e8AdC4D9c21FAe1415D3318A11897,0x646563656e7472616c697a6174696f6e206973206e6f74206f7074696f6e616c)"

The examples in OP:

$ cast abi-encode "UpgradeProposal((uint256)[],uint256)" "[(1)]" "123"
0x0000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000007b00000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000001
$ cast abi-encode "UpgradeProposal(((uint256)[],uint256))" "([(1)],123)"
0x00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000007b00000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000001

Note the extra 0x00..20 at the start

@pcaversaccio
Copy link
Contributor

pcaversaccio commented Sep 12, 2024

Hmm I see, but:

cast abi-encode "UpgradeProposal(((address,uint256,bytes)[],address,bytes32))" "([(0xD7f9f54194C633F36CCD5F3da84ad4a1c38cB2cB,0,0x79ba5097),(0xD7f9f54194C633F36CCD5F3da84ad4a1c38cB2cB,0,0x79ba5097)],0x8f7a9912416e8AdC4D9c21FAe1415D3318A11897,0x646563656e7472616c697a6174696f6e206973206e6f74206f7074696f6e616c)"

produces the following:

0x000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000600000000000000000000000008f7a9912416e8adc4d9c21fae1415d3318a11897646563656e7472616c697a6174696f6e206973206e6f74206f7074696f6e616c0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000e0000000000000000000000000d7f9f54194c633f36ccd5f3da84ad4a1c38cb2cb00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000479ba509700000000000000000000000000000000000000000000000000000000000000000000000000000000d7f9f54194c633f36ccd5f3da84ad4a1c38cb2cb00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000479ba509700000000000000000000000000000000000000000000000000000000

But the correct encoding would be:

0x00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000060000000000000000000000000defd1edee3e8c5965216bd59c866f7f5307c9b29646563656e7472616c697a6174696f6e206973206e6f74206f7074696f6e616c00000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000012000000000000000000000000000000000000000000000000000000000000001c00000000000000000000000000000000000000000000000000000000000000260000000000000000000000000d7f9f54194c633f36ccd5f3da84ad4a1c38cb2cb00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000479ba509700000000000000000000000000000000000000000000000000000000000000000000000000000000303a465b659cbb0ab36ee643ea362c509eeb521300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000479ba509700000000000000000000000000000000000000000000000000000000000000000000000000000000c2ee6b6af7d616f6e27ce7f4a451aedc2b0f5f5c00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000479ba5097000000000000000000000000000000000000000000000000000000000000000000000000000000005d8ba173dc6c3c90c8f7c04c9288bef5fdbad06e00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000479ba509700000000000000000000000000000000000000000000000000000000

If you want to replicate, use this:

// SPDX-License-Identifier: WTFPL
pragma solidity ^0.8.19;

contract ProposalId {
    struct Call {
        address target;
        uint256 value;
        bytes data;
    }

    struct UpgradeProposal {
        Call[] calls;
        address executor;
        bytes32 salt;
    }

    function computeProposalId() external pure returns (bytes memory) {
        Call[] memory calls = new Call[](4);
        calls[0] = Call({target: 0xD7f9f54194C633F36CCD5F3da84ad4a1c38cB2cB, value: 0, data: hex"79ba5097"});
        calls[1] = Call({target: 0x303a465B659cBB0ab36eE643eA362c509EEb5213, value: 0, data: hex"79ba5097"});
        calls[2] = Call({target: 0xc2eE6b6af7d616f6e27ce7F4A451Aedc2b0F5f5C, value: 0, data: hex"79ba5097"});
        calls[3] = Call({target: 0x5D8ba173Dc6C3c90C8f7C04C9288BeF5FDbAd06E, value: 0, data: hex"79ba5097"});

        address executor = 0xdEFd1eDEE3E8c5965216bd59C866f7f5307C9b29;
        bytes32 salt = hex"646563656e7472616c697a6174696f6e206973206e6f74206f7074696f6e616c";

        UpgradeProposal memory upgradeProposal = UpgradeProposal({calls: calls, executor: executor, salt: salt});

        return abi.encode(upgradeProposal);
    }
}

@DaniPopes
Copy link
Member

DaniPopes commented Sep 12, 2024

the arguments are not the same as in the solidity script: calls is length 2 in cast and 4 in your "correct encoding"

@pcaversaccio
Copy link
Contributor

pcaversaccio commented Sep 12, 2024

the arguments are not the same as in the solidity script: calls is length 2 in cast and 4 in your "correct encoding"

I'm stupid - wrong executor address in the above example as well. Now everything works well. Can we please have as actionable item that we document how to do such nested structs correctly in the docs?

@zerosnacks zerosnacks changed the title bug(cast): abi-encode / abi-decode with nested structs that contain arrays does not work as expected docs(cast): document abi-encode / abi-decode with nested structs that contain arrays Sep 12, 2024
@zerosnacks zerosnacks added good first issue Good for newcomers and removed T-to-fix Type: issue in docs labels Sep 12, 2024
@zerosnacks
Copy link
Member Author

Thanks @DaniPopes!

Moving this to foundry-rs/book as the actionable item is related to the documentation and confirmed to not be a bug

@zerosnacks zerosnacks transferred this issue from foundry-rs/foundry Sep 12, 2024
@zerosnacks zerosnacks added A-cast Area: cast T-enhancement Type: enhance existing docs labels Sep 12, 2024
@zerosnacks zerosnacks added this to the v1.0.0 milestone Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cast Area: cast good first issue Good for newcomers T-enhancement Type: enhance existing docs
Projects
No open projects
Status: Todo
Development

No branches or pull requests

3 participants