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

migrate(forge-bind): to alloy #7919

Merged
merged 46 commits into from
Jun 14, 2024
Merged

migrate(forge-bind): to alloy #7919

merged 46 commits into from
Jun 14, 2024

Conversation

yash-atreya
Copy link
Collaborator

@yash-atreya yash-atreya commented May 13, 2024

Motivation

Final part of alloy migration

Solution

In order to do this, we have to make use of parts of code that make up the sol! macro to create a SolMacroGen type that is similar to the Abigen type in ethers.

We're reading the JSON abi files (the artifacts) and retrieving the ABI from it then converting that to a Solidity TokenStream and ultimately convert that to a SolInput that should be passed to the expander that is used by the sol! macro here: https://github.com/alloy-rs/core/blob/85aecdf54c56da20f274a21f372d3d37b8c118ca/crates/sol-macro/src/lib.rs#L281.

However the issue is that the expand module is private in the sol-macro crate and cannot be made public since it's not a macro.

@DaniPopes @mattsse I can move the expand module to a new crate sol-macro-expander, make it public and use it in foundry??

TODO

  • Artifacts and files filter
  • Make expand public by moving to a new crate.
  • Write to crate and module

@yash-atreya yash-atreya marked this pull request as draft May 13, 2024 14:54
@yash-atreya
Copy link
Collaborator Author

Proposing to make the utils method tokens_for_sol https://github.com/alloy-rs/core/blob/85aecdf54c56da20f274a21f372d3d37b8c118ca/crates/sol-macro-input/src/json.rs#L74 also public in alloy-rs/core, we can then remove the syn, proc_macro2 deps.

@yash-atreya
Copy link
Collaborator Author

yash-atreya commented May 15, 2024

TODO:

  • Filter for selecting and excluding ABIs in alloy
  • Adding derive and sol attributes
  • Cleanup nits and error handling todos

@yash-atreya yash-atreya marked this pull request as ready for review May 21, 2024 07:53
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

@DaniPopes is best equipped to give directions here, but I think this sounds reasonable

crates/forge/bin/cmd/bind.rs Outdated Show resolved Hide resolved
crates/forge/src/sol_macro_gen.rs Outdated Show resolved Hide resolved
crates/forge/src/sol_macro_gen.rs Outdated Show resolved Hide resolved
crates/forge/src/sol_macro_gen.rs Outdated Show resolved Hide resolved
crates/sol-macro-gen/src/sol_macro_gen.rs Outdated Show resolved Hide resolved
crates/sol-macro-gen/src/sol_macro_gen.rs Outdated Show resolved Hide resolved
crates/sol-macro-gen/src/sol_macro_gen.rs Outdated Show resolved Hide resolved
@yash-atreya
Copy link
Collaborator Author

@DaniPopes merge?

crates/sol-macro-gen/src/sol_macro_gen.rs Outdated Show resolved Hide resolved
crates/sol-macro-gen/src/sol_macro_gen.rs Outdated Show resolved Hide resolved
crates/sol-macro-gen/src/sol_macro_gen.rs Outdated Show resolved Hide resolved
crates/sol-macro-gen/src/sol_macro_gen.rs Outdated Show resolved Hide resolved
crates/sol-macro-gen/src/sol_macro_gen.rs Outdated Show resolved Hide resolved
crates/sol-macro-gen/src/sol_macro_gen.rs Outdated Show resolved Hide resolved
crates/sol-macro-gen/src/sol_macro_gen.rs Outdated Show resolved Hide resolved
crates/sol-macro-gen/src/sol_macro_gen.rs Show resolved Hide resolved
crates/sol-macro-gen/src/sol_macro_gen.rs Outdated Show resolved Hide resolved
yash-atreya and others added 4 commits June 13, 2024 06:33
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
crates/sol-macro-gen/src/sol_macro_gen.rs Outdated Show resolved Hide resolved
crates/forge/bin/cmd/bind.rs Outdated Show resolved Hide resolved
crates/sol-macro-gen/src/sol_macro_gen.rs Show resolved Hide resolved
@DaniPopes DaniPopes requested a review from mattsse June 14, 2024 16:45
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

nice work!

@mattsse mattsse merged commit 4240376 into master Jun 14, 2024
20 checks passed
@mattsse mattsse deleted the yash/alloy-forge-bind branch June 14, 2024 16:55
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.

None yet

3 participants