[joanestebanr]

🔄 Changes Summary

• exit_certificate: new standalone step --step pending_claims (not part of all) that lists the L2→L1 asset bridges (leaf_type=0) emitted before the exit certificate that are still unclaimed on L1 (isClaimed on the L1 bridge) and writes step-pending-l2-bridges.json. It reads the Step G local exit tree DB, so it needs step g first.
• exit_certificate_claimer: optional pendingBridgesPath (auto-detected in the exit_certificate output dir). /bridges and /claim-params also serve those bridges, with proofs verified against the certificate's new_local_exit_root. Pending bridges are matched by deposit_count, so identical leaves are not collapsed.
• e2e (test/e2e-exit_certificate): new 46-check_pending_bridges.sh; 60-claim_exit_certificate_funds.sh checks that the claimer lists the pending bridge, claims it, and that isClaimed is true on L1.
• Docs: READMEs, CLAUDE.md and SPEC.md updated.

⚠️ Breaking Changes

• 🛠️ Config: none (pendingBridgesPath is optional).
• 🔌 API/CLI: none. New --step pending_claims value.
• 🗑️ Deprecated Features: none.

📋 Config Updates

• 🧾 Claimer: new optional pendingBridgesPath (derived from the exit_certificate output dir when the file exists).

🔌 API Updates

🔌 Bridge service API

• None.

🔌 Proxy API

• None.

🔌 Others API

• Exit certificate claimer: GET /bridges and GET /claim-params append the pending pre-certificate bridges after the certificate's exits when the pending file is present. Not a breaking change; without the file the responses are unchanged. metadata of pending entries in /bridges is the raw bridge metadata.

✅ Testing

• 🤖 Automatic: go test ./tools/exit_certificate/... ./tools/exit_certificate_claimer/... (new unit tests for the step and the claimer, with a real lite DB). New e2e step 46-check_pending_bridges.sh and extra checks in 60-claim_exit_certificate_funds.sh (not run yet).
• 🖱️ Manual: not yet validated against zkEVM mainnet data / a real L1 (isClaimed(depositCount, l2NetworkId) for rollup leaves).

🐞 Issues

• Closes [BUG] exit_certificate_claimer cannot claim unclaimed bridges emitted before the exit certificate #1928

🔗 Related PRs

• None.

📝 Notes

• Messages (leaf_type=1) are out of scope: the claimer does not support claimMessage.
• Draft: pending e2e run, make lint and validation with real data.

🤖 Generated with Claude Code

[joanestebanr]

@claude review

[claude]

Claude finished @joanestebanr's task in 55s —— View job

---

Review of PR #1929: exit_certificate pending step and claimer support for unclaimed pre-certificate bridges

• Read the PR diff and the surrounding code (step_e.go, step_g2.go, bridgesyncerlite, claimer)
• Review for bugs, security, performance and style
• Post the review

I did not run make lint, the unit tests or the e2e scripts. This is a read-only review of the diff, so everything below is from reading the code.

---

🚀 What's New

• exit_certificate --step pending_claims is a standalone step that is not part of all and not in orderedSteps.
  • It reads the Step G2 lite DB (step-g-l2bridgesyncerlite.sqlite) and drops the last BridgeExitCount leaves, which are the certificate's own exits.
  • It keeps asset leaves destined to L1 and calls isClaimed(depositCount, l2NetworkId) on the L1 bridge. It reuses the batching, encodeIsClaimed and parseClaimedResults helpers from Step E.
  • It writes step-pending-l2-bridges.json.
• Claimer:
  • There is a new optional pendingBridgesPath, auto-detected in the exit_certificate output dir.
  • /bridges and /claim-params append the pending bridges after the certificate's exits, de-duplicated by deposit count.
  • Pending proofs are built by deposit count and checked with tree.VerifyProof against the certificate's NewLocalExitRoot.
  • Claim building is refactored into anchorClaim, shared by both paths.
• e2e and docs: a new 46-check_pending_bridges.sh, extra checks in 60-claim_exit_certificate_funds.sh, and updated READMEs, CLAUDE.md and SPEC.md.

🐛 Bug Fixes

None.

📋 Config Updates

Claimer config gains an optional pendingBridgesPath. It is resolved relative to the config file and defaults to <exit_certificate outputDir>/step-pending-l2-bridges.json when that file exists. The default is empty, meaning the feature is off.

{ "pendingBridgesPath": "./output/step-pending-l2-bridges.json" }

⚠️ Breaking Changes

None. Without the pending file, the claimer responses are unchanged.

---

Overall

The design is sound and the code is tidy. Looking up leaves by deposit count rather than leaf hash is the right call for identical exits. Verifying each pending proof against NewLocalExitRoot before serving it is a good safety net. The isClaimed(depositCount, l2NetworkId) encoding matches how the L1 bridge computes leafIndex + sourceBridgeNetwork * 2^32 for rollup leaves, and Step E's parseClaimedResults already fails loudly on a missing or unparseable result. I found no blocking issues. The points below are what I would address before taking it out of draft.

🟡 Suggested

1. A missing step-g DB is not reported (step_pending.go:51). bridgesyncerlite.New with only DBPath runs openDatabase, which in all likelihood creates the sqlite file if it is absent. If the DB is missing but step-g-new-local-exit-root.json exists, the step opens an empty DB and either fails with the misleading "certificate has N exits but the tree has 0 leaves" error, or returns an empty list silently when BridgeExitCount == 0. A short os.Stat(dbPath) before opening would give a clear "run step g first" error and would not create stray files. The BridgeExitCount == 0 path (G2 returns early and builds no tree) deserves a test or an explicit log message.

2. Pending bridges are not sanity-checked in Claimer.Check (run.go:85). Proofs are verified lazily, per request, in pendingClaim. A stale or mismatched step-pending-l2-bridges.json (for example from a previous run, or one generated before step g was re-run) only surfaces as a 500 on /claim-params, and /bridges would still list the entries. Consider verifying all pending entries once at startup, which is cheap, or at least failing fast in Check.

3. isClaimed is queried at "latest" (step_pending.go:131). This is fine for a snapshot, but the result goes stale as soon as anyone claims, so the file is a point-in-time view. The README should say so, and /claim-params could usefully re-check isClaimed on L1 at request time. Otherwise a stale entry produces a claim tx that reverts with AlreadyClaimed. Optionally pin the call to a block (for example finalized, using BlockNumberFinality per the repo conventions) so reruns are reproducible.

4. Amount *big.Int in the JSON (step_pending.go:25). big.Int marshals as a bare JSON number. That is fine for Go consumers, but amounts above 2^53 lose precision in jq and JS, which is why the e2e script has to compare both string and number forms (46-check_pending_bridges.sh:67). A decimal string would be more robust for operators who inspect the file. A nil amount unmarshals to nil, and bigToString handles it but the leaf hash does not, so a hand-edited file could panic in Hash(). A cheap guard in LoadPendingBridges would avoid that.

5. ListBridges and BuildClaimParams duplicate the dedupe logic (claimer.go). Both build certDepositCounts and then filter pending. A small helper, such as pendingFor(destAddr, depositCount, exclude), would remove the duplication. ListBridges also does not filter unclaimed or claimed state, which is fine because it is documented as a snapshot.

6. Naming and the parseClaimedResults contract. checkClaimedOnL1 builds a throwaway []L1Deposit only to satisfy parseClaimedResults's signature. Passing deposit counts directly would be a small cleanup, but it touches Step E, so it is optional.

🟢 Optional

• selectPendingCandidates takes b := bridges[i], which copies the whole BridgeLeaf. Indexing bridges[i] directly, or ranging by pointer, avoids that.
• run.go adds a floating comment about orderedSteps directly above lastAutoStep, and it reads as if it belongs to the next declaration. Moving it next to orderedSteps would be clearer.
• The --step usage string and the "unknown step" error list grow long. A shared constant for the valid steps would avoid the two copies drifting.
• Messages (leaf_type=1) pending on L1 are silently excluded, which is documented. It would be useful to log how many were skipped, so operators know funds exist that the claimer cannot claim.
• Extend the pending test coverage with the error paths: the L1 RPC returning an error or a missing result, certificateExits > len(bridges), and a pending entry whose leaf does not match the tree (the VerifyProof failure branch).

Security

There is nothing exploitable in the diff. The claimer serves data derived from local files and an L1 call, and the pending file is operator-supplied with proofs re-verified against the certificate root. The main risk is operational: serving stale or already-claimed bridges (see points 2 and 3).

Testing and status

The PR description says the e2e run, make lint and real-data validation are still pending. I'd treat those as the gate before merging, in particular isClaimed against a real L1 for rollup leaves.

---

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-02T08:53:49.429526Z	f7aef56	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f7aef56c08

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Handle certificates with no new bridge exits

When bridgeExitCount is zero, RunStepG2 returns early without creating step-g-l2bridgesyncerlite.sqlite; opening that path here creates and migrates a new empty SQLite database. Consequently, a valid certificate with no newly synthesized exits but with unclaimed pre-certificate L2→L1 leaves produces an empty pending list instead of reading those leaves from the Step G1 database. Use the G1 database in this case or ensure G2 always copies/builds its database before this step reads it.

Useful? React with 👍 / 👎.

[joanestebanr]

Addressed in e9b1f90, with a different approach than reading the G1 DB. When the certificate has no bridge exits, G2 returns early with NewLocalExitRoot = EmptyLER, so the pre-certificate leaves could not be proven against that root anyway (the claimer would reject them in VerifyProof). The step now fails explicitly in that case, and also checks with os.Stat that the step-g DB exists before opening it, so it no longer creates an empty sqlite or reports an empty list. Both are covered by tests.

[joanestebanr]

Follow-up on the review comments (commits e9b1f90 and 2051947).

Done

• Missing step-g DB: pending_claims now checks the file exists and fails with "run step g first" instead of creating an empty DB (test added).
• Certificate with no bridge exits: explicit error instead of an empty list. G2 builds no tree in that case and NewLocalExitRoot is EmptyLER, so the old leaves could not be proven (test added).
• selectPendingCandidates no longer copies BridgeLeaf.
• New test: an unparseable isClaimed result aborts the step.
• The pending_claims comment now sits next to orderedSteps.

Intentionally not changed

• Verifying all pending entries in Claimer.Check: a stale file would stop the whole claimer from starting, including the certificate's own exits. Today the failure stays scoped to the affected request.
• Re-checking isClaimed at request time or pinning a block: the claimer has no L1 RPC, and the README already says the file is a snapshot and to rerun the step on AlreadyClaimed.
• Amount as a decimal string / nil guard: would change the file format and break existing files and the e2e script; a nil amount only happens with a hand-edited file.
• Dedupe helper in ListBridges/BuildClaimParams, parseClaimedResults signature (touches Step E) and the shared steps constant: low value for the churn or out of scope for this PR.

Happy to revisit any of these if you feel strongly.