feat(exit-certificate): split Step G into G1/G2 with off-chain LER + shadow-fork verification

[joanestebanr]

🔄 Changes Summary

Speeds up and reworks Step G (NewLocalExitRoot), splitting it into G1 and G2 and adding an off-chain computation path.

Step G split

• G1 (step_g1.go): lite-syncs the L2 bridge history from genesis up to the target block into a persistent lite DB, using the new bridgesyncerlite package (reads BridgeEvent logs in parallel and builds a bridge exit tree byte-for-byte compatible with bridgesync). Resolves the shadow-fork block.
• G2 (step_g2.go, formerly step_g.go): computes NewLocalExitRoot.
  • Default (verifyNewLocalExitRootUsingShadowFork=true): spins up the Anvil shadow-fork, replays every bridge exit in parallel (send/collect pipeline), reorders the certificate to the on-chain deposit order, and verifies the lite exit tree root against the contract's getRoot().
  • Off-chain (=false): computes the root purely from the lite tree (G1 bridges + the certificate's exits, in order) — no Anvil.

Step I always uses the reordered certificate

• In single-step mode, Step I now always reads step-g-reordered-certificate.json (run Step G first) instead of falling back to the capped/Step-E certificates, so the final certificate always matches the computed NewLocalExitRoot. (runAll already flowed the in-memory reordered cert.)

Removed

• options.depositOrderSource (the events/bridgesync modes) and the production-bridgesync recovery (step_g_bridgesync.go). Deposit order now comes from the replay's BridgeEvents (shadow-fork) or the certificate order (off-chain). StepGResult.ShadowForkFirstBlock dropped.

New bridgesyncerlite package

• Minimal bridge syncer: parallel eth_getLogs, persists BridgeEvent leaves and builds the exit tree. Supports a DB-only mode (no RPC) so G2 can insert pre-collected leaves and build the tree without touching Anvil. Aborts on events that invalidate a BridgeEvent-only reconstruction (SetSovereignTokenAddress, MigrateLegacyToken, RemoveLegacySovereignTokenAddress, BackwardLET, ForwardLET) unless ignoreUnsupportedL2Events is set.

⚠️ On mainnet Step G replays ~915k bridge exits; the previous serial execution took ~4 days (~2.8 bridges/s). The parallel replay + off-chain option address this.

⚠️ Breaking Changes

• 🛠️ Config: exitAddress is now mandatory — LoadConfig errors when it is missing or set to the zero address (0x00…00). Configs that previously omitted it (it defaulted to the zero address) now fail. SC-locked value is bridged to this address and can only be recovered by signing from an address whose private key the operator controls.
• 🛠️ Config: option renames (to the ignore* convention) — abortOnGenesisBalance → ignoreGenesisBalance (polarity inverted: default false = abort), continueOnTraceError → ignoreOnTraceError, continueIfBalanceMismatch → ignoreBalanceMismatch.
• 🗑️ Deprecated Features: removed options.depositOrderSource; removed the config-examples/ .json variants (converted to .toml).

📋 Config Updates

Config accepts JSON or TOML

• LoadConfig selects the format by file extension: .toml is parsed as TOML, anything else (.json/no extension) as JSON. TOML is normalized to JSON internally (tomlToJSON) so both formats share one parsing/validation path, including signerConfig (json.RawMessage) and agglayerClient. Field names are identical in both formats.
• Added parameters.toml.example (each field commented with its description + default) and converted the config-examples/ to TOML (zkevm-cardona.toml, zkevm-mainnet.toml); removed the .json variants. .gitignore now also ignores parameters.toml.

exitAddress validation

• LoadConfig now rejects a missing or zero-address exitAddress. Docs/examples updated (the field was previously documented as optional, defaulting to the zero address) and exitAddress ships commented-out in the example configs so the operator must set their own.

New options

• options.verifyNewLocalExitRootUsingShadowFork — true (default). true verifies the LER on the Anvil shadow-fork (requires Anvil); false computes it off-chain from the lite tree (no Anvil, trusts off-chain leaf encoding/metadata).
• options.ignoreUnsupportedL2Events — false (default). Downgrades the lite syncer's abort on unsupported events to a warning.

Renamed options (to the ignore* convention)

• abortOnGenesisBalance → ignoreGenesisBalance (polarity inverted: default false = abort)
• continueOnTraceError → ignoreOnTraceError
• continueIfBalanceMismatch → ignoreBalanceMismatch

Removed

• options.depositOrderSource.

✅ Testing

• 🤖 Automatic: go test ./tools/exit_certificate/... passes (incl. bridgesyncerlite, step_g_order_test.go, and config_test.go with the new TestLoadConfig_MissingExitAddress / TestLoadConfig_ZeroExitAddress). go build, go vet, gofmt, and golangci-lint clean.
• 🖱️ Manual: run --step g (G1+G2) and Step I; confirm step-g-new-local-exit-root.json + step-g-reordered-certificate.json are produced and the lite tree root matches the contract getRoot() in verify mode.
• 🌐 Mainnet: the off-chain computation was tested on mainnet against the shadow-fork and both produced the same LocalExitRoot (shadow-fork verification took 13.5h, with a total of 975,646 bridges generated).

🐞 Issues

• Closes agglayer/pm#352 (https://github.com/agglayer/pm/issues/352)
• Closes agglayer/pm#348 (https://github.com/agglayer/pm/issues/348)

📝 Notes

• --step g runs G1+G2; g1/g2 run individually; g expands to g1,g2 in ranges.
• Anvil (Foundry) is required only in the default shadow-fork verification mode.
• Targets feature/exit-certificate-tool (the exit-certificate integration branch), not develop.
• Step G1 ETA refinement: the fetch-progress ETA now measures throughput over a trailing time window instead of the lifetime average, so it is not skewed by the fast empty low-block windows at the start.

🤖 Generated with Claude Code

[joanestebanr]

@claude

[claude]

Claude finished @joanestebanr's task in 2m 14s —— View job

---

PR Review Complete ✅

Reviewed PR #1633: Split Step G into G1/G2 with off-chain LER + shadow-fork verification

---

🚀 What's New

This PR significantly reworks Step G by splitting it into G1 (lite-sync L2 bridge history) and G2 (compute NewLocalExitRoot), adds a new bridgesyncerlite package for parallel bridge event fetching, introduces an off-chain computation path for faster LER calculation, and makes exitAddress mandatory with validation. The config system now supports both JSON and TOML formats.

🐛 Bug Fixes

None

📋 Config Updates

• exitAddress - Now MANDATORY (was optional). Rejects zero address. Must be an address whose private key the operator controls for SC-locked fund recovery.
• verifyNewLocalExitRootUsingShadowFork - New option (default: true). When true, uses Anvil shadow-fork verification; when false, computes LER purely off-chain (faster, no Anvil required).
• ignoreUnsupportedL2Events - New option (default: false). Downgrades abort on unsupported L2 events to warnings.
• Renamed options (polarity changes):
  • abortOnGenesisBalance → ignoreGenesisBalance (default: false = abort)
  • continueOnTraceError → ignoreOnTraceError
  • continueIfBalanceMismatch → ignoreBalanceMismatch
• Removed: options.depositOrderSource

⚠️ Breaking Changes

1. exitAddress is now mandatory - Config loading fails if missing or set to zero address
2. Config option renames with polarity inversions - See above
3. Removed depositOrderSource feature - Deposit order now comes from replay or certificate order

---

🔍 Review Feedback

🟢 Strengths

1. Excellent Architecture - The G1/G2 split is well-designed: G1 handles the expensive full-history sync once, G2 can be re-run quickly
2. Performance Focus - Parallel bridge fetching with configurable concurrency, send/collect pipeline for Anvil replay, and off-chain computation path address the mainnet scale problem (~915k exits)
3. Robust Error Handling - Receipt timeout retry mechanism in retryDeferredExit (step_g2.go:596-632) prevents false failures from slow fork backends
4. Test Coverage - Good unit test coverage including TestHashMatchesBridgesync ensuring byte-for-byte compatibility, config validation tests for exitAddress, and deposit order reordering tests
5. Documentation - Comprehensive inline comments explaining complex logic (e.g., ETA calculation over trailing window, send/collect pipeline rationale)

🔴 Required Issues

Security: Unchecked Error in Database Transaction - step_g1.go:155

The copyFile function in step_g1.go:141-156 closes the output file but doesn't properly check/handle the out.Close() error when io.Copy fails:

if _, err := io.Copy(out, in); err != nil {
    out.Close()  // ⚠️ Close error is silently ignored
    return err
}
return out.Close()

If the copy fails and the subsequent close also fails (e.g., disk full), you lose the close error. The defer pattern would be safer:

func copyFile(src, dst string) error {
    in, err := os.Open(src)
    if err != nil {
        return err
    }
    defer in.Close()
    out, err := os.Create(dst)
    if err != nil {
        return err
    }
    defer func() {
        if cerr := out.Close(); cerr != nil && err == nil {
            err = cerr
        }
    }()
    _, err = io.Copy(out, in)
    return err
}

Reference: tools/exit_certificate/step_g1.go:141-156

---

🟡 Suggested Improvements

1. Database Transaction Rollback in syncer.go - bridgesyncerlite/syncer.go:209-216, 255-262

Both StoreBridges and BuildTree have identical deferred rollback patterns. The rollback error logging is helpful, but the pattern is repeated. Consider extracting to a helper:

func (s *BridgeSyncerLite) withTx(ctx context.Context, fn func(*sql.Tx) error) error {
    tx, err := db.NewTx(ctx, s.db)
    if err != nil {
        return fmt.Errorf("begin transaction: %w", err)
    }
    committed := false
    defer func() {
        if !committed {
            if rerr := tx.Rollback(); rerr != nil {
                s.log.Errorf("rollback failed: %v", rerr)
            }
        }
    }()
    if err := fn(tx); err != nil {
        return err
    }
    if err := tx.Commit(); err != nil {
        return fmt.Errorf("commit transaction: %w", err)
    }
    committed = true
    return nil
}

References:

• tools/exit_certificate/bridgesyncerlite/syncer.go:205-232
• tools/exit_certificate/bridgesyncerlite/syncer.go:251-287

2. Magic Numbers in Constants - step_g2.go:28-86

Several timeouts and configuration values are well-documented but could benefit from clearer naming or comments about their derivation:

• receiptPollTimeout = 300 * time.Second (5 minutes) - Very generous, but mainnet scale justifies it
• replayInFlightWindow = 2000 - Good value, but document why 2000 specifically (block size? memory? empirical testing?)
• anvilTxGasLimit = "0x4c4b40" (5M) - Comment says ~300k for bridgeAsset, so 5M is ~16x. Document this safety margin.

The comments are excellent, but adding brief justifications for the specific values would help future tuning.

Reference: tools/exit_certificate/step_g2.go:28-86

3. Error Message Context in downloader.go - bridgesyncerlite/downloader.go:208-212

The forbidden event detection provides good detail, but could benefit from actionable guidance:

return nil, fmt.Errorf("unsupported %s event detected at block %d (tx %s, log index %d): "+
    "bridge state cannot be reconstructed from BridgeEvent logs alone. "+
    "Set ignoreUnsupportedL2Events=true to skip this event (may produce incorrect NewLocalExitRoot), "+
    "or use the full bridgesync path if available",
    name, l.BlockNumber, l.TxHash.Hex(), l.Index)

Reference: tools/exit_certificate/bridgesyncerlite/downloader.go:208-212

4. Config Validation Order - config.go:139-151

The exitAddress validation comes after checking l2RpcUrl and l2BridgeAddress. Consider validating all mandatory fields first, then semantic checks (zero address), so users get comprehensive error messages:

var missing []string
if raw.L2RPCURL == "" {
    missing = append(missing, "l2RpcUrl")
}
if raw.L2BridgeAddress == "" {
    missing = append(missing, "l2BridgeAddress")
}
if raw.ExitAddress == "" {
    missing = append(missing, "exitAddress")
}
if len(missing) > 0 {
    return nil, fmt.Errorf("missing required parameters: %s", strings.Join(missing, ", "))
}

Reference: tools/exit_certificate/config.go:139-151

5. Potential Race in Progress Logging - step_g2.go:466-478

The maybeLogProgress function uses atomic operations, but the pattern mixing atomic.AddInt64 with atomic.LoadInt64 and atomic.CompareAndSwapInt64 could be clearer. The logic is correct, but the lastLogNanos updates could race in edge cases where multiple goroutines hit the replayLogMaxGap threshold simultaneously. Not a critical bug (just extra log lines), but worth noting.

Reference: tools/exit_certificate/step_g2.go:466-478

---

🟢 Optional Observations

1. ETA Calculation Refinement - The trailing window ETA (bridgesyncerlite/downloader.go:114-177) is a clever solution to the "fast empty start, slow dense tail" problem. Nice work!

2. Deferred Exit Retry Logic - The retryDeferredExit unbounded retry with re-poll-before-resend ordering (step_g2.go:596-632) correctly prevents double-counting exits. The comment explaining why this ordering matters is excellent.

3. Parallel Testing - All tests use t.Parallel(), which is great for CI performance.

4. Fail-Fast Pattern - The failFast closure with sync.Once in replayBridgeExits (step_g2.go:449-457) correctly captures the first error and cancels context to stop all workers.

---

📝 Summary

This is a high-quality PR that addresses a critical performance bottleneck (mainnet's ~915k bridge exits). The architecture is sound, error handling is robust, and the code is well-tested and documented.

Action Items

1. 🔴 Fix the copyFile error handling (step_g1.go:141-156) - Required before merge
2. 🟡 Consider the suggested improvements for code quality and maintainability

The breaking changes (mandatory exitAddress, config renames) are well-documented in the PR description. The off-chain computation path is a significant optimization while maintaining correctness through optional shadow-fork verification.

Recommendation: Approve with the required fix to copyFile error handling.

---

| Branch: feat/exit_certificate_f15_deposit_order

[Copilot]

Pull request overview

This PR refactors the exit-certificate tool’s Step G (NewLocalExitRoot computation) into a two-phase pipeline (G1/G2) to improve performance and optionally allow an off-chain computation path, while also updating configuration handling (including TOML support) and renaming several options to an ignore* convention.

Changes:

• Split Step G into Step G1 (lite-sync L2 bridge history into a persistent DB) and Step G2 (compute/verify NewLocalExitRoot, reorder certificate by deposit order, and persist reordered output).
• Add bridgesyncerlite (parallel eth_getLogs sync + persisted leaves + exit-tree build) and wire it into Step G1/G2.
• Update config parsing/validation: TOML support via .toml extension, make exitAddress required/non-zero, rename multiple options (abortOnGenesisBalance → ignoreGenesisBalance, etc.).

Reviewed changes

Copilot reviewed 35 out of 35 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
tools/exit_certificate/worker.go	Suppress noisy context.Canceled logging from worker pools after fail-fast cancellation.
tools/exit_certificate/types.go	Update Step F comment for renamed option; add StepG1Result.
tools/exit_certificate/step_g1.go	New Step G1: lite-sync L2 bridge history into a persistent sqlite DB and emit shadow-fork block info.
tools/exit_certificate/step_g2.go	New Step G2: compute NewLocalExitRoot via shadow-fork replay (default) or off-chain lite-tree path; includes replay pipeline + verification.
tools/exit_certificate/step_g.go	Remove legacy monolithic Step G implementation (replaced by G1/G2).
tools/exit_certificate/step_g_order.go	New helper to reorder certificate exits by replayed depositCount.
tools/exit_certificate/step_g_order_test.go	Tests for certificate reordering by deposit count.
tools/exit_certificate/step_g_events.go	Off-chain lite-tree construction from certificate exits; DB-copy + replayed leaf insertion + tree build.
tools/exit_certificate/step_f.go	Rename mismatch-continue option and update messaging accordingly.
tools/exit_certificate/step_f_test.go	Update tests for renamed Step F option.
tools/exit_certificate/step_b.go	Apply ignoreGenesisBalance semantics (polarity inversion) in Step B1.
tools/exit_certificate/step_a.go	Apply ignoreOnTraceError option rename in Step A1.
tools/exit_certificate/step_a_test.go	Update tests/docs for renamed trace-error option.
tools/exit_certificate/scripts/reproduce_sc_locked.sh	Update config option naming and Step G file references.
tools/exit_certificate/scripts/configuration_based_on_kurtosis.sh	Update config option naming and include new ignore flags.
tools/exit_certificate/run.go	Add g1/g2 steps, g alias expansion, and persist reordered certificate output for Step I.
tools/exit_certificate/run_test.go	Update step-range parsing tests for g1/g2 and g alias behavior.
tools/exit_certificate/README.md	Update docs for JSON/TOML config, new Step G split, option renames, and exitAddress validation (but still needs alignment with Step I/G outputs).
tools/exit_certificate/parameters.toml.example	New TOML example config with annotated fields and defaults.
tools/exit_certificate/parameters.json.example	Update JSON example for renamed/new options.
tools/exit_certificate/config.go	Add TOML support, enforce exitAddress required/non-zero, add new options, rename ignore flags.
tools/exit_certificate/config-examples/zkevm-mainnet.toml	Convert mainnet example config to TOML and update required fields/options.
tools/exit_certificate/config-examples/zkevm-mainnet.json	Remove JSON example variant (replaced by TOML).
tools/exit_certificate/config-examples/zkevm-cardona.toml	Convert Cardona example config to TOML and update required fields/options.
tools/exit_certificate/config-examples/zkevm-cardona.json	Remove JSON example variant (replaced by TOML).
tools/exit_certificate/config-examples/README.md	Update examples README for TOML focus and new required fields.
tools/exit_certificate/config_test.go	Add tests for exitAddress validation and TOML parsing/round-tripping of signer config.
tools/exit_certificate/cmd/main.go	Update CLI --config help text to reflect JSON/TOML support.
tools/exit_certificate/CLAUDE.md	Update internal docs for Step G split, option renames, and TOML support.
tools/exit_certificate/bridgesyncerlite/types.go	New lite syncer types + leaf hashing compatible with canonical bridgesync tree.
tools/exit_certificate/bridgesyncerlite/syncer.go	New lite syncer core: persist leaves, build exit tree, DB-only mode support.
tools/exit_certificate/bridgesyncerlite/syncer_test.go	Tests for hashing compatibility, persistence, and tree build correctness.
tools/exit_certificate/bridgesyncerlite/migrations.go	DB schema/migrations for lite syncer bridge table + shared tree tables.
tools/exit_certificate/bridgesyncerlite/downloader.go	Parallel log downloader with forbidden-event detection and trailing-window ETA.
tools/exit_certificate/.gitignore	Ignore parameters.toml in addition to JSON and output artifacts.

[joanestebanr]

Addressed the 🔴 Required Issue (copyFile error handling, step_g1.go) in 84969cc.

copyFile now uses a deferred out.Close() with a named return: the close error is surfaced when the copy succeeded but the flush failed (e.g. full disk), while a copy error still takes precedence. This prevents a partially-written lite DB copy from being reported as success.

The 🟡 suggested improvements (tx helper extraction, constant justifications, richer forbidden-event error, batched config validation, progress-logging atomics) are noted; happy to fold in any you consider blocking — otherwise I'll leave them as follow-ups to keep this PR focused.

[joanestebanr]

Addressed 🟡 suggested improvement #5 (Potential Race in Progress Logging) in 8d60595.

maybeLogProgress previously mixed atomic.AddInt64 on the counter with a plain atomic.StoreInt64 (interval-milestone branch) and a CompareAndSwapInt64 (gap branch) on lastLogNanos. As the review noted, a milestone Store could interleave with the gap CAS and emit a duplicate progress line.

Replaced the atomics with a single sync.Mutex guarding the counter and the last-log timestamp as a unit, collapsing the two branches into one condition. The lock is uncontended in practice (the work between calls is a receipt fetch), so it is not on a hot path. sync/atomic is no longer needed in the file.

[web3security]

LGTM

[sonarqubecloud]

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
81.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud