diff --git a/internal/blockchain/utxocache.go b/internal/blockchain/utxocache.go index b0089c04cf..1a0889a510 100644 --- a/internal/blockchain/utxocache.go +++ b/internal/blockchain/utxocache.go @@ -578,6 +578,28 @@ func (c *UtxoCache) Commit(view *UtxoViewpoint) error { // connected block, so this optimizes for the much more common case. // --------------------------------------------------------------------- + // There is nothing to update in the cache when the output is spent by a + // transaction later in the regular transaction tree of the same block + // since such outputs never exist beyond the scope of the view and + // therefore do not have any cache entries. + // + // Further, it is no longer needed in the view either since it's already + // spent and thus nothing else in future blocks could possibly spend it. + // + // Thus, remove the entry from the view and continue to the next one. + if entry.isSpentByZeroConf() { + // Sanity check zero confirmation spends are also marked as spent. + if !entry.IsSpent() { + return AssertError(fmt.Sprintf("output %v from view@%s is "+ + "simultaneously marked as spent by a transaction later in "+ + "the same block and not spent", outpoint, + view.BestHash())) + } + + delete(view.entries, outpoint) + continue + } + // Mark the spent view entry as modified and spent in the cache and // remove it from the view. // diff --git a/internal/blockchain/utxocache_test.go b/internal/blockchain/utxocache_test.go index 95813a8d6e..3e7aecdd18 100644 --- a/internal/blockchain/utxocache_test.go +++ b/internal/blockchain/utxocache_test.go @@ -152,14 +152,15 @@ func entry85314() *UtxoEntry { // testEntryStates houses a test utxo entry along with various spent, modified, // and fresh states for conenient use throughout the tests. type testEntryStates struct { - unmodified *UtxoEntry - unmodifiedSpent *UtxoEntry - unmodifiedFresh *UtxoEntry - unmodifiedSpentFresh *UtxoEntry - modified *UtxoEntry - modifiedSpent *UtxoEntry - modifiedFresh *UtxoEntry - modifiedSpentFresh *UtxoEntry + unmodified *UtxoEntry + unmodifiedSpent *UtxoEntry + unmodifiedFresh *UtxoEntry + unmodifiedSpentFresh *UtxoEntry + modified *UtxoEntry + modifiedSpent *UtxoEntry + modifiedFresh *UtxoEntry + modifiedSpentFresh *UtxoEntry + modifiedSpentByZeroConf *UtxoEntry } // makeEntryStates creates combinations of the passed test utxo entry in various @@ -181,16 +182,19 @@ func makeEntryStates(entry *UtxoEntry) testEntryStates { modifiedFresh.state |= utxoStateFresh modifiedSpentFresh := modifiedSpent.Clone() modifiedSpentFresh.state |= utxoStateFresh + modifiedSpentByZeroConf := modifiedSpent.Clone() + modifiedSpentByZeroConf.state |= utxoStateSpentByZeroConf return testEntryStates{ - unmodified: unmodified, - unmodifiedSpent: unmodifiedSpent, - unmodifiedFresh: unmodifiedFresh, - unmodifiedSpentFresh: unmodifiedSpentFresh, - modified: modified, - modifiedSpent: modifiedSpent, - modifiedFresh: modifiedFresh, - modifiedSpentFresh: modifiedSpentFresh, + unmodified: unmodified, + unmodifiedSpent: unmodifiedSpent, + unmodifiedFresh: unmodifiedFresh, + unmodifiedSpentFresh: unmodifiedSpentFresh, + modified: modified, + modifiedSpent: modifiedSpent, + modifiedFresh: modifiedFresh, + modifiedSpentFresh: modifiedSpentFresh, + modifiedSpentByZeroConf: modifiedSpentByZeroConf, } } @@ -703,6 +707,16 @@ func TestCommit(t *testing.T) { wantCachedEntries map[wire.OutPoint]*UtxoEntry // expected committed cache err error // expected error }{{ + name: "modified spent by zero conf view entry w/o spent asserts", + viewEntries: map[wire.OutPoint]*UtxoEntry{ + outpoint1200: func() *UtxoEntry { + invalidEntry := entry1200.modifiedSpentByZeroConf.Clone() + invalidEntry.state &^= utxoStateSpent + return invalidEntry + }(), + }, + err: AssertError(""), + }, { name: "modified spent view entry w/ existing spent cache entry asserts", viewEntries: map[wire.OutPoint]*UtxoEntry{ outpoint1200: entry1200.modifiedSpent, @@ -907,6 +921,18 @@ func TestCommit(t *testing.T) { outpoint1200: entry1200.modifiedFresh, outpoint85314: entry85314.modified, }, + }, { + name: "view entry spent by zero conf removed and has no effect on cache", + viewEntries: map[wire.OutPoint]*UtxoEntry{ + outpoint299: entry299.modifiedSpentByZeroConf, + }, + cachedEntries: map[wire.OutPoint]*UtxoEntry{ + outpoint1100: entry1100.unmodified, + }, + wantViewEntries: map[wire.OutPoint]*UtxoEntry{}, + wantCachedEntries: map[wire.OutPoint]*UtxoEntry{ + outpoint1100: entry1100.unmodified, + }, }} for _, test := range tests { diff --git a/internal/blockchain/utxoentry.go b/internal/blockchain/utxoentry.go index c0a1fadf6b..9c05bb713d 100644 --- a/internal/blockchain/utxoentry.go +++ b/internal/blockchain/utxoentry.go @@ -20,19 +20,24 @@ const ( // // The bit representation is: // -// bit 0 - transaction output has been spent -// bit 1 - transaction output has been modified since it was loaded -// bit 2 - transaction output is fresh -// bits 3-7 - unused +// bit 0 - transaction output has been modified since it was loaded +// bit 1 - transaction output has been spent +// bit 2 - transaction output has been spent by tx later in the same block +// bit 3 - transaction output is fresh +// bits 4-7 - unused type utxoState uint8 const ( - // utxoStateSpent indicates that a txout is spent. - utxoStateSpent utxoState = 1 << iota - // utxoStateModified indicates that a txout has been modified since it was // loaded. - utxoStateModified + utxoStateModified utxoState = 1 << iota + + // utxoStateSpent indicates that a txout is spent. + utxoStateSpent + + // utxoStateSpentByZeroConf indicates that a txout was spent by another + // transaction later in the same block. + utxoStateSpentByZeroConf // utxoStateFresh indicates that a txout is fresh, which means that it // exists in the utxo cache but does not exist in the underlying database. @@ -149,6 +154,13 @@ func (entry *UtxoEntry) isFresh() bool { return entry.state&utxoStateFresh == utxoStateFresh } +// isSpentByZeroConf returns whether or not the output is marked as spent by a +// zero confirmation transaction. In other words, it is an output that was +// created in a block and later spent by another transaction in the same block. +func (entry *UtxoEntry) isSpentByZeroConf() bool { + return entry.state&utxoStateSpentByZeroConf == utxoStateSpentByZeroConf +} + // IsCoinBase returns whether or not the output was contained in a coinbase // transaction. func (entry *UtxoEntry) IsCoinBase() bool { diff --git a/internal/blockchain/utxoviewpoint.go b/internal/blockchain/utxoviewpoint.go index 7f4cb54821..63bad2b174 100644 --- a/internal/blockchain/utxoviewpoint.go +++ b/internal/blockchain/utxoviewpoint.go @@ -90,7 +90,7 @@ func (view *UtxoViewpoint) addTxOut(outpoint wire.OutPoint, txOut *wire.TxOut, // The referenced transaction output should always be marked as unspent and // modified when being added to the view. - entry.state &^= utxoStateSpent + entry.state &^= utxoStateSpent | utxoStateSpentByZeroConf entry.state |= utxoStateModified // Deep copy the script when the script in the entry differs from the one in @@ -318,19 +318,26 @@ func (view *UtxoViewpoint) connectStakeTransactions(block *dcrutil.Block, // connectRegularTransaction updates the view by adding all new utxos created by // the passed transaction from the regular transaction tree and marking all -// utxos that the transaction spends as spent. In addition, when the 'stxos' -// argument is not nil, it will be updated to append an entry for each spent -// txout. An error will be returned if the view does not contain the required -// utxos. +// utxos that the transaction spends as spent. The transaction is added to the +// provided in-flight tx map which is used to detect and mark utxos earlier in +// the same regular transaction tree as zero conf spends. +// +// When the 'stxos' argument is not nil, it will be updated to append an entry +// for each spent txout. An error will be returned if the view does not contain +// the required utxos. func (view *UtxoViewpoint) connectRegularTransaction(tx *dcrutil.Tx, - blockHeight int64, blockIndex uint32, stxos *[]spentTxOut, - isTreasuryEnabled bool) error { + blockHeight int64, blockIndex uint32, inFlightTx map[chainhash.Hash]uint32, + stxos *[]spentTxOut, isTreasuryEnabled bool) error { // Coinbase transactions don't have any inputs to spend. msgTx := tx.MsgTx() if standalone.IsCoinBaseTx(msgTx, isTreasuryEnabled) { // Add the transaction's outputs as available utxos. view.AddTxOuts(tx, blockHeight, blockIndex, isTreasuryEnabled) + + // Keep track of in-flight transactions in order detect spends of + // earlier outputs by transactions later in the same block. + inFlightTx[*tx.Hash()] = blockIndex return nil } @@ -355,27 +362,42 @@ func (view *UtxoViewpoint) connectRegularTransaction(tx *dcrutil.Tx, // details have been accessed since spending it might clear the fields // from memory in the future. entry.Spend() + + // Mark txouts that are spent by transaction inputs later in the regular + // transaction tree of the same block as zero conf spends. + if inFlightIdx, ok := inFlightTx[prevOut.Hash]; ok && blockIndex > + inFlightIdx { + + entry.state |= utxoStateSpentByZeroConf + } } // Add the transaction's outputs as available utxos. view.AddTxOuts(tx, blockHeight, blockIndex, isTreasuryEnabled) + // Keep track of in-flight transactions in order detect spends of earlier + // outputs by transactions later in the same block. + inFlightTx[*tx.Hash()] = blockIndex return nil } // connectRegularTransactions updates the view by adding all new utxos created // by the transactions in the regular transaction tree of the block and marking -// all utxos those same transactions spend as spent. In addition, when the -// 'stxos' argument is not nil, it will be updated to append an entry for each -// spent txout. An error will be returned if the view does not contain the -// required utxos. +// all utxos those same transactions spend as spent. It also marks all earlier +// utxos spent by transactions later in the tree as zero confirmation spends. +// +// When the 'stxos' argument is not nil, it will be updated to append an entry +// for each spent txout. An error will be returned if the view does not contain +// the required utxos. func (view *UtxoViewpoint) connectRegularTransactions(block *dcrutil.Block, stxos *[]spentTxOut, isTreasuryEnabled bool) error { // Connect all of the transactions in the regular transaction tree. - for i, tx := range block.Transactions() { + regularTxns := block.Transactions() + inFlightTx := make(map[chainhash.Hash]uint32, len(regularTxns)) + for i, tx := range regularTxns { err := view.connectRegularTransaction(tx, block.Height(), uint32(i), - stxos, isTreasuryEnabled) + inFlightTx, stxos, isTreasuryEnabled) if err != nil { return err } @@ -398,11 +420,20 @@ func (view *UtxoViewpoint) disconnectTransactions(block *dcrutil.Block, // disconnecting stake transactions. stxoIdx := len(stxos) - 1 transactions := block.Transactions() + numSpentRegularOutputs := countSpentRegularOutputs(block) if stakeTree { - stxoIdx = len(stxos) - countSpentRegularOutputs(block) - 1 + stxoIdx = len(stxos) - numSpentRegularOutputs - 1 transactions = block.STransactions() } + // Create a map to keep track of all in-flight spends by the regular + // transaction tree in order detect spends of earlier outputs by + // transactions later in the same block. + var spendsInFlight map[wire.OutPoint]int + if !stakeTree { + spendsInFlight = make(map[wire.OutPoint]int, numSpentRegularOutputs) + } + for txIdx := len(transactions) - 1; txIdx > -1; txIdx-- { tx := transactions[txIdx] txHash := tx.Hash() @@ -481,6 +512,14 @@ func (view *UtxoViewpoint) disconnectTransactions(block *dcrutil.Block, } entry.Spend() + + // Mark txouts that are spent by transaction inputs later in the + // same block as zero conf spends. + if inFlightIdx, ok := spendsInFlight[outpoint]; ok && + txIdx < inFlightIdx { + + entry.state |= utxoStateSpentByZeroConf + } } // Loop backwards through all of the transaction inputs (except for the @@ -524,8 +563,15 @@ func (view *UtxoViewpoint) disconnectTransactions(block *dcrutil.Block, // Mark the existing referenced transaction output as unspent and // modified. - entry.state &^= utxoStateSpent + entry.state &^= utxoStateSpent | utxoStateSpentByZeroConf entry.state |= utxoStateModified + + // Keep track of all in-flight spends by the regular transaction + // tree in order detect spends of earlier outputs by transactions + // later in the same block. + if !stakeTree { + spendsInFlight[txIn.PreviousOutPoint] = txIdx + } } } diff --git a/internal/blockchain/validate.go b/internal/blockchain/validate.go index d4faf9ae7c..9b5262fad4 100644 --- a/internal/blockchain/validate.go +++ b/internal/blockchain/validate.go @@ -3471,6 +3471,10 @@ func (b *BlockChain) checkTransactionsAndConnect(inputFees dcrutil.Amount, node // expensive (though still relatively cheap as compared to running the // scripts) checks against all the inputs when the signature operations // are out of bounds. + var inFlightRegularTx map[chainhash.Hash]uint32 + if !stakeTree { + inFlightRegularTx = make(map[chainhash.Hash]uint32, len(txs)) + } totalFees := int64(inputFees) // Stake tx tree carry forward prevHeader := node.parent.Header() var cumulativeSigOps int @@ -3515,16 +3519,24 @@ func (b *BlockChain) checkTransactionsAndConnect(inputFees dcrutil.Amount, node // and add all of the outputs for this transaction which are not // provably unspendable as available utxos. // + // For the regular transaction tree, keep track of in-flight + // transactions in order detect spends of earlier outputs by + // transactions later in the same block. + // // Also, update the passed spent txos slice to contain an entry for each // output the transaction spends. - connectTransaction := view.connectRegularTransaction - if stakeTree { - connectTransaction = view.connectStakeTransaction - } - err = connectTransaction(tx, node.height, uint32(idx), stxos, - isTreasuryEnabled) - if err != nil { - return err + if !stakeTree { + err := view.connectRegularTransaction(tx, node.height, uint32(idx), + inFlightRegularTx, stxos, isTreasuryEnabled) + if err != nil { + return err + } + } else { + err := view.connectStakeTransaction(tx, node.height, uint32(idx), + stxos, isTreasuryEnabled) + if err != nil { + return err + } } }