Skip to content

Fix race condition in ETH execution confirmation#729

Draft
volovyks wants to merge 1 commit intodevelopfrom
serhii/flacky-eth-test
Draft

Fix race condition in ETH execution confirmation#729
volovyks wants to merge 1 commit intodevelopfrom
serhii/flacky-eth-test

Conversation

@volovyks
Copy link
Copy Markdown
Contributor

@volovyks volovyks commented Mar 25, 2026

AI-generated. Creating this PR for the future discussion with @ppca

Problem

test_solana_eth_bidirectional_flow is flaky because of a race condition between the Solana stream and the ETH indexer.

When the test broadcasts an ETH transaction, Anvil auto-mines it into block N. The ETH indexer processes block N and checks backlog.pending_execution(Ethereum) for watchers — but the Solana stream hasn't registered the watcher yet (it's still processing the `Respond` event). By the time the watcher is registered, block N has already been processed, and in Anvil auto-mine mode, no new blocks arrive, so the watcher is never checked again.

Fix

Replace the block-receipts-based lookup in `collect_execution_confirmations` with a direct `eth_getTransactionReceipt` query per watched tx hash. This reliably finds the receipt regardless of which block the tx was mined in or when the watcher was registered.

Also use the receipt's actual `block_number` instead of the current processing block number, so `eth_call` replay and `ExecutionConfirmed` events reference the correct block.

@ChaoticTempest
Copy link
Copy Markdown
Contributor

When the test broadcasts an ETH transaction, Anvil auto-mines it into block N. The ETH indexer processes block N and checks backlog.pending_execution(Ethereum) for watchers — but the Solana stream hasn't registered the watcher yet (it's still processing the Respond event). By the time the watcher is registered, block N has already been processed, and in Anvil auto-mine mode, no new blocks arrive, so the watcher is never checked again.

Replace the block-receipts-based lookup in collect_execution_confirmations with a direct eth_getTransactionReceipt query per watched tx hash. This reliably finds the receipt regardless of which block the tx was mined in or when the watcher was registered.

Ahh that works. We could also require two "confirmations" for the respond event. One is when the signer generates the signature and one is when we index the respond event. I'm not leaning in either direction, just wanted to comment in this alternative

@ppca
Copy link
Copy Markdown
Contributor

ppca commented Mar 27, 2026

ahh I could see the issue, basically eth indexes the block where the transaction happens before solana indexes the respond() event, so when eth indexes that block it does not know to look for that transaction.
Replacing with eth_getTransactionReceipt makes sense to me, a problem is that this will mean in each eth block we process, the number of eth_getTransactionReceipt calls we make = number of pending_txs, which may cost a considerable amount of increase in alchemy cost. @volovyks
If we want to avoid the above increase cost, we could register watcher when signer generates the signature, as @ChaoticTempest mentioned, basically this would always be happening before eth could mine the block. This is also what the flow looks like when this was initially implemented.
But I'm not sure I understand

We could also require two "confirmations" for the respond event.

what do you mean by requiring two confirmations instead of one or another? @ChaoticTempest

@volovyks
Copy link
Copy Markdown
Contributor Author

I'm ok with the increased Alchemy bill. As long as it adds stability.
We need to make a call for each transaction instead of a block, correct?

@ppca
Copy link
Copy Markdown
Contributor

ppca commented Mar 27, 2026

I'm ok with the increased Alchemy bill. As long as it adds stability. We need to make a call for each transaction instead of a block, correct?

because this logic runs every time we process a block, so we make a call for each transaction in each process_block() for all the blocks before the transaction receipt is found

@volovyks
Copy link
Copy Markdown
Contributor Author

volovyks commented Mar 27, 2026

That does not seam right, let's discuss it on our 1-1
Meanwhile, it worth exploring options

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