Abigen fails with some nested structs

[sveitser]

Version

└── ethers v0.5.3 (https://github.com/gakonst/ethers-rs#eede86df)
    ├── ethers-contract v0.5.3 (https://github.com/gakonst/ethers-rs#eede86df)
    │   ├── ethers-contract-abigen v0.5.3 (https://github.com/gakonst/ethers-rs#eede86df)
    │   │   ├── ethers-core v0.5.4 (https://github.com/gakonst/ethers-rs#eede86df)
    │   ├── ethers-contract-derive v0.5.3 (proc-macro) (https://github.com/gakonst/ethers-rs#eede86df)
    │   │   ├── ethers-contract-abigen v0.5.3 (https://github.com/gakonst/ethers-rs#eede86df) (*)
    │   │   ├── ethers-core v0.5.4 (https://github.com/gakonst/ethers-rs#eede86df) (*)
    │   ├── ethers-core v0.5.4 (https://github.com/gakonst/ethers-rs#eede86df) (*)
    │   ├── ethers-providers v0.5.4 (https://github.com/gakonst/ethers-rs#eede86df)
    │   │   ├── ethers-core v0.5.4 (https://github.com/gakonst/ethers-rs#eede86df) (*)
    ├── ethers-core v0.5.4 (https://github.com/gakonst/ethers-rs#eede86df) (*)
    ├── ethers-middleware v0.5.3 (https://github.com/gakonst/ethers-rs#eede86df)
    │   ├── ethers-contract v0.5.3 (https://github.com/gakonst/ethers-rs#eede86df) (*)
    │   ├── ethers-core v0.5.4 (https://github.com/gakonst/ethers-rs#eede86df) (*)
    │   ├── ethers-providers v0.5.4 (https://github.com/gakonst/ethers-rs#eede86df) (*)
    │   ├── ethers-signers v0.5.3 (https://github.com/gakonst/ethers-rs#eede86df)
    │   │   ├── ethers-core v0.5.4 (https://github.com/gakonst/ethers-rs#eede86df) (*)
    ├── ethers-providers v0.5.4 (https://github.com/gakonst/ethers-rs#eede86df) (*)
    └── ethers-signers v0.5.3 (https://github.com/gakonst/ethers-rs#eede86df) (*)

Platform

Linux lulus 5.10.75 #1-NixOS SMP Wed Oct 20 09:45:06 UTC 2021 x86_64 GNU/Linux

Description
Some combinations of nested structs and arrays are not handled correctly by the abigen.

Example:

use ethers::prelude::abigen;

abigen!(
    Test,
     r#"[
        struct Outer {Inner inner; uint256[] arr;}
        struct Inner {uint256 inner;}
        function myfun(Outer calldata a)
    ]"#,
);

I expected to see this happen: cargo check succeeds.

Instead, this happened:

error: Unable to determine ABI: myfun(((uint256),uint256[]))
  --> src/lib.rs:3:1
   |
3  | / abigen!(
4  | |     Test,
5  | |      r#"[
6  | |         struct Outer {Inner inner; uint256[] arr;}
...  |
9  | |     ]"#,
10 | | );
   | |__^
   |
   = note: this error originates in the macro `abigen` (in Nightly builds, run with -Z macro-backtrace for more info)

[mattsse]

this is an edge case that ethabi::Reader fails to handle,
the function takes a single param so the abi is myfun(((uint256), unit256[])) and within EthCall we try to parse (((uint256), unit256[])) as tuple to implement the call's encoding function.

This can be resolved with:

• improving the abi parser: Tokenized Human Readable ABI instead of String #474
• use the new AbiEncode + AbiDecode + AbiType impls of the struct's fields, making parsing obsolete. this is the preferred solution because it makes parsing of the struct's AST and/or abi attribute redundant.

[gakonst]

IMO if we can avoid parsing altogether that'd be great. So let's try the second solution?

[sveitser]

I'm unfortunately still affected by this issue. As a first step towards fixing it I was trying to find a place in ethers-rs where the failing example could be added as a (regression) test, but I didn't succeed. If that even makes sense, is there a place where it would make sense to add this?

And in the meantime I'm also wondering if anyone was aware of any workarounds. This seems to affect most of my contracts with nested structs.

[sveitser]

I was looking at the code below and wouldn't the example above be parsed as a function in the parse_function function?

edit: I might get it now that the ABI at that point is already incorrect?

ethers-rs/ethers-contract/ethers-contract-derive/src/call.rs

Lines 29 to 62 in 24f9bd0

	let mut function = if let Some((src, span)) = attributes.abi {
	// try to parse as solidity function
	if let Ok(fun) = parse_function(&src) {
	fun
	} else {
	// try as tuple
	if let Some(inputs) = Reader::read(
	src.trim_start_matches("function ")
	.trim_start()
	.trim_start_matches(&function_call_name),
	)
	.ok()
	.and_then(|param| match param {
	ParamType::Tuple(params) => Some(
	params
	.into_iter()
	.map(|kind| Param { name: "".to_string(), kind, internal_type: None })
	.collect(),
	),
	_ => None,
	}) {
	#[allow(deprecated)]
	Function {
	name: function_call_name.clone(),
	inputs,
	outputs: vec![],
	constant: false,
	state_mutability: Default::default(),
	}
	} else {
	return Error::new(span, format!("Unable to determine ABI: {}", src))
	.to_compile_error()
	}
	}

[mattsse]

thanks for the follow up.
all technical blockers for this have been resolved, however supporting this directly in abigen! requires some refactoring, which I haven't gotten to yet.

workaround currently would be to define the input as tuple function myfun((...) a) and the structs standalone with EthAbiType and a helper to either encode and decode as Tuple like
contract.my_fun(outer.encoded().unwrap().decode().unwrap()) or a conversion to tuple directly

[mattsse]

I was looking at the code below and wouldn't the example above be parsed as a function in the parse_function function?

edit: I might get it now that the ABI at that point is already incorrect?

ethers-rs/ethers-contract/ethers-contract-derive/src/call.rs

Lines 29 to 62 in 24f9bd0

	let mut function = if let Some((src, span)) = attributes.abi {
	// try to parse as solidity function
	if let Ok(fun) = parse_function(&src) {
	fun
	} else {
	// try as tuple
	if let Some(inputs) = Reader::read(
	src.trim_start_matches("function ")
	.trim_start()
	.trim_start_matches(&function_call_name),
	)
	.ok()
	.and_then(|param| match param {
	ParamType::Tuple(params) => Some(
	params
	.into_iter()
	.map(|kind| Param { name: "".to_string(), kind, internal_type: None })
	.collect(),
	),
	_ => None,
	}) {
	#[allow(deprecated)]
	Function {
	name: function_call_name.clone(),
	inputs,
	outputs: vec![],
	constant: false,
	state_mutability: Default::default(),
	}
	} else {
	return Error::new(span, format!("Unable to determine ABI: {}", src))
	.to_compile_error()
	}
	}

the parser is really not ideal and now that we've ParamType support, we should be able to bypass that entirely during abigen! expansion, but this is a bit more refactoring effort.

[sveitser]

Thanks for the replies. In terms of workarounds for contracts in development, I noticed that sometimes the failures can be avoided by reordering the fields in the structs. The example above compiles if the order of the fields in Outer is changed.

use ethers::prelude::abigen;

abigen!(
    Test,
     r#"[
        struct Outer {uint256[] arr; Inner inner;}
        struct Inner {uint256 inner;}
        function myfun(Outer calldata a)
    ]"#,
);

[sveitser]

Probably not very useful but in case someone is hitting a similar issue some more information and a reduced real world example (where I can't find a workaround) below. I'm not sure this is hitting the same problem.

The last ethers commit I can expand the abigen macro with is 071a416 , the commit where it started breaking is fb4d9a9.

Fails with

error: Unable to determine ABI: submitBlock((uint64,((((uint256,uint256,uint256,uint256),uint256[]),uint256[],uint256[]))[],((((uint256,uint256,uint256,uint256),uint256[]),uint256[],uint256[]))[]))

pragma solidity ^0.8.0;

contract Example {

    struct EncKey {
        uint256 x;
        uint256 y;
        uint256 t;
        uint256 z;
    }

    struct AuditMemo {
        EncKey ephemeral;
        uint256[] data;
    }

    struct TransferNote {
        AuditMemo auditMemo;
        uint256[] inputsNullifiers;
        uint256[] outputCommitments;
    }

    struct Transaction {
        TransferNote note;
    }

    struct Block {
        uint64 blockHeight;
        Transaction[] txns;
        Transaction[] burnTxns;
    }

    function submitBlock(
        Block memory newBlock
    ) public {
    }
}

[mattsse]

thanks for these,
the whole reason this fails is because abigen! utilizes ethabi's tuple parser that has some troubles with nested tuples apparently,
I'll try to revamp the whole abigen type detection this weekend.

[mfw78]

Hello, I've come across this github issue when attempting to use forge and ethers-rs. After successfully testing the contracts in forge, I used forge bind to generate bindings for the project. When attempting to use one of the generated bindings, the following error was thrown:

133 |         pub diamond_cut: Vec<(ethers::core::types::Address, u8, Vec<[u8; 4]>)>,
    |         ^^^ the trait `AbiArrayType` is not implemented for `u8`
    |
    = note: required because of the requirements on the impl of `AbiArrayType` for `(H160, u8, Vec<[u8; 4]>)`
    = note: required because of the requirements on the impl of `AbiType` for `Vec<(H160, u8, Vec<[u8; 4]>)>`

The solidity source code for this is available at https://github.com/mudgen/diamond-3-hardhat/blob/main/contracts/interfaces/IDiamondCut.sol.

[mattsse]

thanks, it appears there are some AbiArrayType impls missing for tuples
I'll check