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

Feature/yul compilation #1006

Closed
wants to merge 31 commits into from
Closed

Feature/yul compilation #1006

wants to merge 31 commits into from

Conversation

ControlCplusControlV
Copy link

Moving here as this is a continuation of #994 But this adds ABI support for Yul, I've added more comments to explain the flow.

@mattsse for a better explanation

So the main motivation behind all of this is Yul artifacts have no ABI by default, so instead we compile a .abi.sol file with a matching filename which has it's ABI ripped off and attached to the proper Yul artifact. This allows for the use of a user defined ABI on a Yul artifact.

They key issue I was stuck on for the longest time is its incredible difficult to convert from a generic artifact to a Contract, then back. This conversion was needed because I followed the following workflow

  1. Find .abi.sol artifacts, and save the ABI from them
  2. Find .yul artifact with a matching name, convert it to a contract, and modify the ABI. Then replace its artifact with the modified contract (which I converted back to an Artifact)

Detection of .abi.sol just involves chopping the file name and stashing away the ABI with the name of the Yul Artifact its supposed to replace. (Stashed in yul_abi_targets)

yul_contracts is a mapping of each Yul file to the contract object for that file, so that when inserting the ABI later it makes the contract object easy to lookup, then insert the ABI into the looked up contract object. Then the new Contract Object can be inserted in the place of the old Artifact.

The mapping yul_abi_targets contains all the needed artifact pieces (parts needed to turn the contract saved into yul_contracts into a proper ArtifactFile) and since both use the same key it is easy to correlate between the 2.

Happy to answer any other questions you have, but hopefully that and the new comments are able to clear up something!

0xc0drr and others added 29 commits March 7, 2022 10:46
…the matching .yul contract to inject the abi into the yul artifact.
…verwrite the yul_artifact with the newly injected abi
…t instead of an abi, then during yul_abi injection, create an artifact from a contract
@ControlCplusControlV
Copy link
Author

@gakonst you wanted my changes moved to a PR like this one correct?

@gakonst
Copy link
Owner

gakonst commented Mar 13, 2022

@ControlCplusControlV just trying to understand the motivation / solution taken here:

Motivation: We compile Yul files, but they have no ABI artifact
Solution: We detect interface contracts ending in .abi.sol and tie them to the corresponding Yul contract with the same name.

Does that sound right?

Question: Is the .abi.sol + named interface path reasonable? Maybe there's a more generalized way?

Curious what @mattsse thinks given he's been owning ethers-solc.

@ControlCplusControlV
Copy link
Author

@gakonst yeah that's pretty much spot on. As for .abi.sol named path being reasonable I would think so, as without using a Solidity interface it would be difficult for people to actually develop using it, although we could load it from a json file. The only issue I had against doing that to begin is I felt that it took people out of using Foundry as they would have to go through another process to find an ABI to use for their contracts. Using a json file is also harder to iterate with and and actually see the methods you are exposing

@mattsse
Copy link
Collaborator

mattsse commented Mar 14, 2022

@ControlCplusControlV

Find .abi.sol artifacts, and save the ABI from them
Find .yul artifact with a matching name, convert it to a contract, and modify the ABI. Then replace its artifact with the modified contract (which I converted back to an Artifact)

one question before we can find the best solution for this problem:

The *.abi.sol interface contracts are a yul convention and provided by the user?
So it's safe to say that a .yul contract has a matching .abi.sol interface file?

@ControlCplusControlV
Copy link
Author

ControlCplusControlV commented Mar 14, 2022

@mattsse Yes the *.abi.sol would be a new convention we made up, and the files would be provided by the user. It's not always safe to assume there is a match .abi.sol, as the user could just not define an ABI for their Yul contract, which should be taken into account with my approach. It is ok to assume however that generally people would want to write an *.abi.sol for each Yul contract of theirs to make interacting with it easier, with the names of <contract>.yul matching with <contract>.abi.sol

@ControlCplusControlV
Copy link
Author

@mattsse can re-sync with master, but wanted to make sure you didn't have any issues with my design. No worries if you've been busy but just wanted to remind you as the branch is falling behind a bit and waiting to update on approval to not clutter the commit history too much (although this is getting squashed for sure)

This pull request was closed.
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

6 participants