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

Abigen does not work with functions with large structs #23

Open
electricddev opened this issue Aug 8, 2023 · 7 comments
Open

Abigen does not work with functions with large structs #23

electricddev opened this issue Aug 8, 2023 · 7 comments

Comments

@electricddev
Copy link
Contributor

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
Solidity functions with a large struct as a function parameter currently get compiled by Abigen into Rust structs with a long tuple as the field types. However, Rust implements Debug and PartialEq for tuples up to 12 items (see here). Since Debug and PartialEq is derived for the function struct, the implementation will throw an error in the comiler.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem.
Example ABI from Rarible contract:

[
          {
		"inputs": [
			{
				"components": [
					{
						"internalType": "address",
						"name": "bidMaker",
						"type": "address"
					},
					{
						"internalType": "uint256",
						"name": "bidNftAmount",
						"type": "uint256"
					},
					{
						"internalType": "bytes4",
						"name": "nftAssetClass",
						"type": "bytes4"
					},
					{
						"internalType": "bytes",
						"name": "nftData",
						"type": "bytes"
					},
					{
						"internalType": "uint256",
						"name": "bidPaymentAmount",
						"type": "uint256"
					},
					{
						"internalType": "address",
						"name": "paymentToken",
						"type": "address"
					},
					{
						"internalType": "uint256",
						"name": "bidSalt",
						"type": "uint256"
					},
					{
						"internalType": "uint256",
						"name": "bidStart",
						"type": "uint256"
					},
					{
						"internalType": "uint256",
						"name": "bidEnd",
						"type": "uint256"
					},
					{
						"internalType": "bytes4",
						"name": "bidDataType",
						"type": "bytes4"
					},
					{
						"internalType": "bytes",
						"name": "bidData",
						"type": "bytes"
					},
					{
						"internalType": "bytes",
						"name": "bidSignature",
						"type": "bytes"
					},
					{
						"internalType": "uint256",
						"name": "sellOrderPaymentAmount",
						"type": "uint256"
					},
					{
						"internalType": "uint256",
						"name": "sellOrderNftAmount",
						"type": "uint256"
					},
					{
						"internalType": "bytes",
						"name": "sellOrderData",
						"type": "bytes"
					}
				],
				"internalType": "struct LibDirectTransfer.AcceptBid",
				"name": "direct",
				"type": "tuple"
			}
		],
		"name": "directAcceptBid",
		"outputs": [],
		"stateMutability": "payable",
		"type": "function"
	}
]

Results in:

     #[derive(Debug, Clone, PartialEq)]
        pub struct DirectAcceptBid {
            pub direct: (
                Vec<u8>,
                substreams::scalar::BigInt,
                [u8; 4usize],
                Vec<u8>,
                substreams::scalar::BigInt,
                Vec<u8>,
                substreams::scalar::BigInt,
                substreams::scalar::BigInt,
                substreams::scalar::BigInt,
                [u8; 4usize],
                Vec<u8>,
                Vec<u8>,
                substreams::scalar::BigInt,
                substreams::scalar::BigInt,
                Vec<u8>,
            ),
        }

Which leads to:

error[E0369]: binary operation `==` cannot be applied to type `(Vec<u8>, substreams::scalar::BigInt, [u8; 4], Vec<u8>, substreams::scalar::BigInt, Vec<u8>, substreams::scalar::BigInt, substreams::scalar::BigInt, substreams::scalar::BigInt, [u8; 4], Vec<u8>, Vec<u8>, substreams::scalar::BigInt, substreams::scalar::BigInt, Vec<u8>)`
  --> substreams-trades/src/abi/rarible.rs:8:13
   |
6  |           #[derive(Debug, Clone, PartialEq)]
   |                                  --------- in this derive macro expansion
7  |           pub struct DirectAcceptBid {
8  | /             pub direct: (
9  | |                 Vec<u8>,
10 | |                 substreams::scalar::BigInt,
11 | |                 [u8; 4usize],
...  |
23 | |                 Vec<u8>,
24 | |             ),
   | |_____________^
   |
   = note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)

What is the expected behavior?
I think the solution would be one of:

  • Generating implementations of Debug and PartialEq for long tuples
  • Generating named structs for struct arguments
@maoueh
Copy link
Collaborator

maoueh commented Aug 22, 2023

I will check if we could remove the == usage, it's probably not required actually, maybe just for tests.

@maoueh
Copy link
Collaborator

maoueh commented Aug 22, 2023

Try https://github.com/streamingfast/substreams-ethereum/tree/fix/tuple-with-lot-of-elements by adding this at the end of your Cargo.toml:

[patch.crates-io]
substreams-ethereum = { git = "https://github.com/streamingfast/substreams-ethereum", branch = "fix/tuple-with-lot-of-elements" }

@maoueh
Copy link
Collaborator

maoueh commented Aug 22, 2023

It's a breaking change, so I need to wait the pros/cons of merging this.

I don't think == has been used a lot, so I think it could be fair to to merge in.

@electricddev
Copy link
Contributor Author

Unfortunately, this won't work because Debug faces the same difficulties with tuples larger than 12 items. I was working on a fork of Abigen in which each tuple got its own struct but I abandoned it. I can give it a go again?

@maoueh
Copy link
Collaborator

maoueh commented Aug 28, 2023

Unfortunately, this won't work because Debug faces the same difficulties with tuples larger than 12 items. I was working on a fork of Abigen in which each tuple got its own struct but I abandoned it. I can give it a go again?

Oh what a bummer! I should have added a unit test, I would have seen that, damn it. Yeah the other solution would be to send them to their own struct indeed. If you have the bandwidth, yeah you can continue it, I'm ok merging such work. If you don't have the time, at least push a branch somewhere and link it here, someone could start from that.

Will be a good breaking change however, so maybe will put that under an option.

@maoueh
Copy link
Collaborator

maoueh commented Mar 25, 2024

I re-think of that today and maybe we could remove generating Debug and PartialEq when the struct has > 12 fields. Would need to check where Debug generated methods were used exactly.

@nabaruns
Copy link

nabaruns commented Jan 4, 2025

Any update regarding same. Facing 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

No branches or pull requests

3 participants