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

feat: Add contracts to compile flag to zksolcconfig #253

Merged
merged 8 commits into from
Feb 9, 2024

Conversation

Jrigada
Copy link
Contributor

@Jrigada Jrigada commented Feb 7, 2024

What πŸ’»

  • Added contracts_to_compile to Zksolcconfig and to TestArgs, BuildArgs and ScriptArgs

Why βœ‹

  • Required to be able to compile the desired files when testing, building and scripting.

Evidence πŸ“·

  1. zkforge test --contracts-to-compile "Sign.t.sol"

  2. image

  3. image

@Jrigada Jrigada marked this pull request as ready for review February 8, 2024 13:16
@Jrigada Jrigada marked this pull request as draft February 8, 2024 14:31
@Jrigada Jrigada self-assigned this Feb 8, 2024
@Jrigada Jrigada marked this pull request as ready for review February 8, 2024 17:21
@dutterbutter
Copy link
Collaborator

dutterbutter commented Feb 8, 2024

@Jrigada thinking about this more deeply. We should also provide the inverse of this.

For example --no-contracts-to-compile which will compile everything but the specified contract(s). I think this is far more useful.

For a practical example, compiling https://github.com/bgd-labs/aave-delivery-infrastructure will fail at first due to the use of EXTCODECOPY in the tests/PayloadScripts.t.sol#L142. Ideally, users would then just run something like zkforge zkbuild --no-contracts-to-compile=PayloadScripts.t.sol. Let's also use ^^ this as a test case.

Thoughts?

@dutterbutter
Copy link
Collaborator

@Jrigada works great. Just the one comment.

@dutterbutter dutterbutter merged commit f22b588 into main Feb 9, 2024
9 of 10 checks passed
@dutterbutter dutterbutter deleted the add-contracts-to-compile-flag branch February 9, 2024 15:33
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

2 participants