feat(forge-bind): add_derives for serde in contract bindings

[Autoparallel]

Motivation

The change was made to close #5835 which would allow users to get more functionality out of enums/structs in contract bindings, e.g., parse contract events from a chain into a file format such as CSV.

Solution

This is a basic solution where we just add in the serde::Serialize and serde:Deserialize traits via the add_derives method on Abigen. This does not give the end user any option in deriving these traits and, perhaps they may not want them.

I am happy to provide flags as such described in #5835. Please let me know what you prefer.

[mattsse]

this looks fine, however, I think we're missing support for configuring additional Cargo.toml dependencies in Abigen.

ideally serde would be added to the generated Cargo.toml as well.

currently we're only setting ethers:

https://github.com/gakonst/ethers-rs/blob/c702e6cdece63f3c47f3476c626024c11f4b8cec/ethers-contract/ethers-contract-abigen/src/multi.rs#L612-L612

so we need a way to also configure additional deps.

I think this should go here:

https://github.com/gakonst/ethers-rs/blob/c702e6cdece63f3c47f3476c626024c11f4b8cec/ethers-contract/ethers-contract-abigen/src/multi.rs#L367-L368

something like dependencies: Vec<String>

I don't want to add this as an additional arg to write_to_crate since this would break.

[Autoparallel]

Okay got it. Let me work on this.

[Autoparallel]

Would it instead be a simpler change to do this:

https://github.com/gakonst/ethers-rs/blob/c702e6cdece63f3c47f3476c626024c11f4b8cec/ethers-contract/src/lib.rs#L20-L30

In this lib.rs, just add

pub use serde::{Serialize, Deserialize}

And then replace the

add_derive("serde::Serialize")

with

add_derive("::ethers::contract::Serialize")

and of course the respective change for Deserialize. This is pretty minimal change required.

Trying this on my end worked fine in both crate/module bindings.

[mattsse]

public re-exports for third-party crates are hacky, really don't like this solution for this

[mattsse]

another fix would be to append the serde dependency manually in forge bind, but ideal solution would be support in abigen

[Autoparallel]

I was working on this for a while today inside ethers-rs. I am struggling to see a nice way to do it that doesn't involve more sweeping changes.

From what I can gather, there is a process like:

Abigen -> MultiAbigen -> MultiExpansion -> MultiExpansionResult -> MultiBindings -> MultiBindingsInner

where finally the Cargo.toml is written.

I considered adding a:

add_dependency()

for Abigen as this is where we would also be adding a derive as well. However, carrying this dependency throughout as a Vec<String> in each one of these structs seemed to lead to larger changes than I expected. I think this is doable, but does this seem like the approach we want?

Do you have more thoughts on this? I want to do it the right way here!

[mattsse]

hmm,

I think we only need this here:

-> MultiBindings -> MultiBindingsInner

this is where it's written:

foundry/crates/forge/bin/cmd/bind.rs

Lines 171 to 176 in 8c1a569

	bindings.write_to_crate(
	&self.crate_name,
	&self.crate_version,
	self.bindings_root(&artifacts),
	self.single_file,
	)?;

so similar to MultiBindings::format

[Autoparallel]

I made a fix here that pushes the relevant dependency into the bindings when generated via a crate. It will have to come after the change in ethers-rs of course!

[Autoparallel]

Once we get ethers-rs issue 2606 in, can we revisit this as well?

[gakonst]

This has been fixed in Ethers

[mattsse]

ty for this!

sorry for the delay