Skip to content

test(e2e): resolve remaining wait-code REFACTOR markers#24428

Open
spalladino wants to merge 3 commits into
merge-train/spartan-v5from
spl/resolve-e2e-refactor-markers
Open

test(e2e): resolve remaining wait-code REFACTOR markers#24428
spalladino wants to merge 3 commits into
merge-train/spartan-v5from
spl/resolve-e2e-refactor-markers

Conversation

@spalladino

@spalladino spalladino commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Motivation

Cleanup finishing #24404's work. That PR adopted shared wait helpers across the e2e suite but deliberately deferred 17 // REFACTOR: wait-code markers as harder or ambiguous cases. This resolves all of them, leaving zero // REFACTOR: markers under end-to-end/src.

Approach

  • Adopt a shared helper where one genuinely fits.
  • Extract single-use / hard-to-adapt polls into descriptively-named local functions in the same file, then delete the marker.
  • Delete stale markers whose referenced code no longer exists.

All changes are behavior-preserving refactors: comparator direction, timeouts (with seconds-vs-ms conversions where a helper's unit differs), the retryUntil falsy-means-keep-polling contract, and side effects (loops that send txs / advance blocks / rotate the oracle keep doing so every tick) are all retained.

API changes

New/changed test-only helpers (in @aztec/ethereum test utils and the e2e fees harness):

  • ChainMonitor.waitForCheckpoint(match, opts) gains an opt-in flag. When set, it takes a fresh run() snapshot and short-circuits if the current checkpoint already satisfies match before attaching the event listener. This closes an event-vs-poll already-satisfied race: a checkpoint could advance during a preceding wait and be missed by the purely event-driven path. The existing caller is unchanged.
  • New RollupCheatCodes.waitForCheckpointBelow(checkpoint, opts) — a poll-only waiter (mirroring waitForEpoch/waitForSlot) that reads the L1 rollup contract until the pending checkpoint drops below a baseline (rollback detection). No node dependency.
  • snapshot_sync now adopts the existing ChainMonitor.waitUntilCheckpoint (which has the already-satisfied guard) instead of a hand-rolled poll.
  • New FeesTest.waitForEpochProven() encapsulates the recurring advanceToNextEpoch() + catchUpProvenChain() pair (adopted in failures and private_payments).

Finishes #24404's cleanup by resolving the 17 deferred // REFACTOR: wait-code
markers in the e2e suite. Adopts a shared helper where one fits and extracts
single-use polls into named local functions otherwise, preserving behavior.

- ChainMonitor.waitForCheckpoint gains an opt-in guard that short-circuits on a
  fresh run() snapshot, avoiding the event-vs-poll already-satisfied race.
- New RollupCheatCodes.waitForCheckpointBelow poll-only rollback waiter.
- snapshot_sync adopts ChainMonitor.waitUntilCheckpoint.
- FeesTest.waitForEpochProven encapsulates the advance-epoch + catch-up pair.
…tCheckpoint

The opt-in short-circuit option said nothing about what it did. Rename it to
checkCurrentCheckpoint and expand the JSDoc to spell out when to set it (latching
state predicates) versus when not to (predicates that must observe the checkpoint
live, e.g. one published mid-slot and then timed against wall-clock).
The prior commit extracted several single-use polls into functions defined inline
in the test body and called on the spot, which is pure indirection. Move them to
describe scope (closing over the test-context vars, parameterizing the it-locals)
so the extraction actually reads as a named helper rather than a renamed block.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants