lnwallet/btcwallet: remove invalid transactions from the wallet when broadcast fails #2671
Conversation
|
There are two cases left which I cannot determine if they should also be ignoring Line 195 in 8a0d0ec Line 650 in 8a0d0ec cc @joostjager |
|
Yes, they should be ignoring. The mechanism in sweeper is that it often tries to publish and if it is a double spend, it keeps retrying with exponential randomized back-off. This is to avoid making all kinds of mempool assumptions in the sweeper why a tx might be a double spend. |
|
Even though the first case that you refer to is rebroadcasting an exact tx of ours, it may still be that it fails because there is a conflict. |
Roasbeef
added
wallet
wpaulino
force-pushed
the
tx-broadcast-err-handling
branch
from
a777e21
to
ce78399
Compare
|
Needs a rebase now that the dependent PR is in. |
|
Will rebase this after we've caught up lnd with btcwallet and neutrino. |
The checks to determine whether the transaction broadcast failed due to it already existing in the mempool/chain are no longer needed since the underlying btcwallet PublishTransaction call will not return an error when running into these cases.
In this commit, we address a lingering issue within some subsystems that are responsible for broadcasting transactions. Previously, ErrDoubleSpend indicated that a transaction was already included in the mempool/chain. This error was then modified to actually be returned for conflicting transactions, but its callers were not modified accordingly. This would lead to conflicting transactions to be interpreted as valid, when they shouldn't be.
|
Needs rebase now that the dep PR has been merged. |
wpaulino
force-pushed
the
tx-broadcast-err-handling
branch
from
ce78399
to
7946d0a
Compare
|
Rebased. |
In this PR, we update our btcwallet dependency to the latest version, which includes some minor improvements to our error handling when broadcasting transactions.
Along the way, we also address a lingering issue within some subsystems that are responsible for broadcasting transactions. Previously, ErrDoubleSpend indicated that a transaction was already included in the mempool/chain. This error was then modified to actually be returned for conflicting transactions, but its callers were not modified accordingly. This would lead to conflicting transactions to be interpreted as valid, when they shouldn't be.
Depends on btcsuite/btcwallet#597.
Fixes #2657.