From 19eada0b4b3054375d3426d5e30c24d57f9418bc Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Mon, 14 Aug 2017 00:22:40 -0500 Subject: [PATCH] blockchain: Combine ErrDoubleSpend & ErrMissingTx. This replaces the ErrDoubleSpend and ErrMissingTx error codes with a single error code named ErrMissingTxOut and updates the relevant errors and expected test results accordingly. Once upon a time, the code relied on a transaction index, so it was able to definitively differentiate between a transaction output that legitimately did not exist and one that had already been spent. However, since the code now uses a pruned utxoset, it is no longer possible to reliably differentiate since once all outputs of a transaction are spent, it is removed from the utxoset completely. Consequently, a missing transaction could be either because the transaction never existed or because it is fully spent. --- blockchain/chain.go | 17 ++++++-------- blockchain/error.go | 13 ++++------- blockchain/error_test.go | 3 +-- blockchain/fullblocktests/generate.go | 20 ++++++++--------- blockchain/scriptval.go | 2 +- blockchain/validate.go | 32 +++++++++++---------------- blockchain/weight.go | 10 +++++---- mempool/error.go | 2 -- rpcserver.go | 4 +--- 9 files changed, 43 insertions(+), 60 deletions(-) diff --git a/blockchain/chain.go b/blockchain/chain.go index b4fa0e97..c302f481 100644 --- a/blockchain/chain.go +++ b/blockchain/chain.go @@ -348,9 +348,7 @@ type SequenceLock struct { // the candidate transaction to be included in a block. // // This function is safe for concurrent access. -func (b *BlockChain) CalcSequenceLock(tx *btcutil.Tx, utxoView *UtxoViewpoint, - mempool bool) (*SequenceLock, error) { - +func (b *BlockChain) CalcSequenceLock(tx *btcutil.Tx, utxoView *UtxoViewpoint, mempool bool) (*SequenceLock, error) { b.chainLock.Lock() defer b.chainLock.Unlock() @@ -361,9 +359,7 @@ func (b *BlockChain) CalcSequenceLock(tx *btcutil.Tx, utxoView *UtxoViewpoint, // transaction. See the exported version, CalcSequenceLock for further details. // // This function MUST be called with the chain state lock held (for writes). -func (b *BlockChain) calcSequenceLock(node *blockNode, tx *btcutil.Tx, - utxoView *UtxoViewpoint, mempool bool) (*SequenceLock, error) { - +func (b *BlockChain) calcSequenceLock(node *blockNode, tx *btcutil.Tx, utxoView *UtxoViewpoint, mempool bool) (*SequenceLock, error) { // A value of -1 for each relative lock type represents a relative time // lock value that will allow a transaction to be included in a block // at any given height or time. This value is returned as the relative @@ -411,10 +407,11 @@ func (b *BlockChain) calcSequenceLock(node *blockNode, tx *btcutil.Tx, for txInIndex, txIn := range mTx.TxIn { utxo := utxoView.LookupEntry(&txIn.PreviousOutPoint.Hash) if utxo == nil { - str := fmt.Sprintf("unable to find unspent output "+ - "%v referenced from transaction %s:%d", - txIn.PreviousOutPoint, tx.Hash(), txInIndex) - return sequenceLock, ruleError(ErrMissingTx, str) + str := fmt.Sprintf("output %v referenced from "+ + "transaction %s:%d either does not exist or "+ + "has already been spent", txIn.PreviousOutPoint, + tx.Hash(), txInIndex) + return sequenceLock, ruleError(ErrMissingTxOut, str) } // If the input height is set to the mempool height, then we diff --git a/blockchain/error.go b/blockchain/error.go index e4bf8f53..eb30d8ae 100644 --- a/blockchain/error.go +++ b/blockchain/error.go @@ -128,9 +128,9 @@ const ( // range or not referencing one at all. ErrBadTxInput - // ErrMissingTx indicates a transaction referenced by an input is - // missing. - ErrMissingTx + // ErrMissingTxOut indicates a transaction output referenced by an input + // either does not exist or has already been spent. + ErrMissingTxOut // ErrUnfinalizedTx indicates a transaction has not been finalized. // A valid block may only contain finalized transactions. @@ -150,10 +150,6 @@ const ( // coinbase that has not yet reached the required maturity. ErrImmatureSpend - // ErrDoubleSpend indicates a transaction is attempting to spend coins - // that have already been spent. - ErrDoubleSpend - // ErrSpendTooHigh indicates a transaction is attempting to spend more // value than the sum of all of its inputs. ErrSpendTooHigh @@ -242,12 +238,11 @@ var errorCodeStrings = map[ErrorCode]string{ ErrBadTxOutValue: "ErrBadTxOutValue", ErrDuplicateTxInputs: "ErrDuplicateTxInputs", ErrBadTxInput: "ErrBadTxInput", - ErrMissingTx: "ErrMissingTx", + ErrMissingTxOut: "ErrMissingTxOut", ErrUnfinalizedTx: "ErrUnfinalizedTx", ErrDuplicateTx: "ErrDuplicateTx", ErrOverwriteTx: "ErrOverwriteTx", ErrImmatureSpend: "ErrImmatureSpend", - ErrDoubleSpend: "ErrDoubleSpend", ErrSpendTooHigh: "ErrSpendTooHigh", ErrBadFees: "ErrBadFees", ErrTooManySigOps: "ErrTooManySigOps", diff --git a/blockchain/error_test.go b/blockchain/error_test.go index c18abf0f..d9724ebf 100644 --- a/blockchain/error_test.go +++ b/blockchain/error_test.go @@ -38,12 +38,11 @@ func TestErrorCodeStringer(t *testing.T) { {blockchain.ErrDuplicateTxInputs, "ErrDuplicateTxInputs"}, {blockchain.ErrBadTxInput, "ErrBadTxInput"}, {blockchain.ErrBadCheckpoint, "ErrBadCheckpoint"}, - {blockchain.ErrMissingTx, "ErrMissingTx"}, + {blockchain.ErrMissingTxOut, "ErrMissingTxOut"}, {blockchain.ErrUnfinalizedTx, "ErrUnfinalizedTx"}, {blockchain.ErrDuplicateTx, "ErrDuplicateTx"}, {blockchain.ErrOverwriteTx, "ErrOverwriteTx"}, {blockchain.ErrImmatureSpend, "ErrImmatureSpend"}, - {blockchain.ErrDoubleSpend, "ErrDoubleSpend"}, {blockchain.ErrSpendTooHigh, "ErrSpendTooHigh"}, {blockchain.ErrBadFees, "ErrBadFees"}, {blockchain.ErrTooManySigOps, "ErrTooManySigOps"}, diff --git a/blockchain/fullblocktests/generate.go b/blockchain/fullblocktests/generate.go index 71d1ed93..82d3a036 100644 --- a/blockchain/fullblocktests/generate.go +++ b/blockchain/fullblocktests/generate.go @@ -997,7 +997,7 @@ func Generate(includeLargeReorg bool) (tests [][]TestInstance, err error) { acceptedToSideChainWithExpectedTip("b6") g.nextBlock("b8", outs[4]) - rejected(blockchain.ErrMissingTx) + rejected(blockchain.ErrMissingTxOut) // --------------------------------------------------------------------- // Too much proof-of-work coinbase tests. @@ -1078,7 +1078,7 @@ func Generate(includeLargeReorg bool) (tests [][]TestInstance, err error) { // \-> b3(1) -> b4(2) g.setTip("b15") g.nextBlock("b17", &b3Tx1Out) - rejected(blockchain.ErrMissingTx) + rejected(blockchain.ErrMissingTxOut) // Create block that forks and spends a tx created on a third fork. // @@ -1090,7 +1090,7 @@ func Generate(includeLargeReorg bool) (tests [][]TestInstance, err error) { acceptedToSideChainWithExpectedTip("b15") g.nextBlock("b19", outs[6]) - rejected(blockchain.ErrMissingTx) + rejected(blockchain.ErrMissingTxOut) // --------------------------------------------------------------------- // Immature coinbase tests. @@ -1278,11 +1278,11 @@ func Generate(includeLargeReorg bool) (tests [][]TestInstance, err error) { doubleSpendTx := createSpendTx(outs[11], lowFee) g.nextBlock("b37", outs[11], additionalTx(doubleSpendTx)) b37Tx1Out := makeSpendableOut(g.tip, 1, 0) - rejected(blockchain.ErrDoubleSpend) + rejected(blockchain.ErrMissingTxOut) g.setTip("b35") g.nextBlock("b38", &b37Tx1Out) - rejected(blockchain.ErrMissingTx) + rejected(blockchain.ErrMissingTxOut) // --------------------------------------------------------------------- // Pay-to-script-hash signature operation count tests. @@ -1536,7 +1536,7 @@ func Generate(includeLargeReorg bool) (tests [][]TestInstance, err error) { b.Transactions[1].TxIn[0].PreviousOutPoint.Hash = *hash b.Transactions[1].TxIn[0].PreviousOutPoint.Index = 0 }) - rejected(blockchain.ErrMissingTx) + rejected(blockchain.ErrMissingTxOut) // --------------------------------------------------------------------- // Block header median time tests. @@ -1688,7 +1688,7 @@ func Generate(includeLargeReorg bool) (tests [][]TestInstance, err error) { g.nextBlock("b58", outs[17], func(b *wire.MsgBlock) { b.Transactions[1].TxIn[0].PreviousOutPoint.Index = 42 }) - rejected(blockchain.ErrMissingTx) + rejected(blockchain.ErrMissingTxOut) // Create block with transaction that pays more than its inputs. // @@ -1816,7 +1816,7 @@ func Generate(includeLargeReorg bool) (tests [][]TestInstance, err error) { b.AddTransaction(tx3) b.AddTransaction(tx2) }) - rejected(blockchain.ErrMissingTx) + rejected(blockchain.ErrMissingTxOut) // Create block that double spends a transaction created in the same // block. @@ -1831,7 +1831,7 @@ func Generate(includeLargeReorg bool) (tests [][]TestInstance, err error) { b.AddTransaction(tx3) b.AddTransaction(tx4) }) - rejected(blockchain.ErrDoubleSpend) + rejected(blockchain.ErrMissingTxOut) // --------------------------------------------------------------------- // Extra subsidy tests. @@ -2022,7 +2022,7 @@ func Generate(includeLargeReorg bool) (tests [][]TestInstance, err error) { // to effective negate that behavior. b75OpReturnOut.amount++ g.nextBlock("b80", &b75OpReturnOut) - rejected(blockchain.ErrMissingTx) + rejected(blockchain.ErrMissingTxOut) // Create a block that has a transaction with multiple OP_RETURNs. Even // though it's not considered a standard transaction, it is still valid diff --git a/blockchain/scriptval.go b/blockchain/scriptval.go index 07bc0595..38e50bcf 100644 --- a/blockchain/scriptval.go +++ b/blockchain/scriptval.go @@ -65,7 +65,7 @@ out: "transaction %v referenced from "+ "transaction %v", originTxHash, txVI.tx.Hash()) - err := ruleError(ErrMissingTx, str) + err := ruleError(ErrMissingTxOut, str) v.sendResult(err) break out } diff --git a/blockchain/validate.go b/blockchain/validate.go index 64bbfe54..58fcf973 100644 --- a/blockchain/validate.go +++ b/blockchain/validate.go @@ -389,10 +389,11 @@ func CountP2SHSigOps(tx *btcutil.Tx, isCoinBaseTx bool, utxoView *UtxoViewpoint) originTxIndex := txIn.PreviousOutPoint.Index txEntry := utxoView.LookupEntry(originTxHash) if txEntry == nil || txEntry.IsOutputSpent(originTxIndex) { - str := fmt.Sprintf("unable to find unspent output "+ - "%v referenced from transaction %s:%d", - txIn.PreviousOutPoint, tx.Hash(), txInIndex) - return 0, ruleError(ErrMissingTx, str) + str := fmt.Sprintf("output %v referenced from "+ + "transaction %s:%d either does not exist or "+ + "has already been spent", txIn.PreviousOutPoint, + tx.Hash(), txInIndex) + return 0, ruleError(ErrMissingTxOut, str) } // We're only interested in pay-to-script-hash types, so skip @@ -897,12 +898,14 @@ func CheckTransactionInputs(tx *btcutil.Tx, txHeight int32, utxoView *UtxoViewpo for txInIndex, txIn := range tx.MsgTx().TxIn { // Ensure the referenced input transaction is available. originTxHash := &txIn.PreviousOutPoint.Hash + originTxIndex := txIn.PreviousOutPoint.Index utxoEntry := utxoView.LookupEntry(originTxHash) - if utxoEntry == nil { - str := fmt.Sprintf("unable to find unspent output "+ - "%v referenced from transaction %s:%d", - txIn.PreviousOutPoint, tx.Hash(), txInIndex) - return 0, ruleError(ErrMissingTx, str) + if utxoEntry == nil || utxoEntry.IsOutputSpent(originTxIndex) { + str := fmt.Sprintf("output %v referenced from "+ + "transaction %s:%d either does not exist or "+ + "has already been spent", txIn.PreviousOutPoint, + tx.Hash(), txInIndex) + return 0, ruleError(ErrMissingTxOut, str) } // Ensure the transaction is not spending coins which have not @@ -922,15 +925,6 @@ func CheckTransactionInputs(tx *btcutil.Tx, txHeight int32, utxoView *UtxoViewpo } } - // Ensure the transaction is not double spending coins. - originTxIndex := txIn.PreviousOutPoint.Index - if utxoEntry.IsOutputSpent(originTxIndex) { - str := fmt.Sprintf("transaction %s:%d tried to double "+ - "spend output %v", txHash, txInIndex, - txIn.PreviousOutPoint) - return 0, ruleError(ErrDoubleSpend, str) - } - // Ensure the transaction amounts are in range. Each of the // output values of the input transactions must not be negative // or more than the max allowed per transaction. All amounts in @@ -1017,7 +1011,7 @@ func (b *BlockChain) checkConnectBlock(node *blockNode, block *btcutil.Block, vi // an error now. if node.hash.IsEqual(b.chainParams.GenesisHash) { str := "the coinbase for the genesis block is not spendable" - return ruleError(ErrMissingTx, str) + return ruleError(ErrMissingTxOut, str) } // Ensure the view is for the node being checked. diff --git a/blockchain/weight.go b/blockchain/weight.go index f3fd2cfe..286a9197 100644 --- a/blockchain/weight.go +++ b/blockchain/weight.go @@ -91,10 +91,12 @@ func GetSigOpCost(tx *btcutil.Tx, isCoinBaseTx bool, utxoView *UtxoViewpoint, originTxIndex := txIn.PreviousOutPoint.Index txEntry := utxoView.LookupEntry(originTxHash) if txEntry == nil || txEntry.IsOutputSpent(originTxIndex) { - str := fmt.Sprintf("unable to find unspent output "+ - "%v referenced from transaction %s:%d", - txIn.PreviousOutPoint, tx.Hash(), txInIndex) - return 0, ruleError(ErrMissingTx, str) + str := fmt.Sprintf("output %v referenced from "+ + "transaction %s:%d either does not "+ + "exist or has already been spent", + txIn.PreviousOutPoint, tx.Hash(), + txInIndex) + return 0, ruleError(ErrMissingTxOut, str) } witness := txIn.Witness diff --git a/mempool/error.go b/mempool/error.go index c50c3965..b0d42be4 100644 --- a/mempool/error.go +++ b/mempool/error.go @@ -74,8 +74,6 @@ func extractRejectCode(err error) (wire.RejectCode, bool) { switch err.ErrorCode { // Rejected due to duplicate. case blockchain.ErrDuplicateBlock: - fallthrough - case blockchain.ErrDoubleSpend: code = wire.RejectDuplicate // Rejected due to obsolete version. diff --git a/rpcserver.go b/rpcserver.go index 44af8572..331ec2d2 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -1995,7 +1995,7 @@ func chainErrToGBTErrString(err error) string { return "bad-txns-dupinputs" case blockchain.ErrBadTxInput: return "bad-txns-badinput" - case blockchain.ErrMissingTx: + case blockchain.ErrMissingTxOut: return "bad-txns-missinginput" case blockchain.ErrUnfinalizedTx: return "bad-txns-unfinalizedtx" @@ -2005,8 +2005,6 @@ func chainErrToGBTErrString(err error) string { return "bad-txns-overwrite" case blockchain.ErrImmatureSpend: return "bad-txns-maturity" - case blockchain.ErrDoubleSpend: - return "bad-txns-dblspend" case blockchain.ErrSpendTooHigh: return "bad-txns-highspend" case blockchain.ErrBadFees: