Repository navigation
fix: aggkit-proxy: wait for L1 info tree leaf availability before claiming L2->L1 bridges 1823 - #1829
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12afa44789
ℹ️ 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".
Collaborator
Author
|
@claude review |
joanestebanr
added a commit
that referenced
this pull request
Sep 4, 2026
…ridge-service instance Fix a review finding on #1829 (chatgpt-codex-connector): mainnet (network 0) has no bridge-service deployment of its own — bridgeservicefinder.GetURL(0) fails with ErrURLNotFound unless a network-0 URL was explicitly configured, which the shipped default does not do. GERSource.L1InfoTreeIndexForBridge was resolving the queried instance from bridge.NetworkID directly, which is 0 for every L1->L2 bridge — turning StepWaitingL1InfoLeafAvailable into a permanent, repeated error for the whole route instead of letting it reach StepWaitingClaim. When the origin is mainnet, query the destination's own instance instead (a real rollup, always resolvable), with network_id=0 as a query parameter rather than as the resolution key — the same pattern coveringLeafIndex/OriginGER already rely on for this exact case. Add bridgetracker/sources/ger_test.go pinning both branches (destination queried for an L1-originated bridge, origin self-queried for an L2-originated one) plus the not-yet-covered and URL-resolution-failure cases, none of which the existing fake-backed domain/engine tests can catch since they bypass real URL resolution entirely.
…aiming L2->L1 bridges For an L2->L1 bridge, the tracker jumped straight from WaitL1SettledGER to WaitingClaim once the certificate's settlement tx was confirmed on L1. That does not guarantee the destination network's own bridge-service instance has indexed the resulting L1 info tree leaf yet: its own L1 info tree sync can lag behind the finality this tracker uses for WaitL1SettledGER (e.g. it waits for the block to be finalized), so a bridge could look claimable while the destination still could not produce a proof for it. Add a new StepWaitingL1InfoLeafAvailable, only for the L2->L1 path, between WaitL1SettledGER and WaitingClaim. It reuses the same mechanism StepWaitingGERInjection already relies on for L2->L2 (GERSource. InjectedGERAtIndex against the destination's own bridge-service instance), which for a mainnet destination resolves straight against l1infotreesync's own indexing state. Fixes #1823
Add a new claim_status field to TrackingData (both the REST tracker response and every WebSocket "status" message), a simplified claim-readiness summary for clients that only care about "is this claimable" without walking all_steps themselves: - ERROR whenever tracking_status is error (this takes priority over everything else) - CLAIMED once the current step is StepClaimed - READY_TO_CLAIM while the current step is StepWaitingClaim - PENDING otherwise, including before the bridge resolves Computed via a new domain.TrackingData.ClaimStatus method, mirroring TrackingStatus's own derivation pattern, backed by a new types.TrackerClaimStatus enum (distinct from the unrelated types.ClaimStatus used by the Activity endpoint's own on-chain isClaimed() check).
…e claiming StepWaitingL1InfoLeafAvailable was only reached on the L2->L1 path. For L1->L2 and L2->L2, StepWaitingGERInjection was assumed to already imply the leaf is available — but it only confirms the GER's injection tx landed on the destination network's own contract (an L2-side fact), not that the destination's own bridge-service instance has its L1 info tree sync caught up, which is what it actually needs to produce a claim proof. Those are different facts and can legitimately disagree. Move StepWaitingL1InfoLeafAvailable so it is always the step right before StepWaitingClaim, on every route, never skipped and never inferred from StepWaitingGERInjection having just completed. Its resolver now looks up the settled leaf index by scanning AllSteps for whichever earlier step resolved it (StepWaitL1SettledGER for an L2-originated bridge, StepWaitingGERUpdate for an L1-originated one) instead of assuming it sits immediately before, since StepWaitingGERInjection now sits in between on the routes that have it.
…fAvailable TestBridgeTrackerL1ToL2 pinned the exact all_steps shape for the L1->L2 route, which now includes WaitingL1InfoLeafAvailable right before WaitingClaim. Also mirrors the new claim_status field in trackerTrackingData and asserts it at the readyToClaim/claimed transitions this test already exercises.
…st the proof-building instance
StepWaitingL1InfoLeafAvailable previously called the same
InjectedGERAtIndex mechanism StepWaitingGERInjection uses, against the
bridge's destination network. That checks an L2-side fact only (did the
GER's injection tx land on that network's own contract) and, per
docs/bridge_service.md's documented flow and the l1-info-tree-index
endpoint's own network_id validation (network_id must be either mainnet
or the queried instance's own network — confirmed by
bridge_test.go's "Invalid network ID" case), was never actually a valid
combination for an L2-originated bridge's non-mainnet origin anyway.
The bridge-service instance that will actually build the claim proof is
the origin network's own instance (see docs/bridge_service.md's L2->L2
sequence: bridge_getProof is called there, against the same instance
this step now queries). Replace the resolver's mechanism entirely:
- New GERSource.L1InfoTreeIndexForBridge queries GET
/bridge/v1/l1-info-tree-index on the origin network's own
bridge-service instance (network_id=origin, deposit_count=origin's
own deposit count) — the one combination the endpoint always accepts.
- StepWaitingL1InfoLeafAvailable's Resolve no longer needs to look up a
leaf index from a previous step at all; the endpoint resolves
directly from the bridge's own origin network/deposit count.
- Its Result is now types.L1InfoLeafAvailableResult{L1InfoTreeIndex},
replacing the reused types.InjectedGERL1Leaf (GER/block_number/
block_timestamp are not part of this endpoint's response).
Still always the step right before StepWaitingClaim, on every route,
still never skipped or inferred from StepWaitingGERInjection completing
— that remains an L2-side fact, distinct from whether the proof-building
instance's own L1 info tree sync has caught up.
…ridge-service instance Fix a review finding on #1829 (chatgpt-codex-connector): mainnet (network 0) has no bridge-service deployment of its own — bridgeservicefinder.GetURL(0) fails with ErrURLNotFound unless a network-0 URL was explicitly configured, which the shipped default does not do. GERSource.L1InfoTreeIndexForBridge was resolving the queried instance from bridge.NetworkID directly, which is 0 for every L1->L2 bridge — turning StepWaitingL1InfoLeafAvailable into a permanent, repeated error for the whole route instead of letting it reach StepWaitingClaim. When the origin is mainnet, query the destination's own instance instead (a real rollup, always resolvable), with network_id=0 as a query parameter rather than as the resolution key — the same pattern coveringLeafIndex/OriginGER already rely on for this exact case. Add bridgetracker/sources/ger_test.go pinning both branches (destination queried for an L1-originated bridge, origin self-queried for an L2-originated one) plus the not-yet-covered and URL-resolution-failure cases, none of which the existing fake-backed domain/engine tests can catch since they bypass real URL resolution entirely.
Add a new api_revision field to GET /tracker/v1/health's HealthResponse: an integer, backed by a new types.CurrentAPIRevision constant, meant to be bumped by one whenever a change to the tracker's wire API contract could break an existing client — a field added/removed/renamed, an enum's value set changed, a new step inserted into a bridge's expected path, and the like. Purely informational: the tracker never rejects or alters behavior based on it, it just lets a client tell which contract shape a given instance speaks without diffing full response bodies.
joanestebanr
force-pushed
the
fix/1823-l2-to-l1-l1-info-leaf-available
branch
from
September 4, 2026 13:09
c2460db to
2cc57d9
Compare
arnaubennassar
approved these changes
Sep 4, 2026
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The functional change is well-covered by unit/e2e tests and is localized to an additive lifecycle step plus additive API fields, with only minor documentation wording nits remaining.
Pull request overview
This PR updates the bridgetracker (and related docs/tests) to prevent premature “claimable” status by inserting a dedicated wait step that verifies the proof-building bridge-service instance has indexed the relevant L1 info tree leaf before the tracker reports WaitingClaim.
Changes:
- Adds a new lifecycle step
WaitingL1InfoLeafAvailable(always executed immediately beforeWaitingClaim) that polls/bridge/v1/l1-info-tree-indexon the proof-building bridge-service instance. - Adds
claim_statusto tracking responses/messages as a simplified claim-readiness indicator derived fromtracking_status+ current step. - Adds
api_revisionto/tracker/v1/healthto identify wire-contract revisions.
File summaries
| File | Description |
|---|---|
| test/e2e/proxy_tracker_test.go | Updates e2e expectations for the new step and asserts claim_status. |
| docs/bridgetracker/README.md | Documents the inserted step and updates the lifecycle mermaid diagram. |
| docs/bridgetracker/bridge-tracker-infographic.html | Updates infographic to include the new step and claim_status in examples/simulator. |
| docs/bridgetracker/API.md | Extends API docs with claim_status, api_revision, and the new step result shape. |
| docs/bridgetracker.md | Updates high-level bridgetracker docs to include the new step in each route. |
| docs/assets/swagger/bridge_tracker/swagger.json | Regenerated swagger artifact for the tracker docs. |
| bridgetracker/types/tracking.go | Introduces TrackerClaimStatus enum and string mapping. |
| bridgetracker/types/tracking_test.go | Adds unit test coverage for TrackerClaimStatus.String(). |
| bridgetracker/types/status.go | Adds StepWaitingL1InfoLeafAvailable and its result type. |
| bridgetracker/types/status_test.go | Extends step string tests for the new step. |
| bridgetracker/types/health.go | Adds CurrentAPIRevision and exposes it via HealthResponse. |
| bridgetracker/sources/ger.go | Adds L1InfoTreeIndexForBridge to query the proof-building instance for leaf availability. |
| bridgetracker/sources/ger_test.go | Adds httptest-based unit tests covering mainnet-origin vs L2-origin querying behavior. |
| bridgetracker/ports.go | Extends GERSource port with L1InfoTreeIndexForBridge. |
| bridgetracker/engine.go | Registers the resolver for StepWaitingL1InfoLeafAvailable. |
| bridgetracker/engine_test.go | Adds lifecycle tests ensuring the new step is present and never skipped across routes. |
| bridgetracker/domain/tracking_data.go | Adds TrackingData.ClaimStatus() derivation logic. |
| bridgetracker/domain/tracking_data_test.go | Adds derivation table tests for ClaimStatus(). |
| bridgetracker/domain/resolve_steps_test.go | Updates step resolution tests to include the new step and fact queries. |
| bridgetracker/domain/resolve_step_waiting_l1_info_leaf_available.go | Adds the new step resolver that waits until the proof-building instance reports the leaf index. |
| bridgetracker/domain/path.go | Updates expected paths to include WaitingL1InfoLeafAvailable before WaitingClaim. |
| bridgetracker/domain/path_test.go | Updates expected path tests for all bridge routes. |
| bridgetracker/domain/bridge_step_path.go | Extends step result plumbing to carry L1InfoLeafAvailableResult. |
| bridgetracker/bridgetracker_test.go | Extends health handler test to assert api_revision. |
| bridgetracker/api/tracking_data.go | Adds claim_status to wire TrackingData. |
| bridgetracker/api/tracking_data_test.go | Adds JSON and mapping tests for claim_status. |
| bridgetracker/api/health_command.go | Populates api_revision in the health response. |
| bridgetracker/api/docs/swagger.yaml | Updates tracker swagger yaml for new fields and result type. |
| bridgetracker/api/docs/swagger.json | Updates tracker swagger json for new fields and result type. |
| bridgetracker/api/docs/docs.go | Updates embedded swagger template for new fields and result type. |
| bridgetracker/api/bridge_step_path.go | Updates API step-path documentation comment to include the new result type. |
Review details
Files not reviewed (1)
- bridgetracker/api/docs/docs.go: Generated file
- Files reviewed: 30/31 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+26
to
+34
| // WaitingL1InfoLeafAvailableResolver resolves StepWaitingL1InfoLeafAvailable: whether the | ||
| // bridge-service instance that will actually build the claim proof — the origin network's own | ||
| // instance — has its own L1 info tree sync caught up to this bridge's deposit yet, per GET | ||
| // /bridge/v1/l1-info-tree-index. Always the current step right before StepWaitingClaim, on | ||
| // every route: that sync can lag behind whatever this tracker itself uses elsewhere | ||
| // (StepWaitL1SettledGER's own L1 RPC scan, StepWaitingGERInjection's destination-side injection | ||
| // check), so a bridge can look claimable per those checks yet still fail to claim because the | ||
| // proof-building instance cannot produce a proof for it yet — this is why it is never skipped | ||
| // or inferred from a sibling step, on any route (#1823) |
| <span class="routes"><span class="route-tag tag-l1l2">L1→L2</span><span class="route-tag tag-l2l1">L2→L1</span><span class="route-tag tag-l2l2">L2→L2</span></span> | ||
| </div> | ||
| <p class="step-desc">The GER that includes the bridge is available on the destination network. The bridge is ready to be claimed with <code>claimAsset()</code> / <code>claimMessage()</code>.</p> | ||
| <p class="step-desc">The destination network's own bridge-service instance has the covering L1 info tree leaf indexed. The bridge is ready to be claimed with <code>claimAsset()</code> / <code>claimMessage()</code>.</p> |
Comment on lines
495
to
+496
| These values are the <span class="mono">BridgeStep</span> enum, carried in every | ||
| <span class="mono">all_steps[].step</span> entry: <span class="mono">0→WaitingGERUpdate … 7→Claimed</span>. | ||
| <span class="mono">all_steps[].step</span> entry: <span class="mono">0→WaitingGERUpdate … 8→Claimed</span>. |
joanestebanr
added a commit
that referenced
this pull request
Sep 7, 2026
…info tree index lookup ActivitySource.IsReadyToClaim asked the destination network's own bridge-service instance for GET /bridge/v1/l1-info-tree-index, passing the origin network as the network_id query param. The real endpoint only accepts network_id == mainnet or network_id == the queried instance's own network, so every non-mainnet-originated bridge got rejected there and stayed reported as pending/errors.readiness even once claimable. Route it the same way GERSource.L1InfoTreeIndexForBridge already does (the #1829 fix): query the origin's own instance, falling back to the destination's instance only when the origin is mainnet (which has no bridge-service deployment of its own). Extracted the routing rule into a shared bridgeServiceClients.l1InfoTreeIndexClientFor helper used by both. Addresses #1831 (comment)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔄 Changes Summary
WaitL1SettledGERtoWaitingClaimonce the certificate's settlement tx was confirmed on L1 — but that doesn't guarantee the bridge-service instance that will actually build the claim proof has caught up yet, since its own L1 info tree sync can lag behind the finality this tracker uses forWaitL1SettledGER. A bridge could look claimable per this tracker's own settlement check yet still fail to claim because that instance couldn't produce a proof for it.WaitingL1InfoLeafAvailable, always right beforeWaitingClaim, on every route (L1->L2, L2->L1, L2->L2) — never skipped, never inferred from a sibling step. It callsGET /bridge/v1/l1-info-tree-indexagainst the bridge-service instance that will build the claim proof for the deposit — the origin network's own instance, or, when the origin is mainnet (which has no bridge-service deployment of its own), the destination's instance instead (network_id=0sent as a query parameter there, never as the URL-resolution key — mainnet has no resolvable bridge-service URL of its own).StepWaitingGERInjectiononly confirms an L2-side fact (the GER's injection tx landing on the destination's own contract); it says nothing about whether the proof-building instance's own sync has caught up, so this step always runs its own check regardless of what preceded it.claim_statusfield toTrackingData(both the REST tracker response and every WebSocketstatusmessage): a simplified claim-readiness summary —"error"whenevertracking_statusis"error"(takes priority over everything else),"claimed"once the current step isClaimed,"readyToClaim"while it isWaitingClaim,"pending"otherwise — for clients that only care about "is this claimable" without walkingall_stepsthemselves.api_revisionfield toGET /tracker/v1/health's response: an integer, bumped whenever a change to the tracker's wire API contract could break an existing client, so a client can tell which contract shape a given instance speaks without diffing full response bodies. Purely informational./tracker/v1/...responses now include one extra step,WaitingL1InfoLeafAvailable, inall_steps, right beforeWaitingClaim— on every route (L1->L2, L2->L1, L2->L2). Itsresultis{"l1_info_tree_index": <uint32>}. Existing steps'step_names andresultshapes are unchanged.TrackingDataalso gains a newclaim_statusfield, and the health response a newapi_revisionfield (both additive, existing fields unchanged).📋 Config Updates
✅ Testing
WaitingL1InfoLeafAvailable: unit tests inbridgetracker/domain/path_test.go,bridgetracker/domain/resolve_steps_test.go,bridgetracker/types/status_test.go, andTestEngineLifecycleL1ToL2/TestEngineLifecycleL2ToL1/TestEngineLifecycleL2ToL2inbridgetracker/engine_test.gowalking each route's full lifecycle through the new step.GERSource.L1InfoTreeIndexForBridge: real httptest-backed tests inbridgetracker/sources/ger_test.gocovering the mainnet-origin (destination queried) and L2-origin (self-queried) branches, plus not-yet-covered and URL-resolution-failure cases.claim_status:domain.TrackingData.ClaimStatus()derivation table inbridgetracker/domain/tracking_data_test.go, plus wire-level tests inbridgetracker/api/tracking_data_test.goandbridgetracker/types/tracking_test.go.api_revision: extendedTestHealthHandlerinbridgetracker/bridgetracker_test.go.🐞 Issues
🔗 Related PRs
📝 Notes
docs/bridgetracker.md,docs/bridgetracker/README.md(+ mermaid diagram) anddocs/bridgetracker/API.md, and regenerates the bridgetracker swagger docs (make generate-swagger-docs).