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(abigen): add EthAbiCodec proc macro #704

Merged
merged 3 commits into from
Dec 19, 2021

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Dec 18, 2021

Motivation

Add AbiEncode + AbiDecode implementation for custom structs.
Currently, those get derived for EthCall only.

Solution

Add EthAbiCodec as a complementary proc macro to EthAbiType that derives encode/decode.
The reason why this needs to be separate and not included in EthAbiType is because in EthCall we have an additional leading 4byte selector, so EthCall needs to implement the codec traits but requires all the tokenized support that EthAbiType provides.
So we simply add an additional EthAbiCodec derive when we generate AbiEncoderV2 structs within abigen!

PR Checklist

  • Added Tests
  • Added Documentation
  • Updated the changelog

@mattsse
Copy link
Collaborator Author

mattsse commented Dec 18, 2021

Fixed clippy lints on #705

@sveitser
Copy link
Contributor

sveitser commented Dec 18, 2021

Awesome. It works well on the struct I was testing with.

I noticed something that seems a bit strange to me. In order to abi.decode in the contract I am wrapping the struct in a one-element tuple then tokenize and encode.

If I use the new decode method on that I do get and extra 32 in front (compared to doing into_tokens followed by encode).

Tried to illustrate this below

    abigen!(
        Example,
        r#"[
        struct Foo { uint256[] a; }
        function readFoo(bytes b)
        ]"#,
        event_derives(serde::Deserialize, serde::Serialize);
    );

    let a = vec![U256::from(255)];
    let foo = Foo { a };

    fn disp(b: &[u8]) {
        hex::encode(b)
            .chars()
            .collect::<Vec<char>>()
            .chunks(64)
            .for_each(|x| println!("{}", x.iter().collect::<String>()));
        println!("");
    }

    let encoded = foo.clone().encode();
    disp(&encoded);
    // 0000000000000000000000000000000000000000000000000000000000000020
    // 0000000000000000000000000000000000000000000000000000000000000001
    // 00000000000000000000000000000000000000000000000000000000000000ff

    // same as encode above
    let from_tokens = abi::encode(&Tokenize::into_tokens(foo.clone()));
    disp(&from_tokens);
    // 0000000000000000000000000000000000000000000000000000000000000020
    // 0000000000000000000000000000000000000000000000000000000000000001
    // 00000000000000000000000000000000000000000000000000000000000000ff

    let encoded_wrapped = (foo.clone(),).encode();
    disp(&encoded_wrapped);
    // 0000000000000000000000000000000000000000000000000000000000000020  X
    // 0000000000000000000000000000000000000000000000000000000000000020  X
    // 0000000000000000000000000000000000000000000000000000000000000020
    // 0000000000000000000000000000000000000000000000000000000000000001
    // 00000000000000000000000000000000000000000000000000000000000000ff

    // this works with abi.decode(b, (Foo)) in a contract
    let from_tokens_wrapped = abi::encode(&Tokenize::into_tokens((foo.clone(),)));
    disp(&from_tokens_wrapped);
    // 0000000000000000000000000000000000000000000000000000000000000020  X
    // 0000000000000000000000000000000000000000000000000000000000000020
    // 0000000000000000000000000000000000000000000000000000000000000001
    // 00000000000000000000000000000000000000000000000000000000000000ff

@mattsse
Copy link
Collaborator Author

mattsse commented Dec 18, 2021

nice catch!
I think I fixed tuple codec now, could you try again please @sveitser?

@sveitser
Copy link
Contributor

Yes. I'm getting matching outputs now for the tuple 🥳

    let encoded_wrapped = (foo.clone(),).encode();
    disp(&encoded_wrapped);
    // 0000000000000000000000000000000000000000000000000000000000000020  X
    // 0000000000000000000000000000000000000000000000000000000000000020
    // 0000000000000000000000000000000000000000000000000000000000000001
    // 00000000000000000000000000000000000000000000000000000000000000ff

Comment on lines +551 to +553
let tuple = (val,);
let encoded = tuple.clone().encode();
let decoded = <(SomeType,)>::decode(&encoded).unwrap();
Copy link
Owner

Choose a reason for hiding this comment

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

this tuple syntax seems like a footgun? can we get rid of the x,) requirement?

Copy link
Collaborator Author

@mattsse mattsse Dec 19, 2021

Choose a reason for hiding this comment

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

it's a bit weird but this ist the required rust syntax for single-element tuple, otherwise it would just be a parenthesized expression

image
https://doc.rust-lang.org/1.30.0/book/second-edition/appendix-02-operators.html

@gakonst gakonst merged commit 3c164bc into gakonst:master Dec 19, 2021
thasarito added a commit to thasarito/ethers-rs that referenced this pull request Dec 24, 2021
thasarito added a commit to thasarito/ethers-rs that referenced this pull request Dec 24, 2021
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.

3 participants