fix(tests): use correct topic names for account pay_fee events#1995
Closed
0xPepeSilvia wants to merge 2 commits intotari-project:developmentfrom
Closed
fix(tests): use correct topic names for account pay_fee events#19950xPepeSilvia wants to merge 2 commits intotari-project:developmentfrom
0xPepeSilvia wants to merge 2 commits intotari-project:developmentfrom
Conversation
The indexer_scans_network_events step queried getEvents once with no retry. If the indexer hadn't finished scanning/indexing events yet, the query returned an incomplete set and the assertion failed immediately. Now retries for up to 30 seconds, checking that all expected topics are present before asserting. On timeout, still reports the specific missing topic for debugging. Closes tari-project#1975 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a retry mechanism to the indexer_scans_network_events integration test. The updated logic polls the GraphQL indexer for up to 30 seconds to ensure all expected event topics are present, addressing potential timing issues where events might not be immediately available. I have no feedback to provide.
The previous attempt wrapped the event assertion in a 30-second retry loop, but the underlying bug was that the feature file expected `std.vault.pay_fee` events while the step definition filters events by `substate_id == component_address`. `std.vault.pay_fee` is emitted by the runtime with `substate_id = vault_id` (see crates/engine/src/runtime/impl.rs:1784-1790), so the filter can never match it. The retry would always time out. The `Account.pay_fee` custom event (emitted via `emit_event` in templates/account/src/lib.rs) carries the component substate id and does match the filter. Revert the topic names so the assertion is looking for events that can actually be found. The retry loop is kept as defence-in-depth against indexer catch-up timing.
0xPepeSilvia
changed the title
Member
|
Firstly, thanks for thr PR @0xPepeSilvia! Unfortunately, this PR was first #1983 |
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
3 participants
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.
Description
Fixes #1975.
The feature file
integration_tests/tests/features/indexer.featurewas asserting that the indexer sawstd.vault.pay_feetopics for the account component. The step definitionindexer_scans_network_eventsfilters events bysubstate_id == component_address, but the runtime emitsstd.vault.pay_feewithsubstate_id = vault_id(seecrates/engine/src/runtime/impl.rs:1784-1790). The filter therefore can never match, and the assertion fails every run.The
Account.pay_feecustom event (emitted viaemit_eventincrates/template_builtin/templates/account/src/lib.rs:178) is recorded with the account component's substate id (see the customemit_eventpath incrates/engine/src/runtime/impl.rs:466) and does match the filter.Fix
std.vault.pay_feeback toAccount.pay_fee.Why the previous attempt on this branch was insufficient
The first commit on this branch (
fix(tests): retry event topic assertion until indexer has scanned) wrapped the assertion in a retry. That masks indexer timing flakiness but does not address the root cause: with the filter looking for component-scoped events and the expected topics being vault-scoped, the retry simply sleeps until the timeout. The topic-name revert is the actual fix.Verification
crates/engine/src/runtime/impl.rsfor bothemit_std_event(line 256) and customemit_event(line 466).Account.pay_feeis still emitted by the account template (unchanged).cargo check -p tari_ootle_wallet_storage_sqliteand-p tari_ootle_walletdboth clean (touching unrelated crates just to spot-check).