feat: add bridge_loop_tester long-running bridge soak tool - #1833
Draft
arnaubennassar wants to merge 25 commits into
Draft
arnaubennassar wants to merge 25 commits into
arnaubennassar wants to merge 25 commits into
Conversation
Add initial scaffold for bridge_loop_tester tool with stubbed CLI commands (run, validate, deploy-token, claim, status) and build integration. - Create tools/bridge_loop_tester package with root package functions - Add cmd/main.go with urfave/cli/v2 app and all command stubs - Create README.md with feature overview and command documentation - Add config-examples/example.toml template file - Wire build targets into Makefile and build-tools prerequisite - All commands return "not implemented yet" errors for later implementation - Binary builds successfully; make lint passes; version flag works Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Verifies (read-only) the exact /bridge/v1 + /tracker/v1 wire contract the bridge_loop_tester tool depends on and specifies the resumable hop state machine S6 will implement against: network_id routing semantics (query vs path param), the l1-info-tree-index -> injected-l1-info-leaf -> claim-proof retry sequence, tracker tx-status response shape, native-ETH-vs-gas-token rules per network, BridgeEvent-based deposit-count extraction, globalIndex computation, idempotent already-claimed detection, and reuse of test/contracts/mintableerc20 from tools/. Lists four gaps found (client 503 handling, tracker registration side effect, no live env to confirm gas token, and network_id=0 routing being config-dependent) with recommended resolutions.
Implements the standalone Config schema (Global/Networks/Loops/Hops), LoadConfig (viper/TOML, no aggkit template pipeline), and (*Config).Validate() with aggregated, actionable errors. Wires LoadConfig+Validate into CmdValidate (network preflight still stubbed for a later step). Extends config-examples/example.toml into a real 3-network closed-ring fixture (ETH + ERC20 loops, mixed auto/manual claims).
Add the per-network on-chain layer the hop engine will build on, behind small, behaviour-focused interfaces so later steps can mock it without a fake chain: - NetworkClient: chain identity, signer, native-balance reads and a transaction sender that serializes nonce reservation/estimation/signing/ submission under a per-client mutex, so several loops sharing one EOA on one network get distinct, gap-free nonces. A rejected submission clears the cached nonce instead of leaving a permanent hole. - Bridge: bridgeAsset (native and ERC20 variants), claimAsset/claimMessage, isClaimed, gasTokenAddress (a real runtime call, per DESIGN.md gap G3), computeTokenProxyAddress and BridgeEvent decoding from a receipt. - Token: deploy/mint/approve/balanceOf/allowance over mintableerc20. - RevertError: decodes Error(string), Panic(uint256) and the bridge ABI's custom errors, so a failed claim reports "AlreadyClaimed()" rather than "execution reverted"; IsAlreadyClaimed is the branch the hop state machine needs. Global index encoding is delegated to bridgesync.GenerateGlobalIndexForNetworkID rather than re-implemented. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Implement one hop of a loop end to end as the resumable state machine specified in DESIGN.md §4, driven entirely by injected dependencies (NetworkClient/Bridge/Token/Proxy) so a whole hop runs against mocks. - HopEngine.RunHop resolves the asset on both endpoints (native, the origin ERC20, or its bridge-computed wrapped representation), checks the source balance against Amount + MinNativeReserve, approves the bridge once per (network, token), bridges, decodes depositCount from the receipt's BridgeEvent, computes the global index through the shared bridgesync helper, waits the three readiness gates, settles the claim per the hop's claim mode, and verifies the destination balance delta. - Leaf-index threading is explicit: the claim proof is fetched for the actually-injected index I' returned by WaitInjectedLeaf, never for the index I returned by WaitL1InfoTreeIndex, and the result records both plus whether they diverged. - Claim modes are assertions, not hints: an auto hop nobody claimed and a manual hop something else claimed both fail with a *ClaimModeViolationError naming the offending claimer, and are never retried. - Every gate's deadline is the hop budget still remaining, so a stalled hop always fails with a *DeadlineExceededError naming the gate rather than with a bare timeout. - Resumability is externally observable: a checkpoint is persisted on entry to every state, before that state's side effect, and each state is re-derived from the bridge tx receipt, the /bridge/v1 reads and an on-chain isClaimed call. The one unobservable state (HopStateBridging) is refused with *AmbiguousResumeError plus a recovery procedure rather than risking a double bridge. - Balance verification is exact for ERC20 and gas-tolerant for native, with the reason documented and the tolerated shortfall recorded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The hop engine checkpointed HopStateBridging before submitting the deposit,
but had no way to learn the transaction's hash until its receipt arrived, so a
crash in that window left a checkpoint that named nothing. Resuming it was
refused outright with *AmbiguousResumeError and a manual recovery procedure,
which made a restart at the wrong moment wedge a loop until a human looked at
it - and the tool is meant to run unattended for days.
A signed transaction's hash is fixed by its signature, so it is knowable before
the broadcast. The network layer now hands it over there:
- TxRequest.OnSigned is called with a PendingTx (hash, nonce, from, network)
after signing and strictly before SendTransaction. Returning an error
aborts the submission with the nonce unconsumed, which is the safe
direction. It runs inside the nonce-serialization critical section, so it
gets a context bounded by WithPreBroadcastTimeout (30s) and its panics are
recovered rather than unwound through the held mutex.
- EthBackend gains NonceAt, the account's *mined* nonce - distinct from
PendingNonceAt, and the fact that makes an unreceipted submission
decidable.
The engine checkpoints hash and nonce from that hook, and a resume from
HopStateBridging now decides:
- no hash: the crash preceded the signature, nothing was broadcast, restart;
- a receipt: the deposit is real, continue from it;
- no receipt, nonce neither mined nor queued: the node never saw it,
re-submit (and the re-submission reuses the same nonce, so even a
submission to a node this tool cannot see could not double-deposit);
- no receipt, nonce queued: it may be ours and about to land, so wait out its
receipt (HopTimings.BridgeResumeReceiptWait, capped by the hop budget);
- no receipt, nonce mined by something else: a deposit may exist under a hash
the tool never learned, so refuse.
That last case is all that is left of *AmbiguousResumeError, which now carries
the evidence (hash, tx nonce, mined nonce, pending nonce, how long it waited)
and points the operator at the bridge's own indexed deposits. Nothing infers
"no deposit exists" from an absent indexed bridge: an indexer that merely
trails the chain would turn that into the double-bridge this path prevents.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Drive every enabled loop of the config in its own goroutine, hop by hop around its circular route, and finish the CLI on top of it. Orchestrator - Exactly one NetworkClient per (network, signing key) pair, built at startup and shared by every loop touching that network. SendTx serializes nonces per instance, so a second client for the same key would collide and wedge the account for the rest of a multi-day run. - LoopDelay between cycles, Iterations cycles per run (0 = forever), context cancellation stopping every loop at its next safe point and flushing the state file before Run returns. - Failure policy, one fixed response per class (FailureClass): transient -> retry the same hop in place, resuming from the checkpoint the failed attempt reached, then leave the cycle and resume the ring at that hop next cycle (the value is stranded there; restarting at hop 0 would spend value that is not present); claim-mode violation, ambiguous resume, insufficient balance and configuration -> fatal to that loop, persisted, never retried. An AmbiguousResumeError is logged with its full decision evidence and the hop is neither skipped nor re-driven. - Because a route is circular, LoopReport.ValueLocation (and `status`) says whether a loop is at rest on its origin or holds value stranded part-way round its ring, and whether that value is in flight. - ERC20 lifecycle: deploy and mint once on TokenOriginNetwork, persist the address, reuse it across restarts (redeploying only when there is no code at the recorded address). - Prometheus metrics through aggkit/prometheus on MetricsAddr. State file - JSON at StatePath, written temp-file + fsync + rename + directory fsync, so a reader (including this tool after a crash) sees either the previous or the new snapshot, never a mixture. S6B's resume safety depends on it: the engine reads "state bridging with no bridge_tx_hash" as proof the transaction was never signed. Preflight - Live, read-only, zero transactions. Refuses an "eth" loop on a network whose gasTokenAddress() is not 0x0 (DESIGN.md G3) and a route through network 0 when the proxy reports no L1 bridge service (G4) in both strict and run mode; `validate` additionally refuses balances below MinNativeReserve and bridge addresses the proxy disagrees with. - Config now refuses a run with no enabled loop: Enabled has no implicit true-default, so an omitted or typo'd Enabled would otherwise produce a run that does nothing and exits successfully. CLI and library API - run / validate / deploy-token / claim / status, plus DryRun (every read and fundability check, no transaction). - Run(ctx, *Config) (*Report, error) is library-callable: no file load, no signal handler, no package-level state, no os.Exit, injectable logger and dependencies. Report aggregates every HopResult per loop and per cycle plus run-level totals, and round-trips through JSON. - Proxy gains BridgeByDepositCount, for the one-off recovery claim of a deposit this process did not make (mock regenerated). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Close S8's unit-test gaps and one real defect surfaced while closing them: - Add orchestrator tests that exercise the production HopRunner wiring (newHopEngine, hopNetworks, persistCheckpoint, State) that every prior orchestrator test bypassed with a fake HopRunner, plus the runner-construction-failure halt path (haltLoop) and the ERC20 half of DryRun planning (planERC20Hop, resolveTokenOn), which only the ETH case covered before. - Add a ProxyClient-level test for BridgeByDepositCount and WithHTTPTimeout, whose real implementations were only reachable through the mocked Proxy interface in operations_test.go before. - Add a hop-engine resume test for the no-recorded-baseline degraded balance check (verifyWithoutBaseline), reachable from a legacy or hand-edited checkpoint that predates DestinationBalanceBefore. - Fix a real gap: DeployToken discarded the deployment receipt, so TokenState.DeployTxHash and DeployTokenResult were always the zero hash. DeployToken now returns the deploy tx hash alongside the Token, threaded through both call sites (Orchestrator.DeployToken and ensureTokens). Coverage for tools/bridge_loop_tester rose from 82.1% to 85.2%, in line with the repo's other tool packages (exit_certificate 85.5%). The remaining 0% functions are CLI-only print/formatting glue (printPreflight, printReportSummary, printClaimResult), matching the convention every other tool's cmd/ layer already leaves untested. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Rewrite tools/bridge_loop_tester/README.md as an operator-facing reference (circular-route model, auto/manual claim-mode contract and what a failure of each proves, full config schema, every CLI command with example output, the per-class failure policy, the state file format/atomicity, metrics, gas-drain mechanics, and DryRun semantics), and add tools/bridge_loop_tester/config-examples/README.md explaining how to adapt the existing generic example.toml. Documentation only, no behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ter config Network.GasOffset was typed WeiAmount but the gas package already treated it as a plain gas-unit margin added to eth_estimateGas's result, not wei; retype it to GasLimitOffset uint64 so the type matches what the code does. HopAttempts (the transient-hop-retry count, default 3) was only reachable via OrchestratorDeps in process; expose it as Global.HopAttempts, validated > 0 and defaulted to 3, so an operator can tune it for a long soak run without recompiling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
HopAttempts was documented as "retried N times" which suggests N retries after an initial attempt (N+1 total). The code actually performs exactly N total attempts. This misalignment was confirmed in the retry loop and the test TestRunHonoursConfiguredHopAttempts which shows that HopAttempts=5 results in 5 total attempts, not 6. Updated all mentions of HopAttempts to use unambiguous language: "attempted in total (the first attempt plus any retries)" rather than "retried N times". Files updated: - config.go: doc comments for HopAttempts field and defaultHopAttempts - orchestrator.go: doc comments for OrchestratorDeps.HopAttempts, failure policy, and runHopWithRetries - README.md: Global config table and failure policy table - config-examples/example.toml: HopAttempts field comment Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
/bridge/v1/claims never populates from_address: bridgeservice.NewClaimResponse does not set ClaimResponse.FromAddress, and claimsync's Claim record has no such field to set it from. Confirmed live against the anvil-2chains env, where every claim comes back with "from_address": "" while tx_hash is populated. So every claim the tool did not submit itself was attributed to ClaimActorUnknown, which is precisely the attribution an "auto" hop exists to report: the hop succeeded but the report could not say who claimed it. Recover the claimant from the claim transaction's own signature instead. The network layer gains EthBackend.TransactionByHash and TransactionSender, which reads the transaction back and recovers its sender against the network's chain ID; the hop engine calls it whenever the claim record named a transaction but no sender. It stays best-effort, exactly like the record it supplements: a node that cannot answer leaves the claimant unknown rather than failing the hop, since the on-chain isClaimed read has already decided the hop's outcome. Also raise claimRecordLookupBudget from 2s to 30s. At 2s the claim syncer had usually not indexed the claim yet, so not even its transaction hash was known, which would have defeated the recovery above. The wait is still bounded and still capped by whatever is left of the hop's own budget; measured against this env the syncer indexes a fresh claim within a few seconds. Two unit tests cover both outcomes: a record without a sender is attributed to ClaimActorExternal with the address recovered from the signature, and an unreadable claim transaction leaves ClaimActorUnknown with the hop still successful. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…log line (*Orchestrator).Run stamps Report.FinishedAt and Report.Duration in a deferred function, but logFinish -- which prints Report.Summary(), including the run's duration -- runs before that defer. Every completed run therefore closed its log with "duration=0s" while the returned Report carried the correct value. Stamp both fields before logging; the defer still refreshes them afterwards, so the returned Report keeps the later, marginally more accurate value. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
GasLimitOffset defaulted to 0 and the README and example config presented that as a neutral choice. It is not: bridgeAsset is submitted with forceUpdateGlobalExitRoot = true, so it also writes the global exit root, and when two bridges land in the same block the second costs materially more than its own eth_estimateGas result predicted (the first already warmed the storage the estimate was priced against) and reverts OutOfGas inside updateExitRoot. Observed on the anvil-2chains e2e env: an L1 bridge reverted with gas used 158025 against an estimate of 161947, with cast run placing the revert inside updateExitRoot. test/e2e/bridge_utils.go works around the same race by pinning l1BridgeGasLimit to 500000. Document it as something to set on any network that will see concurrent bridges, and give the example config a working 300000 rather than 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Add TestBridgeLoopFullCycle, which drives tools/bridge_loop_tester's library API through one full closed ring 0 -> 1 -> 2 -> 0 against the anvil-2chains env, so a single cycle covers L1->L2, L2->L2 and L2->L1. The ring is walked twice concurrently: once for ETH and once for an ERC20 the tool deploys and mints itself on L1. The Config is built programmatically from the loaded env -- network IDs, chain IDs, bridge addresses and pre-funded keys all come from envs.LoadEnv and the env's own summary.json, and Enabled is set explicitly on both loops. Each network's signer is a real go_signer "local" keystore config written at test time, so the run exercises NewNetworkClient's dial-plus-signer path; the background priming engines use NewNetworkClientWithBackend over the env's already-dialed clients, covering the other constructor. Both loops share the orchestrator's own pool (one NetworkClient per network and signing key), so the run also exercises the per-instance nonce serialization concurrent loops depend on. The ring mixes claim modes: 0 -> 1 and 2 -> 0 are "manual" and 1 -> 2 is "auto". Auto Claim is enabled for the test on the aggkit-002 node alone, via the same harness machinery TestAutoClaimL2ToL2AllowAll uses, so no claimer exists for either manual hop's destination -- which is what makes the manual hops' negative assertion (nothing claimed the deposit for the whole grace period, asserted through the recorded manual-grace-period phase duration) meaningful. Assertions are made on the returned Report: the preflight's live gas-token and proxy findings, run totals, per-hop readiness gates, InjectedLeafSkipped and InjectedLeafAdvanced, the canonical global index, who claimed each hop, exact ERC20 and tolerant native balance deltas, the per-phase timing list, the final checkpoint and its JSON round-trip, the persisted state file, ring closure, and a live replay of the last hop's checkpoint through HopRequest.Resume. Because the two-chain Anvil env runs no batcher or proposer, an aggsender only certifies up to min(lastBridgeSyncBlock, lastClaimSyncBlock), so a hop whose source is an L2 needs an unrelated claim to land on that L2 after its bridge or its local exit root never settles to L1. The test drives that background L1->L2 bridge-and-claim activity on both L2s through the tool's own hop engine with its own keys, the same environment precondition TestAutoClaimL2ToL1AllowAll and TestAutoClaimL2ToL2AllowAll already establish. Wire it into .github/workflows/test-go-e2e.yml as its own anvil-2chains / bridge-loop matrix group (it restarts aggkit-002, so it needs an isolated stack for the same reason the autoclaim group does) and document it in docs/e2e_tests.md. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rately After the tool's own claimAsset/claimMessage receipt comes back successful and isClaimed confirms it, the GET /bridge/v1/claims cross-check has nothing left to decide: ClaimedBy is already ClaimActorTool and ClaimTxHash already comes from that receipt. It nonetheless shared claimRecordLookupBudget (30s) with the attribution path, so every manual hop could spend up to 30s waiting out a trailing claim syncer for a diagnostic that cannot influence the hop's verdict. Observed live: a manual hop that reached "claimed" in 17s did not complete for another 30s, and a three-hop manual ring paid that twice. Give that one call site its own short budget (claimRecordConfirmBudget, 2s) so the cross-check still lands whenever the syncer is caught up, while the attribution path -- where the record is the only thing that can name an external claimer, and which an "auto" hop's whole result rests on -- keeps the 30s budget unchanged. Measured effect on a three-hop manual ring against the anvil-2chains env: 1m55s -> 1m7s. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TestMain's post-test network-health probe was a hand-rolled ERC20 mint/approve plus a parallel BridgeL1ToL2 / BridgeL2ToL1 pair. Replace it with one tool-driven cycle of a closed ring, so the tool the repo ships is the probe the repo uses. The ring is derived from the loaded env's topology: 0 -> 1 -> 0 on a single-L2 env (the two directions the old check covered), 0 -> 1 -> 2 -> 0 when env.L2B != nil (adding L2->L2). Networks, chain IDs, RPC URLs, bridge addresses and signing keys all come off the env; nothing is hardcoded. Because the ring is closed the value returns to the L1 account it started from, so the check also asserts conservation, which the old pair could not. The loop is deliberately ETH-only, one cycle, one hop attempt: an ERC20 loop would need a token deploy and a mint on every suite run, and a retry would double the worst case of something that runs after every suite. TestBridgeLoopFullCycle is where the ERC20 ring, the "auto" claim mode and the full assertion surface live. Every hop uses Claim = "manual". Whether an autoclaim service is running on a given aggkit node here depends on which tests just ran (autoclaim_test.go enables it and restores the node's config on cleanup), so an "auto" hop would fail whenever the suite left autoclaim off -- a property of the preceding test, not of the network's health. Everything around the check is unchanged: it runs only when the suite passed, honours E2E_SKIP_POSTTEST_BRIDGE_CHECK=true, is bounded by the same 8 minutes, and still log.Fatalf's with the "env will not be cleaned for further debugging" semantics. The verdict reads the tool's Report rather than trusting Run's error alone, because a hop that exhausts its attempts on a transient failure ends its cycle without halting the loop -- a ring that did not close is exactly what this check exists to catch. envs.Env now publishes what the check needs instead of it re-reading summary.json: L1Config.RPCURL, L1Contracts.BridgeAddress, L2Config.RPCURL and Env.ProxyRESTURL. ProxyRESTURL is "" for envs that run no aggkit-proxy (op-pp), and those fall back to the hand-rolled check: the tool observes everything through the proxy's /bridge/v1 + /tracker/v1 surface and the /tracker/v1 half exists only in the aggkit-proxy binary. bridge_utils.go and its other callers are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…claim-mode violation A loop halted by a claim-mode violation stops precisely because something other than the tool claimed the deposit, yet the value-location detail reported alongside it asserted the deposit "was not claimed on network N" -- flatly contradicting the violation the line above had just reported, complete with the external claim's transaction hash and claimant address. Both the live path ((*Orchestrator).valueLocation) and the offline `status` path (describeValueLocation) derive that sentence from the state file alone, which records how far this tool got and not what the destination bridge says, so neither can assert either way. They now say the tool had not claimed it and that the deposit is therefore either still unclaimed on the bridge or was claimed by something else, which is true in both cases and still names the destination network an operator has to look at. Found live on the anvil-2chains env by inducing a real claim-mode violation: a hop configured Claim = "manual" into a network whose aggkit node runs Auto Claim. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tools/bridge_loop_tester's headline claim is that it detects a bridge on a manual-mode route being claimed by something else - i.e. that an autoclaim service is active on a route configured to have none. TestBridgeLoopFullCycle only ever observes that assertion holding, so CI covered the passing case only. An assertion never seen failing is weak protection, and this is the one that matters. TestBridgeLoopClaimModeViolation induces the failure for real: Auto Claim is enabled on aggkit-002 exactly as TestAutoClaimL2ToL2AllowAll and TestBridgeLoopFullCycle enable it, and the same ring 0 -> 1 -> 2 -> 0 then declares its 1 -> 2 hop Claim = "manual" - the hop the full-cycle test declares "auto". Nothing else about the topology changes, so the only difference between a pass there and a violation here is the expectation the config states. It asserts the run fails and the loop halts with halt_class = claim-mode-violation; that the violating hop was attempted exactly once even though HopAttempts = 3 (a violation is a test result, never a transient to retry); that the ring is reported as not closed with the value stranded in flight, with a detail that does not contradict the violation; that the halt is persisted in the state file so a restart cannot paper over it; that the typed *ClaimModeViolationError is reachable and satisfies errors.Is(err, ErrClaimModeViolation); and that the hop's error, the loop's error and the error the run returns all carry the same diagnosis, naming the bridge transaction plus the claim transaction and the claimant address recovered from its signature. That last part is asserted when the tool could resolve it and that it says so plainly when it could not. The claimant is recovered from the claim transaction, whose hash comes from the destination's GET /bridge/v1/claims record, and on anvil-2chains that record is usually served within a poll or two but in roughly one run in four is never served at all (verified live with the tool's lookup budget raised to five minutes). The violation itself does not depend on it - it rests on the destination bridge's own isClaimed read - so the tool degrades the attribution to ClaimActorUnknown rather than failing the hop, and requiring it unconditionally would make the test flaky for something it does not test. The test runs in its own anvil-2chains / bridge-loop-violation matrix group: it halts mid-ring by design, so it leaves a restarted node, a deposit the tool never claimed and extra claim traffic on network 2 behind, and test-go-e2e.yml keeps state-mutating suites in separate stacks. What checks that it puts the env back is TestMain's post-test bridge health-check, whose ring declares every hop manual - including 1 -> 2 - so a leaked Auto Claim service there fails it loudly. Verified live against anvil-2chains: four consecutive passes, and the negative path confirmed by neutering the manual-hop violation detection in hop.go, whereupon the run reported the whole ring as a success and the test failed on its first assertion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…Event log The hop engine named a claim's transaction from the proxy's GET /bridge/v1/claims record, and recovered the claimant from that transaction's signature. S12B measured that record absent for 2 of 9 genuinely claimed deposits, with a five-minute budget not helping (one hop spent 5m23s being told "not found" for a deposit the bridge itself reported claimed), which made an "auto" hop's ClaimActorExternal verdict - the result the tool exists to report - non-deterministic on a healthy network, and TestBridgeLoopFullCycle's ClaimedExternally assertion flaky with it. Attribution now reads the destination bridge's own ClaimEvent/DetailedClaimEvent log over the destination's JSON-RPC, which is written in the very block that flips isClaimed and so cannot lag the observation that already decided the hop. Bridge gains FindClaimEvent, scanning [anchor, head] for the log matching the deposit's global index, where the anchor is the destination head read once when the hop starts (a claim of a deposit the hop has not made yet cannot precede it, so the window is guaranteed to contain the claim while staying no wider than the hop). The proxy's record stays as the fallback for the case the log cannot cover - a resumed hop, whose claim predates that window - and HopResult.ClaimAttribution records which source answered, so a run's attributions are auditable rather than merely plausible. The claim-mode violation detection is untouched: it rests on isClaimed, not on any claim record. Skipping the proxy poll also takes ~6s off an externally claimed hop. Separately, TestBridgeLoopFullCycle carried a second, unrelated flake this repetition exposed: its two loops share one signing key per network, so a native hop's destination balance check could see a sibling loop's claim gas as a shortfall (one run failed with a 1.354e15 wei shortfall against a 1.1295e15 tolerance). OrchestratorDeps gains NativeGasSlack - the override HopDeps already documented for exactly this - and the test sets 1e16, still 1% of the 1e18 it moves. Live on anvil-2chains: 3 of 4 TestBridgeLoopFullCycle runs passed, the failure being the balance race above, and both runs after the slack fix passed with every auto hop attributed external via the chain to the aggkit-002 claimer. Where the proxy's record was served it named exactly the transaction the tool's own receipt did - absent, never wrong. TestBridgeLoopClaimModeViolation still passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
arnaubennassar
marked this pull request as draft
September 8, 2026 15:21
The tool-driven post-test probe walked the whole ring the loaded env could express, so on a two-L2 env it did three hops (0 -> 1 -> 2 -> 0). Measured on anvil-2chains that cost 59.6s and 67.6s on two runs, against 17.0s for the hand-rolled BridgeL1ToL2 / BridgeL2ToL1 pair it replaced. That regression is not worth paying after every passing suite, so the ring is now two hops on every env: 0 -> 1 -> 0, the same directional coverage the hand-rolled pair had. Measured after this change: 36.1s and 44.1s on two runs. Everything else about the probe is unchanged: it is still driven through tools/bridge_loop_tester's library API, still ETH-only, one cycle, one hop attempt, every hop Claim = "manual", still gated on code == 0, still honours E2E_SKIP_POSTTEST_BRIDGE_CHECK, still bounded by the same 8-minute timeout, still ends in log.Fatalf leaving the env standing, still reads the Report rather than only Run's error, and still falls back to the hand-rolled check on envs with no aggkit-proxy (Env.ProxyRESTURL == ""). L2 -> L2 is not lost from the repo's coverage: TestBridgeLoopFullCycle walks the full 0 -> 1 -> 2 -> 0 ring with both an ETH and an ERC20 loop. The one thing the shorter ring does give up is documented in docs/e2e_tests.md -- the probe no longer touches network 2, so it no longer catches an Auto Claim service that a preceding test left running on aggkit-002. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TestBridgeLoopFullCycle had a matrix group of its own, which made it a new
long pole in the e2e matrix (857s on this PR's run, against 324-487s for every
other anvil-2chains group). It now shares the anvil-2chains / bridge group and
the bridge-loop group is removed.
The bridge group was chosen because it is measurably the cheapest one. Job
durations over five recent workflow runs (34236587646, 34224103830,
34114889814, 33876555021, 33862428008), averaged:
bridge 324s <- chosen
removeger-b1 349s
removeger-category-a 350s
removeger-fast 397s
backward-forward-let 421s
removeger-b2 441s
autoclaim 454s
bridge-loop-violation 487s
bridge-loop 857s
bridge was also the shortest job outright in four of those five runs.
Verified locally that the four tests coexist in one go-test process against one
compose stack, which is the thing a shared group risks:
go test -count=1 -v -timeout 60m \
-run '^(TestJustBridge|TestBridgeL2ToL2|TestBridgeTrackerL1ToL2|TestBridgeLoopFullCycle)$' \
./test/e2e/
--- PASS: TestBridgeL2ToL2 (19.06s)
--- PASS: TestJustBridge (0.00s)
--- PASS: TestBridgeLoopFullCycle (433.96s)
--- PASS: TestBridgeTrackerL1ToL2 (15.56s)
ok github.com/agglayer/aggkit/test/e2e 527.446s
TestBridgeLoopClaimModeViolation keeps its own group. Sharing a stack with it
was tried and destabilised the full-cycle test: it halts mid-ring by design and
leaves a restarted aggkit-002, an unclaimed deposit and extra claim traffic on
network 2 behind.
No test's coverage or assertions changed; only which stack runs it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The comment quoted only the pre-change numbers and described the saving as "roughly a third of the extra cost", which the measurements do not support: the two-hop ring came in at 36.1s and 44.1s, i.e. roughly half the extra cost over the 17.0s hand-rolled pair, not a third. Record both measurements and name the reason a ring cannot reach 17.0s: it is sequential, where the hand-rolled pair ran its two flows in parallel. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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
A new long-running bridge soak-test tool,
tools/bridge_loop_tester, that continuously moves value around circular routes covering all three bridge directions (L1→L2, L2→L1, L2→L2) using ETH and a deployed ERC20 token. Every observation goes through the aggkit proxy REST API (/bridge/v1+/tracker/v1) and each network's JSON-RPC endpoint; nothing accesses databases or internal aggkit storage directly.Because routes are circular, value returns to its origin each cycle, allowing multi-day runs on a fixed balance (only gas is consumed). The config declares per-hop whether a claim is expected from an autoclaim service (
Claim = "auto") or performed by the tool (Claim = "manual"), making the tool a direct test of per-network autoclaim policies.The tool includes:
run,validate,deploy-token,claim,status,--versionFor aggkit itself: None. The tool lives in
tools/bridge_loop_tester/and does not modify any existing aggkit packages or their public APIs.Shared-package addition: Commit
75baf094addsErrServiceUnavailable(HTTP 503 sentinel) tobridgeservice/client/client.go. This is backward-compatible; bothdoRequestanddoRequestAllowNotFoundnow return this error for 503s, allowing the hop engine to retry transient syncer reorgs instead of treating them as hard failures.📋 Config Updates
The tool carries its own TOML config schema (
tools/bridge_loop_tester/config.toml). No changes to aggkit's config. Repo-level change:.mockery.yamlnow registersbridgelooptesterinterfaces with mockery (new tool's public interfaces:EthBackend,TxSigner,NetworkClient,Bridge,Token,HopRunner,Proxy).✅ Testing
Verified Live (Against
anvil-2chainsDocker Compose)halt_class=claim-mode-violationwhen a hop configuredClaim="manual"is claimed externally, names the transaction and recovered claimant address, does not retry on violation (HopAttempts=3 but HopsRetried=0)bridgedgasTokenAddress() == 0x0(ether is gas token); native and ERC20 bridging on L1, L2A, L2B confirmed successfulNOT Verified (Deferred to This PR's CI and Future Runs)
bridgegroup has not yet run withTestBridgeLoopFullCyclein it in CI — it was verified locally instead (four tests, one process, one stack; see below)HopAttemptsretry path end to end: No transient error occurred during testing, so only the negative (that claim-mode violations are never retried) is proven; successful transient recovery on a real transient remains untestedDryRunmode: Not exercised live; code paths exist and are tested viaTestRunDryRunSubmitsNothingAndReportsAPlan, but real network invocation never occurreddeploy-tokenandclaimsubcommands: Unit-tested but never invoked against live networksop-ppenv: The tool structurally cannot run there (noaggkit-proxymeans no/tracker/v1/healthendpoint for preflight); S11 retained the hand-rolled check as a fallback for envs without a proxyTest Coverage
bridgelooptester_test(external test package) imports mocks; in-package tests use injected dependencies. Coverage includes: config parsing and validation, network layer (signing, RPC calls, gas estimation), proxy client (backoff on 404/503, deadline tracking), hop state machine (all state transitions, resume paths, error classification), orchestrator (loop lifecycle, preflight, metrics), operations (token deployment, claim recovery, status), state file (atomic writes, JSON round-trip)TestBridgeLoopFullCycle: Full ring with both loops (ETH and ERC20) running concurrently, both auto and manual hops, 3 complete cycles, balance verification exact for ERC20 and within gas tolerance for nativeTestBridgeLoopClaimModeViolation: Manual hop receives claim externally during grace period; tool detects, halts, and reports the violationTestMain's probe (postTestBridgeCheck, not aTest*function) now drives the tool around the closed ring0 → 1 → 0instead of the hand-rolled parallelBridgeL1ToL2+BridgeL2ToL1pairanvil-2chains / bridge:TestBridgeLoopFullCycle(both loops, both claim modes, all three directions) joinsTestJustBridge,TestBridgeL2ToL2andTestBridgeTrackerL1ToL2— see the group-rebalance note belowanvil-2chains / bridge-loop-violation(new): runsTestBridgeLoopClaimModeViolationin its own stack (the violation test halts mid-ring, and sharing a stack with the full-cycle test was tried and destabilised it)Statistical Note on Flake
TestBridgeLoopFullCyclewas run 4 times total during S12C: runs 1–2 with pre-fix native-balance race (both failed), runs 3–4 withNativeGasSlack=1e16fix (both passed). Confidence in de-flaking is limited to two passing runs; CI will expand this.📝 Notes
Shared-package change (commit
75baf094— separate, self-contained):ErrServiceUnavailable in
bridgeservice/clientis returned for HTTP 503 by both request paths. This allows the hop engine (S6) to retry transient 503s during syncer reorg recovery instead of treating them as hard failures. Backward-compatible: new error type, no other status-code behavior changed.Claim attribution is derived from the destination chain's own
ClaimEvent/DetailedClaimEventlog, not from/bridge/v1/claims:eth_getLogsto the destination's JSON-RPC, recovering the claimant from the claim transaction's own signature.mockery.yamlentries (additive change, no other repo files modified outside the tool):Registers new tool's interfaces with mockery:
EthBackend,TxSigner,NetworkClient,Bridge,Token,HopRunner,Proxy. Repo convention requires mocks to be generated via mockery, not hand-rolled.GasLimitOffset = 0is a trap:A concurrent
bridgeAssetrevertedOutOfGasinsideupdateExitRootwhen two bridges landed in the same block (158,025 gas used vs 161,947 estimated). The field was retyped from wei-scale touint64(it is a gas-unit margin) and now exposed in config. Tool README and example config recommend a non-zero value (e.g., 300000).NativeGasSlackadded (field added toOrchestratorDeps, threaded to hop engines):The e2e test's two loops share one signing key per network; while the ETH loop sat out gates, the ERC20 loop's claim gas appeared as a native-balance shortfall on the destination.
NativeGasSlackis an override (default 0.001 ETH perDefaultNativeGasSlack) accepted inOrchestratorDepsand applied to each network's balance check. The e2e test sets1e16wei; this is an accepted known-issue fix for concurrent sign-key scenarios and does not mask missing-value failures (which still halt the test).Post-test health-check timing: ~17s → 36–44s:
0 → 1 → 0via the tool, 36.1s and 44.1s on two measured runs. Same directional coverage as the hand-rolled pair, plus the closure assertion (the value comes home) that a parallel pair structurally cannot make. A ring is sequential by definition, which is the whole of the remaining gap to 17sTestBridgeLoopFullCyclewalks the full0 → 1 → 2 → 0ring with both an ETH and an ERC20 loop3f618211(S11's808d4550before the rebase) cut the three-hop check from ~1m55s to ~1m7s, i.e. the cost was measured and reduced rather than silently acceptedaggkit-002. That guard mattered forTestBridgeLoopClaimModeViolation, which now runs alone in its own group with the stack torn down after itop-ppenv limitation — hand-rolled fallback retained:op-pphas noaggkit-proxy(no/tracker/v1endpoint for preflight health checks). The tool structurally cannot run there. Commit02c78df4retained the old hand-rolled check as a fallback whenEnv.ProxyRESTURL == "", and that fallback is unchanged —op-ppstill takes it, and the fallback path itself has never been exercised live (it is the previous inline code moved verbatim into a function,log.Fatalf→return err). Note that the tool-driven ring is now0 → 1 → 0on every env, so there is no longer a per-topology branch in the ring derivation: what used to be the single-L2 shape is now the only shape.CI matrix: one new group, and the full-cycle test folded into the cheapest existing one:
anvil-2chains / bridge-loop-violation(the only new group): claim-mode violation test, on its own stack because it halts mid-ring by design and leaves a restartedaggkit-002, a deposit the tool never claimed and extra claim traffic on network 2 behind — sharing a stack with the full-cycle test was tried and destabilised itanvil-2chains / bridge:TestBridgeLoopFullCycleruns here rather than in a group of its own, so it does not become a new long pole in the matrix.bridgewas chosen because it is measurably the cheapestanvil-2chainsgroup. Job durations over five recent workflow runs (34236587646,34224103830,34114889814,33876555021,33862428008), averaged: bridge 324s, removeger-b1 349s, removeger-category-a 350s, removeger-fast 397s, backward-forward-let 421s, removeger-b2 441s, autoclaim 454s, bridge-loop-violation 487s, bridge-loop (now removed) 857s.bridgewas also the shortest job outright in four of those five runsgo testprocess against one compose stack —TestBridgeL2ToL219.06s,TestJustBridge0.00s,TestBridgeLoopFullCycle433.96s,TestBridgeTrackerL1ToL215.56s,ok … 527.446s, and the post-test probe passed after themTestBridgeLoopFullCycle's own coverage and assertions are untouched by the move:test/e2e/bridgeloop_test.gois not modified by either rebalance commitDeveloper setup note (host-specific, not committed):
On hosts where Cloudflare WARP excludes
172.16-18.0.0/16but not172.19-20.x, Docker's auto-assigned172.20.0.0/16foranvil-2chains_defaultbecomes unreachable. Workaround: createtest/e2e/envs/anvil-2chains/docker-compose.override.yml(git-excluded via.git/info/exclude) to pin the subnet to172.16.0.0/16. CI and WARP-free hosts are unaffected.