Skip to content

chore(protocol): add lookahead store in protocol deployment#21349

Open
AnshuJalan wants to merge 4 commits intopermissionless-preconfs/lookahead-slashing-pathfrom
permissionless-preconfs/scripts
Open

chore(protocol): add lookahead store in protocol deployment#21349
AnshuJalan wants to merge 4 commits intopermissionless-preconfs/lookahead-slashing-pathfrom
permissionless-preconfs/scripts

Conversation

@AnshuJalan
Copy link
Copy Markdown
Collaborator

No description provided.

@claude
Copy link
Copy Markdown

claude bot commented Feb 19, 2026

Claude finished @AnshuJalan's task in 2m 3s —— View job


PR Review

  • Gather context
  • Read changed files
  • Review deployment script changes
  • Post review feedback

Overall the restructuring is clean and the two-phase deployment pattern (deploy with zero inbox → upgrade with real inbox) is well-commented. A few issues worth addressing:

Issues

1. Redundant third return value in _deployProposerChecker

packages/protocol/script/layer1/core/DeployProtocolOnL1.s.sol:220

The function returns lookaheadStoreProxy_ as its third return value, but the caller always discards it and re-derives the same information:

(address proposerChecker, address whitelist,) = _deployProposerChecker(config);
lookaheadStoreProxy = proposerChecker != whitelist ? proposerChecker : address(0);

The third return value is set to lookaheadStoreProxy_ inside the function when useLookaheadStore == true, which is exactly when proposerChecker != whitelist. Just use the returned value directly and drop the indirect inference: Fix this →

(address proposerChecker, address whitelist, lookaheadStoreProxy) = _deployProposerChecker(config);

2. Missing input validation when useLookaheadStore = true

packages/protocol/script/layer1/core/DeployProtocolOnL1.s.sol:197

When useLookaheadStore is enabled, neither urc nor lookaheadOverseer are validated. If urc is left as address(0) (the shell script default), LookaheadStore.isOperatorActive will call into address(0) — which will silently treat all URC operators as inactive (or revert depending on the EVM state). This is a footgun. Add a guard: Fix this →

if (config.useLookaheadStore) {
    require(config.urc != address(0), "URC not set for LookaheadStore");
    // deploy...
}

3. preconfSlasherL1 is always address(0)

packages/protocol/script/layer1/core/DeployProtocolOnL1.s.sol:202,228

Both the initial and upgraded LookaheadStore deployments hardcode address(0) for _preconfSlasherL1:

new LookaheadStore(address(0), address(0), whitelist_, config.urc)  // initial
new LookaheadStore(shastaInbox, address(0), whitelist, config.urc)  // upgrade

preconfSlasherL1 is an immutable that's used in isOperatorActive (checks slasher opt-in) and buildLookaheadCommitment (sets slasher field). With it at address(0):

  • isOperatorActive will always return false for the slasher commitment check (no operator will ever be opted into address(0))
  • Commitments will be signed against slasher address(0)

If this is intentional for the devnet phase (slasher not yet deployed), it should be documented prominently with a // TODO: add preconfSlasherL1 to config when available comment, and the PR description should call this out. It otherwise looks like an accidental omission. Fix this →

Nits

  • deploy_protocol_on_l1.sh already has DUMMY_VERIFIERS and PROPOSER_ADDRESS exported twice (pre-existing). The new env vars are correctly placed.
  • The _loadConfig method reads config.preconfWhitelist twice — once via vm.envAddress (line 113) and once via vm.envOr (line 117) — the second overwriting the first. Pre-existing, but worth cleaning up separately.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

if (vm.envBool("ACTIVATE_INBOX")) {
Inbox(payable(shastaInbox)).activate(config.l2GenesisHash);
}

P2 Badge Upgrade lookahead checker before activating inbox

In the new flow, _deployInbox can activate the inbox before _deployRollupContracts upgrades LookaheadStore from the placeholder implementation constructed with inbox = address(0) to one with the real inbox address. If ACTIVATE_INBOX=true, there is a deployment window where propose calls can start but checkProposer still enforces msg.sender == address(0), so proposals will revert until the later upgrade transaction lands.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +204 to +205
LookaheadStore.init, (config.contractOwner, config.lookaheadOverseer)
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Initialize LookaheadStore with deployer before upgrading

When USE_LOOKAHEAD_STORE is enabled, the proxy is initialized with config.contractOwner as owner, but the script then immediately calls LookaheadStore(lookaheadStoreProxy).upgradeTo(...) from the broadcaster account in _deployRollupContracts. Because upgrades are onlyOwner via EssentialContract, deployments where CONTRACT_OWNER differs from the broadcasting key (for example, a multisig owner) will revert on that upgrade step and cannot complete.

Useful? React with 👍 / 👎.

@AnshuJalan AnshuJalan force-pushed the permissionless-preconfs/scripts branch from 35a34a4 to 37723aa Compare February 19, 2026 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant