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

protocol: forge scripts deploy in Go #61

Merged
merged 2 commits into from
Aug 13, 2024
Merged

Conversation

protolambda
Copy link
Contributor

Description

This introduces the idea of running the Forge scripts, primarily the chain deployment / genesis setup scripts, in Go.
This improves the integration with Go tooling and tests, to unlock the customization we need for things like Interop, and to generally improve test coverage of our deployments.

Metadata

Fix ethereum-optimism/optimism#11360

@protolambda protolambda requested review from tynes, mds1 and sbvegan August 6, 2024 22:51
## Devrel feedback

Historically devrel has not been included sufficiently in the deployment-flow design.
Known pain-points like inconsistency between the Go and forge genesis generation,
Copy link

Choose a reason for hiding this comment

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

Good call out here, for example, a recent partner was having trouble using the solidity genesis allocs creation on the op-contracts/v1.3.0 release so they opted for something ~200 commits into the future that played nicely with the tooling instead of using the legacy genesis creation method.

With that being called out, if this lands after a L1 contract release, it would be nice to have this type of functionality cut in a different minor release so its accessible for new chain deployments between say op-contracts/v1.6.0 and op-contracts/v1.7.0.


Historically devrel has not been included sufficiently in the deployment-flow design.
Known pain-points like inconsistency between the Go and forge genesis generation,
unclear allocs, and monolithic deploy-config are being addressed, but more feedback may still improve the design.
Copy link

Choose a reason for hiding this comment

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

The modular deploy configuration will be nice improvement, especially for our beta features (CGT and Alt-DA Mode). Similar to my comment above, it would be nice to have this worked into the CGT contract releases.

Copy link

@sbvegan sbvegan left a comment

Choose a reason for hiding this comment

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

I'm very excited for any contract deployment quality of life improvements. Beyond my initial comments, I'm going to work on collecting additional customer contract deployment pain in this notion document. I'll reach out to developer support who may have additional insight.

@tynes tynes mentioned this pull request Aug 7, 2024
This document proposes the latter: run the Forge scripts within Go,
such that we do not need multiple steps of Forge script sub-process to build a state for testing / genesis tooling.

## Running Forge scripts in Go
Copy link
Contributor

Choose a reason for hiding this comment

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

I have slight concern about possible bugs in the foundry Go precompile implementation resulting in e2e tests not running against "the same" contracts. I suppose the precompile isn't super complex, but adding layers of abstraction does make me feel worried


For Go tests, this would be the local FS.

For Go genesis tooling, this might be a bundled FS, or one from a versioned release tarball.
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you proposing that this is used outside of testing, ie for L2 genesis generation?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see in the follow up section this is mentioned

@tynes
Copy link
Contributor

tynes commented Aug 7, 2024

I think that this could be a good approach although I would need more convincing to think its a good idea to use this for anything other than testing. I would much rather contribute features directly to foundry that help make our deployments better rather than us building tooling that subsumes foundry

Cheatcodes are essentially responses to `STATICCALL`s to a special system address, acting like a precompile,
with functions that interact with the EVM environment.

We do not have to support all Forge cheatcodes; just the subset used by the OP-Stack scripts would be sufficient.
Copy link
Contributor

Choose a reason for hiding this comment

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

Aside from implementation risk, another risk is the subset used by OP Stack scripts will continue to grow, and I worry contract devs might want to use a new cheat, but then PRs get blocked by needing go support for that cheat.

Ultimately, is the resulting state from a script what the go tests need? If so, I think it will be a lot simpler to call the forge script from go, with a way conditionally call vm.dumpState at the end. Go can then read in the resulting file that was written and apply that state. This is effectively what kontrol does to initialize proofs so it doesn't need to execute the EVM/cheats

If we don't want to use the filesystem as an interface, forge has a --json arg to output JSON to stdout. We can probably get a forge script --json --dumpstate feature added so that it automatically dumps state to the JSON as part of stdout


Using an FS helps simplify the way we interact with forge-artifacts.

## Semver the scripts
Copy link
Contributor

Choose a reason for hiding this comment

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

Great idea

Co-authored-by: Matt Solomon <matt@mattsolomon.dev>
@protolambda
Copy link
Contributor Author

protolambda commented Aug 8, 2024

Summary of design-review call:

The primary concern is if whether or not we can use forge script as-is to serve tooling and op-e2e.

Arguments for:

Arguments against:

  • Need for well-defined inputs/outputs
    • The more we side-channel data through files, the more the Go integration has to prepare and manage edge-cases of (caches, missing data, IO errors, etc.).
    • Difficult to bundle Go L2 genesis generation and Forge L2 genesis generation, do to multi-step process and intermediate files. Hurts devrel experience.
  • Performance concerns
    • Intermediate disk access
    • Encoding the configs in JSON (or toml), split between 32 byte leafs, reconstructing them multiple times, JSON parsing again, etc.
    • Multiple state dumps / loads per test setup.
    • FFI calls to forge processes (and forge versioning etc. affecting tests)
  • No cache-invalidation complexity

And L1 deployments can be unified by making a broadcast a no-op in the Go context, so we can run the same deploy functions, but generate state in-place.

There is some hesitation, but we can try the Go forge-script approach, and fall back to the other approach if the Go approach does not work.

@tynes tynes merged commit efb33c6 into main Aug 13, 2024
@tynes tynes deleted the forge-scripts-deploy-in-go branch August 13, 2024 05:02
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.

Interop: design doc for refactor of deployer tooling, using Go forge script execution
4 participants