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

Support custom backends in forge crate #9262

Open
vanruch opened this issue Nov 5, 2024 · 1 comment
Open

Support custom backends in forge crate #9262

vanruch opened this issue Nov 5, 2024 · 1 comment
Labels
A-forge-builder Area: forge-builder A-internals Area: internals T-feature Type: feature T-post-V1 Area: to tackle after V1

Comments

@vanruch
Copy link

vanruch commented Nov 5, 2024

Component

Forge

Describe the feature you would like

❓ What are we doing?

We are building tooling that gives smart contract developers ways to create and verify zk-proofs in Solidity. We are utilizing revm with custom revm inspectors. We want to make it easy for developers to test their smart contracts using forge.

The problem

We have a few inspectors that are necessary for the vlayer contracts to work. There's currently no easy way to run forge with with custom revm inspectors.

What we had to do was copy a bunch of code to replace backend in test executor with our custom backend with custom revm inspector.

Unfortinately, it's very deep in the call tree with nested builders, which made it necessary for us to copy a lot of very slightly modified forge code.

main
  TestArgs::run()
    TestArgs::execute_tests()
      multi_runner = MultiRunnerBuilder::build()
      TestArgs::run_tests(multi_runner, ...)
        multi_runner.test()
          for each contract:
          multi_runner.run_test_suite()
            executor = ExecutorBuilder::build()
            runner = ContractRunner(executor, ...)
            runner.run_tests()
              for each test:
              runner.run_unit_test()
                executor.call()
                ... more calls inside executor...
                executor.call_with_env()
    We need to patch this  
    ----->        Create backend with inspector
                  backend.call()

This makes it hard for us to keep it up to date with the current foundry codebase.

Another big issue is the fact that there're a lot of large files in the /bin dir, with makes it impossible to import them in any other crate.

Can it be improved?

Yes, we believe so! And we think that it would be useful for other developers, if the forge crate was more modular and easy to use.

But talking solely about our use case, here's a minimal set of changes that would make our life much easier:

  • Move (ideally all files from) /bin/cmd to /src/cmd
  • Create Executor and ExecutorBuilder traits
  • Accept dyn ExecutorBuilder in TestArgs::execute_tests and pass it down the tree
  • ExecutorBuilder::build() would return Box<dyn Executor>
  • ContractRunner would also keep Box<dyn Executor>

This way, we'd only need to reimplement call_with_env in the executor.

We will be happy to discuss this proposal and prepare the pull request!

Additional context

No response

@vanruch vanruch added T-feature Type: feature T-needs-triage Type: this issue needs to be labelled labels Nov 5, 2024
@github-project-automation github-project-automation bot moved this to Todo in Foundry Nov 5, 2024
@zerosnacks zerosnacks added A-internals Area: internals A-forge-builder Area: forge-builder and removed T-needs-triage Type: this issue needs to be labelled labels Nov 6, 2024
@zerosnacks
Copy link
Member

Post-1.0 we are planning to work on forge-builder to greatly improve the way developers can interact with Foundry as a library. I would be supportive of an external PR but please be aware that it is not an area currently in focus pre-1.0.

@zerosnacks zerosnacks added the T-post-V1 Area: to tackle after V1 label Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-forge-builder Area: forge-builder A-internals Area: internals T-feature Type: feature T-post-V1 Area: to tackle after V1
Projects
Status: Todo
Development

No branches or pull requests

2 participants