Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: Wait for transaction to be in a finalized block #1236

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

lexnv
Copy link
Collaborator

@lexnv lexnv commented Oct 30, 2023

The test fetch_block_and_decode_extrinsic_details failed on this CI job:

Rpc(ClientError(Call(ErrorObject { code: ServerError(2001), message: "Invalid block hash", data: None })))

The test is submitting an extrinsic and waiting for inclusion in any block.

Submitting an extrinsic with the unstable backend (rpc v2) implies:

  • subscribing to chainHead for related block hashes
  • calling transaction rpc method

It is entirely possible that the block hash reported by the transaction method was not reported yet by the chainHead class yet:

rpc_methods::TransactionStatus::BestChainBlockIncluded {
block: Some(block),
} => {
// Look up a pinned block ref if we can, else return a non-pinned
// block that likely isn't accessible. We have no guarantee that a best
// block on the node a tx was sent to will ever be known about on the
// chainHead_follow subscription.
let block_ref = match seen_blocks.get(&block.hash).cloned() {
Some(block_ref) => block_ref.into(),
None => BlockRef::from_hash(block.hash),
};
TransactionStatus::InBestBlock { hash: block_ref }

Given that the test was waiting for any block inclusion (wait_for_in_block), the test attempted to call chainHead_header before the block hash was reported.

To mitigate this, the wait_for_in_block is replaced by wait_for_finalized. The submit_transaction will return TransactionStatus::InFinalizedBlock only after the block hash has been reported by chainHead.
The downside of this approach is that the test may wait for a longer time (until the block becomes finalized + overhead of reporting the block via chainHead).

I was unable to reproduce the CI error in ~900 runs.

@lexnv lexnv self-assigned this Oct 30, 2023
@lexnv lexnv requested a review from a team as a code owner October 30, 2023 18:59
Copy link
Collaborator

@jsdw jsdw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Yup, mostly wait_for_in_block should be avoided for exactly this reason :)

@niklasad1
Copy link
Member

I was unable to reproduce the CI error in ~900 runs.

🤣

@niklasad1 niklasad1 merged commit e53df87 into master Oct 31, 2023
@niklasad1 niklasad1 deleted the lexnv/test-finalized branch October 31, 2023 09:35
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.

3 participants