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

Periodically rebroadcast transactions that aren't confirmed #3667

Open
cdecker opened this issue Apr 26, 2020 · 2 comments
Open

Periodically rebroadcast transactions that aren't confirmed #3667

cdecker opened this issue Apr 26, 2020 · 2 comments

Comments

@cdecker
Copy link
Member

cdecker commented Apr 26, 2020

We currently have a bit of a fire-and-forget attitude to publishing
transactions, which results in us trusting our bitcoin backend (local
bitcoind instance to attempt a rebroadcast if possible). This can lead to
some strange situations like orphaned transactions (#2171) and or transactions
being crowded out of the mempool due to fee pressure (#3590). Since we already
store all sent transactions, with annotations, in the transactions table,
along with the blockheight, we can easily identify unconfirmed transactions
(those with blockheight=0) and could reattempt a broadcast every once in a
while.

The rebroadcast could be triggered whenever a block is found. As an extra
precaution against forever broadcasting the same transactions over and over
without a chance of them getting confirmed (double spends), we can use the
utxoset table to filter out transactions whose input UTXOs aren't available,
though this can be added in a second step.

Fixes #3590
Fixes #2171

@ZmnSCPxj
Copy link
Collaborator

ZmnSCPxj commented Jul 27, 2020

As an extra precaution against forever broadcasting the same transactions over and over
without a chance of them getting confirmed (double spends), we can use the
utxoset table to filter out transactions whose input UTXOs aren't available,
though this can be added in a second step.

utxoset currently only reliably records P2WSH outpoints (if I understand the code around it correctly), and some rebroadcastable txes might not spend any P2WSH outpoints. Further, plans in #3858 would be to remove utxoset table from lightningd and instead put the get-UTXO-by-SCID logic in the backend plugin, since if we have an SPV backend we will not be able to track the entire utxoset.

A better idea might be to have the plugin sendrawtransaction return a specific error code if the backend fails due to double-spend. Backend plugins then parse the corresponding API, and if the reason it is rejected from mempool is because it is double-spent, returns the specific double-spend error code. When using the bitcoin-cli backend I believe matching code: -26\n should be what double-spendedness looks like (I think?).

#3870 changes the sendrawtransaction interface, maybe it should package the new double-spend-error-code change as well even if it does not use it currently, so that changes to the interface are fewer.

@ZmnSCPxj
Copy link
Collaborator

Okay, here is the bitcoind error codes that I think we should consider as "this transaction will never be confirmable":

https://github.com/bitcoin/bitcoin/blob/ccef10261efc235c8fcc8aad54556615b0cc23be/src/rpc/protocol.h#L46-L48

So I think we can define static const errcode_t BCLI_TX_INVALID = xx;, and in bcli parse the error message and check for error code: for any of -25, -26, or -27 followed by a newline, and if so return an RPC error of BCLI_TX_INVALID. Then lightningd can stop rebroadcasting the TX if the backend returns BCLI_TX_INVALID.

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

No branches or pull requests

2 participants