Implement build.rs bindings generation #3827
Conversation
This reverts commit 9f08fac.
crates/contracts-generate/src/lib.rs
Outdated
There was a problem hiding this comment.
Is there a reason why this needs to live in a separate crate and can't be made part of the existing contracts crate? Seems appropriate to have this in one place, no?
There was a problem hiding this comment.
This is build.rs code only, IMO it doesn't make sense to litter the main crate with this. Though I can move it to the build.rs of the contracts crate
There was a problem hiding this comment.
I think having this code in the contracts crate makes the most sense. It's not like it would be useful for anything else.
There was a problem hiding this comment.
Agree on keeping everything in the contracts crate.
There was a problem hiding this comment.
Done, please review the new build.rs. Keep in mind that I can't move any of the code inside build.rs to the crate because the build.rs is what builds the crate
crates/contracts-generate/src/lib.rs
Outdated
| all_derives: bool, | ||
| ) -> eyre::Result<proc_macro2::TokenStream> { | ||
| let bindings_path = self.bindings_path(bindings_folder.as_ref()); | ||
| let mut macrogen = SolMacroGen::new(bindings_path, self.name.clone()); |
There was a problem hiding this comment.
Could you check if this also supports json5? Would be nice to have the ability to add comments in the ABI file. This is just a nice to have so I wouldn't change the PR if this is not supported out of the box.
There was a problem hiding this comment.
The new dependencies pull in hundreds of additional transient dependencies. If there is a way to just expand the sol! marcro we've been using before this should do the trick, right?
There was a problem hiding this comment.
This is no longer the case. Furthermore, expanding the sol! macro on the side and moving the code without external tools doesn't seem to be possible
cc @squadgazzz @m-sz
jmg-duarte
force-pushed
the
jmgd/alloy/generate
branch
from
14fd890 to
0583047
Compare
crates/contracts-generate/src/lib.rs
Outdated
| struct NetworkArm(u64, (String, Option<u64>)); | ||
|
|
||
| impl ToTokens for NetworkArm { | ||
| fn to_tokens(&self, tokens: &mut proc_macro2::TokenStream) { | ||
| let chain_id = self.0; | ||
| let address = &self.1.0; | ||
| let block_number = match self.1.1 { | ||
| Some(block) => quote::quote! {Some (#block)}, | ||
| None => quote::quote! {None}, | ||
| }; | ||
| tokens.extend(quote::quote! { | ||
| #chain_id => Some((::alloy::primitives::address!(#address), #block_number)) | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| impl From<(u64, (String, Option<u64>))> for NetworkArm { | ||
| fn from((chain_id, info): (u64, (String, Option<u64>))) -> Self { | ||
| Self(chain_id, info) | ||
| } | ||
| } |
There was a problem hiding this comment.
Is it possible to split this PR? It contains too many changes that are hard to review. Can the initial version live without these helpers?
There was a problem hiding this comment.
This one here is to solve Martin's query in #3827 (comment)
I can split them though
There was a problem hiding this comment.
Ah, I didn't notice this comment. Alright, no worries. I'll try to review again.
There was a problem hiding this comment.
I have no clue how to review this other than by asking AI.
There was a problem hiding this comment.
I can do them one by one. I did this whole with AI too and did multilple rounds with different contexts. But this was more about proving that I was able to migrate everything with minimal changes on the main codebase
There was a problem hiding this comment.
Pull Request Overview
This PR refactors contract binding generation to use a build script approach with auto-generated code. The migration moves from runtime contract instance creation to compile-time code generation.
Key changes:
- Introduces
contracts-generatecrate with custom binding generation - Replaces runtime
InstanceExttrait with compile-time generated deployment info - Removes legacy
web3dummy transport and macros - Updates type references from
contracts::alloy::Providertoalloy::providers::DynProvider
Reviewed Changes
Copilot reviewed 29 out of 30 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/contracts-generate/Cargo.toml | New crate for build-time contract generation |
| crates/contracts-generate/src/lib.rs | Core generation logic with network deployment mapping |
| crates/contracts/build.rs | Build script defining all contract deployments |
| crates/contracts/src/lib.rs | Simplified to export generated bindings only |
| crates/contracts/src/alloy.rs | Deleted - replaced by generated code |
| crates/contracts/.gitignore | Ignores generated src/alloy directory |
| Multiple usage files | Updated to remove InstanceExt imports and use DynProvider |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Description
Implements bindings generation in
build.rs, adding this allowsChanges
contracts::alloy::Provider->alloy::providers::DynProvider)How to test
Run existing tests & compile