Skip to content

Fix broadcasting to a peer who already has tx#556

Open
randomlogin wants to merge 1 commit into2140-dev:masterfrom
randomlogin:tx-broadcast-fix
Open

Fix broadcasting to a peer who already has tx#556
randomlogin wants to merge 1 commit into2140-dev:masterfrom
randomlogin:tx-broadcast-fix

Conversation

@randomlogin
Copy link
Contributor

Previously if we broadcast to a peer who already has the tx it would hang indefinetely, as there is no subsequent getdata message.

@rustaceanrob
Copy link
Collaborator

This is by design. There is a possibility the first peer simply drops the inv announcement the first time we attempt to broadcast a transaction. It's the client responsibility to determine if they have already broadcast a transaction, they still want to try rebroadcasting, and to add timeouts accordingly. It would make sense to add a Builder method or direct argument on broadcast_tx to timeout.

@randomlogin
Copy link
Contributor Author

It's the client responsibility to determine if they have already broadcast a transaction

What if we ask the same peer for the tx we've just broadcast? Of course, it doesn't mean it has propagated further, but we know that the broadcast succeeded?

And also to have a fallback with timeout?

@randomlogin
Copy link
Contributor Author

randomlogin commented Mar 18, 2026

Is there a deep purpose of broadcast_tx only erring if the node stops?
Can adding a timeout error break some guarantees? It seems not, as it could just hang.

@randomlogin
Copy link
Contributor Author

I should not have squashed commits in this PR yet.

@randomlogin
Copy link
Contributor Author

I think this PR may be closed, it was just a surprising behaviour to me in comparison to how other things broadcast txs, now it seems it is a valid design decision.

Not sure about builder method or explicit timeout argument, is it needed or not?

@rustaceanrob
Copy link
Collaborator

rustaceanrob commented Mar 20, 2026

The only way to evaluate if a transaction is in a miner's memory pool is by heuristics, as we don't know if the transaction propagates beyond our peers. I think the intuition that we can improve broadcasting is correct. For instance, in Bitcoin Core's private broadcast feature, a transaction is advertised to peer A, and we wait for any other connection to broadcast the Wtxid back to us. This is a strong indication the transaction has propagated. The problem here is that this crate allows for one connection, so the logic would be different depending on the connections. I'd like to get #532 sorted first, but a combination of better heuristics and a timeout would be an improvement.

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.

2 participants