-
Notifications
You must be signed in to change notification settings - Fork 40
Make L2 predeploys configurable via --l2-predeploy-json for opstack genesis #218
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a configurable mechanism for deploying contracts at genesis via JSON files for the opstack recipe. It replaces the previous --enable-entrypoint flag with a more generic and extensible --l2-predeploy-json flag that accepts JSON file paths describing L2 predeploy accounts.
Key changes:
- Introduces a JSON-driven predeploy mechanism allowing users to inject custom contracts into L2 genesis
- Adds
PredeployAllocstruct andloadPredeployAlloc()function to parse and validate JSON predeploy configurations - Wires the existing ERC-4337 EntryPoint v0.7 JSON through this new mechanism
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
playground/utils/entrypoint_v0.7.json |
Adds JSON configuration for ERC-4337 EntryPoint v0.7 contract to be deployed at genesis |
playground/artifacts.go |
Implements PredeployAlloc type, loadPredeployAlloc() parser, and integrates predeploy JSON files into L2 genesis alloc |
playground/recipe_opstack.go |
Adds --l2-predeploy-json flag and wires it to the artifacts builder |
README.md |
Documents the new --l2-predeploy-json flag with usage examples |
π‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
playground/artifacts.go
Outdated
| } | ||
| if p.Address == "" { | ||
| return "", nil, fmt.Errorf("predeploy JSON missing address") | ||
| } |
Copilot
AI
Dec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The address field should be validated to ensure it's a valid Ethereum address. Currently, any string can be passed and will be directly inserted into the genesis alloc, which could lead to invalid genesis configuration if an improperly formatted address is provided.
Consider adding validation:
if !gethcommon.IsHexAddress(p.Address) {
return "", nil, fmt.Errorf("predeploy JSON has invalid address format: %q", p.Address)
}| } | |
| } | |
| if !gethcommon.IsHexAddress(p.Address) { | |
| return "", nil, fmt.Errorf("predeploy JSON has invalid address format: %q", p.Address) | |
| } |
playground/artifacts.go
Outdated
| account := map[string]interface{}{ | ||
| "balance": p.Balance, | ||
| "nonce": p.Nonce, |
Copilot
AI
Dec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Balance and Nonce fields are always included in the genesis alloc account, even when they're empty strings. This could potentially lead to invalid genesis data. Consider handling empty values more explicitly:
account := map[string]interface{}{}
if p.Balance != "" {
account["balance"] = p.Balance
}
if p.Nonce != "" {
account["nonce"] = p.Nonce
}Or set default values if these fields should always be present:
account := map[string]interface{}{
"balance": p.Balance,
"nonce": p.Nonce,
}
if p.Balance == "" {
account["balance"] = "0x0"
}
if p.Nonce == "" {
account["nonce"] = "0x0"
}| account := map[string]interface{}{ | |
| "balance": p.Balance, | |
| "nonce": p.Nonce, | |
| balance := p.Balance | |
| if balance == "" { | |
| balance = "0x0" | |
| } | |
| nonce := p.Nonce | |
| if nonce == "" { | |
| nonce = "0x0" | |
| } | |
| account := map[string]interface{}{ | |
| "balance": balance, | |
| "nonce": nonce, |
playground/artifacts.go
Outdated
| var p PredeployAlloc | ||
| if err := json.Unmarshal(raw, &p); err != nil { | ||
| return "", nil, fmt.Errorf("failed to unmarshal predeploy JSON: %w", err) | ||
| } | ||
| if p.Address == "" { | ||
| return "", nil, fmt.Errorf("predeploy JSON missing address") | ||
| } | ||
|
|
||
| account := map[string]interface{}{ | ||
| "balance": p.Balance, | ||
| "nonce": p.Nonce, | ||
| } | ||
| if p.Code != "" { | ||
| account["code"] = p.Code | ||
| } | ||
| if len(p.Storage) > 0 { | ||
| account["storage"] = p.Storage | ||
| } | ||
|
|
||
| return p.Address, account, nil |
Copilot
AI
Dec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hex values (balance, nonce, code) from the JSON are not validated to ensure they're properly formatted hex strings. Invalid hex values would be silently passed through to the genesis alloc, which could cause issues when the genesis is processed.
Consider adding validation for hex string fields:
// After unmarshaling
if p.Balance != "" && !isValidHex(p.Balance) {
return "", nil, fmt.Errorf("predeploy JSON has invalid balance hex format: %q", p.Balance)
}
if p.Nonce != "" && !isValidHex(p.Nonce) {
return "", nil, fmt.Errorf("predeploy JSON has invalid nonce hex format: %q", p.Nonce)
}
if p.Code != "" && !isValidHex(p.Code) {
return "", nil, fmt.Errorf("predeploy JSON has invalid code hex format: %q", p.Code)
}Where isValidHex checks for "0x" prefix and valid hex characters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicee
| if err != nil { | ||
| return nil, fmt.Errorf("failed to load L2 predeploy from %q: %w", path, err) | ||
| } | ||
|
|
Copilot
AI
Dec 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If multiple predeploy JSON files specify the same address, the later file will silently overwrite the earlier one without any warning. This could lead to unexpected behavior where one predeploy contract unintentionally replaces another.
Consider adding a check for duplicate addresses:
for _, path := range b.predeployJSONFiles {
data, err := os.ReadFile(path)
if err != nil {
return nil, fmt.Errorf("failed to read L2 predeploy JSON %q: %w", path, err)
}
addr, account, err := loadPredeployAlloc(data)
if err != nil {
return nil, fmt.Errorf("failed to load L2 predeploy from %q: %w", path, err)
}
if _, exists := allocs[addr]; exists {
return nil, fmt.Errorf("duplicate predeploy address %s found in %q", addr, path)
}
allocs[addr] = account
}| if _, exists := allocs[addr]; exists { | |
| return nil, fmt.Errorf("duplicate predeploy address %s found in %q", addr, path) | |
| } |
| @@ -0,0 +1,7 @@ | |||
| { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be great if we could deploy v06 - v09, and either a factory or an account to test on for each.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thinking is users, can have there own jsons that they pass the path in via params on startup.
WDYT ?
README.md
Outdated
|
|
||
| - `--external-builder`: URL of an external builder to use (enables rollup-boost) | ||
| - `--enable-latest-fork` (int): Enables the latest fork (isthmus) at startup (0) or n blocks after genesis. | ||
| - `--l2-predeploy-json` (string[]): One or more paths to JSON files describing L2 predeploy accounts to inject into the L2 genesis (e.g. EntryPoint, paymasters, custom system contracts). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think just calling it use-predeploys or something (L2 seems irrelevant)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, main thinking was because this adds it to the genesis of the l2 config, not l1. The repo deploys both an L1 and an L2. This mechanism only affects the L2 genesis config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But since this flag is unique for the L2 recipe, that already includes the context that this flag only runs there so it makes more sense to me to call it --predeploy-json only and remove that namespace since it is inferred.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm ok, sure!
playground/artifacts.go
Outdated
|
|
||
| // PredeployAlloc is a generic JSON schema for describing a single L2 | ||
| // predeploy account to be injected into the L2 genesis alloc. | ||
| type PredeployAlloc struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is already an alloc.GeneissAccount object in the go-ethereum repo with this data and the unmarshal/marshal methods. Ideally we can use that instead of this custom format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh didn't know about that one, nice find catch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated!
| @@ -0,0 +1,9 @@ | |||
| { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This a config for launching an entry point v07 contract for 4437
Description π¨βπ» ;
This PR adds support for passing in JSON files that define contracts you want predeployed at genesis. For the opstack recipe, we sometimes need to inject contract bytecode + metadata directly into the L2 alloc, and this gives us a simple, configurable way to do that.
We now have a generic JSON-driven predeploy flow, and the existing EntryPoint v0.7 JSON is wired through this new path. Anyone who needs things like ERC-4337 EntryPoint, paymasters, or other system contracts can just pass in JSON paths and have them included deterministically during genesis.