Skip to content

feat: new tool - exit_certificate#1582

Open
krlosMata wants to merge 5 commits intodevelopfrom
feature/exit-certificate-tool
Open

feat: new tool - exit_certificate#1582
krlosMata wants to merge 5 commits intodevelopfrom
feature/exit-certificate-tool

Conversation

@krlosMata
Copy link
Copy Markdown
Member

@krlosMata krlosMata commented Apr 14, 2026

🔄 Changes Summary

  • New exit-certificate CLI tool under tools/exit_certificate/ that generates exit certificates for aggchain migration. It scans L2 state from genesis to a target block, discovers all addresses with value (ETH + wrapped tokens), computes smart-contract-locked balances, detects unclaimed L1→L2 bridge deposits, and produces a standard agglayer Certificate with BridgeExit entries that bridge all value back to L1.
  • The tool runs a 6-step pipeline (0 → A → B → C → D → E): LBT generation, address collection via debug_traceTransaction, EOA balance scanning, SC-locked value computation, certificate building, and unclaimed bridge detection.
  • Steps can be run individually or as a full pipeline via --step flag. Intermediate results are persisted to an output directory, enabling step-by-step execution and resumability.
  • 3,830 lines of new code across 27 files, including ~1,315 lines of unit/integration tests.

⚠️ Breaking Changes

  • None. This is a new standalone tool with no impact on existing aggkit components.

📋 Config Updates

  • 🧾 New config file: tools/exit_certificate/parameters.json (standalone JSON, not part of aggkit's main config). Example:
{
    "l2RpcUrl": "https://your-l2-rpc.example.com",
    "l1RpcUrl": "https://your-l1-rpc.example.com",
    "l2BridgeAddress": "0x2a3DD3EB832aF982ec71669E178424b10Dca2EDe",
    "l1BridgeAddress": "0x2a3DD3EB832aF982ec71669E178424b10Dca2EDe",
    "l2NetworkId": 1,
    "targetBlock": "latest",
    "exitAddress": "0x0000000000000000000000000000000000000001",
    "destinationNetwork": 0,
    "options": {
        "blockRange": 10000,
        "concurrencyLimit": 200,
        "rpcBatchSize": 200,
        "rpcDelayMs": 10,
        "outputDir": "./output",
        "l1StartBlock": 0
    }
}

Close #1584

@claude
Copy link
Copy Markdown

claude bot commented Apr 14, 2026

Claude finished @krlosMata's task in 4m 37s —— View job


PR Review: exit_certificate Tool ✅


🚀 What's New

A new standalone CLI tool (exit-certificate) that generates exit certificates for aggchain migration. The tool scans L2 state from genesis to a target block, discovers all value (ETH + wrapped tokens), computes smart-contract-locked balances, detects unclaimed L1→L2 bridge deposits, and produces a standard agglayer Certificate with BridgeExit entries that bridge all value back to L1. Includes a 6-step resumable pipeline with ~1,315 lines of tests (34% of total code).

🐛 Bug Fixes

None.

📋 Config Updates

New config file: tools/exit_certificate/parameters.json (standalone JSON, not part of aggkit's main config).

Required fields:

  • l2RpcUrl (string): L2 JSON-RPC endpoint (must support debug_traceTransaction)
  • l2BridgeAddress (hex address): L2 bridge contract address
  • targetBlock (string): Target block number or "latest"

Optional fields:

  • l1RpcUrl (string): L1 JSON-RPC endpoint (required for Step E)
  • l1BridgeAddress (hex address): L1 bridge contract (defaults to l2BridgeAddress)
  • l2NetworkId (uint32): L2 network ID (default: 1)
  • exitAddress (hex address): Address for SC-locked value exits (default: zero address)
  • destinationNetwork (uint32): Destination network (default: 0)
  • lbtFile (string): Path to pre-generated LBT JSON file (optional)
  • options.blockRange (int): Block range per eth_getLogs (default: 5000)
  • options.concurrencyLimit (int): Max concurrent RPC requests (default: 20)
  • options.rpcBatchSize (int): Max calls per batch (default: 200)
  • options.rpcDelayMs (int): Delay between batches (default: 0)
  • options.outputDir (string): Output directory (default: "./output")
  • options.l1StartBlock (uint64): L1 scan start block (default: 0)

⚠️ Breaking Changes

None. This is a new standalone tool with no impact on existing aggkit components.


Code Quality Assessment

✅ Strengths

  1. Excellent Architecture: Clean 6-step pipeline with clear separation of concerns. Each step has well-defined inputs/outputs and can run independently.

  2. Strong Test Coverage: ~1,315 lines of tests (34% of codebase) with good unit and integration coverage. Tests use proper table-driven patterns and edge case validation.

  3. Resumability: Intermediate results persisted to disk enable step-by-step execution and recovery from failures.

  4. Performance-Conscious: Custom HTTP client with unlimited connections, worker pools with proper concurrency control, and batched RPC calls.

  5. Error Handling: Comprehensive error wrapping with context throughout. Retry logic with exponential backoff for RPC calls.

  6. Good Documentation: Detailed README with clear usage examples, config documentation, and pipeline explanations.


🔴 Required Issues

1. Security: Unchecked Big Number Conversions

Location: step_0.go:163, step_e.go:229-236, hex.go:10-25

The code uses unchecked type conversions from *big.Int to uint32/uint64 which can silently truncate values:

// step_0.go:163
OriginNetwork: uint32(new(big.Int).SetBytes(data[0:32]).Uint64())

// step_e.go:229
LeafType: uint8(new(big.Int).SetBytes(data[0:32]).Uint64())

// hex.go - no bounds checking
func hexToUint64(s string) uint64 {
    var n uint64
    for _, c := range s {
        n <<= 4  // Can overflow silently
        // ...
    }
}

Impact: If a malicious or corrupted event contains a value larger than uint32_MAX or uint64_MAX, it will be silently truncated, potentially causing incorrect certificate generation or missing funds.

Recommendation: Add overflow checks before conversions:

val := new(big.Int).SetBytes(data[0:32])
if val.Uint64() > math.MaxUint32 {
    return wrappedTokenEvent{}, fmt.Errorf("originNetwork value too large: %s", val)
}
originNetwork := uint32(val.Uint64())

Fix this →

2. Security: Unvalidated Array Bounds in Event Decoding

Location: step_0.go:156-166, step_e.go:209-239

Event decoding functions check minimum data length but don't validate upper bounds when accessing dynamic metadata:

// step_e.go:218-225
metadataOffset := new(big.Int).SetBytes(data[192:224]).Uint64()
var metadata []byte
if metadataOffset+32 <= uint64(len(data)) {  // Check offset+32
    metadataLen := new(big.Int).SetBytes(data[metadataOffset : metadataOffset+32]).Uint64()
    metadataStart := metadataOffset + 32
    if metadataStart+metadataLen <= uint64(len(data)) {  // Check length
        metadata = make([]byte, metadataLen)
        copy(metadata, data[metadataStart:metadataStart+metadataLen])  // But no max size limit!
    }
}

Impact: A malicious event with an extremely large metadataLen could cause excessive memory allocation (DoS) or integer overflow in the offset calculation.

Recommendation: Add maximum size limits:

const maxMetadataSize = 1024 * 1024 // 1MB
if metadataLen > maxMetadataSize {
    return L1Deposit{}, fmt.Errorf("metadata too large: %d bytes", metadataLen)
}

Fix this →

3. Bug: Potential Integer Overflow in Batch Size Calculation

Location: step_0.go:189

batchSize := max(len(calls)/concurrency, 1)

If len(calls) is very large and concurrency is 1, this could create an extremely large batch that exceeds RPC limits.

Recommendation: Cap batch size to the configured RPCBatchSize:

batchSize := min(max(len(calls)/concurrency, 1), cfg.Options.RPCBatchSize)

Fix this →

4. Bug: Step C Doesn't Handle Native Token (Zero Address) Correctly

Location: step_c.go:83-88

The native token has WrappedTokenAddress == common.Address{} (zero address), but the indexing function uses strings.ToLower(e.WrappedTokenAddress.Hex()) which converts it to "0x0000000000000000000000000000000000000000". This is fine, but Step C doesn't explicitly handle the native token case:

func indexByAddress(entries []LBTEntry) map[string]LBTEntry {
    m := make(map[string]LBTEntry, len(entries))
    for _, e := range entries {
        m[strings.ToLower(e.WrappedTokenAddress.Hex())] = e  // Zero address becomes "0x000...000"
    }
    return m
}

In step_d.go:54-56, there's special handling for native tokens, but Step C doesn't distinguish between native and wrapped tokens when computing SC-locked values.

Recommendation: Add explicit handling for the native token entry or add a comment explaining that native tokens are intentionally treated the same way.

5. Bug: Missing Validation for TargetBlock Format

Location: run.go:69-79

The parseBlockNumber function silently returns 0 for invalid inputs:

func parseBlockNumber(s string) uint64 {
    // ... parse logic ...
    var n uint64
    if _, err := fmt.Sscanf(s, "%d", &n); err == nil {
        return n
    }
    return 0  // Returns 0 on error!
}

Impact: If a user provides an invalid targetBlock value (e.g., "abc"), it silently becomes block 0 instead of returning an error.

Recommendation: Return an error from this function and propagate it.

Fix this →


🟡 Suggested Improvements

6. Performance: RPC Batch Size Not Respected in Step A

Location: step_a.go:61, step_a.go:96

Step A uses concurrentBatchRPC with a hardcoded batch size calculation, ignoring cfg.Options.RPCBatchSize:

headerResults, err := concurrentBatchRPC(ctx, rpcURL, headerCalls, batchSize, concurrency)

But batchSize is set to cfg.Options.RPCBatchSize in the caller. However, in Step 0, the batch size is calculated dynamically. This inconsistency could lead to performance issues.

Recommendation: Consistently use cfg.Options.RPCBatchSize across all steps or document why some steps calculate it differently.

7. Code Quality: Inconsistent Error Handling in Worker Pools

Location: worker.go:66-83

The worker pool returns only the first error encountered but continues processing:

if r.err != nil {
    if firstErr == nil {
        firstErr = r.err
    }
    log.Warnf("%s job failed: %v", label, r.err)
    continue  // Keeps going even after first error
}

Behavior Concern: If many jobs fail, the tool continues processing and only returns the first error at the end. This could waste time on a fundamentally broken operation.

Recommendation: Consider adding a failFast parameter or error threshold to stop early on repeated failures.

8. Code Quality: Magic Numbers in hex.go

Location: hex.go:14-24, step_e.go:114

Several magic numbers lack clarity:

leafIndexMask := new(big.Int).SetUint64(0xFFFFFFFF) //nolint:mnd
mainnetFlag := new(big.Int).Lsh(big.NewInt(1), 64) //nolint:mnd

While the //nolint:mnd comment suppresses the linter, it would be clearer to define these as package-level constants with descriptive names:

const (
    globalIndexLeafMask    = 0xFFFFFFFF
    globalIndexMainnetBit  = 64
)

Fix this →

9. Observability: Missing Progress Logging in Step D

Location: step_d.go:17-75

Step D processes EOA balances and SC-locked values but doesn't log progress during iteration (unlike other steps). For large migrations with thousands of exits, this could leave users wondering if the process is hung.

Recommendation: Add progress logging similar to other steps.

10. Code Quality: Inconsistent Nil Check Pattern

Location: step_b.go:220-228

func unmarshalHexBigInt(result json.RawMessage) *big.Int {
    if result == nil {
        return nil
    }
    var hex string
    if json.Unmarshal(result, &hex) != nil || hex == "" || hex == "0x" {
        return nil
    }
    return hexToBigInt(hex)
}

This function returns nil for invalid inputs, which is then checked in step_0.go:198-200:

supply := unmarshalHexBigInt(result)
if supply == nil {
    supply = new(big.Int)  // Convert nil to zero
}

Pattern Concern: Treating nil as "zero" is semantically different from "error". A missing RPC result vs. a zero balance should be distinguished.

Recommendation: Consider returning (*big.Int, error) to distinguish between zero values and errors.


🟢 Optional Enhancements

11. Go Best Practices: Use crypto/rand for Worker Pool Channel Cap

Location: worker.go:9

const workerPoolChannelCap = 10000

This is a reasonable constant, but the value seems arbitrary. Consider documenting why 10,000 was chosen or making it configurable.

12. Testing: Missing Tests for Edge Cases

While test coverage is good overall, some edge cases lack explicit tests:

  • Step A with chain that has never had a transaction
  • Step E with L2ClaimEvents that have non-mainnet globalIndex flags
  • Boundary condition where metadataOffset is exactly at the end of data

13. Documentation: Add Sequence Diagram

The README is excellent, but a sequence diagram showing the data flow through all 6 steps would help users understand dependencies.

14. Code Style: Inconsistent Use of Named Return Values

Most functions don't use named return values, but classifyAddresses (step_b.go:75) does:

func classifyAddresses(...) (eoas, contracts []common.Address, err error)

For consistency with the rest of the codebase, consider using unnamed returns.

15. Performance: Consider Adding Rate Limiting

The tool has rpcDelayMs but it's applied between batches, not per request. For RPC providers with strict rate limits, adding a proper rate limiter (e.g., golang.org/x/time/rate) would be more robust.


📊 CLAUDE.md Compliance

Formatting: Uses gofmt, goimports
Imports: Properly grouped (stdlib, external, internal)
Naming: CamelCase for exports, no stuttering
Errors: Uses fmt.Errorf with %w for wrapping
Line length: All lines under 120 chars
Comments: Exported functions have doc comments
Tests: Uses testify/require (not assert) ✅
File naming: All test files use _test.go suffix

Minor deviation: Uses require consistently (good), but CLAUDE.md says "Use require (not assert)" which is satisfied.


🏗️ Architecture Assessment

The tool demonstrates excellent software engineering:

  1. Modularity: Each step is independently testable and resumable
  2. Concurrency: Thoughtful use of worker pools, semaphores, and batching
  3. Reliability: Retry logic, error handling, and graceful degradation
  4. Usability: Clear CLI, good defaults, comprehensive README
  5. Maintainability: Clean code structure, good test coverage

The main areas for improvement are security validations around untrusted input (event decoding) and more consistent error handling patterns.


🎯 Summary

This is a high-quality implementation of a complex migration tool. The code is well-structured, thoroughly tested, and production-ready with some security hardening.

Priority fixes:

  1. 🔴 Add overflow checks for big.Int conversions (Unify L1 syncers #1)
  2. 🔴 Add bounds validation for event metadata (Split L1 / L2 responsibilities for lastgersync #2)
  3. 🔴 Cap batch size to respect RPCBatchSize (Add UTs for lastgersync #3)
  4. 🔴 Add validation for targetBlock format (Add monitoring #5)

Recommended improvements:

Test status: CI checks are currently in progress. I'll monitor for any failures.


@krlosMata krlosMata force-pushed the feature/exit-certificate-tool branch from 6d18df4 to 85a1fa9 Compare April 14, 2026 08:58
@krlosMata krlosMata changed the title new tool: exit_certificate feat: new tool - exit_certificate Apr 14, 2026
@joanestebanr joanestebanr requested a review from Copilot April 15, 2026 09:01
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new standalone exit-certificate CLI tool under tools/exit_certificate/ to generate Agglayer exit certificates for aggchain migration by scanning L2/L1 state and producing an agglayer/types.Certificate with BridgeExit entries.

Changes:

  • Implements a 6-step pipeline (0 → A → B → C → D → E) for LBT generation, address discovery, balance scanning, SC-locked computation, certificate building, and unclaimed L1→L2 deposit detection.
  • Adds JSON-RPC batching/concurrency utilities plus step-wise resumable output persisted to an output directory.
  • Adds unit/integration tests and documentation/config examples for running the tool.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
tools/exit_certificate/worker.go Generic worker-pool helper used to parallelize step workloads.
tools/exit_certificate/types.go Shared data structures for step outputs and deposit/event models.
tools/exit_certificate/step_0.go Step 0 implementation: generate LBT by scanning bridge logs + supplies.
tools/exit_certificate/step_a.go Step A implementation: scan blocks/txs and trace touched addresses.
tools/exit_certificate/step_b.go Step B implementation: EOA/contract classification and balance scanning.
tools/exit_certificate/step_c.go Step C implementation: compute SC-locked values from LBT vs EOA totals.
tools/exit_certificate/step_d.go Step D implementation: build the exit certificate BridgeExits.
tools/exit_certificate/step_e.go Step E implementation: scan L1 BridgeEvents and add unclaimed deposits.
tools/exit_certificate/rpc.go JSON-RPC client utilities: batch/single RPC, retries, concurrency batching.
tools/exit_certificate/hex.go Hex/decimal parsing helpers and ABI safety conversions.
tools/exit_certificate/config.go Config loading/validation, defaults, and LBT file parsing helpers.
tools/exit_certificate/run.go CLI execution wiring: full pipeline + step-by-step resumability and I/O.
tools/exit_certificate/cmd/main.go CLI binary entrypoint using urfave/cli.
tools/exit_certificate/README.md Tool documentation: config, steps, usage, outputs, testing.
tools/exit_certificate/.gitignore Ignore local parameters/output/binary artifacts for this tool.
tools/exit_certificate/parameters.json.example Example standalone JSON config for running the tool.
tools/exit_certificate/step_a_test.go Unit tests for hex block parsing helper(s) used in Step A.
tools/exit_certificate/step_b_test.go Unit tests for hex-to-bigint helper used in Step B.
tools/exit_certificate/step_c_test.go Unit tests for SC-locked computation behavior and edge cases.
tools/exit_certificate/step_d_test.go Unit tests for certificate construction from EOA + SC-locked inputs.
tools/exit_certificate/step_e_test.go Unit tests for BridgeEvent decoding and claimed-set filtering logic.
tools/exit_certificate/rpc_test.go Unit tests for batch/single RPC, retry behavior, and error handling.
tools/exit_certificate/run_test.go Unit tests for block parsing and JSON save/load helpers.
tools/exit_certificate/config_test.go Unit tests for config parsing, defaults, and LBT parsing helpers.
tools/exit_certificate/integration_test.go Integration-style tests for production-like config/data shapes (skippable).

Comment thread tools/exit_certificate/step_d.go Outdated
Comment thread tools/exit_certificate/run.go
Comment thread tools/exit_certificate/step_b.go Outdated
Comment thread tools/exit_certificate/rpc.go Outdated
Comment thread tools/exit_certificate/step_e.go Outdated
Comment thread tools/exit_certificate/step_a.go Outdated
Comment thread tools/exit_certificate/run.go Outdated
Comment thread tools/exit_certificate/step_e.go Outdated
var bridgeEventTopic = common.HexToHash("0x501781209a1f8899323b96b4ef08b168df93e0a90c673d1e4cce39f97571d4d7")

// claimEventTopic is keccak256("ClaimEvent(uint256,uint32,address,address,uint256)").
var claimEventTopic = common.HexToHash("0x25308c93ceeed162da955b3f7ce3e3f93606579e40fb92029faa9efe27545983")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It ignores old zkevm claimEvent

claimEventSignaturePreEtrog = crypto.Keccak256Hash([]byte("ClaimEvent(uint32,uint32,address,address,uint256)"))

It's the expected behaviour?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch !!
I'm thinking that is worth it to check directly the nullififer (via de isClaimed) SC call rather than ClaimEvent

- Add overflow checks for big.Int to uint32/uint64 conversions (safeUint32, safeUint8)
- Add max metadata size validation (1MB) in decodeBridgeEvent to prevent DoS
- Cap batch size to RPCBatchSize in fetchTotalSupplies
- Return error from parseBlockNumber on invalid input instead of silent zero
- Extract globalIndex magic numbers to named constants
- Add progress logging to Step D
- Document native token handling in step_c indexByAddress
- Fix all golangci-lint issues (errcheck, gci, gosec, lll, mnd, prealloc, unparam)

Made-with: Cursor
- Scan L2 bridge for ClaimEvent logs so Step E correctly identifies
  already-claimed deposits instead of treating all as unclaimed (joanestebanr)
- Fail on trace/scan errors instead of warn+continue: traceTransactions,
  fetchL1BridgeEvents now propagate errors (partial scans are unsafe)
- Fix encodeBalanceOf: use zero-padding (LeftPadBytes) instead of
  space-padding (%064s) which produced invalid hex calldata
- Use strconv.ParseUint instead of fmt.Sscanf to reject trailing
  non-numeric input like "123abc"
- Set MaxIdleConnsPerHost=100 instead of 0 (0 defaults to 2 in net/http)
- Preserve OriginNetwork/OriginTokenAddress from LBT for native token
  entries (supports chains with custom gas tokens)
- Add decodeClaimEvent tests

Made-with: Cursor
- Extract magic number 32 to named constant abiWordSize in step_b.go
- Pre-allocate claims slice in fetchClaimEventsInRange

Made-with: Cursor
- Replace ClaimEvent log scanning with isClaimed(depositCount, 0)
  eth_call on L2 bridge contract (authoritative claimed bitmap)
- Extract helper functions across all steps to bring every function
  under diffguard thresholds (complexity ≤ 10, size ≤ 50 lines)

Made-with: Cursor
@krlosMata krlosMata force-pushed the feature/exit-certificate-tool branch from 4936ffa to 63a43be Compare April 17, 2026 09:47
@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

implement exit cert tool

4 participants