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

fix: correctly treat confirmation for watch_pending_transaction #381

Merged
merged 4 commits into from
Mar 26, 2024

Conversation

klkvr
Copy link
Member

@klkvr klkvr commented Mar 23, 2024

Motivation

Current logic was not treating confirmations in the expected way (tx being included = 1 confirmation)
Also split_off splits off keys including the one at which split is being performed, thus we actually notified TxWatcher about txses one block later

Two open questions:

  1. Currently if tx is sent, included into block and one more block is mined (basically tx included into block X and latest block is X+1), then call to watch_pending_transaction will indefinitely wait for a block with targeted tx. Is this expected? Feels like a footgun, especially considering that we don't have a default timeout
  2. It feels like watch_pending_transaction users specifying confirmations > 1 would also be interested in ensuring that no reorg has happened and that tx is actually on-chain, should we add a parameter for that?

@prestwich
Copy link
Member

please put open questions in issues 👍

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

I believe this is correct

@prestwich prestwich merged commit 73d648b into alloy-rs:main Mar 26, 2024
17 checks passed
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