Skip to content

fix: aggkit-proxy: bali integration: bridge tracker activity endpoint, GER settlement/injection fixes - #1815

Merged
joanestebanr merged 16 commits into
developfrom
feat/fix_bali_integration
Sep 2, 2026
Merged

fix: aggkit-proxy: bali integration: bridge tracker activity endpoint, GER settlement/injection fixes#1815
joanestebanr merged 16 commits into
developfrom
feat/fix_bali_integration

Conversation

@joanestebanr

@joanestebanr joanestebanr commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

🔄 Changes Summary

  • feat(bridgetracker): new GET /tracker/v1/activity/from/{from_address} endpoint that scans every bridge service known to the bridgeservicefinder (via new Finder.NetworkIDs()) for bridges sent by an address and resolves each one's claim state (ActivityCache, ActivitySource, ActivityQuerier/ActivityBridgeScanner/ActivityClaimChecker ports). Opt-in via includeTracking=true to also register still-unclaimed bridges with the tracker. Endpoint is only registered when wired (proxy/cmd/run.go), and swagger docs are regenerated.
  • feat(bridgetracker): StepClaimed is now its own tracked step with a dedicated ClaimedResolver that fetches the claim tx/block from the destination network's bridge service, decoupled from StepWaitingClaim, which now checks isClaimed() on-chain directly through a new ClaimChecker port instead of waiting on the indexed claim record — faster and authoritative, at the cost of not carrying claim tx details itself (that's what the new Claimed step result is for). The on-chain binding/cache logic is factored out of ActivitySource into sources/claim_checker.go (contractClaimCheckers) and shared by both the tracker engine and the activity endpoint.
  • fix(bridgetracker): introduce domain.ScannedBridge to track which network's bridge service actually reported a scanned bridge (NetworkID), distinct from Bridge.OriginNetwork (the bridged asset's origin network) — the two diverge when an asset is re-bridged across more than one hop, which was feeding the wrong sourceBridgeNetwork into isClaimed() and the wrong network into TrackingID/the activity endpoint's bridge_network_id for such bridges.
  • feat(bridgetracker): new GET /bridge-address[/{network_id}] endpoint, resolving the bridge contract address for one network or every network currently known — opt-in via Config.BridgeAddressResolver, wired in proxy/cmd/run.go off bridgeservicefinder.Finder directly. Backed by a new Config.BridgeAddress override map on bridgeservicefinder.Finder itself (priority: per-network entry, then [0] as the default, then the rollup manager's own on-chain BridgeAddress()).
  • feat(bridgetracker): ClaimResult now also carries BlockTimestamp alongside BlockNumber. InjectedGERResult is restructured into L1InfoTreeLeaf (the L1 UpdateL1InfoTree/UpdateL1InfoTreeV2 event that produced the covering leaf) and an optional L2InjectedGER (the actual L2 block/timestamp the GER was injected at on the destination network) — fixing bug: proxy: step WaitingGERInjection as result have the L1 block #1818, where the L1 block was returned in the L2 field's place. When the destination's bridge-service instance doesn't report the L2 injection block itself (injected_l2_block_num/injected_l2_block_timestamp on GET /bridge/v1/injected-l1-info-leaf), GERSource falls back to scanning that network's own GlobalExitRootManagerL2 contract for the UpdateHashChainValue event backwards in chunks, via the new Tracker.L2GlobalExitRootAddress per-network contract address map (a workaround-only fallback, see its doc) — bounded by a new Tracker.L2InjectionLookbackBlocks (default 1,000 blocks) instead of always walking back to genesis.
  • fix(bridgetracker): resolve the settled GER correctly when the settlement tx has no UpdateL1InfoTree event: walk backwards on L1 for the most recent earlier one instead of surfacing "not ready" forever, excluding same-block logs from a later transaction, and querying eth_getLogs in chunks that respect providers' 10,000-block range cap.
  • fix(bridgetracker): gate certificate settlement on the settlement tx actually being visible on L1 (not just the agglayer client reporting Settled), avoiding a premature TransactionReceipt lookup that would otherwise fail.

⚠️ Breaking Changes

  • None. Config.BridgeAddressResolver and the new bridgetracker activity/bridge-address ports are additive/opt-in.

📋 Config Updates

  • 🧾 New optional BridgeServiceFinder.BridgeAddress map (networkID → address override), default empty — consulted by the new GET /bridge-address[/{network_id}] endpoint before falling back to the rollup manager's on-chain BridgeAddress(). That endpoint itself is gated by Config.BridgeAddressResolver (Go-level wiring, not a TOML key); unset leaves both routes unregistered.

  • 🧾 New optional Tracker.ActivityIdleTimeout (defaults to IdleTimeout if unset) — idle timeout for the activity endpoint's own cache.

  • 🧾 New optional Tracker.L2GlobalExitRootAddress (networkID → GlobalExitRootManagerL2 address map, default empty) and Tracker.L2InjectionLookbackBlocks (default 1000) — workaround-only fallback for a destination network whose bridge-service instance predates L2 injection block reporting (fix(bridgeservice): expose real L2 injection block/timestamp on injected-l1-info-leaf #1819); defaults leave behavior unchanged for everyone else.

    [BridgeServiceFinder.BridgeAddress]
    # optional per-network override, consulted in this priority order by Finder.BridgeAddress:
    # BridgeAddress[networkID], then BridgeAddress[0] (default for every other network), then the
    # rollup manager's own on-chain BridgeAddress()
    0 = "0xShared_L1_bridge_address_used_as_the_default"
    5 = "0xNetwork5s_own_bridge_address"
    
    [Tracker]
    ActivityIdleTimeout = "30m"
    L2InjectionLookbackBlocks = 1000
    
    # Workaround only: uncomment for a destination network whose bridge-service instance does not
    # report the L2 block a covering GER was injected at.
    # [Tracker.L2GlobalExitRootAddress]
    # 1 = "0x..."

✅ Testing

  • 🤖 Automatic: go build ./... and go test ./bridgetracker/... ./bridgeservicefinder/... ./autoclaim/... ./proxy/... pass, including new/updated regression tests for the activity endpoint (activity_test.go, sources/activity_test.go, cache_test.go), the claimed-step split (resolve_steps_test.go, engine_test.go), the new bridge-address endpoint (bridge_address_test.go), the settlement GER backwards-search fix (settlement_test.go), and the L2 injection fallback/lookback cap (sources_test.go's TestGERSourceInjectedGER_FallsBackToL2Scan and TestFindL2InjectionBlockBackwards, proxy/config/config_test.go).

🐞 Issues

🔗 Related PRs

  • None.

📝 Notes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3712ed8ae8

ℹ️ 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".

Comment thread bridgetracker/sources/activity.go Outdated
Comment thread bridgetracker/sources/settlement.go Outdated
Comment thread bridgetracker/sources/settlement.go
@joanestebanr joanestebanr changed the title fix(bali): bridge tracker activity endpoint, GER settlement/removal fixes, dead-network handling fix: aggkit-proxy: bali integration: bridge tracker activity endpoint, GER settlement/removal fixes, dead-network handling Aug 27, 2026
joanestebanr and others added 13 commits September 1, 2026 17:40
…l-clock

op-pp's L1 image (arnaubennassar/geth:op-pp) has its chain data baked in at
build time and never advances past that snapshot. The op-geth entrypoint was
patching the L2 genesis timestamp to date +%s (real wall-clock) on every
start, so the gap between the L2 genesis and its L1 origin block grows by one
day for every day that passes since the L1 image was built. Once that drift
exceeded rollup.json's max_sequencer_drift (600s), op-node's sequencer could
never find a valid L1 origin for the first post-genesis block and the L2
chain stalled forever at block 0 -- surfacing as
"wait for MintableERC20 deployment: context deadline exceeded" during
LoadEnv, since op-pp's L1 snapshot is from Feb 2026 (~6 months of drift by
now).

Fix: read L1's actual head timestamp and use it to patch the L2 genesis
instead of wall-clock time, keeping L2 genesis anchored to L1's frozen origin
regardless of what day the test actually runs.

Verified locally: op-geth-001/op-node-001 went from stuck at block 0 to
actively sequencing new L2 blocks.

Note: since L1 never advances, the chain still stalls again once L2's
virtual time drifts past max_sequencer_drift from the anchored origin
(~1800s of L2 time in local testing) -- well past LoadEnv/MintableERC20
deployment, but a longer-running test could still hit it. Left as a known
follow-up rather than widening scope here.
L1's chain data is baked into its image at build time and never advances
past block 384. Anchoring L2 genesis to L1's head (previous commit) fixes
LoadEnv, but once the sequencer has produced ~600s (max_sequencer_drift)
worth of L2 blocks since genesis, op-node's origin-selector needs a newer
L1 origin than block 384 to keep going and never finds one, stalling the
chain forever mid-test-run.

Raise max_sequencer_drift to a week so the sequencer never needs to look
for a newer L1 origin within the lifetime of a test run.

Found while investigating CI failures on #1810.
…ateL1InfoTree event

Fixes #1811.

A cert's settlement tx on L1 doesn't always emit UpdateL1InfoTree itself — when
the settlement doesn't move the GER, it just propagates whatever GER an earlier
update already established. StepWaitL1SettledGER treated the missing event as
"not ready yet" and stalled forever instead of recognizing this case.

SettlementSource now:
- Fails fast (domain.ErrBadSettlementTx, permanent) when the receipt is missing
  VerifyBatchesTrustedAggregator, instead of silently returning "not ready".
- When UpdateL1InfoTree is missing, walks L1 backwards in bounded chunks
  (findEventUpdateL1InfoTreeBackwards) to find the closest earlier
  UpdateL1InfoTree event and uses its GER.
- Requires the L1 GlobalExitRoot contract address (NewSettlementSource) to
  scope that backwards search.

resolve_steps.UpdateStep now distinguishes permanent step failures
(IsPermanent) from transient ones: a permanent error marks the step
StepErrorPermanent immediately instead of accumulating a retry history that
will never be retried.

L1SettledGERResult now carries where each piece of evidence was found
(SettlementBlockNumber/SettlementLogIndex, GERBlockNumber/GERLogIndex)
instead of a single BlockNumber, since the GER-producing event can now live
in a different block than the settlement tx itself.

Also logs the set of resolved network entries once bridgeservicefinder
finishes building its initial cache, to aid diagnosing network-resolution
issues like the one reported in bali.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds a new activity endpoint to the bridge tracker that answers "what
bridges has this address sent, and what is their claim state", across
every bridge service the bridgeservicefinder currently knows about
rather than one network at a time:

- bridgeservicefinder.Finder gains NetworkIDs(), enumerating every
  network currently resolved (i.e. every network GetURL would presently
  succeed for), backed by a new cache.networkIDs() read.
- bridgetracker/domain/activity.go defines the ActivityEntry model and
  the driven ports (ActivityBridgeScanner, ActivityClaimChecker,
  ActivityQuerier) the endpoint depends on; bridgetracker/activity.go
  implements ActivityCache, composing a scan across networks with claim
  resolution and (optionally, via includeTracking=true) registering
  still-unclaimed bridges with the tracker.
- bridgetracker/sources/activity.go implements ActivitySource, the
  adapter over the per-network bridge-service/JSON-RPC clients used
  elsewhere in the tracker.
- bridgetracker/api/activity_command.go + api.go wire
  GET /tracker/v1/activity/from/{from_address}; the route is only
  registered when both Config.ActivityScanner and Config.ActivityClaims
  are set, so the endpoint is entirely opt-in.
- proxy/cmd/run.go wires the new sources.ActivitySource into the
  tracker config using the existing finder/rpcClients/BridgeAddrs.
- bridgetracker/types/claim_status.go adds the claim-status vocabulary
  shared between the activity endpoint and its sources.
- Regenerated swagger docs (bridgetracker/api/docs,
  docs/assets/swagger/bridge_tracker) for the new route.
- Mocks for the new ports generated under bridgetracker/mocks; unrelated
  autoclaim call sites updated for the new
  bridgeservicefinder.Finder.NetworkIDs() method on the interface.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…k IDs, auto bridge address, incremental cache

Builds on the GET /activity/from/{from_address} endpoint (3712ed8) with:

- filterBridges query param (all|claimed|pending|error, default all): lets
  a caller ask for only claimed, only pending, or only errored bridges.
  Requesting pending/error skips fetching a claimed bridge's claim record
  (it would be filtered out anyway) — the entry simply stays unsettled and
  is fetched normally once a filter that needs it is used.
- claimed becomes a tri-state string ("false"/"true"/"error") instead of a
  bool, via types.ClaimStatus, so a failed isClaimed() check (e.g. no
  bridge contract address configured) is never confused with "not
  claimed"; the failure message is reported under errors["claim"].
- bridge_network_id / claim_network_id sit alongside the raw bridge/claim
  payloads (kept byte-for-byte as the bridge service returned them)
  instead of wrapping them, so callers know which bridge service produced
  each one without altering the response shape.
- bridgeservicefinder.Finder gains BridgeAddress(ctx, networkID): defaults
  to the rollup manager's own on-chain BridgeAddress() (an immutable
  constructor parameter, resolved once and cached forever), overridable
  per network via the new BridgeServiceFinder.BridgeAddress config map —
  and a BridgeAddress[0] override doubles as the default for every network
  without its own entry. ActivitySource now resolves destination bridge
  contracts through this instead of a manually maintained address map.
- ActivityCache no longer re-scans every page of every network on every
  call: ActivityBridgeScanner.BridgesFrom takes the caller's already-known
  global indexes and each network's scan stops at the first already-known
  bridge, relying on the bridge service's own newest-first order. Once a
  bridge is confirmed claimed, isClaimed() is never asked again for it
  (only its claim record may still need fetching); once a claim record is
  fetched, it is cached for good. A from_address idle for
  Config.ActivityIdleTimeout (default 30m, mirroring IdleTimeout) is
  forgotten entirely on the next request, freeing everything cached for
  it — swept lazily on access rather than a dedicated ticker/goroutine.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… endpoint, document it

- ActivityEntry gains CreatedAt/UpdatedAt: CreatedAt is stamped once (or
  carried forward from the previous cache entry) and never changes;
  UpdatedAt is stamped on every refresh, so it freezes once a bridge
  settles (claimed with its claim record fetched) since it is never
  refreshed again from that point on.
- ActivityItem exposes them as creation_timestamp/last_updated_timestamp
  (unix seconds, matching the rest of the API's timestamp fields).
- docs/bridgetracker/API.md: documents the whole activity endpoint end to
  end (it had none before) — request params (includeTracking,
  filterBridges), response shape (ActivityResponse/ActivityItem), the
  pass-through BridgeResponse/ClaimResponse shapes, an example, and the
  caching/eviction behavior.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…block timestamps

- StepClaimed now gets its own resolver (ClaimedResolver) fetching the claim tx/block
  from the destination bridge service, decoupled from StepWaitingClaim, which now checks
  isClaimed() on-chain directly via the new ClaimChecker port instead of waiting on the
  claim record.
- Factor the on-chain isClaimed() binding/cache logic out of ActivitySource into
  sources/claim_checker.go (contractClaimCheckers), shared by both the tracker engine's
  ClaimChecker and the activity endpoint's ActivitySource.
- Introduce domain.ScannedBridge to track which network's bridge service actually reported
  a scanned bridge (NetworkID), distinct from Bridge.OriginNetwork (the bridged asset's
  origin network) — they diverge for a re-bridged asset across more than one hop, which
  was feeding the wrong sourceBridgeNetwork into isClaimed() and the wrong network into
  TrackingID for such bridges.
- Add GET /bridge-address[/{network_id}], resolving the bridge contract address for one
  network or every network currently known (opt-in via Config.BridgeAddressResolver; wired
  in proxy/cmd/run.go off bridgeservicefinder.Finder).
- ClaimResult and InjectedGERResult now also carry BlockTimestamp alongside BlockNumber.
- Regenerate swagger docs and update API.md accordingly.
A certificate can flip to Settled in the agglayer before its settlement tx
is actually visible on L1, letting StepCertificatePending resolve early.

- CertificateSource now resolves the settlement tx's block number/timestamp
  on L1 (settlementBlockInfo) and exposes them as CertificateData.BlockNumber/
  BlockTimestamp; both stay nil while the receipt is not mined/visible yet.
- CertificatePendingResolver only treats the step as done once the
  certificate is settled AND BlockNumber is known, otherwise it keeps
  returning ErrCertificateNotSettled.
- SettlementSource now also surfaces SettlementBlockTimestamp/
  GERBlockTimestamp on L1SettledGERResult.
- Wire the new EthClientResolver dependency into NewCertificateSource
  (proxy/cmd/run.go).
- Update docs/bridgetracker/API.md and tests accordingly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ly fallback

Document that Tracker.L2GlobalExitRootAddrs should only be set for a
destination network whose bridge-service instance does not report the
L2 block a covering GER was injected at.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…activity endpoint

Previously ActivitySource.BridgesFrom aborted the whole GET /activity/from/{address}
scan on the first network it could not reach, even though every other network's
bridges were still available. Now it skips the failing network, logs it, and the
endpoint reports it in a new "warnings" field instead — bridges found on every
other network are still returned.

- domain: new ActivityWarning type; ActivityBridgeScanner.BridgesFrom and
  ActivityQuerier.GetActivity now also return []ActivityWarning
- sources: ActivitySource.BridgesFrom continues past a failing network instead of
  returning early; gained a logger
- api: ActivityResponse gains an omitempty "warnings" field ([]ActivityWarningItem)
- docs/swagger regenerated for the new field
…back and lookback cap

InjectedGERResult now splits into L1InfoTreeLeaf (the L1 UpdateL1InfoTree
event that produced the leaf) and the new, optional L2InjectedGER (the
actual L2 block/timestamp the GER was injected at on the destination
network) -- fixing #1818, where the L1 block was returned in the L2
field's place.

- bridgeservice.L1InfoTreeLeafResponse carries the new
  injected_l2_block_num/injected_l2_block_timestamp fields when the
  destination's bridge-service instance reports them.
- When it doesn't (an older instance), GERSource falls back to scanning
  the destination network's own GlobalExitRootManagerL2 contract for the
  UpdateHashChainValue event, backwards in chunks, via the new
  Tracker.L2GlobalExitRootAddress per-network contract address map.
- That backward scan is now bounded by the new
  Tracker.L2InjectionLookbackBlocks (default 1,000 blocks) instead of
  always walking back to genesis, so a network where the fallback never
  finds the event doesn't cost an unbounded eth_getLogs scan on every
  lookup.
- Document both new Tracker config fields (docs/bridgetracker.md,
  proxy/config/default.go) and the new InjectedGERResult response shape
  (docs/bridgetracker/API.md).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@joanestebanr joanestebanr self-assigned this Sep 1, 2026
@joanestebanr
joanestebanr force-pushed the feat/fix_bali_integration branch from 1fd1119 to 23e68c8 Compare September 1, 2026 15:44
arnaubennassar
arnaubennassar previously approved these changes Sep 2, 2026
@joanestebanr

Copy link
Copy Markdown
Collaborator Author

@claude review

@claude

claude Bot commented Sep 2, 2026

Copy link
Copy Markdown

Claude finished @joanestebanr's task in 4m 1s —— View job


✅ Code Review Complete

Reviewed PR #1815: Bali integration fixes and activity endpoint


🚀 What's New

This PR introduces a comprehensive activity tracking endpoint (GET /tracker/v1/activity/from/{from_address}) that scans all known bridge services for user bridges and tracks claim status. It also fixes several critical issues in GER settlement/injection tracking, adds a bridge address resolution endpoint, and introduces dead-network handling to skip unreachable networks during startup.

🐛 Bug Fixes

  • Fixed bug: proxy: step WaitingGERInjection as result have the L1 block #1818: L2 GER injection block/timestamp now correctly reports actual L2 values instead of L1 block data
  • Fixed GER settlement resolution for settlement transactions without UpdateL1InfoTree events (with new backwards search)
  • Fixed isGERRemovedFromL2 to handle RPC providers that cap eth_getLogs block ranges via chunked scanning
  • Introduced ScannedBridge domain model to correctly track source network ID (where bridge tx occurred) vs asset origin network ID
  • Split StepClaimed into separate step with dedicated ClaimedResolver that fetches claim tx details from bridge service
  • StepWaitingClaim now uses authoritative on-chain isClaimed() check via new ClaimChecker port

📋 Config Updates

  • IgnoreNetworkIDs = [] under [BridgeServiceFinder] / [AutoClaim.BridgeServiceFinder] - Skips known-dead networks during enumeration. Empty array means no networks ignored. Example: IgnoreNetworkIDs = [1001, 1002]
  • L2GlobalExitRootAddress = {} under [Tracker] - Maps networkID → GlobalExitRootManagerL2 contract address for L2 injection fallback when bridge service doesn't report it. Empty map disables fallback. Example: L2GlobalExitRootAddress = {1 = "0x...", 2 = "0x..."}
  • L2InjectionLookbackBlocks = 1000 under [Tracker] - Limits backwards scan for L2 GER injection to prevent full chain scan on misconfigured networks. Default: 1000 blocks
  • ActivityIdleTimeout under [Tracker] - Idle timeout for activity cache entries. Defaults to IdleTimeout value if not set
  • Go-level wiring: Config.BridgeAddressResolver, Config.ActivityScanner, Config.ActivityClaims (no TOML keys)

⚠️ Breaking Changes

None - All new features are opt-in or additive with backward-compatible defaults.


🔴 Critical Issues (Required Fixes)

Issue 1: Off-by-one error in backwards GER lookup produces 10,001-block queries

Location: bridgetracker/sources/settlement.go:192
Severity: 🔴 P1 - Causes immediate query failures on strict RPC providers
Reported by: Codex automated review

Problem: The backwards search uses fromBlockChunk = toBlock - l1InfoTreeBackwardsSearchChunkSize, which produces an inclusive range [fromBlockChunk, toBlock] of 10,001 blocks (e.g., [15000, 25000]). RPC providers enforcing a 10,000-block eth_getLogs limit reject these queries immediately.

Impact: Every settlement without UpdateL1InfoTree in its own receipt fails to resolve, remaining stuck in error state indefinitely.

Fix:

// Line 192 - Current (WRONG):
fromBlockChunk = toBlock - l1InfoTreeBackwardsSearchChunkSize

// Should be:
fromBlockChunk = toBlock - l1InfoTreeBackwardsSearchChunkSize + 1

This produces exactly 10,000 blocks: [15001, 25000].

Fix this →


Issue 2: Missing log position filter allows selecting GER from events AFTER settlement

Location: bridgetracker/sources/settlement.go:206-209
Severity: 🔴 P1 - Returns wrong GER for certificates
Reported by: Codex automated review

Problem: When findEventUpdateL1InfoTreeBackwards finds UpdateL1InfoTree logs in the same block as the settlement, it selects the last log in the entire block without checking whether that log occurred before or after the settlement transaction:

// Line 206-209 - Current code:
if len(logs) > 0 {
    last := logs[len(logs)-1]  // ❌ Could be AFTER settlement!
    // ... extracts GER from last log
}

Impact: If another transaction emits UpdateL1InfoTree later in the settlement block, the tracker associates the certificate with a GER that didn't exist when the settlement executed. This causes wrong GER/leaf index associations and breaks claim resolution.

Fix: Filter logs to only those at or before SettlementLogIndex (analogous to the position filtering in GERSource):

if len(logs) > 0 {
    // Filter to logs before or at settlement position in the same block
    var filtered []gethtypes.Log
    for _, l := range logs {
        if l.BlockNumber < receipt.BlockNumber.Uint64() ||
           (l.BlockNumber == receipt.BlockNumber.Uint64() && l.Index <= result.SettlementLogIndex) {
            filtered = append(filtered, l)
        }
    }
    if len(filtered) == 0 {
        // No UpdateL1InfoTree before settlement in this chunk - continue to next chunk
        if fromBlockChunk == 0 { /* error handling */ }
        toBlock = fromBlockChunk - 1
        continue
    }
    last := filtered[len(filtered)-1]
    // ... extract GER
}

Fix this →


Issue 3: Bridge network ID tracking in activity endpoint ✅ FALSE POSITIVE

Location: bridgetracker/sources/activity.go:127
Reported by: Codex automated review

Status:Code is correct - Codex flagged this incorrectly.

The code properly preserves networkID via ScannedBridge{Bridge: b, NetworkID: networkID} and uses it correctly in IsClaimed() at line 140. The distinction between ScannedBridge.NetworkID (where tx was sent) and Bridge.OriginNetwork (asset origin) is maintained throughout.


🟡 Suggestions (Code Quality & Best Practices)

4. Consider adding explicit comment for underflow protection

File: bridgetracker/sources/ger.go:387-390

The lookback calculation already handles underflow correctly:

floor := uint64(0)
if head.Number > lookback {
    floor = head.Number - lookback
}

Suggest adding a comment: // Guard against underflow when chain is shorter than lookback


5. Activity cache settled state transitions deserve documentation

File: bridgetracker/activity.go

The logic for when an activity entry stops being refreshed (claimed + claim record fetched) is implicit. Consider adding a state diagram comment or explicit documentation of the "settled" conditions.


6. Edge case test coverage for claim step split

Files: bridgetracker/domain/resolve_steps_test.go, bridgetracker/engine_test.go

The split between StepWaitingClaim (on-chain) and StepClaimed (bridge service indexer) is well-tested, but consider adding a test for the window where isClaimed() returns true but the bridge service hasn't indexed the claim tx yet (should stay StepWaitingClaim.Done → StepClaimed.Pending).


7. Consider logging or metrics for L2 injection fallback usage

File: bridgetracker/sources/ger.go:341-349

The fallback to scanning L2 for GER injection logs when already logged as a warning, but adding a metric counter would help ops teams track how often old bridge-service instances are encountered.


🟢 Positive Observations

  1. Excellent domain modeling - ScannedBridge cleanly separates "reporting network" from "asset origin network"
  2. Graceful degradation - Activity endpoint returns warnings for failed networks instead of full failure
  3. Comprehensive testing - Dedicated test files for new features with good edge case coverage
  4. Clear inline documentation - Complex logic (GER fallback, backwards search) has helpful comments
  5. Backward compatibility - All new features are opt-in with sensible defaults
  6. Pragmatic configuration - IgnoreNetworkIDs handles real-world dead-network scenarios
  7. Security-conscious - Block finality requirements prevent reorg issues
  8. API design - Activity endpoint properly carries warnings in response, not as errors

📊 Testing Assessment

Coverage: ✅ Good

  • New endpoints have dedicated test files
  • Domain logic changes have corresponding test updates
  • Edge cases (malformed logs, missing events) are tested
  • Mock-based testing for external dependencies

Gaps (Minor):

  • L2 injection fallback with RPC failures during scan
  • Activity cache concurrent access patterns
  • Bridge address endpoint with empty finder results

📝 Summary

This PR delivers significant value with the activity endpoint and fixes important bugs in GER handling. The two critical issues (off-by-one in backwards search and missing log position filter) are must-fix before merge, as they cause incorrect behavior under normal operation. The third Codex issue is a false positive.

Overall code quality is high, with good separation of concerns, comprehensive error handling, and thoughtful backward compatibility.

Recommendation: 🟡 Approve after addressing the 2 critical issues


| Branch: feat/fix_bali_integration

…k range

findEventUpdateL1InfoTreeBackwards computed fromBlockChunk as
toBlock - l1InfoTreeBackwardsSearchChunkSize, which spans
chunkSize+1 blocks since FromBlock/ToBlock are both inclusive (e.g.
[15000,25000] for a 10_000 chunk). RPC providers enforcing a strict
10_000-block eth_getLogs limit rejected every such query, leaving any
settlement without its own UpdateL1InfoTree event stuck in error
state.

Add the missing +1 (and widen the guard to toBlock >= chunkSize
accordingly) so each chunk is exactly l1InfoTreeBackwardsSearchChunkSize
blocks. Update the settlement_test.go helper/paginated test that
mirrored the same calculation.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@joanestebanr

Copy link
Copy Markdown
Collaborator Author

Fixed the off-by-one in findEventUpdateL1InfoTreeBackwards (Issue 1 in the review above, same as Codex's discussion_r3872865287) in 0c021aa — each backwards chunk now spans exactly l1InfoTreeBackwardsSearchChunkSize blocks (toBlock - chunkSize + 1), instead of chunkSize+1.

…nt log

findEventUpdateL1InfoTreeBackwards picked the last UpdateL1InfoTree log
in its query range as "most recent", even when that log came from a
later transaction in the settlement's own block. If another tx in that
block emits UpdateL1InfoTree after the settlement's own log index, its
GER did not exist yet when the settlement executed, so the tracker was
associating the certificate with the wrong GER/leaf index.

Thread settlementLogIndex through to findEventUpdateL1InfoTreeBackwards
and filter out any log sharing fromBlock with the settlement at or
after that index before picking the latest one — mirroring the
position filtering GERSource.FindFirstL1InfoTreeAfterBlock already
does in ger.go, just looking backwards instead of forwards.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@joanestebanr

Copy link
Copy Markdown
Collaborator Author

Fixed the same-block GER exclusion issue (also flagged by Codex as discussion_r3872865292) in ce78dabfindEventUpdateL1InfoTreeBackwards now filters out any UpdateL1InfoTree log sharing the settlement's block at or after its own log index before picking the latest one.

…indow

Addresses review feedback (item 6, issuecomment-5506110812) on the
StepWaitingClaim/StepClaimed split: nothing pinned the tick where the
on-chain isClaimed() check goes true but the destination bridge
service has not indexed the claim tx yet, so StepWaitingClaim
completes while StepClaimed stays its own current step (InProgress,
no result) instead of being auto-completed alongside it.

- domain/resolve_steps_test.go: new TestResolveStepsClaimedNotIndexedYet,
  exercising ResolveSteps directly against the fakeFacts port.
- engine_test.go: TestEngineLifecycleL2ToL2 now ticks through that
  window (claimed=true, claim=nil) before the bridge service indexes
  the claim tx on the following tick, asserting both steps' status at
  each point.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@joanestebanr

Copy link
Copy Markdown
Collaborator Author

Addressed item 6 (edge case test coverage for the claim step split) in a603660: added TestResolveStepsClaimedNotIndexedYet in domain/resolve_steps_test.go, and extended TestEngineLifecycleL2ToL2 in engine_test.go to tick through the window where isClaimed() goes true but the bridge service hasn't indexed the claim tx yet — pinning that StepWaitingClaim completes (Done) while StepClaimed stays its own current step (InProgress, no result) until ClaimFor resolves on a later tick.

@joanestebanr joanestebanr changed the title fix: aggkit-proxy: bali integration: bridge tracker activity endpoint, GER settlement/removal fixes, dead-network handling fix: aggkit-proxy: bali integration: bridge tracker activity endpoint, GER settlement/injection fixes Sep 2, 2026
@joanestebanr
joanestebanr enabled auto-merge (squash) September 2, 2026 08:29
@joanestebanr
joanestebanr merged commit fe25576 into develop Sep 2, 2026
31 checks passed
@joanestebanr
joanestebanr deleted the feat/fix_bali_integration branch September 2, 2026 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: proxy: step WaitingGERInjection as result have the L1 block

2 participants