test: poll Horizon instead of asserting immediately in clawback tests - #2698
Open
Galmanus wants to merge 1 commit into
Open
test: poll Horizon instead of asserting immediately in clawback tests#2698Galmanus wants to merge 1 commit into
Galmanus wants to merge 1 commit into
Conversation
Two recurring rpc-tests failure modes come from the same race: the CLI returns on the RPC acknowledgment, but Horizon ingestion lags behind it, so a read issued immediately afterwards can observe stale state. - `tx::clawback::clawback` read the holder's balance once right after the clawback and asserted 500 USDC; on a slow runner it saw the pre-clawback balance. - `tx::claimable_balance::clawback_claimable_balance` fetched the balance id once right after creating it and panicked when no record existed yet. Both now poll through a shared `poll_horizon_until` helper (200ms interval, ~30s cap) that on timeout returns the last observed value, so a real failure still shows what Horizon reported instead of a bare miss. Contributes to stellar#2689 (the ledger-fetch failures tracked there are a separate, deterministic XDR-JSON decode issue).
Contributor
There was a problem hiding this comment.
Pull request overview
Adds bounded Horizon polling to reduce integration-test races after CLI transaction submission.
Changes:
- Adds a reusable Horizon polling helper.
- Polls for updated clawback balances.
- Polls for claimable-balance creation before clawback.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
integration/util.rs |
Adds the shared polling helper. |
tx/clawback.rs |
Polls until the expected balance appears. |
tx/claimable_balance.rs |
Polls until the balance ID appears. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+28
to
+42
| let mut last = None; | ||
| for _ in 0..150 { | ||
| if let Ok(response) = reqwest::get(url).await { | ||
| if let Ok(json) = response.json::<serde_json::Value>().await { | ||
| if let Some(value) = extract(&json) { | ||
| if accept(&value) { | ||
| return Some(value); | ||
| } | ||
| last = Some(value); | ||
| } | ||
| } | ||
| } | ||
| tokio::time::sleep(std::time::Duration::from_millis(200)).await; | ||
| } | ||
| last |
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.
What
Adds a shared
poll_horizon_untilhelper (200ms interval, ~30s cap) to the integration test utils and uses it in the two tests that read Horizon immediately after a CLI submit:tx::clawback::clawback— asserted the post-clawback balance from a single read; on a slow runner it observed the pre-clawback balance (a recurring macOS failure on main).tx::claimable_balance::clawback_claimable_balance— fetched the balance id from a single read; when no record was ingested yet it panicked.On timeout the helper returns the last observed value, so a genuine failure still shows what Horizon actually reported rather than a bare miss.
Why
Contributes to #2689: these are two of the recurring failure modes in
rpc-tests.yml. The race is real — the CLI returns on the RPC acknowledgment, while Horizon ingestion lags behind it. This intentionally does not claim to fix #2689: the dominantledger_fetchfailures since 08-18 are a separate, deterministic issue (the quickstart image now emitsContractEvent's discriminant astypein XDR-JSON while the pinned stellar-xdr 27 expectstype_; stellar-xdr 28 fixes it but stellar-rpc-client 28 is unreleased), and retry/quarantine CI policy is a maintainer call.Note: current CI red on main is pre-existing (
ledger_fetch) and unrelated to this change.Testing
Test-only change;
cargo check -p soroban-test --features it --testsandcargo fmt --checkclean locally. The behavior change is only exercisable in CI's quickstart environment — which is the point: it removes the ingestion race those runs keep hitting.