op-deployer: Add design doc on using Forge #325
Conversation
mslipper
force-pushed
the
feat/op-deployer-forge-engine
branch
from
bdcd3b1 to
dbcd0f1
Compare
| ## Risks & Uncertainties | ||
|
|
||
| None really. This is a pretty straightforward change that reduces risk overall. |
There was a problem hiding this comment.
I am definitely supportive of this. One flag is that we (and everyone else in the ecosystem) increasingly rely heavily on foundry, making it a good target for attackers, similar to the Safe UI. Out of scope for this design doc, but we should consider implementing a foundry update policy similar to our solidity upgrade policy to mitigate this risk
|
|
||
| ## Proposed Solution | ||
|
|
||
| We propose switching all contract calls in OP Deployer to use Forge instead of the Golang EVM. OP Deployer would |
There was a problem hiding this comment.
Would this allow us to delete some Golang evm code that was previously used by op-deployer? Or is that same code used elsewhere (e.g. in-memory go acceptance tests and one-click deployments?)
There was a problem hiding this comment.
I think we can delete all of the Golang EVM code.
| We originally adopted the Golang solution because we wanted to avoid adding a dependency on Forge. However, there | ||
| are many ways around this that do not incur the maintenance burdens described above. | ||
|
|
||
| ## Proposed Solution |
There was a problem hiding this comment.
Tactically, we could start by shelling out to forge for contract verification. That is a nice, isolated feature/cli-command. That would help us setup the forge invocation framework. Then we could extend it to invoke our solidity scripts via forge instead of via our golang evm code.
| OP Deployer will autodetect Forge based on the following heuristics: | ||
|
|
||
| 1. If Forge is already on the user's machine, and it is a supported version, use it. | ||
| 2. Otherwise, download a pinned version of Forge and use that. |
There was a problem hiding this comment.
Should this check/download happen as part of the just build recipe for op-deployer, or as part of the op-deployer runtime?
There was a problem hiding this comment.
I was thinking at runtime.
|
|
||
| ## Risks & Uncertainties | ||
|
|
||
| None really. This is a pretty straightforward change that reduces risk overall. |
There was a problem hiding this comment.
While I think the benefits far outweigh the negatives in this proposal, its probably still good to call out some of the risks/uncertainties:
-
parsing forge std output - not all forge commands provide the option to output results to json, therefore we will have to parse std output in some cases. Typically std output is not considered a reliable api, therefore it is more likely to change. This means we may take on the maintenance burden of adjusting to breaking changes to std output. This can be mitigated by pinning to known forge versions though, and the maintenance is likely much less effort than having to reimplement entire features
-
cross-platform compatibility complexity: go builds automatically handle cross-platform compilation for the go binary, but if we add forge binary as a dependency, then we must also ensure the forge binary supports the same OS the go binary is built for - do we want to embed the forge binary in the go binary, or treat forge as an external dependency? Do we want to leverage mise to help us manage the forge version so its consistent with the version used in other parts of the monorepo (maybe this would help reduce supply-chain risk if the versions managed by mise go through any sort of security check)? Or do we want op-deployer's forge version to be independent?
-
forge scriptworks within the context of a full forge project: will it work with just the embeddedforge-artifactstar file that is currently embedded within op-deployer? Since we have theuse_literal_content = "true"(ref) setting within our foundry.toml file, the solidity source code (including dependencies) is included in each forge-artifact. Hopefully this givesforge scripteverything it needs, but we should confirm
There was a problem hiding this comment.
- See the note in the doc - we'll need to modify contracts to output JSON (or some other parseable output) so that we don't need to parse the Forge output.
- You're correct, we'll need to pull in the right binary based on the user's architecture/OS. This can happen at runtime and doesn't need
mise. - It should work - forge has a
--no-compileoption that we can use.
No description provided.