feat(aggkit): aggkit bridge-service client + multi-network aggregator (incl. bridge tracker, rc5) - #28
Open
arnaubennassar wants to merge 16 commits into
Open
feat(aggkit): aggkit bridge-service client + multi-network aggregator (incl. bridge tracker, rc5)#28arnaubennassar wants to merge 16 commits into
arnaubennassar wants to merge 16 commits into
Conversation
…egator Add a self-contained `src/aggkit/` module for the aggkit bridge service REST API (`/bridge/v1`), leaving the NATIVE module untouched: - `AggkitBridgeClient`: typed single-network client (bridges, claims, claim-proof, l1-info-tree-index, token-mappings, sync-status, health) with snake_case query serialization, page_size<=200 handling, BigInt-safe global_index parsing, and typed `AggkitApiError`. - `AggkitBridgeAggregator`: multi-network fan-out with partial-failure reporting, tiered status derivation (BRIDGED/LEAF_INCLUDED/ READY_TO_CLAIM/CLAIMED), ready-to-claim counting, claim-proof orchestration, and token metadata composition. - `ERC20.getMetadata()` on-chain read helper. - Fixture-driven unit tests + a live-enclave smoke script. Status derivation probes l1-info-tree-index by recording network (not token origin) and confirms READY_TO_CLAIM candidates via a targeted global_index claims query to avoid page-1-only false positives. Legacy bridge-hub `fetchMerkleProof` marked @deprecated (unused by dev-ui). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E4tXyxxSecCKHA4uqsiN81
…lt tolerance design.md (aggkit-proxy-l2l2 round) §2/§3 delta: - getInjectedL1InfoLeaf (client.ts): new destination-side GER-injection probe, message-matched 404 "not injected" vs the proxy's unrelated 404 "bridge service url not found" (§3.2). - getSyncStatus now sends network_id explicitly — unqualified requests 400 through aggkit-proxy (§2.3 gap G2). - resolveInjectedLeafIndex + Tier-2b gate in toTransaction: an L2-destination row only reaches READY_TO_CLAIM once the destination's GER is actually injected, closing the F1/F2 premature-READY window (measured >=8.02s live) that would otherwise build a claim proof that reverts GlobalExitRootInvalid on-chain (§3.1-§3.4). - getClaimInputs rewritten to build /claim-proof on the destination-injected index M (>= source index N), not the deposit's own L1-info-tree index — this was wrong for every L2-destination claim, including mainnet-relevant L1->L2 manual claims (§3.5). Adds an additive `sourceL1InfoTreeIndex` field. - getActivity/getReadyToClaimCount: guard every per-row Tier-2 probe so one network's failure degrades to `failedNetworks` instead of rejecting the whole call (§2.2 gap G1). - errors.ts: AggkitApiError.httpStatus JSDoc adds 502 (§2.5). Additive only: one new client method, one added return field, two behavioural fixes, one JSDoc line. No exports removed or renamed. Fixture-backed regression tests added per §3.7 (8 required cases): the L2->L2 premature-window/post-injection transition, L2->L1 skip-Tier-2b, synthetic M>N (S2 shape), the 404 message-matching hazard, the proxy-502 partial-failure path, and getSyncStatus's network_id. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PCzWpoQMLj1HTXksvLbmpG
…ross {0,1,2}
design.md §8 S8 item 7: point AGGKIT_URL at aggkit-proxy (direct port or
haproxy /aggkitapi), configure the aggregator with networks {1,2}, and skip
the getHealth check in proxy mode (§2.4 gap G3 — no root health route behind
the proxy).
Adds: full fan-out activity across the configured L2s, an L2->L2 row's
derived status (asserts CLAIMED for the known-autoclaimed L2-1->L2-2
deposit), getClaimInputs for a real L2->L2 AND a real L2->L1 deposit
(printing both sourceL1InfoTreeIndex and leafIndex), and an opt-in
(RUN_PARTIAL_FAILURE_TEST=true) proxy-502 partial-failure section that stops
aggkit-002-bridge, asserts getActivity resolves with failedNetworks naming
only network 2 while network 1's rows still render, then restarts it.
Verified live against the cdk enclave (direct proxy port + haproxy
/aggkitapi, both byte-identical) — all checks pass, enclave healthy
afterward. See handoff-sdk.md for the recorded output.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PCzWpoQMLj1HTXksvLbmpG
Document AggKit proxy usage and transaction status state machine: src/aggkit/index.ts: - Add multi-network proxy configuration example showing same base URL under multiple networkId keys (enables devnet proxy multiplexing) - Clarify that routing happens via ?network_id= query parameter src/aggkit/types.ts: - Document AggkitTransactionStatus lifecycle (BRIDGED → LEAF_INCLUDED → READY_TO_CLAIM → CLAIMED) - Explain LEAF_INCLUDED state: source settled, destination GER not yet injected (common on fresh enclaves where L2 block height exceeds L1) - Clarify that LEAF_INCLUDED only occurs for L2 destinations (L1 has no injection) These docs describe the correct configuration pattern for 2-L2 devnets with aggkit-proxy frontends, and explain the timing windows developers may observe during claim-readiness polling (design.md §0.1–§3.3). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PCzWpoQMLj1HTXksvLbmpG
…M_ADDRESS Re-running the S8 smoke script against the current enclave (recreated for the S12 E2E round) produced "4 CHECK(S) FAILED" with no indication why: the default FROM_ADDRESS is the EOA from one earlier round's lifecycle deposits, and bridge history does not survive `kurtosis enclave rm`. Sections 3/5/6/7 all failed for want of data, which reads identically to a genuine SDK regression. The script now prints an explicit diagnostic when the fan-out returns an empty activity page, naming the address and telling the operator to pass FROM_ADDRESS, and the header documents the default as enclave-specific. The assertions still fail (an empty page IS a smoke failure when you expected data) -- only the diagnosis is added. Confirmed with FROM_ADDRESS set to the dev-ui E2E wallet against the live 2-L2 enclave: every section passes, including the destination-injected getClaimInputs checks (L2->L2 sourceL1InfoTreeIndex=16/leafIndex=16, L2->L1 18/18). typecheck, lint and 93 unit tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PCzWpoQMLj1HTXksvLbmpG
Types for aggkit v0.11.0-rc4's tracker/v1 REST API (docs/bridgetracker/API.md), carrying raw numeric enums alongside their _string companions verbatim. Adds AggkitBridgeClient.getBridgeTracking(txHash, networkId?) over the same fetchRawText/AggkitApiError plumbing as the bridge/v1 endpoints, and AggkitBridgeAggregator.getBridgeTracking(networkId, txHash), which routes L1 through a configured L2 instance via clientForNetworkOrL1 and passes networkId through explicitly to the URL path (that routed-through instance's own networkId is not 0). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014GNMDLHYqUSAJ3qb2GKfK3
…ormat S4's tracker types were derived verbatim from docs/bridgetracker/API.md; a live capture off a real rc4 enclave (plans/bridge-tracker/fixtures/) proved the wire deviates further than that doc's own "numeric + _string companion" convention suggests. tracking_status, bridge_type, step_name (not `step`), and per-step status all ship as bare strings with no numeric/companion fields; BridgeStatus nests leaf_type/origin/destination/ amount under an undocumented `event` object; per-step start_date/end_date/ result/error are omitted keys (not null) until applicable; and CertificateData.settlement_tx_hash is omitted (not null) pre-Candidate. error_type and CertificateData.status keep their documented numeric + _string pairs, confirmed unchanged by the same fixtures. Copies a representative fixture subset into src/aggkit/__fixtures__ and adds src/aggkit/__tests__/tracker.test.ts covering URL construction (including network-0 L1 routing through the aggregator), the registered-only and giving-up-error shapes, and the resolved route (step count/order/per-step results) for all three typologies. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014GNMDLHYqUSAJ3qb2GKfK3
Document getBridgeTracking's operational contract on both entry points (AggkitBridgeClient and AggkitBridgeAggregator) and in the module's top-level doc comment, matching the style of the earlier multi-network-proxy/status-lifecycle docs (2ec0d5e): - Terminal semantics: tracking_status === 'finished', or 'error' with bridge_status still null (the tracker giving up entirely). Per-step errors inside all_steps[i].error are explicitly non-terminal -- the tracker retries those on its own. - Polling guidance: no push/subscription channel exists, so callers must poll; ~5s is a good default (matches the dev-ui consumer), stopping once the terminal condition above is met. - Server-side registration/eviction: the first call for a (networkId, txHash) pair registers it (tracking_status: 'registered', all_steps: null); a RetentionPeriod eviction of a still-pending bridge causes the same 'registered'/null-steps response on the next poll, not an error -- treat it like the original registration, not a regression. - Cross-reference the aggregator's getBridgeTracking to AggkitBridgeClient's for the wire-format deviations already documented in types.ts (bare string enums, step_name vs step, etc.) rather than duplicating them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014GNMDLHYqUSAJ3qb2GKfK3
… semantics Two JSDoc claims contradicted aggkit v0.11.0-rc4's actual behavior: - A per-step error does NOT leave tracking_status at 'running' -- aggkit derives tracking_status from the step at step_index (bridgetracker/domain/tracking_data.go), so a step in 'error' reports tracking_status 'error' with bridge_status populated. This is exactly why the documented terminal check needs the bridge_status === null conjunct rather than tracking_status alone. - TrackingData.error is not "set only if the tracker gave up": the tracker_registered.json fixture carries a transient error at retry_count 1 while tracking_status is still 'registered' (retries in progress). Only the exhausted (2) case is the giving-up terminal. Doc-only; no runtime change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014GNMDLHYqUSAJ3qb2GKfK3
v0.11.0-rc5 (agglayer/aggkit#1784) rewrote docs/bridgetracker/API.md to match the tracker's wire format exactly, closing #1781; the serializer itself never changed between rc4 and rc5, and live rc5 fixtures confirm identical shapes (plans/bridge-tracker-rc5/verification.md §4). Update the tracker JSDoc in types.ts/client.ts/aggregator.ts so it no longer frames the bare-string enums, event nesting, and omitted-optional-keys behavior as an open API.md drift/bug — upstream docs now agree with what the SDK already modeled from fixtures. No type-shape changes. Add an rc5 provenance note to the tracker fixture-loading test file confirming the rc4 captures remain valid, live-verified unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014GNMDLHYqUSAJ3qb2GKfK3
`ChainRegistry.getChainByNetworkId()` previously returned the first insertion-order match, so the SDK's pre-seeded default chains (e.g. Ethereum mainnet at networkId 0) shadowed a consumer-registered chain sharing the same networkId (e.g. a devnet L1, also at networkId 0). Dormant in practice (console-triage.md) but real: an ERC-20 bridged from devnet L1 whose token isn't in a UI's static list would resolve mainnet's rpcUrl and hit https://eth.llamarpc.com instead of the devnet RPC. Track which chainIds are built-in defaults at construction time and prefer a non-default (consumer-registered) match on networkId lookup collisions, independent of registration order. No override still resolves defaults exactly as before. Public API is unchanged. Reconciles the TODO(aggkit-migration) at aggregator.ts:653-666. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014GNMDLHYqUSAJ3qb2GKfK3
- Add a "Bridge Transaction Tracking" README subsection: getBridgeTracking usage sample, polling/terminal-state guidance, the bare-string enum shape note, and the #1786 claim-readiness caveat. - Replace local-only plans/ path citations in types.ts and tracker.test.ts with durable content: fixture provenance stated inline (no path), and agglayer/aggkit#1781/#1784 issue/PR references for the rc4->rc5 API.md correction, instead of a /home/brolygon/... verification doc path. - Opportunistically strip the /home/brolygon/... absolute-path prefix from the pre-existing design.md/handoff-sdk.md citations in types.ts and scripts/aggkit-smoke.ts (username leak fix; the broader design.md sweep remains deferred). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014GNMDLHYqUSAJ3qb2GKfK3
fea252e reframed the per-type JSDoc in types.ts after aggkit v0.11.0-rc5 (PR #1784) corrected docs/bridgetracker/API.md to match the wire format, but missed the module-level comment in this barrel, which still framed the SDK's types as "deviating" from upstream docs. Post-rc5 the docs agree with the types; only rc4's API.md described them differently. Comment-only change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014GNMDLHYqUSAJ3qb2GKfK3
arnaubennassar
added a commit
to agglayer/agglayer-dev-ui
that referenced
this pull request
Aug 10, 2026
…ailure cause The old comment said "revert to a semver range once the aggkit SDK surface ships to npm (tracked for S15)" but didn't mention that the file:../sdk override is the direct reason `pnpm install --frozen-lockfile` fails on GitHub CI (ENOENT scandir on the missing sibling ../sdk checkout) for both the lint/typecheck/test and e2e workflows on PR #24. Rewrite the comment to state that plainly and spell out the dependency ordering to clear it: agglayer/sdk#28 merges -> a tracker-API sdk version is published via workflow_dispatch -> swap this override to that published semver. Local dev and local CI-equivalent checks are unaffected since a sibling ../sdk checkout is present here; only GitHub's runners lack one. Comment-only change, no functional effect: lint/typecheck/test (54 tests) stay green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014GNMDLHYqUSAJ3qb2GKfK3
CodeQL flagged `config.baseUrl.replace(/\/+$/, '')` (client.ts:70) as a polynomial regex over library-supplied input (bridge check on PR #28): a `+` quantifier anchored at `$` against an attacker/consumer-controlled `baseUrl` is a recognized ReDoS shape, even though this specific pattern has no exploitable nested-quantifier or overlapping-alternation blowup. Replace the regex with `stripTrailingSlashes()`, a manual backward scan that strips trailing '/' characters in a single linear pass -- same semantics (trim any number of trailing slashes before appending /bridge/v1 or /tracker/v1), no regex engine involved. The tracker not-ready pattern matching (L1_INFO_TREE_INDEX_NOT_READY_PATTERNS / INJECTED_L1_INFO_LEAF_NOT_READY_PATTERNS) was already plain `.includes()` on a lowercased string -- not implicated by this alert -- and is unchanged. Add regression tests: multiple trailing slashes, and a 50k-slash input to cover the ReDoS-adjacent long-input case the original alert worried about. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014GNMDLHYqUSAJ3qb2GKfK3
arnaubennassar
marked this pull request as draft
August 11, 2026 10:11
This was referenced Aug 11, 2026
…comments Removes citations to machine-local planning documents (design.md, handoff-sdk.md, manual-validation.md, RELEASE.md, console-triage.md, enclave-notes.md and their §-numbered sections) from comments, test titles, and diagnostic strings across the aggkit module. Every citation is replaced with the underlying fact inlined directly, or dropped where the surrounding text already carried it — no behavior change. Comment-only sweep; dist/ rebuild confirms every line that differs in emitted output is itself a comment or JSDoc line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014GNMDLHYqUSAJ3qb2GKfK3
Missed by the design-doc citation sweep (3879690): a bare `§` symbol with no section number, left over from before that PR. Names the actual regression coverage instead of a dangling symbol. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014GNMDLHYqUSAJ3qb2GKfK3
arnaubennassar
marked this pull request as ready for review
August 14, 2026 14:17
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.
What & why
Adds a self-contained
src/aggkit/module: a typed client and multi-network aggregator for the aggkit bridge service REST API (/bridge/v1). This is the API surface behind the dev-ui's migration off the legacy Bridge Hub API (companion PR agglayer/agglayer-dev-ui#24): everything the UI needs — activity, status derivation, claim proofs, ready-to-claim counts, token metadata, and (later in this PR) bridge tracking — is served from aggkit through this module. The NATIVE module is untouched; the legacy bridge-hub proof path is retained (still used by the NATIVE claim flow) and marked@deprecated.14 commits, oldest layer first:
1. Bridge-service client + multi-network aggregator (
0fa7c77)AggkitBridgeClient— typed single-network client (bridges, claims, claim-proof, l1-info-tree-index, token-mappings, sync-status, health) with snake_case query serialization,page_size <= 200handling, BigInt-safeglobal_indexparsing (raw-text parse), and a typedAggkitApiError.AggkitBridgeAggregator— multi-network fan-out with partial-failure reporting (failedNetworks), tiered status derivation (BRIDGED → LEAF_INCLUDED → READY_TO_CLAIM → CLAIMED, with a targetedglobal_indexclaims probe to avoid page-1-only false positives), ready-to-claim counting, claim-proof orchestration, and token-metadata composition.ERC20.getMetadata()on-chain read helper (aggkit/token-mappingscarries no name/symbol/decimals).2. Claim correctness + proxy fault tolerance (
c472bd0)getInjectedL1InfoLeaf) gates READY_TO_CLAIM for L2-destination rows, closing a measured ≥8s premature-READY window whose claim proof would revertGlobalExitRootInvalidon-chain.getClaimInputsrebuilt on the destination-injected leaf index rather than the deposit's own L1-info-tree index (previously wrong for every L2-destination claim, incl. L1→L2 manual claims); additivesourceL1InfoTreeIndexfield.failedNetworksinstead of rejecting the whole call;getSyncStatussendsnetwork_idexplicitly (unqualified requests 400 through aggkit-proxy).3. Live smoke script + docs (
fef61f2,b706110,2ec0d5e)scripts/aggkit-smoke.tsextended to run against aggkit-proxy across networks {0,1,2} (full fan-out activity, real L2→L2 and L2→L1 claim-input checks, opt-in proxy-502 partial-failure section); multi-network proxy configuration and the status lifecycle documented.4. Bridge tracker API (
a8a44a0,c254ec2,3c6d475,5ad31b0)getBridgeTracking— tracker types + client method + aggregator wiring, reconciled against the live-captured wire format (not the docs, which were wrong pre-rc5 — see #1781); terminal-state, step-error, and transient-resolve semantics plus polling guidance documented. This is what the dev-ui tracker UI (companion PR) polls.5. rc5 phase (
fea252e,76fe6a4,8094f92,9a5a5b9,5bbda2b)fix(native): consumer-registered chains now win networkId collisions in the chain registry (see breaking-change note below) — previously the SDK's pre-seeded Ethereum mainnet (networkId 0) shadowed a consumer-registered devnet L1.Upstream issue context
expected_durationnever emitted). Fixed upstream in rc5 (fix: bridgetracker API.md drift, fail-fast on unset L1GlobalExitRootAddress, unlimited default rate limit (#1781, #1782, #1783) aggkit#1784). Our tracker types were never derived from the buggy docs — they're fixture-derived from live-captured rc4/rc5 wire payloads (30+ payloads checked against 0 violations during review) — so no functional fix was needed here, only a JSDoc pass (76fe6a4) to stop citing the pre-fix API.md text.WaitingClaimprecedes real claimability by a measured ~16s gap. No SDK-side change; the client surfaces tracker status as-is, and dev-ui handles the UX mitigation ("Finalizing claim data").Testing evidence
npm run typecheck,npm run lint: clean.npm run test:run: 113/113 unit tests passing across 5 files, including new registry-precedence tests (registry.test.ts, 6 tests) and the full tracker/aggregator/client suites (33 + 13 + 31 tests). The aggregator suite includes fixture-backed regressions for the L2→L2 premature-window/post-injection transition, the L2→L1 skip path, the 404 message-matching hazard, and the proxy-502 partial-failure path.npm run build: green (ESM + CJS +.d.tsall build successfully).scripts/aggkit-smoke.ts) exercises the client + aggregator against a real kurtosis devnet, both direct and through aggkit-proxy across networks {0,1,2} (migration-phase baseline run: 117 checks PASS / 0 FAIL).76fe6a4and re-verified (typecheck/lintclean, 113/113 tests). Full attack-list writeup lives in a local plans workspace, not linkable — summarized here.Breaking-change notes
None for existing consumers — the entire aggkit module is additive (new type/value exports only), and the NATIVE surface is untouched. The chain-registry fix (
8094f92) only changes behavior when a consumer explicitly registers a chain whosenetworkIdcollides with one of the SDK's built-in defaults — previously the built-in default silently won that collision; now the consumer's registration wins, which is the documented/expected precedence. No default-registry behavior changes for consumers who don't register overrides. LegacyfetchMerkleProofis retained (the NATIVE claim flow still uses it) and marked@deprecated.Reviewer notes
workflow_dispatchworkflow, not triggered on merge tomain.file:../sdklocal snapshot dependency during development, so it should be reviewed/merged in this order: sdk → dev-ui. kurtosis-cdk's companion change is independent of both (deploys the enclave-side backend + tracker).Citation sweep (agglayer/aggkit#1791)
Contributes to agglayer/aggkit#1791.
Local design-doc citations in this branch were replaced with self-contained comments in commits
3879690(sweep) +5680d83(review fix), with no behavior change: dist proven byte-identical after comment-stripping, 115/115 tests passing.Merging this PR does not complete agglayer/aggkit#1793 — the sdk release is a separate manual
workflow_dispatchstep (.github/workflows/release.yml), and the dev-ui dependency swap to the published version must follow it. agglayer/aggkit#1793 is tracked independently and gets closed manually once both of those steps have happened.Other issues to close
Closes agglayer/aggkit#1800.