Merge pull request #647 from halseth/rbf-reject-matching

Cleanup publication error matching, add ErrDoubleSpend/ErrReplacment

wallet/wallet.go

@@ -3294,6 +3294,44 @@ func (w *Wallet) SignTransaction(tx *wire.MsgTx, hashType txscript.SigHashType,
 	return signErrors, err
 }
 
+// ErrDoubleSpend is an error returned from PublishTransaction in case the
+// published transaction failed to propagate since it was double spending a
+// confirmed transaction or a transaction in the mempool.
+type ErrDoubleSpend struct {
+	backendError error
+}
+
+// Error returns the string representation of ErrDoubleSpend.
+//
+// NOTE: Satisfies the error interface.
+func (e *ErrDoubleSpend) Error() string {
+	return fmt.Sprintf("double spend: %v", e.backendError)
+}
+
+// Unwrap returns the underlying error returned from the backend.
+func (e *ErrDoubleSpend) Unwrap() error {
+	return e.backendError
+}
+
+// ErrReplacement is an error returned from PublishTransaction in case the
+// published transaction failed to propagate since it was double spending a
+// replacable transaction but did not satisfy the requirements to replace it.
+type ErrReplacement struct {
+	backendError error
+}
+
+// Error returns the string representation of ErrReplacement.
+//
+// NOTE: Satisfies the error interface.
+func (e *ErrReplacement) Error() string {
+	return fmt.Sprintf("unable to replace transaction: %v", e.backendError)
+}
+
+// Unwrap returns the underlying error returned from the backend.
+func (e *ErrReplacement) Unwrap() error {
+	return e.backendError
+}
+
 // PublishTransaction sends the transaction to the consensus RPC server so it
 // can be propagated to other nodes and eventually mined.
 //
@@ -3369,6 +3407,12 @@ func (w *Wallet) publishTransaction(tx *wire.MsgTx) (*chainhash.Hash, error) {
 		return nil, err
 	}
 
+	// match is a helper method to easily string match on the error
+	// message.
+	match := func(err error, s string) bool {
+		return strings.Contains(strings.ToLower(err.Error()), s)
+	}
+
 	_, err = chainClient.SendRawTransaction(tx, false)
 
 	// Determine if this was an RPC error thrown due to the transaction
@@ -3378,7 +3422,11 @@ func (w *Wallet) publishTransaction(tx *wire.MsgTx) (*chainhash.Hash, error) {
 		rpcTxConfirmed = rpcErr.Code == btcjson.ErrRPCTxAlreadyInChain
 	}
 
-	txid := tx.TxHash()
+	var (
+		txid      = tx.TxHash()
+		returnErr error
+	)
+
 	switch {
 	case err == nil:
 		return &txid, nil
@@ -3390,16 +3438,14 @@ func (w *Wallet) publishTransaction(tx *wire.MsgTx) (*chainhash.Hash, error) {
 	//
 	// This error is returned when broadcasting/sending a transaction to a
 	// btcd node that already has it in their mempool.
-	case strings.Contains(
+	// https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/mempool/mempool.go#L953
-		strings.ToLower(err.Error()), "already have transaction",
+	case match(err, "already have transaction"):
-	):
 		fallthrough
 
 	// This error is returned when broadcasting a transaction to a bitcoind
 	// node that already has it in their mempool.
-	case strings.Contains(
+	// https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L590
-		strings.ToLower(err.Error()), "txn-already-in-mempool",
+	case match(err, "txn-already-in-mempool"):
-	):
 		return &txid, nil
 
 	// If the transaction has already confirmed, we can safely remove it
@@ -3409,19 +3455,21 @@ func (w *Wallet) publishTransaction(tx *wire.MsgTx) (*chainhash.Hash, error) {
 	//
 	// This error is returned when sending a transaction that has already
 	// confirmed to a btcd/bitcoind node over RPC.
+	// https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/rpcserver.go#L3355
+	// https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/node/transaction.cpp#L36
 	case rpcTxConfirmed:
 		fallthrough
 
 	// This error is returned when broadcasting a transaction that has
 	// already confirmed to a btcd node over the P2P network.
-	case strings.Contains(
+	// https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/mempool/mempool.go#L1036
-		strings.ToLower(err.Error()), "transaction already exists",
+	case match(err, "transaction already exists"):
-	):
 		fallthrough
 
 	// This error is returned when broadcasting a transaction that has
 	// already confirmed to a bitcoind node over the P2P network.
-	case strings.Contains(strings.ToLower(err.Error()), "txn-already-known"):
+	// https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L648
+	case match(err, "txn-already-known"):
 		dbErr := walletdb.Update(w.db, func(dbTx walletdb.ReadWriteTx) error {
 			txmgrNs := dbTx.ReadWriteBucket(wtxmgrNamespaceKey)
 			txRec, err := wtxmgr.NewTxRecordFromMsgTx(tx, time.Now())
@@ -3437,29 +3485,108 @@ func (w *Wallet) publishTransaction(tx *wire.MsgTx) (*chainhash.Hash, error) {
 
 		return &txid, nil
 
-	// If the transaction was rejected for whatever other reason, then we'll
+	// If the transactions is invalid since it attempts to double spend a
-	// remove it from the transaction store, as otherwise, we'll attempt to
+	// transaction already in the mempool or in the chain, we'll remove it
-	// continually re-broadcast it, and the UTXO state of the wallet won't
+	// from the store and return an error.
-	// be accurate.
+	//
-	default:
+	// This error is returned from btcd when there is already a transaction
-		dbErr := walletdb.Update(w.db, func(dbTx walletdb.ReadWriteTx) error {
+	// not signaling replacement in the mempool that spends one of the
-			txmgrNs := dbTx.ReadWriteBucket(wtxmgrNamespaceKey)
+	// referenced outputs.
-			txRec, err := wtxmgr.NewTxRecordFromMsgTx(tx, time.Now())
+	// https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/mempool/mempool.go#L591
-			if err != nil {
+	case match(err, "already spent"):
-				return err
+		fallthrough
-			}
+
-			return w.TxStore.RemoveUnminedTx(txmgrNs, txRec)
+	// This error is returned from btcd when a referenced output cannot be
-		})
+	// found, meaning it etiher has been spent or doesn't exist.
-		if dbErr != nil {
+	// https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/blockchain/chain.go#L405
-			log.Warnf("Unable to remove invalid transaction %v: %v",
+	case match(err, "already been spent"):
-				tx.TxHash(), dbErr)
+		fallthrough
-		} else {
+
-			log.Infof("Removed invalid transaction: %v",
+	// This error is returned from btcd when a transaction is spending
-				spew.Sdump(tx))
+	// either output that is missing or already spent, and orphans aren't
+	// allowed.
+	// https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/mempool/mempool.go#L1409
+	case match(err, "orphan transaction"):
+		fallthrough
+
+	// Error returned from bitcoind when output was spent by other
+	// non-replacable transaction already in the mempool.
+	// https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L622
+	case match(err, "txn-mempool-conflict"):
+		fallthrough
+
+	// Returned by bitcoind on the RPC when broadcasting a transaction that
+	// is spending either output that is missing or already spent.
+	// https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/node/transaction.cpp#L49
+	case match(err, "missing inputs"):
+		returnErr = &ErrDoubleSpend{
+			backendError: err,
 		}
 
-		return nil, err
+	// Returned by bitcoind if the transaction spends outputs that would be
+	// replaced by it.
+	// https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L790
+	case match(err, "bad-txns-spends-conflicting-tx"):
+		fallthrough
+
+	// Returned by bitcoind when a replacement transaction did not have
+	// enough fee.
+	// https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L830
+	// https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L894
+	// https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L904
+	case match(err, "insufficient fee"):
+		fallthrough
+
+	// Returned by bitcoind in case the transaction would replace too many
+	// transaction in the mempool.
+	// https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L858
+	case match(err, "too many potential replacements"):
+		fallthrough
+
+	// Returned by bitcoind if the transaction spends an output that is
+	// unconfimed and not spent by the transaction it replaces.
+	// https://github.com/bitcoin/bitcoin/blob/9bf5768dd628b3a7c30dd42b5ed477a92c4d3540/src/validation.cpp#L882
+	case match(err, "replacement-adds-unconfirmed"):
+		fallthrough
+
+	// Returned by btcd when replacement transaction was rejected for
+	// whatever reason.
+	// https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/mempool/mempool.go#L841
+	// https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/mempool/mempool.go#L854
+	// https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/mempool/mempool.go#L875
+	// https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/mempool/mempool.go#L896
+	// https://github.com/btcsuite/btcd/blob/130ea5bddde33df32b06a1cdb42a6316eb73cff5/mempool/mempool.go#L913
+	case match(err, "replacement transaction"):
+		returnErr = &ErrReplacement{
+			backendError: err,
+		}
+
+	// We received an error not matching any of the above cases.
+	default:
+		returnErr = fmt.Errorf("unmatched backend error: %v", err)
 	}
+
+	// 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.
+	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, returnErr
 }
 
 // ChainParams returns the network parameters for the blockchain the wallet