fix(tests): wait for archive indexing before asserting on emitted blocks - #205
Merged
Conversation
The live-network resolver tests queried the archive immediately after `pendingTx.wait()` resolved. That wait is the *daemon* confirming inclusion; the archive node ingests the block into Postgres asynchronously afterward. When the query won that race, the "latest block" was still the previous suite's block, so the assertion saw the previous block's entry count. This is what failed CI on PR #203 (expected 3, actual 1) — the previous suite emitted exactly one event. All emits for a given suite go into a single transaction, so a genuine split across blocks was never possible. Adds `waitForNewBlock`, plus `emitAndQueryEvents` / `emitAndQueryActions` wrappers, and routes all seven emit-then-query hooks through them. The poll predicate is deliberately content-independent: it waits for "a new block for this address is visible", not "the count equals what the test expects". Polling on the expected value would make the assertions tautological and would convert a real wrong-count regression into an opaque timeout instead of a clean assertion diff. Also rethrows in `sendTransaction`/`sendTransactions` after logging. The swallowed error meant a genuinely rejected transaction surfaced as the same confusing stale-block count mismatch, far from its cause. Closes #204 Co-Authored-By: Claude Opus 5 (1M context) <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
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.
Closes #204.
Problem
The live-network resolver tests query the archive immediately after
pendingTx.wait()resolves. That wait is the daemon confirming inclusion — the archive node ingests the block into Postgres asynchronously afterward. When the query wins that race,events[events.length - 1]is still the previous suite's block, so the assertion reads the previous block's entry count.This is what failed CI on #203 (run 30536209372):
expected 3, actual 1, where the preceding suite had emitted exactly one event. A re-run of the identical commit passed, and #203 changed only a version string — see #204 for the full evidence.Note that "the emits got split across blocks" was never possible:
emitSingleEventputs allnumberOfEmitsemits inside a singleMina.transaction(zkapp/utils.ts:106), so once included they are necessarily in one block.Changes
tests/resolvers.test.ts— addswaitForNewBlock, plusemitAndQueryEvents/emitAndQueryActionswrappers, and routes all seven emit-then-querybefore()hooks through them (5 event suites, 2 action suites).zkapp/utils.ts—sendTransactionandsendTransactionsnow rethrow after logging.Design note: why the poll predicate is content-independent
waitForNewBlockwaits for "a new block for this address is visible" — it captures the block count before emitting and polls until it increases. It deliberately does not poll until the count matches what the test expects.Polling on the expected value would make these assertions tautological: the test would spin until it passed, and a genuine wrong-count regression would surface as an opaque 30s timeout instead of a clean
expected 3, actual 2diff. The bound is 30 attempts at 1s, and the timeout message states that the daemon already confirmed inclusion, so a failure points at archive lag rather than at the transaction.Why rethrowing matters
sendTransactionpreviously swallowedwait()failures:A genuinely rejected or timed-out transaction produced exactly the same stale-block count mismatch as the race above, several lines away from the real cause. In the #203 failure the absence of that log line is what proved the transaction had actually succeeded — but that only worked because someone read the log closely. Rethrowing surfaces it at the origin.
Verification
npm run build(tsc) andnpm run lintclean. The behavioral proof is theRun-Testsjob on this PR, which exercises these paths againstmina-local-network.Since this fixes a race, a single green run is evidence but not proof. The change is structural — there is no longer a query that can observe pre-ingestion state — rather than a timing tweak that widens a window.
Not addressed here
The
NetworkStatesuite (tests/resolvers.test.ts:261-308) has a related weakness: a hard-codedsetTimeout(25000)and a direct archive-height vs daemon-height equality assertion that is racy in the same way. It is currently passing and fixing it would widen this diff considerably, so it is left for a follow-up — noted in #204.🤖 Generated with Claude Code