fix: deflake should_flag_only_deposits_at_or_above_the_per_token_minimum - #11408
Open
basvandijk wants to merge 1 commit into
Open
fix: deflake should_flag_only_deposits_at_or_above_the_per_token_minimum#11408basvandijk wants to merge 1 commit into
basvandijk wants to merge 1 commit into
Conversation
The minter's balance scan reads every balance at its cached latest_block_height, refreshed only by a separate 30-second timer - so a scan can run after the harness funded the deposit addresses on anvil and still eth_call at a pre-funding block, recording scan_count = 1 without having seen the funds. The live harness' await_scan accepted any Scanning status with scan_count >= 1 as terminal and returned that transient verdict, so the test's AwaitingSweep assertion flaked. An address now counts as scanned only once its status is AwaitingSweep (terminal by construction) or Scanning with a last_scanned_block at or past the chain head as of await_scan entry - the funding is mined by then, so such a scan cannot have missed the balance. SCAN_TICKS grows from 4 to 8 (a budget, not a cost): the pair's block-based backoff gap can demand up to 300 block-seconds before it is due for the scan the new predicate waits for. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Deflakes the CEX deposit test by ensuring balance scans include the funded block.
Changes:
- Requires scans to reach the Anvil head captured when waiting begins.
- Increases the scan-tick budget to accommodate backoff.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
✅ No security or compliance issues detected. Reviewed everything up to 04e0051. Security Overview
Detected Code Changes
|
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Sep 3, 2026
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.
Problem
//rs/ethereum/cketh/minter:deposit_from_cexkept flaking after #11383 (which removed the dominant stop-canister race). Both post-#11383masterflakes share one new signature:should_flag_only_deposits_at_or_above_the_per_token_minimumfails at its first assertion withright after
credit_depositsfunded the deposit addresses on anvil (its own balance assertions passed, so the funds were readable).Root cause
The minter's balance scan does not fetch a block number: it
eth_calls at its cachedlatest_block_height(rs/ethereum/cketh/minter/src/balance_scan/mod.rs, pinned asBlockTag::Number), and that cache is refreshed only by the separate 30-secondREFRESH_LATEST_BLOCK_HEIGHT_INTERVALtimer (deposit.rsis its sole writer; the log scrape writes a different field). So a scan firing after the harness mined the funding block can still read every balance at a pre-funding block: the call succeeds (the token code, set throughanvil_setCode, is visible to historicaleth_calls — mined transfers are not), finds nothing, andrecord_scanstoresscan_count = 1with the stalelast_scanned_block.The failing logs show exactly this: an ETH log scrape covering blocks
1..=11(head ≥ 11 pre-funding), the funding mined after it, then one balance scan —scanned 3 (address, token) pair(s), found 0 candidate(s), 0 decode error(s), 0 call error(s)— pinned at block 11.The harness'
await_scanaccepted anyScanningstatus withscan_count >= 1as terminal, so it returned that transient verdict instead of driving on until a scan that could actually see the funds — and the test'sAwaitingSweepassertion failed. (The next scan corrects the verdict within a couple of blocks; the predicate just never waited for it.)Fix
await_scannow snapshots the anvil chain head at entry (the funding is always mined by then — every caller funds throughcredit_depositsfirst) and counts an address as scanned only once its status is:AwaitingSweep— terminal by construction (a detection only results from a scan that read a balance at or above the minimum, and a queued pair never returns toScanning), orScanningwithscan_count >= 1andlast_scanned_blockat or past that entry head — a scan pinned there cannot have missed the balance, so a below-minimum verdict is genuine.SCAN_TICKSgrows from 4 to 8 (a budget, not a cost — driving stops the moment the condition holds): the pair's block-based backoff can demand up to 300 block-seconds before it is due for the scan the new predicate waits for, against the ~10 blocks each tick's settle mines.Validation
scan_count = 1at a block where the funding is invisible) and forcing one such scan:AwaitingSweepand the test passes end to end.cargo check,cargo fmt,./ci/scripts/rust-lint.sh— clean.integration_tests_tests/{ckerc20,cketh}_test,minter:lib_tests,test_utils:lib_tests.bazel test --runs_per_test=3 --jobs=3 //rs/ethereum/cketh/minter:deposit_from_cex— 3/3 full-suite runs pass.The remaining non-success runs in the window since #11383 merged all ran on commits that predate it (branch/merge-queue checkouts from before the merge) and show the old, already-fixed stop-canister signature.
This PR was created following the steps in
.claude/skills/fix-flaky-tests/SKILL.md.🤖 Generated with Claude Code