wallet/wallet: remove invalid transactions when broadcast fails

In this commit, we rework how publishTransaction works in order to
correctly handle removing invalid transactions from the wallet's
unconfirmed transaction store. This is crucial as otherwise, invalid
transactions can remain within the wallet and be used for further
transactions, causing a chain of inaccurate transactions.

publishTransaction will now only return an error if the transaction
fails to be broadcast and it has not been previously seen in the
mempool/chain. This is intended in order to provide an easier API to
callers. Any other errors when broadcasting the transaction will cause
it to be removed from the wallet's unconfirmed transaction store to
ensure it maintains an accurate view of the chain.
This commit is contained in:
Wilmer Paulino 2019-02-20 12:59:11 -08:00
parent 7e00d1843e
commit 7a2b5cef76
No known key found for this signature in database
GPG key ID: 6DF57B9F9514972F

View file

@ -3444,24 +3444,39 @@ func (w *Wallet) publishTransaction(tx *wire.MsgTx) (*chainhash.Hash, error) {
case err == nil: case err == nil:
return txid, nil return txid, nil
// The following are errors returned from btcd's mempool. // Since we have different backends that can be used with the wallet,
case strings.Contains(err.Error(), "spent"): // we'll need to check specific errors for each one.
fallthrough //
case strings.Contains(err.Error(), "orphan"): // If the transaction is already in the mempool, we can just return now.
fallthrough //
case strings.Contains(err.Error(), "conflict"): // This error is returned when broadcasting/sending a transaction to a
// btcd node that already has it in their mempool.
case strings.Contains(err.Error(), "already have transaction"):
fallthrough fallthrough
// The following errors are returned from bitcoind's mempool. // This error is returned when broadcasting a transaction to a bitcoind
case strings.Contains(err.Error(), "fee not met"): // node that already has it in their mempool.
case strings.Contains(err.Error(), "txn-already-in-mempool"):
return txid, nil
// If the transaction has already confirmed, we can safely remove it
// from the unconfirmed store as it should already exist within the
// confirmed store. We'll avoid returning an error as the broadcast was
// in a sense successful.
//
// This error is returned when broadcasting/sending a transaction that
// has already confirmed to a btcd node.
case strings.Contains(err.Error(), "transaction already exists"):
fallthrough fallthrough
case strings.Contains(err.Error(), "Missing inputs"):
// This error is returned when broadcasting a transaction that has
// already confirmed to a bitcoind node.
case strings.Contains(err.Error(), "txn-already-known"):
fallthrough fallthrough
case strings.Contains(err.Error(), "already in block chain"):
// If the transaction was rejected, then we'll remove it from // This error is returned when sending a transaction that has already
// the txstore, as otherwise, we'll attempt to continually // confirmed to a bitcoind node over RPC.
// re-broadcast it, and the utxo state of the wallet won't be case strings.Contains(err.Error(), "transaction already in block chain"):
// accurate.
dbErr := walletdb.Update(w.db, func(dbTx walletdb.ReadWriteTx) error { dbErr := walletdb.Update(w.db, func(dbTx walletdb.ReadWriteTx) error {
txmgrNs := dbTx.ReadWriteBucket(wtxmgrNamespaceKey) txmgrNs := dbTx.ReadWriteBucket(wtxmgrNamespaceKey)
txRec, err := wtxmgr.NewTxRecordFromMsgTx(tx, time.Now()) txRec, err := wtxmgr.NewTxRecordFromMsgTx(tx, time.Now())
@ -3471,13 +3486,33 @@ func (w *Wallet) publishTransaction(tx *wire.MsgTx) (*chainhash.Hash, error) {
return w.TxStore.RemoveUnminedTx(txmgrNs, txRec) return w.TxStore.RemoveUnminedTx(txmgrNs, txRec)
}) })
if dbErr != nil { if dbErr != nil {
return nil, fmt.Errorf("unable to broadcast tx: %v, "+ log.Warnf("Unable to remove confirmed transaction %v "+
"unable to remove invalid tx: %v", err, dbErr) "from unconfirmed store: %v", tx.TxHash(), dbErr)
} }
return nil, err return txid, nil
// If the transaction was rejected for whatever other reason, then we'll
// remove it from the transaction store, as otherwise, we'll attempt to
// continually re-broadcast it, and the UTXO state of the wallet won't
// be accurate.
default: default:
dbErr := walletdb.Update(w.db, func(dbTx walletdb.ReadWriteTx) error {
txmgrNs := dbTx.ReadWriteBucket(wtxmgrNamespaceKey)
txRec, err := wtxmgr.NewTxRecordFromMsgTx(tx, time.Now())
if err != nil {
return err
}
return w.TxStore.RemoveUnminedTx(txmgrNs, txRec)
})
if dbErr != nil {
log.Warnf("Unable to remove invalid transaction %v: %v",
tx.TxHash(), dbErr)
} else {
log.Infof("Removed invalid transaction: %v",
spew.Sdump(tx))
}
return nil, err return nil, err
} }
} }