From 93fb5daa2a9ebf6fa9dd40396251f4a6d27954b4 Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Thu, 23 Jun 2022 18:01:32 -0500
Subject: [PATCH 01/17] [blockchain-v4] blockchain: Address some linter
 complaints.

---
 blockchain/chainio_test.go | 6 +++---
 blockchain/error.go        | 9 +++++++++
 blockchain/upgrade_test.go | 2 +-
 blockchain/utxoio_test.go  | 6 +++---
 4 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/blockchain/chainio_test.go b/blockchain/chainio_test.go
index 88cbb58ea5..6dbc35badb 100644
--- a/blockchain/chainio_test.go
+++ b/blockchain/chainio_test.go
@@ -309,7 +309,7 @@ func TestBlockIndexDecodeErrors(t *testing.T) {
 		// Ensure the expected error type is returned.
 		gotBytesRead, err := decodeBlockIndexEntry(test.serialized,
 			&test.entry)
-		if !errors.As(err, &test.errType) {
+		if !errors.Is(err, test.errType) {
 			t.Errorf("%q: expected error type does not match - got %T, want %T",
 				test.name, err, test.errType)
 			continue
@@ -525,7 +525,7 @@ func TestStxoDecodeErrors(t *testing.T) {
 		gotBytesRead, err := decodeSpentTxOut(test.serialized,
 			&test.stxo, test.stxo.amount, test.stxo.blockHeight, test.stxo.blockIndex,
 			test.txOutIndex)
-		if !errors.As(err, &test.errType) {
+		if !errors.Is(err, test.errType) {
 			t.Errorf("%q: expected error type does not match - got %T, want %T",
 				test.name, err, test.errType)
 			continue
@@ -777,7 +777,7 @@ func TestSpendJournalErrors(t *testing.T) {
 		// slice is nil.
 		stxos, err := deserializeSpendJournalEntry(test.serialized,
 			test.blockTxns, noTreasury)
-		if !errors.As(err, &test.errType) {
+		if !errors.Is(err, test.errType) {
 			t.Errorf("%q: expected error type does not match - got %T, want %T",
 				test.name, err, test.errType)
 			continue
diff --git a/blockchain/error.go b/blockchain/error.go
index 8f20ffc5cb..a370a5f0ef 100644
--- a/blockchain/error.go
+++ b/blockchain/error.go
@@ -24,6 +24,15 @@ func (e AssertError) Error() string {
 	return "assertion failed: " + string(e)
 }
 
+// Is implements the interface to work with the standard library's errors.Is.
+//
+// It returns true in the following cases:
+// - The target is AssertError
+func (e AssertError) Is(target error) bool {
+	var err AssertError
+	return errors.As(target, &err)
+}
+
 // ErrorKind identifies a kind of error.  It has full support for errors.Is and
 // errors.As, so the caller can directly check against an error kind when
 // determining the reason for an error.
diff --git a/blockchain/upgrade_test.go b/blockchain/upgrade_test.go
index 23d60aa292..1df91c4deb 100644
--- a/blockchain/upgrade_test.go
+++ b/blockchain/upgrade_test.go
@@ -142,7 +142,7 @@ func TestBlockIndexDecodeErrorsV2(t *testing.T) {
 		// Ensure the expected error type is returned.
 		gotBytesRead, err := decodeBlockIndexEntryV2(test.serialized,
 			&test.entry)
-		if !errors.As(err, &test.errType) {
+		if !errors.Is(err, test.errType) {
 			t.Errorf("decodeBlockIndexEntry (%s): expected error "+
 				"type does not match - got %T, want %T",
 				test.name, err, test.errType)
diff --git a/blockchain/utxoio_test.go b/blockchain/utxoio_test.go
index 6fa660639a..fe59dbf9a2 100644
--- a/blockchain/utxoio_test.go
+++ b/blockchain/utxoio_test.go
@@ -303,7 +303,7 @@ func TestUtxoEntryDeserializeErrors(t *testing.T) {
 		// Ensure the expected error type is returned and the returned
 		// entry is nil.
 		entry, err := deserializeUtxoEntry(test.serialized, test.txOutIndex)
-		if !errors.As(err, &test.errType) {
+		if !errors.Is(err, test.errType) {
 			t.Errorf("%q: expected error type does not match - got %T, want %T",
 				test.name, err, test.errType)
 			continue
@@ -397,7 +397,7 @@ func TestUtxoSetStateDeserializeErrors(t *testing.T) {
 		// Ensure the expected error type is returned and the returned
 		// utxo set state is nil.
 		entry, err := deserializeUtxoSetState(test.serialized)
-		if !errors.As(err, &test.errType) {
+		if !errors.Is(err, test.errType) {
 			t.Errorf("%q: expected error type does not match - got %T, want %T",
 				test.name, err, test.errType)
 			continue
@@ -509,7 +509,7 @@ func TestDecodeOutpointKeyErrors(t *testing.T) {
 		// Ensure the expected error type is returned.
 		var gotOutpoint wire.OutPoint
 		err := decodeOutpointKey(test.serialized, &gotOutpoint)
-		if !errors.As(err, &test.errType) {
+		if !errors.Is(err, test.errType) {
 			t.Errorf("%q: expected error type does not match - got %T, want %T",
 				test.name, err, test.errType)
 			continue

From f0315387b1d1adfde0e2217b85f18c910fd0a1bd Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Tue, 13 Sep 2022 05:24:06 -0500
Subject: [PATCH 02/17] [blockchain-v4] blockchain: Misc consistency cleanup
 pass.

This makes some minor and non-functional misc changes to improve code
consistency.
---
 blockchain/chain.go                    |   3 +-
 blockchain/indexers/indexsubscriber.go |   4 +-
 blockchain/indexers/txindex_test.go    |   3 +-
 blockchain/utxoviewpoint.go            |  55 +++++-----
 blockchain/validate.go                 | 140 +++++++++++++------------
 blockchain/validate_test.go            |   2 +-
 6 files changed, 102 insertions(+), 105 deletions(-)

diff --git a/blockchain/chain.go b/blockchain/chain.go
index df89b998a2..1576612490 100644
--- a/blockchain/chain.go
+++ b/blockchain/chain.go
@@ -1020,7 +1020,8 @@ func countSpentStakeOutputs(block *dcrutil.Block, isTreasuryEnabled bool) int {
 			continue
 		}
 
-		// Exclude TreasuryBase and TSpend.
+		// Exclude treasurybase and treasury spends since neither have any
+		// inputs.
 		if stake.IsTreasuryBase(stx) || stake.IsTSpend(stx) {
 			continue
 		}
diff --git a/blockchain/indexers/indexsubscriber.go b/blockchain/indexers/indexsubscriber.go
index 21e85ebba4..becb3d879e 100644
--- a/blockchain/indexers/indexsubscriber.go
+++ b/blockchain/indexers/indexsubscriber.go
@@ -238,8 +238,8 @@ func (s *IndexSubscriber) findLowestIndexTipHeight(queryer ChainQueryer) (int64,
 	return lowestHeight, bestHeight, nil
 }
 
-// CatchUp syncs all subscribed indexes to the the main chain by connecting
-// blocks from after the lowest index tip to the current main chain tip.
+// CatchUp syncs all subscribed indexes to the main chain by connecting blocks
+// from after the lowest index tip to the current main chain tip.
 //
 // This should be called after all indexes have subscribed for updates.
 func (s *IndexSubscriber) CatchUp(ctx context.Context, db database.DB, queryer ChainQueryer) error {
diff --git a/blockchain/indexers/txindex_test.go b/blockchain/indexers/txindex_test.go
index ad2d04ba7e..cab92b159c 100644
--- a/blockchain/indexers/txindex_test.go
+++ b/blockchain/indexers/txindex_test.go
@@ -109,8 +109,7 @@ func (tc *testChain) MainChainHasBlock(hash *chainhash.Hash) bool {
 	return ok
 }
 
-// Best returns the height and block hash of the the current
-// chain tip.
+// Best returns the height and block hash of the current chain tip.
 func (tc *testChain) Best() (int64, *chainhash.Hash) {
 	tc.mtx.Lock()
 	defer tc.mtx.Unlock()
diff --git a/blockchain/utxoviewpoint.go b/blockchain/utxoviewpoint.go
index 6180d5226a..a41783e899 100644
--- a/blockchain/utxoviewpoint.go
+++ b/blockchain/utxoviewpoint.go
@@ -50,6 +50,13 @@ func (view *UtxoViewpoint) LookupEntry(outpoint wire.OutPoint) *UtxoEntry {
 	return view.entries[outpoint]
 }
 
+// RemoveEntry removes the given transaction output from the current state of
+// the view.  It will have no effect if the passed output does not exist in the
+// view.
+func (view *UtxoViewpoint) RemoveEntry(outpoint wire.OutPoint) {
+	delete(view.entries, outpoint)
+}
+
 // addTxOut adds the specified output to the view if it is not provably
 // unspendable.  When the view already has an entry for the output, it will be
 // marked unspent.  All fields will be updated for existing entries since it's
@@ -147,9 +154,8 @@ func (view *UtxoViewpoint) AddTxOut(tx *dcrutil.Tx, txOutIdx uint32,
 func (view *UtxoViewpoint) AddTxOuts(tx *dcrutil.Tx, blockHeight int64,
 	blockIndex uint32, isTreasuryEnabled, isAutoRevocationsEnabled bool) {
 
-	msgTx := tx.MsgTx()
-
 	// Set encoded flags for the transaction.
+	msgTx := tx.MsgTx()
 	isCoinBase := standalone.IsCoinBaseTx(msgTx, isTreasuryEnabled)
 	hasExpiry := msgTx.Expiry != wire.NoExpiryValue
 	txType := stake.DetermineTxType(msgTx, isTreasuryEnabled,
@@ -235,18 +241,15 @@ func (view *UtxoViewpoint) connectTransaction(tx *dcrutil.Tx, blockHeight int64,
 	// Spend the referenced utxos by marking them spent in the view and, if a
 	// slice was provided for the spent txout details, append an entry to it.
 	isVote := stake.IsSSGen(msgTx, isTreasuryEnabled)
-	var isTSpend bool
-	if isTreasuryEnabled {
-		isTSpend = stake.IsTSpend(msgTx)
-	}
+	isTreasurySpend := isTreasuryEnabled && stake.IsTSpend(msgTx)
 	for txInIdx, txIn := range msgTx.TxIn {
 		// Ignore stakebase since it has no input.
 		if isVote && txInIdx == 0 {
 			continue
 		}
 
-		// Ignore TSpend since it has no input.
-		if isTSpend {
+		// Ignore treasury spends since they have no inputs.
+		if isTreasurySpend {
 			continue
 		}
 
@@ -319,12 +322,11 @@ func (view *UtxoViewpoint) disconnectTransactions(block *dcrutil.Block,
 		}
 		isVote := txType == stake.TxTypeSSGen
 
-		var isTreasuryBase, isTSpend bool
-
+		var isTreasuryBase, isTreasurySpend bool
 		if isTreasuryEnabled {
-			isTSpend = txType == stake.TxTypeTSpend && stakeTree
-			isTreasuryBase = txType == stake.TxTypeTreasuryBase &&
-				stakeTree && txIdx == 0
+			isTreasurySpend = txType == stake.TxTypeTSpend && stakeTree
+			isTreasuryBase = txType == stake.TxTypeTreasuryBase && stakeTree &&
+				txIdx == 0
 		}
 
 		tree := wire.TxTreeRegular
@@ -387,10 +389,10 @@ func (view *UtxoViewpoint) disconnectTransactions(block *dcrutil.Block,
 		}
 
 		// Loop backwards through all of the transaction inputs (except for the
-		// coinbase and treasurybase which have no inputs) and unspend the
-		// referenced txos.  This is necessary to match the order of the spent
-		// txout entries.
-		if isCoinBase || isTreasuryBase || isTSpend {
+		// coinbase, treasurybase, and treasury spends which have no inputs) and
+		// unspend the referenced txos.  This is necessary to match the order of
+		// the spent txout entries.
+		if isCoinBase || isTreasuryBase || isTreasurySpend {
 			continue
 		}
 		for txInIdx := len(msgTx.TxIn) - 1; txInIdx > -1; txInIdx-- {
@@ -435,13 +437,6 @@ func (view *UtxoViewpoint) disconnectTransactions(block *dcrutil.Block,
 	return nil
 }
 
-// RemoveEntry removes the given transaction output from the current state of
-// the view.  It will have no effect if the passed output does not exist in the
-// view.
-func (view *UtxoViewpoint) RemoveEntry(outpoint wire.OutPoint) {
-	delete(view.entries, outpoint)
-}
-
 // disconnectRegularTransactions updates the view by removing all utxos created
 // by the transactions in regular tree of the provided block and unspending all
 // of the txos spent by those same transactions by using the provided spent txo
@@ -524,8 +519,8 @@ func (view *UtxoViewpoint) connectBlock(db database.DB, block, parent *dcrutil.B
 	// Disconnect the transactions in the regular tree of the parent block if
 	// the passed block disapproves it.
 	if !headerApprovesParent(&block.MsgBlock().Header) {
-		err := view.disconnectDisapprovedBlock(db, parent,
-			isTreasuryEnabled, isAutoRevocationsEnabled)
+		err := view.disconnectDisapprovedBlock(db, parent, isTreasuryEnabled,
+			isAutoRevocationsEnabled)
 		if err != nil {
 			return err
 		}
@@ -646,8 +641,8 @@ func (view *UtxoViewpoint) Entries() map[wire.OutPoint]*UtxoEntry {
 	return view.entries
 }
 
-// ViewFilteredSet represents a set of utxos to fetch from the database that are
-// not already in a view.
+// ViewFilteredSet represents a set of utxos to fetch from the cache/backend
+// that are not already in a view.
 type ViewFilteredSet map[wire.OutPoint]struct{}
 
 // add conditionally adds the provided outpoint to the set if it does not
@@ -712,8 +707,8 @@ func (view *UtxoViewpoint) addRegularInputUtxos(block *dcrutil.Block,
 				i >= inFlightIndex {
 
 				originTx := regularTxns[inFlightIndex]
-				view.AddTxOuts(originTx, block.Height(),
-					uint32(inFlightIndex), isTreasuryEnabled, isAutoRevocationsEnabled)
+				view.AddTxOuts(originTx, block.Height(), uint32(inFlightIndex),
+					isTreasuryEnabled, isAutoRevocationsEnabled)
 				continue
 			}
 
diff --git a/blockchain/validate.go b/blockchain/validate.go
index fe5e8a9b65..17b7666d1a 100644
--- a/blockchain/validate.go
+++ b/blockchain/validate.go
@@ -95,8 +95,8 @@ const (
 	// checkForDuplicateHashes checks for duplicate hashes when validating
 	// blocks.  Because of the rule inserting the height into the second (nonce)
 	// txOut, there should never be a duplicate transaction hash that overwrites
-	// another. However, because there is a 2^128 chance of a collision, the
-	// paranoid user may wish to turn this feature on.
+	// another. However, because there is a 1 in 2^128 chance of a collision,
+	// the paranoid user may wish to turn this feature on.
 	checkForDuplicateHashes = false
 
 	// testNet3MaxDiffActivationHeight is the height that enforcement of the
@@ -738,8 +738,8 @@ func standaloneToChainRuleError(err error) error {
 // target difficulty as claimed.
 //
 // The flags modify the behavior of this function as follows:
-//  - BFNoPoWCheck: The check to ensure the block hash is less than the target
-//    difficulty is not performed.
+//   - BFNoPoWCheck: The check to ensure the block hash is less than the target
+//     difficulty is not performed.
 func checkProofOfWork(header *wire.BlockHeader, powLimit *big.Int, flags BehaviorFlags) error {
 	// Only ensure the target difficulty bits are in the valid range when the
 	// the flag to avoid proof of work checks is set.
@@ -928,10 +928,8 @@ func checkBlockSanity(block *dcrutil.Block, timeSource MedianTimeSource, flags B
 
 	// Do some preliminary checks on each regular transaction to ensure they
 	// are sane.
-	transactions := block.Transactions()
-	for _, tx := range transactions {
-		msgTx := tx.MsgTx()
-		err := checkTransactionSanity(msgTx, chainParams)
+	for _, tx := range msgBlock.Transactions {
+		err := checkTransactionSanity(tx, chainParams)
 		if err != nil {
 			return err
 		}
@@ -963,8 +961,9 @@ func checkBlockSanity(block *dcrutil.Block, timeSource MedianTimeSource, flags B
 
 	// Check for duplicate transactions.
 	existingTxHashes := make(map[chainhash.Hash]struct{})
+	regularTransactions := block.Transactions()
 	stakeTransactions := block.STransactions()
-	allTransactions := append(transactions, stakeTransactions...)
+	allTransactions := append(regularTransactions, stakeTransactions...)
 	for _, tx := range allTransactions {
 		hash := tx.Hash()
 		if _, exists := existingTxHashes[*hash]; exists {
@@ -1028,8 +1027,8 @@ func isDCP0005Violation(network wire.CurrencyNet, header *wire.BlockHeader, bloc
 // on having the full block data of all ancestors available.
 //
 // The flags modify the behavior of this function as follows:
-//  - BFFastAdd: All checks except those involving comparing the header against
-//    the checkpoints and expected height are not performed.
+//   - BFFastAdd: All checks except those involving comparing the header against
+//     the checkpoints and expected height are not performed.
 //
 // This function MUST be called with the chain state lock held (for reads).
 func (b *BlockChain) checkBlockHeaderPositional(header *wire.BlockHeader, prevNode *blockNode, flags BehaviorFlags) error {
@@ -1169,7 +1168,7 @@ func (b *BlockChain) checkBlockHeaderPositional(header *wire.BlockHeader, prevNo
 // available.
 //
 // The flags modify the behavior of this function as follows:
-//  - BFFastAdd: The transactions are not checked to see if they are expired.
+//   - BFFastAdd: The transactions are not checked to see if they are expired.
 //
 // This function MUST be called with the chain state lock held (for reads).
 func (b *BlockChain) checkBlockDataPositional(block *dcrutil.Block, prevNode *blockNode, flags BehaviorFlags) error {
@@ -1251,7 +1250,7 @@ func (b *BlockChain) checkBlockPositional(block *dcrutil.Block, prevNode *blockN
 // vote will necessarily need to be transitioned to this function.
 //
 // The flags modify the behavior of this function as follows:
-//  - BFFastAdd: No check are performed.
+//   - BFFastAdd: No check are performed.
 //
 // This function MUST be called with the chain state lock held (for writes).
 func (b *BlockChain) checkBlockHeaderContext(header *wire.BlockHeader, prevNode *blockNode, flags BehaviorFlags) error {
@@ -1678,13 +1677,13 @@ func (b *BlockChain) checkMerkleRoots(block *wire.MsgBlock, prevNode *blockNode)
 // necessarily need to be transitioned to this function.
 //
 // The flags modify the behavior of this function as follows:
-//  - BFFastAdd:
-//      - The max block size is not checked
-//      - The calculated merkle root(s) of the transaction trees are not checked
-//        against the associated entries in the header
-//      - Transactions are not checked to see if they are finalized
-//      - The included votes, revocations, and treasury spend transactions are
-//        not verified to be allowed
+//   - BFFastAdd:
+//   - The max block size is not checked
+//   - The calculated merkle root(s) of the transaction trees are not checked
+//     against the associated entries in the header
+//   - Transactions are not checked to see if they are finalized
+//   - The included votes, revocations, and treasury spend transactions are
+//     not verified to be allowed
 //
 // The flags are also passed to checkBlockHeaderContext.  See its documentation
 // for how the flags modify its behavior.
@@ -3313,20 +3312,19 @@ func CountP2SHSigOps(tx *dcrutil.Tx, isCoinBaseTx bool, isStakeBaseTx bool, view
 // checkNumSigOps Checks the number of P2SH signature operations to make
 // sure they don't overflow the limits.  It takes a cumulative number of sig
 // ops as an argument and increments will each call.
-// TxTree true == Regular, false == Stake
-func checkNumSigOps(tx *dcrutil.Tx, view *UtxoViewpoint, index int, txTree bool, cumulativeSigOps int, isTreasuryEnabled bool) (int, error) {
+func checkNumSigOps(tx *dcrutil.Tx, view *UtxoViewpoint, index int, stakeTree bool, cumulativeSigOps int, isTreasuryEnabled bool) (int, error) {
 	msgTx := tx.MsgTx()
 	isSSGen := stake.IsSSGen(msgTx, isTreasuryEnabled)
-	numsigOps := CountSigOps(tx, (index == 0) && txTree, isSSGen,
-		isTreasuryEnabled)
+	isCoinbaseTx := (index == 0) && !stakeTree
+	numsigOps := CountSigOps(tx, isCoinbaseTx, isSSGen, isTreasuryEnabled)
 
 	// Since the first (and only the first) transaction has already been
 	// verified to be a coinbase transaction, use (i == 0) && TxTree as an
 	// optimization for the flag to countP2SHSigOps for whether or not the
 	// transaction is a coinbase transaction rather than having to do a
 	// full coinbase check again.
-	numP2SHSigOps, err := CountP2SHSigOps(tx, (index == 0) && txTree,
-		isSSGen, view, isTreasuryEnabled)
+	numP2SHSigOps, err := CountP2SHSigOps(tx, isCoinbaseTx, isSSGen, view,
+		isTreasuryEnabled)
 	if err != nil {
 		log.Tracef("CountP2SHSigOps failed; error returned %v", err)
 		return 0, err
@@ -3430,7 +3428,7 @@ func getStakeBaseAmounts(txs []*dcrutil.Tx, view *UtxoViewpoint, isTreasuryEnabl
 }
 
 // getStakeTreeFees determines the amount of fees for in the stake tx tree of
-// some node given a transaction store.
+// some node given a utxo view.
 func getStakeTreeFees(subsidyCache *standalone.SubsidyCache, height int64,
 	txs []*dcrutil.Tx, view *UtxoViewpoint, isTreasuryEnabled,
 	isSubsidySplitEnabled bool) (dcrutil.Amount, error) {
@@ -3440,11 +3438,8 @@ func getStakeTreeFees(subsidyCache *standalone.SubsidyCache, height int64,
 	for _, tx := range txs {
 		msgTx := tx.MsgTx()
 		isSSGen := stake.IsSSGen(msgTx, isTreasuryEnabled)
-		var isTSpend, isTreasuryBase bool
-		if isTreasuryEnabled {
-			isTSpend = stake.IsTSpend(msgTx)
-			isTreasuryBase = stake.IsTreasuryBase(msgTx)
-		}
+		isTreasuryBase := isTreasuryEnabled && stake.IsTreasuryBase(msgTx)
+		isTreasurySpend := isTreasuryEnabled && stake.IsTSpend(msgTx)
 
 		for i, in := range msgTx.TxIn {
 			// Ignore stakebases.
@@ -3452,13 +3447,9 @@ func getStakeTreeFees(subsidyCache *standalone.SubsidyCache, height int64,
 				continue
 			}
 
-			// Ignore TSpend.
-			if isTSpend && i == 0 {
-				continue
-			}
-
-			// Ignore treasury base.
-			if isTreasuryBase && i == 0 {
+			// Ignore treasury spends and treasurybases since they have no
+			// inputs.
+			if isTreasuryBase || isTreasurySpend {
 				continue
 			}
 
@@ -3489,7 +3480,7 @@ func getStakeTreeFees(subsidyCache *standalone.SubsidyCache, height int64,
 				isSubsidySplitEnabled)
 		}
 
-		if isTSpend {
+		if isTreasurySpend {
 			totalOutputs -= msgTx.TxIn[0].ValueIn
 		}
 
@@ -3508,10 +3499,10 @@ func getStakeTreeFees(subsidyCache *standalone.SubsidyCache, height int64,
 }
 
 // checkTransactionsAndConnect is the local function used to check the
-// transaction inputs for a transaction list given a predetermined TxStore.
+// transaction inputs for a transaction list given a predetermined utxo view.
 // After ensuring the transaction is valid, the transaction is connected to the
-// UTXO viewpoint.  TxTree true == Regular, false == Stake
-func (b *BlockChain) checkTransactionsAndConnect(inputFees dcrutil.Amount, node *blockNode, txs []*dcrutil.Tx, view *UtxoViewpoint, stxos *[]spentTxOut, txTree bool) error {
+// utxo view.
+func (b *BlockChain) checkTransactionsAndConnect(inputFees dcrutil.Amount, node *blockNode, txs []*dcrutil.Tx, view *UtxoViewpoint, stxos *[]spentTxOut, stakeTree bool) error {
 	isTreasuryEnabled, err := b.isTreasuryAgendaActive(node.parent)
 	if err != nil {
 		return err
@@ -3545,21 +3536,27 @@ func (b *BlockChain) checkTransactionsAndConnect(inputFees dcrutil.Amount, node
 		// Ensure that the number of signature operations is not beyond
 		// the consensus limit.
 		var err error
-		cumulativeSigOps, err = checkNumSigOps(tx, view, idx, txTree,
+		cumulativeSigOps, err = checkNumSigOps(tx, view, idx, stakeTree,
 			cumulativeSigOps, isTreasuryEnabled)
 		if err != nil {
 			return err
 		}
 
-		// This step modifies the txStore and marks the tx outs used
-		// spent, so be aware of this.
-		txFee, err := CheckTransactionInputs(b.subsidyCache, tx,
-			node.height, view, true, /* check fraud proofs */
-			b.chainParams, &prevHeader, isTreasuryEnabled,
-			isAutoRevocationsEnabled, isSubsidySplitEnabled)
+		// Perform a series of checks on the inputs to the transaction to ensure
+		// they are valid and calculate the total fees for it.
+		//
+		// An example of some of the checks include verifying all inputs exist,
+		// ensuring the coinbase seasoning requirements are met, detecting
+		// double spends, validating all values and fees are in the legal range,
+		// the total output amount doesn't exceed the input amount, and
+		// verifying the signatures to prove the spender was the owner of the
+		// coins and therefore allowed to spend them.
+		const checkFraudProof = true
+		txFee, err := CheckTransactionInputs(b.subsidyCache, tx, node.height,
+			view, checkFraudProof, b.chainParams, &prevHeader,
+			isTreasuryEnabled, isAutoRevocationsEnabled, isSubsidySplitEnabled)
 		if err != nil {
-			log.Tracef("CheckTransactionInputs failed; error "+
-				"returned: %v", err)
+			log.Tracef("CheckTransactionInputs failed; error returned: %v", err)
 			return err
 		}
 
@@ -3572,10 +3569,14 @@ func (b *BlockChain) checkTransactionsAndConnect(inputFees dcrutil.Amount, node
 				"overflows accumulator")
 		}
 
-		// Connect the transaction to the UTXO viewpoint, so that in
-		// flight transactions may correctly validate.
-		err = view.connectTransaction(tx, node.height, uint32(idx),
-			stxos, isTreasuryEnabled, isAutoRevocationsEnabled)
+		// Update the view to mark all utxos spent by the transaction as spent
+		// and add all of the outputs for this transaction which are not
+		// provably unspendable as available utxos.
+		//
+		// Also, update the passed spent txos slice to contain an entry for each
+		// output the transaction spends.
+		err = view.connectTransaction(tx, node.height, uint32(idx), stxos,
+			isTreasuryEnabled, isAutoRevocationsEnabled)
 		if err != nil {
 			return err
 		}
@@ -3585,8 +3586,8 @@ func (b *BlockChain) checkTransactionsAndConnect(inputFees dcrutil.Amount, node
 	// the expected subsidy value plus total transaction fees gained from
 	// mining the block.  It is safe to ignore overflow and out of range
 	// errors here because those error conditions would have already been
-	// caught by checkTransactionSanity.
-	if txTree { //TxTreeRegular
+	// caught by the transaction sanity checks.
+	if !stakeTree { //TxTreeRegular
 		// Apply penalty to fees if we're at stake validation height.
 		if node.height >= b.chainParams.StakeValidationHeight {
 			totalFees *= int64(node.voters)
@@ -3920,8 +3921,8 @@ func (b *BlockChain) checkConnectBlock(node *blockNode, block, parent *dcrutil.B
 	// Disconnect all of the transactions in the regular transaction tree of
 	// the parent if the block being checked votes against it.
 	if node.height > 1 && !voteBitsApproveParent(node.voteBits) {
-		err = view.disconnectDisapprovedBlock(b.db, parent,
-			isTreasuryEnabled, isAutoRevocationsEnabled)
+		err = view.disconnectDisapprovedBlock(b.db, parent, isTreasuryEnabled,
+			isAutoRevocationsEnabled)
 		if err != nil {
 			return err
 		}
@@ -3931,7 +3932,7 @@ func (b *BlockChain) checkConnectBlock(node *blockNode, block, parent *dcrutil.B
 	// that 'overwrite' older transactions which are not fully spent.
 	err = b.checkDupTxs(block.STransactions(), view, wire.TxTreeStake)
 	if err != nil {
-		log.Tracef("checkDupTxs failed for cur TxTreeStake: %v", err)
+		log.Tracef("checkDupTxs failed for stake tree: %v", err)
 		return err
 	}
 
@@ -3946,11 +3947,11 @@ func (b *BlockChain) checkConnectBlock(node *blockNode, block, parent *dcrutil.B
 		return err
 	}
 
+	const stakeTreeTrue = true
 	err = b.checkTransactionsAndConnect(0, node, block.STransactions(),
-		view, stxos, false)
+		view, stxos, stakeTreeTrue)
 	if err != nil {
-		log.Tracef("checkTransactionsAndConnect failed for "+
-			"TxTreeStake: %v", err)
+		log.Tracef("checkTransactionsAndConnect failed for stake tree: %v", err)
 		return err
 	}
 
@@ -3961,7 +3962,7 @@ func (b *BlockChain) checkConnectBlock(node *blockNode, block, parent *dcrutil.B
 	stakeTreeFees, err := getStakeTreeFees(b.subsidyCache, node.height,
 		block.STransactions(), view, isTreasuryEnabled, isSubsidySplitEnabled)
 	if err != nil {
-		log.Tracef("getStakeTreeFees failed for TxTreeStake: %v", err)
+		log.Tracef("getStakeTreeFees failed for stake tree: %v", err)
 		return err
 	}
 
@@ -4009,15 +4010,16 @@ func (b *BlockChain) checkConnectBlock(node *blockNode, block, parent *dcrutil.B
 	// that 'overwrite' older transactions which are not fully spent.
 	err = b.checkDupTxs(block.Transactions(), view, wire.TxTreeRegular)
 	if err != nil {
-		log.Tracef("checkDupTxs failed for cur TxTreeRegular: %v", err)
+		log.Tracef("checkDupTxs failed for cur regular tree: %v", err)
 		return err
 	}
 
+	const stakeTreeFalse = false
 	err = b.checkTransactionsAndConnect(stakeTreeFees, node,
-		block.Transactions(), view, stxos, true)
+		block.Transactions(), view, stxos, stakeTreeFalse)
 	if err != nil {
-		log.Tracef("checkTransactionsAndConnect failed for cur "+
-			"TxTreeRegular: %v", err)
+		log.Tracef("checkTransactionsAndConnect failed for regular tree: %v",
+			err)
 		return err
 	}
 
diff --git a/blockchain/validate_test.go b/blockchain/validate_test.go
index ce2bdce0a0..4983bb81a6 100644
--- a/blockchain/validate_test.go
+++ b/blockchain/validate_test.go
@@ -1997,7 +1997,7 @@ func TestModifiedSubsidySplitSemantics(t *testing.T) {
 		trsySubsidy := cache.CalcTreasurySubsidy(height, numVotes, noTreasury)
 		powSubsidy := cache.CalcWorkSubsidyV2(height, numVotes, withDCP0010)
 
-		// Update the input value to the the new expected subsidy sum.
+		// Update the input value to the new expected subsidy sum.
 		coinbaseTx := b.Transactions[0]
 		coinbaseTx.TxIn[0].ValueIn = trsySubsidy + powSubsidy
 

From 48c0fbe953bd8fbe3fd1f549f5e3310abb76da49 Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Tue, 13 Sep 2022 05:24:07 -0500
Subject: [PATCH 03/17] [blockchain-v4] blockchain: Pre-allocate in-flight
 utxoview tx map.

This modifies the code that tracks the in-flight transactions when
determining the input utxos that need to be loaded to pre-allocate the
map to avoid some additional overhead associated with growing the map
multiple times when a lot of transactions are involved.
---
 blockchain/utxoviewpoint.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/blockchain/utxoviewpoint.go b/blockchain/utxoviewpoint.go
index a41783e899..edadd7144c 100644
--- a/blockchain/utxoviewpoint.go
+++ b/blockchain/utxoviewpoint.go
@@ -680,8 +680,8 @@ func (view *UtxoViewpoint) addRegularInputUtxos(block *dcrutil.Block,
 	// Build a map of in-flight transactions because some of the inputs in the
 	// regular transaction tree of this block could be referencing other
 	// transactions earlier in the block which are not yet in the chain.
-	txInFlight := map[chainhash.Hash]int{}
 	regularTxns := block.Transactions()
+	txInFlight := make(map[chainhash.Hash]int, len(regularTxns))
 	for i, tx := range regularTxns {
 		txInFlight[*tx.Hash()] = i
 	}

From fd8c4452b1810083751493f708d5485cf0efb444 Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Tue, 13 Sep 2022 05:24:09 -0500
Subject: [PATCH 04/17] [blockchain-v4] blockchain: Remove unused utxo cache
 add entry err.

This removes the error return from the UtxoCache.addEntry method since
it does not return any errors.
---
 blockchain/utxocache.go      | 14 ++++----------
 blockchain/utxocache_test.go | 11 ++---------
 2 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/blockchain/utxocache.go b/blockchain/utxocache.go
index d8427ed733..402e7bc1d8 100644
--- a/blockchain/utxocache.go
+++ b/blockchain/utxocache.go
@@ -265,7 +265,7 @@ func (c *UtxoCache) hitRatio() float64 {
 // it to the cache.
 //
 // This function MUST be called with the cache lock held.
-func (c *UtxoCache) addEntry(outpoint wire.OutPoint, entry *UtxoEntry) error {
+func (c *UtxoCache) addEntry(outpoint wire.OutPoint, entry *UtxoEntry) {
 	// Attempt to get an existing entry from the cache.
 	cachedEntry := c.entries[outpoint]
 
@@ -287,8 +287,6 @@ func (c *UtxoCache) addEntry(outpoint wire.OutPoint, entry *UtxoEntry) error {
 		c.totalEntrySize -= cachedEntry.size()
 	}
 	c.totalEntrySize += entry.size()
-
-	return nil
 }
 
 // AddEntry adds the specified output to the cache.  The entry being added MUST
@@ -301,9 +299,9 @@ func (c *UtxoCache) addEntry(outpoint wire.OutPoint, entry *UtxoEntry) error {
 // This function is safe for concurrent access.
 func (c *UtxoCache) AddEntry(outpoint wire.OutPoint, entry *UtxoEntry) error {
 	c.cacheLock.Lock()
-	err := c.addEntry(outpoint, entry)
+	c.addEntry(outpoint, entry)
 	c.cacheLock.Unlock()
-	return err
+	return nil
 }
 
 // spendEntry marks the specified output as spent.
@@ -488,11 +486,7 @@ func (c *UtxoCache) Commit(view *UtxoViewpoint) error {
 
 		// If we passed all of the conditions above, the entry is modified or
 		// fresh, but not spent, and should be added to the cache.
-		err := c.addEntry(outpoint, entry)
-		if err != nil {
-			c.cacheLock.Unlock()
-			return err
-		}
+		c.addEntry(outpoint, entry)
 
 		// All entries that are added to the cache should be removed from the
 		// provided view.  This is an optimization to allow the cache to take
diff --git a/blockchain/utxocache_test.go b/blockchain/utxocache_test.go
index a6fd3a3e6c..841e6766e6 100644
--- a/blockchain/utxocache_test.go
+++ b/blockchain/utxocache_test.go
@@ -193,10 +193,7 @@ func createTestUtxoCache(t *testing.T, entries map[wire.OutPoint]*UtxoEntry) *Ut
 		// Add the entry to the cache.  The entry is cloned before being added
 		// so that any modifications that the cache makes to the entry are not
 		// reflected in the provided test entry.
-		err := utxoCache.AddEntry(outpoint, entry.Clone())
-		if err != nil {
-			t.Fatalf("unexpected error when adding entry: %v", err)
-		}
+		utxoCache.addEntry(outpoint, entry.Clone())
 
 		// Set the state of the cached entries based on the provided entries.
 		// This is allowed for tests to easily simulate entries in the cache
@@ -343,11 +340,7 @@ func TestAddEntry(t *testing.T) {
 		}
 
 		// Add the entry specified by the test.
-		err := utxoCache.AddEntry(test.outpoint, test.entry)
-		if err != nil {
-			t.Fatalf("%q: unexpected error when adding entry: %v", test.name,
-				err)
-		}
+		utxoCache.addEntry(test.outpoint, test.entry)
 		wantTotalEntrySize += test.entry.size()
 
 		// Attempt to get the added entry from the cache.

From 4298a395413df836609a874d81af3f15abb1b5b9 Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Tue, 13 Sep 2022 05:24:11 -0500
Subject: [PATCH 05/17] [blockchain-v4] blockchain: Allow tests to override
 cache flushing.

This modifies the utxo cache tests to ensure the overridden MaybeFlush
method is called for all utxo cache methods as opposed to only when
accessed via the UtxoCacher interface.

Perhaps most notably, the overridden method is now called when the cache
is being initialized which allows its logic to be better tested.

It accomplishes this by adding a new closure to the UtxoCache struct
that defaults to the exported UtxoCache.MaybeAccept, updates all
internal calls to make use of it, and then overrides it when creating
the test harness.
---
 blockchain/utxocache.go      | 20 ++++++++++++++++----
 blockchain/utxocache_test.go |  7 +++++--
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/blockchain/utxocache.go b/blockchain/utxocache.go
index 402e7bc1d8..a45414031f 100644
--- a/blockchain/utxocache.go
+++ b/blockchain/utxocache.go
@@ -186,6 +186,12 @@ type UtxoCache struct {
 	// defaults to time.Now but an alternative function can be provided for
 	// testing purposes.
 	timeNow func() time.Time
+
+	// maybeFlushFn defines the function to use for potentially flushing the
+	// cache to the backend.  It defaults to the exported MaybeFlush method of
+	// this cache instance, but an alternative can be provided for testing
+	// purposes.
+	maybeFlushFn func(*chainhash.Hash, uint32, bool, bool) error
 }
 
 // Ensure UtxoCache implements the UtxoCacher interface.
@@ -222,7 +228,7 @@ func NewUtxoCache(config *UtxoCacheConfig) *UtxoCache {
 		p2pkhScriptLen
 	maxEntries := math.Ceil(float64(config.MaxSize) / float64(avgEntrySize))
 
-	return &UtxoCache{
+	c := &UtxoCache{
 		backend:       config.Backend,
 		flushBlockDB:  config.FlushBlockDB,
 		maxSize:       config.MaxSize,
@@ -230,6 +236,8 @@ func NewUtxoCache(config *UtxoCacheConfig) *UtxoCache {
 		lastFlushTime: time.Now(),
 		timeNow:       time.Now,
 	}
+	c.maybeFlushFn = c.MaybeFlush
+	return c
 }
 
 // totalSize returns the total size of the cache on a 64-bit platform, in bytes.
@@ -442,7 +450,7 @@ func (c *UtxoCache) FetchBackendState() (*UtxoSetState, error) {
 func (c *UtxoCache) FetchStats(bestHash *chainhash.Hash, bestHeight uint32) (*UtxoStats, error) {
 	// Force a UTXO cache flush.  This is required in order for the backend to
 	// fetch statistics on the full UTXO set.
-	err := c.MaybeFlush(bestHash, bestHeight, true, false)
+	err := c.maybeFlushFn(bestHash, bestHeight, true, false)
 	if err != nil {
 		return nil, err
 	}
@@ -836,7 +844,9 @@ func (c *UtxoCache) Initialize(ctx context.Context, b *BlockChain, tip *blockNod
 		// Conditionally flush the utxo cache to the backend.  Don't force flush
 		// since many blocks may be disconnected and connected in quick
 		// succession when initializing.
-		err = c.MaybeFlush(&n.hash, uint32(n.height), false, true)
+		const forceFlush = false
+		const logFlush = true
+		err = c.maybeFlushFn(&n.hash, uint32(n.height), forceFlush, logFlush)
 		if err != nil {
 			return err
 		}
@@ -918,7 +928,9 @@ func (c *UtxoCache) Initialize(ctx context.Context, b *BlockChain, tip *blockNod
 		// Conditionally flush the utxo cache to the backend.  Don't force flush
 		// since many blocks may be connected in quick succession when
 		// initializing.
-		err = c.MaybeFlush(&n.hash, uint32(n.height), false, true)
+		const forceFlush = false
+		const logFlush = true
+		err = c.maybeFlushFn(&n.hash, uint32(n.height), forceFlush, logFlush)
 		if err != nil {
 			return err
 		}
diff --git a/blockchain/utxocache_test.go b/blockchain/utxocache_test.go
index 841e6766e6..34315b6261 100644
--- a/blockchain/utxocache_test.go
+++ b/blockchain/utxocache_test.go
@@ -172,9 +172,12 @@ func (c *testUtxoCache) MaybeFlush(bestHash *chainhash.Hash, bestHeight uint32,
 // newTestUtxoCache returns a testUtxoCache instance using the provided
 // configuration details.
 func newTestUtxoCache(config *UtxoCacheConfig) *testUtxoCache {
-	return &testUtxoCache{
-		UtxoCache: NewUtxoCache(config),
+	utxoCache := NewUtxoCache(config)
+	c := &testUtxoCache{
+		UtxoCache: utxoCache,
 	}
+	utxoCache.maybeFlushFn = c.MaybeFlush
+	return c
 }
 
 // Ensure testUtxoCache implements the UtxoCacher interface.

From cfb451e9eeeae6ee0ddea71e9a96e24c28694f27 Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Tue, 13 Sep 2022 05:24:11 -0500
Subject: [PATCH 06/17] [blockchain-v4] blockchain: Improve utxo cache
 initialize tests.

Currently, the utxo cache initialize tests don't actually test that the
initialize code itself flushes the expected data to the backend rather
they do a flush from the tests themselves using the best chain tip which
may or may not match whatever initialize actually ordinarily
conditionally flushed.

This modifies the utxo cache initialize tests force flushing internally
when the cache is being initialized accordingly to better ensure it is
flushing the expected data.
---
 blockchain/common_test.go    |  6 ++++--
 blockchain/utxocache_test.go | 17 ++++++++++-------
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/blockchain/common_test.go b/blockchain/common_test.go
index 54343de2d9..460f1c19b5 100644
--- a/blockchain/common_test.go
+++ b/blockchain/common_test.go
@@ -926,8 +926,10 @@ func (g *chaingenHarness) ExpectUtxoSetState(blockName string) {
 		lastFlushHash:   block.BlockHash(),
 	}
 	if !reflect.DeepEqual(gotState, wantState) {
-		g.t.Fatalf("mismatched utxo set state:\nwant: %+v\n got: %+v\n", wantState,
-			gotState)
+		g.t.Fatalf("mismatched utxo set state:\nwant: hash %s, height %d\n "+
+			"got: hash %s, height %d\n", wantState.lastFlushHash,
+			wantState.lastFlushHeight, gotState.lastFlushHash,
+			gotState.lastFlushHeight)
 	}
 }
 
diff --git a/blockchain/utxocache_test.go b/blockchain/utxocache_test.go
index 34315b6261..32cb9d4a02 100644
--- a/blockchain/utxocache_test.go
+++ b/blockchain/utxocache_test.go
@@ -152,7 +152,8 @@ func entry85314() *UtxoEntry {
 // simulate various scenarios.
 type testUtxoCache struct {
 	*UtxoCache
-	disableFlush bool
+	disableFlush bool // disable flushing unconditionally (even if forced)
+	forceFlush   bool // force flushing even when not requested by caller
 }
 
 // MaybeFlush conditionally flushes the cache to the backend.  If the disable
@@ -166,6 +167,9 @@ func (c *testUtxoCache) MaybeFlush(bestHash *chainhash.Hash, bestHeight uint32,
 		return nil
 	}
 
+	// Force a flush if either the flag provided by the caller is true or the
+	// test cache overrides it.
+	forceFlush = forceFlush || c.forceFlush
 	return c.UtxoCache.MaybeFlush(bestHash, bestHeight, forceFlush, logFlush)
 }
 
@@ -1072,12 +1076,13 @@ func TestInitialize(t *testing.T) {
 	if err != nil {
 		t.Fatalf("error initializing backend info: %v", err)
 	}
-	resetTestUtxoCache := func() *testUtxoCache {
+	resetTestUtxoCache := func(forceFlush bool) *testUtxoCache {
 		testUtxoCache := newTestUtxoCache(&UtxoCacheConfig{
 			Backend:      backend,
 			FlushBlockDB: g.chain.db.Flush,
 			MaxSize:      100 * 1024 * 1024, // 100 MiB
 		})
+		testUtxoCache.forceFlush = forceFlush
 		g.chain.utxoCache = testUtxoCache
 		err := testUtxoCache.Initialize(context.Background(), g.chain,
 			g.chain.bestChain.Tip())
@@ -1103,7 +1108,7 @@ func TestInitialize(t *testing.T) {
 
 	// Replace the utxo cache in the test chain with a test utxo cache so that
 	// flushing can be toggled on and off for testing.
-	testUtxoCache := resetTestUtxoCache()
+	testUtxoCache := resetTestUtxoCache(false)
 
 	// Validate that the tip and utxo set state are currently at the genesis
 	// block.
@@ -1120,8 +1125,7 @@ func TestInitialize(t *testing.T) {
 	g.ExpectUtxoSetState("genesis")
 
 	// Reset the cache and force a flush.
-	testUtxoCache = resetTestUtxoCache()
-	forceFlush(testUtxoCache)
+	testUtxoCache = resetTestUtxoCache(true)
 
 	// Validate that the utxo cache is now caught up to the tip.
 	g.ExpectUtxoSetState(g.TipName())
@@ -1203,8 +1207,7 @@ func TestInitialize(t *testing.T) {
 	g.ExpectUtxoSetState("b1")
 
 	// Reset the cache and force a flush.
-	testUtxoCache = resetTestUtxoCache()
-	forceFlush(testUtxoCache)
+	testUtxoCache = resetTestUtxoCache(true)
 
 	// Validate that the cache recovered and is now caught up to b1a.
 	g.ExpectUtxoSetState("b1a")

From 3d67a8021ff8c51b5f39f07e6ff072748168e24e Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Tue, 13 Sep 2022 05:24:12 -0500
Subject: [PATCH 07/17] [blockchain-v4] blockchain: Fix rare unclean utxo cache
 recovery.

This corrects an extremely hard to hit corner case involving a mix of
manual block invalidation, conditional flushing, and successive unclean
shutdowns and adds a test to ensure proper behavior.

Specifically, the recovery code from an unclean shutdown that unwinds
the cache potentially periodically flushes the state to the backend was
incorrectly using the block being disconnected as opposed to its parent
which is now the best tip.

In practice, this is exceedingly difficult to hit without intentionally
trying because it requires a mix of highly unlikely events and manual
invalidation:

- Two unclean shutdowns at precise inopportune times
- Manual invalidation of blocks by the operator such that blocks are
  only disconnected without reconnecting a side chain
- The recovery taking long enough or involving enough entries to trigger
  a conditional flush, again, at exactly the right time prior to a
  second unclean shutdown
---
 blockchain/utxocache.go      |  3 +-
 blockchain/utxocache_test.go | 68 ++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/blockchain/utxocache.go b/blockchain/utxocache.go
index a45414031f..e0d42b4cbe 100644
--- a/blockchain/utxocache.go
+++ b/blockchain/utxocache.go
@@ -846,7 +846,8 @@ func (c *UtxoCache) Initialize(ctx context.Context, b *BlockChain, tip *blockNod
 		// succession when initializing.
 		const forceFlush = false
 		const logFlush = true
-		err = c.maybeFlushFn(&n.hash, uint32(n.height), forceFlush, logFlush)
+		err = c.maybeFlushFn(&n.parent.hash, uint32(n.parent.height),
+			forceFlush, logFlush)
 		if err != nil {
 			return err
 		}
diff --git a/blockchain/utxocache_test.go b/blockchain/utxocache_test.go
index 32cb9d4a02..e844644608 100644
--- a/blockchain/utxocache_test.go
+++ b/blockchain/utxocache_test.go
@@ -14,6 +14,7 @@ import (
 	"github.com/decred/dcrd/chaincfg/chainhash"
 	"github.com/decred/dcrd/chaincfg/v3"
 	"github.com/decred/dcrd/database/v3"
+	"github.com/decred/dcrd/dcrutil/v4"
 	"github.com/decred/dcrd/wire"
 )
 
@@ -1138,10 +1139,12 @@ func TestInitialize(t *testing.T) {
 
 	outs := g.OldestCoinbaseOuts()
 	g.NextBlock("b0", &outs[0], outs[1:])
+	g.SaveTipCoinbaseOuts()
 	g.AcceptTipBlock()
 
 	outs = g.OldestCoinbaseOuts()
 	b1 := g.NextBlock("b1", &outs[0], outs[1:])
+	g.SaveTipCoinbaseOuts()
 	g.AcceptTipBlock()
 
 	// Force a cache flush and validate that the cache is caught up to block b1.
@@ -1211,6 +1214,71 @@ func TestInitialize(t *testing.T) {
 
 	// Validate that the cache recovered and is now caught up to b1a.
 	g.ExpectUtxoSetState("b1a")
+
+	// -------------------------------------------------------------------------
+	// Simulate an unclean shutdown such that the utxocache was last flushed at
+	// the tip block which is later invalidated and ensure the cache and backend
+	// recover back to the parent block as expected.
+	//
+	// This is accomplished by creating a new block that becomes the new best
+	// chain tip, disabling flushing, and invalidating that new block to ensure
+	// that the flushed utxo cache state is the invalidated block.  Then the
+	// cache is reset while forcing flushing during initialization.
+	//
+	//   ... -> b0 -> b1
+	//            |      \-> b2bad
+	//            \-> b1a    -----
+	//                       ^ (marked invalid with no flush to cache backend)
+	// -------------------------------------------------------------------------
+
+	outs = g.OldestCoinbaseOuts()
+	g.SetTip("b1")
+	b2bad := g.NextBlock("b2bad", &outs[0], outs[1:])
+	g.AcceptTipBlock()
+	forceFlush(testUtxoCache)
+	g.ExpectUtxoSetState("b2bad")
+
+	// Disable flushing to ensure the block that is about to be invalidated is
+	// not flushed to the backend.
+	//
+	// Ordinarily, the spend journal entry for disconnected blocks is not
+	// removed when there is an error in flushing, however, since this bypasses
+	// the error, the entry needs to be saved and restored after the block is
+	// invalidated.
+	testUtxoCache.disableFlush = true
+	var b2badStxos []spentTxOut
+	err = g.chain.db.View(func(dbTx database.Tx) error {
+		prevHash := b2bad.Header.PrevBlock
+		isTrsyEnabled, err := g.chain.IsTreasuryAgendaActive(&prevHash)
+		if err != nil {
+			return err
+		}
+		block := dcrutil.NewBlock(b2bad)
+		b2badStxos, err = dbFetchSpendJournalEntry(dbTx, block, isTrsyEnabled)
+		return err
+	})
+	if err != nil {
+		t.Fatalf("unexpected error getting spend journal entry: %v", err)
+	}
+
+	// Invalidate the previously connected block so that it is disconnected and
+	// ensure the flushed utxo cache state is still at the invalidated block.
+	g.InvalidateBlockAndExpectTip("b2bad", nil, "b1")
+	g.ExpectUtxoSetState("b2bad")
+
+	// Restore the spend journal entry for the invalidated block per the above.
+	err = g.chain.db.Update(func(dbTx database.Tx) error {
+		b2badHash := b2bad.BlockHash()
+		return dbPutSpendJournalEntry(dbTx, &b2badHash, b2badStxos)
+	})
+	if err != nil {
+		t.Fatalf("unexpected error restoring spend journal entry: %v", err)
+	}
+
+	// Reset the cache while forcing flushing during the initialization and
+	// ensure the cache and backend recover back to b1 as expected.
+	resetTestUtxoCache(true)
+	g.ExpectUtxoSetState("b1")
 }
 
 // TestShutdownUtxoCache validates that a cache flush is forced when shutting

From 0140aa1f39082a3bd43664efb5395bc553903c44 Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Tue, 13 Sep 2022 05:24:13 -0500
Subject: [PATCH 08/17] [blockchain-v4] blockchain: Don't fetch
 trsy{base,spend} inputs.

This modifies the code that deal with fetching input utxos to avoid
needlessly looking up inputs for treasurybase and treasury spend
transactions since they are both similar to coinbase transactions in
that they do not reference any real previous outputs and therefore are
guaranteed to result in a cache miss.
---
 blockchain/utxoviewpoint.go | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/blockchain/utxoviewpoint.go b/blockchain/utxoviewpoint.go
index edadd7144c..458aa08ff8 100644
--- a/blockchain/utxoviewpoint.go
+++ b/blockchain/utxoviewpoint.go
@@ -760,9 +760,20 @@ func (view *UtxoViewpoint) fetchInputUtxos(block *dcrutil.Block,
 	// stake tree are not allowed to access outputs of transactions earlier in
 	// the block.  This applies to both transactions earlier in the stake tree
 	// as well as those in the regular tree.
-	for _, stx := range block.STransactions() {
-		isVote := stake.IsSSGen(stx.MsgTx(), isTreasuryEnabled)
-		for txInIdx, txIn := range stx.MsgTx().TxIn {
+	for txIdx, stx := range block.STransactions() {
+		// Ignore treasurybases and treasury spends since they have no inputs.
+		msgTx := stx.MsgTx()
+		shouldBeTreasuryBase := isTreasuryEnabled && txIdx == 0
+		if shouldBeTreasuryBase && stake.IsTreasuryBase(msgTx) {
+			continue
+		}
+		isTreasurySpend := isTreasuryEnabled && stake.IsTSpend(msgTx)
+		if isTreasurySpend {
+			continue
+		}
+
+		isVote := stake.IsSSGen(msgTx, isTreasuryEnabled)
+		for txInIdx, txIn := range msgTx.TxIn {
 			// Ignore stakebase since it has no input.
 			if isVote && txInIdx == 0 {
 				continue
@@ -885,8 +896,13 @@ func (b *BlockChain) FetchUtxoView(tx *dcrutil.Tx, includeRegularTxns bool) (*Ut
 		outpoint.Index = uint32(txOutIdx)
 		filteredSet.add(view, &outpoint)
 	}
-	if !standalone.IsCoinBaseTx(msgTx, isTreasuryEnabled) {
-		isVote := stake.IsSSGen(msgTx, isTreasuryEnabled)
+	// Ignore coinbases, treasurybases, and treasury spends since none of them
+	// have any inputs.
+	isCoinBase := standalone.IsCoinBaseTx(msgTx, isTreasuryEnabled)
+	isTreasuryBase := txType == stake.TxTypeTreasuryBase
+	isTreasurySpend := txType == stake.TxTypeTSpend
+	if !isCoinBase && !isTreasuryBase && !isTreasurySpend {
+		isVote := txType == stake.TxTypeSSGen
 		for txInIdx, txIn := range msgTx.TxIn {
 			// Ignore stakebase since it has no input.
 			if isVote && txInIdx == 0 {

From e8fcff4b43d4211aef7e0cfa1833dc35b302047c Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Tue, 13 Sep 2022 05:24:14 -0500
Subject: [PATCH 09/17] [blockchain-v4] blockchain: Don't add treasurybase
 utxos.

This updates the code when connecting transactions in a utxo view to
check for treasurybases prior to the coinbase since a treasurybase is
also identified as a coinbase, but unlike coinbases, treasurybase
outputs are never spendable and thus should not be added to the utxo
view.
---
 blockchain/utxoviewpoint.go | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/blockchain/utxoviewpoint.go b/blockchain/utxoviewpoint.go
index 458aa08ff8..c1ad46c791 100644
--- a/blockchain/utxoviewpoint.go
+++ b/blockchain/utxoviewpoint.go
@@ -224,8 +224,17 @@ func (view *UtxoViewpoint) connectTransaction(tx *dcrutil.Tx, blockHeight int64,
 	blockIndex uint32, stxos *[]spentTxOut, isTreasuryEnabled,
 	isAutoRevocationsEnabled bool) error {
 
-	// Coinbase transactions don't have any inputs to spend.
+	// Treasurybase transactions don't have any inputs to spend.
+	//
+	// NOTE: This check MUST come before the coinbase check because a
+	// treasurybase is identified as a coinbase as of the time this comment is
+	// being written.
 	msgTx := tx.MsgTx()
+	if isTreasuryEnabled && standalone.IsTreasuryBase(msgTx) {
+		return nil
+	}
+
+	// Coinbase transactions don't have any inputs to spend.
 	if standalone.IsCoinBaseTx(msgTx, isTreasuryEnabled) {
 		// Add the transaction's outputs as available utxos.
 		view.AddTxOuts(tx, blockHeight, blockIndex, isTreasuryEnabled,
@@ -233,11 +242,6 @@ func (view *UtxoViewpoint) connectTransaction(tx *dcrutil.Tx, blockHeight int64,
 		return nil
 	}
 
-	// Treasurybase transactions don't have any inputs to spend.
-	if isTreasuryEnabled && standalone.IsTreasuryBase(msgTx) {
-		return nil
-	}
-
 	// Spend the referenced utxos by marking them spent in the view and, if a
 	// slice was provided for the spent txout details, append an entry to it.
 	isVote := stake.IsSSGen(msgTx, isTreasuryEnabled)

From 25b7ec521e930f9319409d0e4e2c88a65d8bbe0f Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Tue, 13 Sep 2022 05:24:15 -0500
Subject: [PATCH 10/17] [blockchain-v4] blockchain: Consolidate utxo cache test
 entries.

This simplifies the various utxo cache tests and improves their
legibility by consolidating the utxo test entry state creation logic and
updating the tests to make use of it accordingly.

In particular, it introduces a struct to house entries with the various
states along with a method to create them.
---
 blockchain/utxocache_test.go | 208 +++++++++++++++++------------------
 1 file changed, 103 insertions(+), 105 deletions(-)

diff --git a/blockchain/utxocache_test.go b/blockchain/utxocache_test.go
index e844644608..342e674bf8 100644
--- a/blockchain/utxocache_test.go
+++ b/blockchain/utxocache_test.go
@@ -148,6 +148,51 @@ 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
+}
+
+// makeEntryStates creates combinations of the passed test utxo entry in various
+// states for convenient use in the tests below.
+func makeEntryStates(entry *UtxoEntry) testEntryStates {
+	unmodified := entry.Clone()
+	unmodifiedSpent := unmodified.Clone()
+	unmodifiedSpent.state |= utxoStateSpent
+	unmodifiedFresh := unmodified.Clone()
+	unmodifiedFresh.state |= utxoStateFresh
+	unmodifiedSpentFresh := unmodifiedSpent.Clone()
+	unmodifiedSpentFresh.state |= utxoStateFresh
+
+	modified := entry.Clone()
+	modified.state |= utxoStateModified
+	modifiedSpent := modified.Clone()
+	modifiedSpent.state |= utxoStateSpent
+	modifiedFresh := modified.Clone()
+	modifiedFresh.state |= utxoStateFresh
+	modifiedSpentFresh := modifiedSpent.Clone()
+	modifiedSpentFresh.state |= utxoStateFresh
+
+	return testEntryStates{
+		unmodified:           unmodified,
+		unmodifiedSpent:      unmodifiedSpent,
+		unmodifiedFresh:      unmodifiedFresh,
+		unmodifiedSpentFresh: unmodifiedSpentFresh,
+		modified:             modified,
+		modifiedSpent:        modifiedSpent,
+		modifiedFresh:        modifiedFresh,
+		modifiedSpentFresh:   modifiedSpentFresh,
+	}
+}
+
 // testUtxoCache provides a mock utxo cache by implementing the UtxoCacher
 // interface.  It allows for toggling flushing on and off to more easily
 // simulate various scenarios.
@@ -305,13 +350,8 @@ func TestAddEntry(t *testing.T) {
 	t.Parallel()
 
 	// Create test entries to be used throughout the tests.
-	outpoint := outpoint299()
-	entry := entry299()
-	entryModified := entry.Clone()
-	entryModified.amount++
-	entryModified.state |= utxoStateModified
-	entryFresh := entry.Clone()
-	entryFresh.state |= utxoStateModified | utxoStateFresh
+	outpoint, entry := outpoint299(), makeEntryStates(entry299())
+	entry.modified.amount++
 
 	tests := []struct {
 		name            string
@@ -322,16 +362,16 @@ func TestAddEntry(t *testing.T) {
 	}{{
 		name:      "add an entry that does not already exist in the cache",
 		outpoint:  outpoint,
-		entry:     entry,
-		wantEntry: entryFresh,
+		entry:     entry.unmodified,
+		wantEntry: entry.modifiedFresh,
 	}, {
 		name: "add an entry that overwrites an existing entry",
 		existingEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint: entry,
+			outpoint: entry.unmodified,
 		},
 		outpoint:  outpoint,
-		entry:     entryModified,
-		wantEntry: entryModified,
+		entry:     entry.modified,
+		wantEntry: entry.modified,
 	}}
 
 	for _, test := range tests {
@@ -386,12 +426,7 @@ func TestSpendEntry(t *testing.T) {
 	t.Parallel()
 
 	// Create test entries to be used throughout the tests.
-	outpoint := outpoint299()
-	entry := entry299()
-	entryFresh := entry.Clone()
-	entryFresh.state |= utxoStateModified | utxoStateFresh
-	entrySpent := entry.Clone()
-	entrySpent.Spend()
+	outpoint, entry := outpoint299(), makeEntryStates(entry299())
 
 	tests := []struct {
 		name            string
@@ -401,28 +436,28 @@ func TestSpendEntry(t *testing.T) {
 	}{{
 		name:     "spend an entry that does not exist in the cache",
 		outpoint: outpoint,
-		entry:    entry,
+		entry:    entry.unmodified,
 	}, {
 		name: "spend an entry that exists in the cache but is already spent",
 		existingEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint: entrySpent,
+			outpoint: entry.unmodifiedSpent,
 		},
 		outpoint: outpoint,
-		entry:    entrySpent,
+		entry:    entry.unmodifiedSpent,
 	}, {
 		name: "spend an entry that exists in the cache and is fresh",
 		existingEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint: entryFresh,
+			outpoint: entry.modifiedFresh,
 		},
 		outpoint: outpoint,
-		entry:    entryFresh,
+		entry:    entry.modifiedFresh,
 	}, {
 		name: "spend an entry that exists in the cache and is not fresh",
 		existingEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint: entry,
+			outpoint: entry.unmodified,
 		},
 		outpoint: outpoint,
-		entry:    entry,
+		entry:    entry.unmodified,
 	}}
 
 	for _, test := range tests {
@@ -483,10 +518,7 @@ func TestFetchEntry(t *testing.T) {
 	backend := createTestUtxoBackend(t)
 
 	// Create test entries to be used throughout the tests.
-	outpoint := outpoint299()
-	entry := entry299()
-	entryModified := entry.Clone()
-	entryModified.state |= utxoStateModified
+	outpoint, entry := outpoint299(), makeEntryStates(entry299())
 
 	tests := []struct {
 		name           string
@@ -501,18 +533,18 @@ func TestFetchEntry(t *testing.T) {
 	}, {
 		name: "entry is in the cache",
 		cachedEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint: entry,
+			outpoint: entry.unmodified,
 		},
 		outpoint:  outpoint,
 		cacheHit:  true,
-		wantEntry: entry,
+		wantEntry: entry.unmodified,
 	}, {
 		name: "entry is not in the cache but is in the backend",
 		backendEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint: entryModified,
+			outpoint: entry.modified,
 		},
 		outpoint:  outpoint,
-		wantEntry: entry,
+		wantEntry: entry.unmodified,
 	}}
 
 	for _, test := range tests {
@@ -529,15 +561,15 @@ func TestFetchEntry(t *testing.T) {
 		}
 
 		// Attempt to fetch the entry for the outpoint specified by the test.
-		entry, err = utxoCache.FetchEntry(test.outpoint)
+		gotEntry, err := utxoCache.FetchEntry(test.outpoint)
 		if err != nil {
 			t.Fatalf("%q: unexpected error fetching entry: %v", test.name, err)
 		}
 
 		// Ensure that the fetched entry matches the expected entry.
-		if !reflect.DeepEqual(entry, test.wantEntry) {
+		if !reflect.DeepEqual(gotEntry, test.wantEntry) {
 			t.Fatalf("%q: mismatched entry:\nwant: %+v\n got: %+v\n", test.name,
-				test.wantEntry, entry)
+				test.wantEntry, gotEntry)
 		}
 
 		// Ensure that the entry is now cached.
@@ -578,12 +610,8 @@ func TestFetchEntries(t *testing.T) {
 
 	// Create test entries to be used throughout the tests.
 	outpoint299 := outpoint299()
-	outpoint1100 := outpoint1100()
-	entry1100 := entry1100()
-	outpoint1200 := outpoint1200()
-	entry1200 := entry1200()
-	entry1200Modified := entry1200.Clone()
-	entry1200Modified.state |= utxoStateModified
+	outpoint1100, entry1100 := outpoint1100(), makeEntryStates(entry1100())
+	outpoint1200, entry1200 := outpoint1200(), makeEntryStates(entry1200())
 
 	tests := []struct {
 		name           string
@@ -594,10 +622,10 @@ func TestFetchEntries(t *testing.T) {
 	}{{
 		name: "entries are fetched from the cache and the backend",
 		cachedEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint1100: entry1100,
+			outpoint1100: entry1100.unmodified,
 		},
 		backendEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint1200: entry1200Modified,
+			outpoint1200: entry1200.modified,
 		},
 		filteredSet: ViewFilteredSet{
 			outpoint299:  struct{}{},
@@ -606,8 +634,8 @@ func TestFetchEntries(t *testing.T) {
 		},
 		wantEntries: map[wire.OutPoint]*UtxoEntry{
 			outpoint299:  nil,
-			outpoint1100: entry1100,
-			outpoint1200: entry1200,
+			outpoint1100: entry1100.unmodified,
+			outpoint1200: entry1200.unmodified,
 		},
 	}}
 
@@ -646,15 +674,9 @@ func TestCommit(t *testing.T) {
 
 	// Create test entries to be used throughout the tests.
 	outpoint299 := outpoint299()
-	outpoint1100 := outpoint1100()
-	entry1100Unmodified := entry1100()
-	outpoint1200 := outpoint1200()
-	entry1200 := entry1200()
-	entry1200Spent := entry1200.Clone()
-	entry1200Spent.Spend()
-	outpoint85314 := outpoint85314()
-	entry85314Modified := entry85314()
-	entry85314Modified.state |= utxoStateModified
+	outpoint1100, entry1100 := outpoint1100(), makeEntryStates(entry1100())
+	outpoint1200, entry1200 := outpoint1200(), makeEntryStates(entry1200())
+	outpoint85314, entry85314 := outpoint85314(), makeEntryStates(entry85314())
 
 	tests := []struct {
 		name              string
@@ -666,12 +688,12 @@ func TestCommit(t *testing.T) {
 		name: "view contains nil, unmodified, spent, and modified entries",
 		viewEntries: map[wire.OutPoint]*UtxoEntry{
 			outpoint299:   nil,
-			outpoint1100:  entry1100Unmodified,
-			outpoint1200:  entry1200Spent,
-			outpoint85314: entry85314Modified,
+			outpoint1100:  entry1100.unmodified,
+			outpoint1200:  entry1200.modifiedSpent,
+			outpoint85314: entry85314.modified,
 		},
 		cachedEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint1200: entry1200,
+			outpoint1200: entry1200.unmodified,
 		},
 		// outpoint299 is removed from the view since the entry is nil.
 		// entry1100Unmodified remains in the view since it is unmodified.
@@ -679,13 +701,13 @@ func TestCommit(t *testing.T) {
 		// entry85314Modified is removed from the view since it is modified and
 		// added to the cache.
 		wantViewEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint1100: entry1100Unmodified,
+			outpoint1100: entry1100.unmodified,
 		},
 		// entry1200Spent remains in the cache but is now spent.
 		// entry85314Modified is added to the cache.
 		wantCachedEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint1200:  entry1200Spent,
-			outpoint85314: entry85314Modified,
+			outpoint1200:  entry1200.modifiedSpent,
+			outpoint85314: entry85314.modified,
 		},
 	}}
 
@@ -855,34 +877,10 @@ func TestMaybeFlush(t *testing.T) {
 	block2000Hash := mustParseHash("0000000000000c8a886e3f7c32b1bb08422066dcf" +
 		"d008de596471f11a5aff475")
 
-	// entry299Fresh is from block height 299 and is modified and fresh.
-	outpoint299 := outpoint299()
-	entry299Fresh := entry299()
-	entry299Fresh.state |= utxoStateModified | utxoStateFresh
-
-	// entry299Unmodified is from block height 299 and is unmodified.
-	entry299Unmodified := entry299()
-
-	// entry1100Spent is from block height 1100 and is modified and spent.
-	outpoint1100 := outpoint1100()
-	entry1100Spent := entry1100()
-	entry1100Spent.Spend()
-
-	// entry1100Modified is from block height 1100 and is modified and unspent.
-	entry1100Modified := entry1100()
-	entry1100Modified.state |= utxoStateModified
-
-	// entry1100Unmodified is from block height 1100 and is unspent and
-	// unmodified.
-	entry1100Unmodified := entry1100()
-
-	// entry1200Fresh is from block height 1200 and is modified and fresh.
-	outpoint1200 := outpoint1200()
-	entry1200Fresh := entry1200()
-	entry1200Fresh.state |= utxoStateModified | utxoStateFresh
-
-	// entry1200Unmodified is from block height 1200 and is unmodified.
-	entry1200Unmodified := entry1200()
+	// Create test entries to be used throughout the tests.
+	outpoint299, entry299 := outpoint299(), makeEntryStates(entry299())
+	outpoint1100, entry1100 := outpoint1100(), makeEntryStates(entry1100())
+	outpoint1200, entry1200 := outpoint1200(), makeEntryStates(entry1200())
 
 	tests := []struct {
 		name                     string
@@ -907,22 +905,22 @@ func TestMaybeFlush(t *testing.T) {
 		bestHash:           block2000Hash,
 		bestHeight:         2000,
 		cachedEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint299:  entry299Fresh,
-			outpoint1100: entry1100Spent,
-			outpoint1200: entry1200Fresh,
+			outpoint299:  entry299.modifiedFresh,
+			outpoint1100: entry1100.modifiedSpent,
+			outpoint1200: entry1200.modifiedFresh,
 		},
 		backendEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint1100: entry1100Modified,
+			outpoint1100: entry1100.modified,
 		},
 		// The cache should remain unchanged since a flush is not required.
 		wantCachedEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint299:  entry299Fresh,
-			outpoint1100: entry1100Spent,
-			outpoint1200: entry1200Fresh,
+			outpoint299:  entry299.modifiedFresh,
+			outpoint1100: entry1100.modifiedSpent,
+			outpoint1200: entry1200.modifiedFresh,
 		},
 		// The backend should remain unchanged since a flush is not required.
 		wantBackendEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint1100: entry1100Unmodified,
+			outpoint1100: entry1100.unmodified,
 		},
 		wantLastEvictionHeight:   0,
 		wantLastFlushHash:        block1000Hash,
@@ -935,27 +933,27 @@ func TestMaybeFlush(t *testing.T) {
 		bestHash:           block2000Hash,
 		bestHeight:         2000,
 		cachedEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint299:  entry299Fresh,
-			outpoint1100: entry1100Spent,
-			outpoint1200: entry1200Fresh,
+			outpoint299:  entry299.modifiedFresh,
+			outpoint1100: entry1100.modifiedSpent,
+			outpoint1200: entry1200.modifiedFresh,
 		},
 		backendEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint1100: entry1100Modified,
+			outpoint1100: entry1100.modified,
 		},
 		// entry299Fresh should be evicted from the cache due to its height.
 		// entry1100Spent should be evicted since it is spent.
 		// entry1200Fresh should remain in the cache but should now be
 		// unmodified.
 		wantCachedEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint1200: entry1200Unmodified,
+			outpoint1200: entry1200.unmodified,
 		},
 		// entry299Unmodified should be added to the backend during the flush.
 		// entry1100Unmodified should be removed from the backend since it now
 		// spent.
 		// entry1200Unmodified should be added to the backend during the flush.
 		wantBackendEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint299:  entry299Unmodified,
-			outpoint1200: entry1200Unmodified,
+			outpoint299:  entry299.unmodified,
+			outpoint1200: entry1200.unmodified,
 		},
 		wantLastEvictionHeight:   300,
 		wantLastFlushHash:        block2000Hash,

From 1b5a2a699038f7a9c6c1f90b6cf9c8c474421499 Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Tue, 13 Sep 2022 05:24:15 -0500
Subject: [PATCH 11/17] [blockchain-v4] blockchain: Rework utxo cache spend
 entry tests.

This reworks the utxo cache spend entry tests to simplify them and make
them more flexible by moving the primary logic for all of the expected
results into the test definitions themselves as oppossed to special
casing depending on flags in the test body and also adds an additional
test case.

It also updates the test cache creation method to allow nil entries in
the initial cache.
---
 blockchain/utxocache_test.go | 107 +++++++++++++++--------------------
 1 file changed, 46 insertions(+), 61 deletions(-)

diff --git a/blockchain/utxocache_test.go b/blockchain/utxocache_test.go
index 342e674bf8..695c1dada5 100644
--- a/blockchain/utxocache_test.go
+++ b/blockchain/utxocache_test.go
@@ -246,7 +246,11 @@ func createTestUtxoCache(t *testing.T, entries map[wire.OutPoint]*UtxoEntry) *Ut
 		// Add the entry to the cache.  The entry is cloned before being added
 		// so that any modifications that the cache makes to the entry are not
 		// reflected in the provided test entry.
-		utxoCache.addEntry(outpoint, entry.Clone())
+		if entry != nil {
+			utxoCache.addEntry(outpoint, entry.Clone())
+		} else {
+			utxoCache.entries[outpoint] = nil
+		}
 
 		// Set the state of the cached entries based on the provided entries.
 		// This is allowed for tests to easily simulate entries in the cache
@@ -428,83 +432,64 @@ func TestSpendEntry(t *testing.T) {
 	// Create test entries to be used throughout the tests.
 	outpoint, entry := outpoint299(), makeEntryStates(entry299())
 
+	type entriesMap map[wire.OutPoint]*UtxoEntry
 	tests := []struct {
-		name            string
-		existingEntries map[wire.OutPoint]*UtxoEntry
-		outpoint        wire.OutPoint
-		entry           *UtxoEntry
+		name           string        // test description
+		cachedEntries  entriesMap    // existing cache entries
+		backendEntries entriesMap    // existing backend entries
+		outpoint       wire.OutPoint // outpoint for the entry to spend
+		wantEntry      *UtxoEntry    // expected entry in cache after spend
+		wantCacheSize  uint64        // expected size of the cache after spend
 	}{{
-		name:     "spend an entry that does not exist in the cache",
-		outpoint: outpoint,
-		entry:    entry.unmodified,
+		name:          "spending nil (aka pruned) cache entry is a noop",
+		cachedEntries: entriesMap{outpoint: nil},
+		outpoint:      outpoint,
+		wantEntry:     nil,
+		wantCacheSize: 0,
 	}, {
-		name: "spend an entry that exists in the cache but is already spent",
-		existingEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint: entry.unmodifiedSpent,
-		},
-		outpoint: outpoint,
-		entry:    entry.unmodifiedSpent,
+		name:          "spending entry not in cache is a noop",
+		outpoint:      outpoint,
+		wantEntry:     nil,
+		wantCacheSize: 0,
 	}, {
-		name: "spend an entry that exists in the cache and is fresh",
-		existingEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint: entry.modifiedFresh,
-		},
-		outpoint: outpoint,
-		entry:    entry.modifiedFresh,
+		name:          "spending cache entry already marked spent is a noop",
+		cachedEntries: entriesMap{outpoint: entry.modifiedSpent},
+		outpoint:      outpoint,
+		wantEntry:     entry.modifiedSpent,
+		wantCacheSize: entry.modifiedSpent.size(),
 	}, {
-		name: "spend an entry that exists in the cache and is not fresh",
-		existingEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint: entry.unmodified,
-		},
-		outpoint: outpoint,
-		entry:    entry.unmodified,
+		name:          "spending cache entry marked fresh makes it nil",
+		cachedEntries: entriesMap{outpoint: entry.modifiedFresh},
+		outpoint:      outpoint,
+		wantEntry:     nil,
+		wantCacheSize: 0,
+	}, {
+		name:          "spending cache entry not marked fresh makes it spent",
+		cachedEntries: entriesMap{outpoint: entry.modified},
+		outpoint:      outpoint,
+		wantEntry:     entry.modifiedSpent,
+		wantCacheSize: entry.modifiedSpent.size(),
 	}}
 
 	for _, test := range tests {
 		// Create a utxo cache with the existing entries specified by the test.
-		utxoCache := createTestUtxoCache(t, test.existingEntries)
-		wantTotalEntrySize := utxoCache.totalEntrySize
-
-		// Attempt to get an existing entry from the cache.
-		entry := utxoCache.entries[test.outpoint]
-		var entryAlreadySpent bool
-		if entry != nil {
-			entryAlreadySpent = entry.IsSpent()
-		}
+		utxoCache := createTestUtxoCache(t, test.cachedEntries)
 
 		// Spend the entry specified by the test.
 		utxoCache.SpendEntry(test.outpoint)
 
-		// If the existing entry was nil or spent, continue as there is nothing
-		// else to validate.
-		if entry == nil || entryAlreadySpent {
-			continue
-		}
-
-		// If the entry is fresh, validate that it was removed from the cache
-		// when spent.
-		if entry.isFresh() {
-			wantTotalEntrySize -= test.entry.size()
-			if utxoCache.entries[test.outpoint] != nil {
-				t.Fatalf("%q: entry for outpoint %v was not removed from the "+
-					"cache", test.name, test.outpoint)
-			}
+		// Get the entry associated with the output from the cache and ensure it
+		// matches the expected one.
+		gotEntry := utxoCache.entries[test.outpoint]
+		if !reflect.DeepEqual(gotEntry, test.wantEntry) {
+			t.Fatalf("%q: unexpected entry after spend -- got: %+v, want %+v",
+				test.name, gotEntry, test.wantEntry)
 		}
 
 		// Validate that the total entry size was updated as expected.
-		if utxoCache.totalEntrySize != wantTotalEntrySize {
+		if utxoCache.totalEntrySize != test.wantCacheSize {
 			t.Fatalf("%q: unexpected total entry size -- got %v, want %v",
-				test.name, utxoCache.totalEntrySize, wantTotalEntrySize)
-		}
-
-		// If entry is not fresh, validate that it still exists in the cache and
-		// is now marked as spent.
-		if !entry.isFresh() {
-			cachedEntry := utxoCache.entries[test.outpoint]
-			if cachedEntry == nil || !cachedEntry.IsSpent() {
-				t.Fatalf("%q: expected entry for outpoint %v to exist in the "+
-					"cache and be marked spent", test.name, test.outpoint)
-			}
+				test.name, utxoCache.totalEntrySize, test.wantCacheSize)
 		}
 	}
 }

From b4b207901c71ea5d20e0c6921fcdef52efde7605 Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Tue, 13 Sep 2022 05:24:16 -0500
Subject: [PATCH 12/17] [blockchain-v4] blockchain: Rework utxo cache commit
 tests.

This reworks the utxo cache commit tests to simplify them a bit, make
them more flexible, and test a lot more conditions.  It accomplishes
this by introducing an expected error so the tests themselves can
specify the expected result as opposed to assuming it should be nil.  It
also clones all of the view entries to ensure none of the shared entries
are modified when the cache takes ownership of them during the tests.

Finally, it improves the robustness of Commit by correcting the logic
regarding unmodified spent and unspent fresh view entries discovered
while adding the tests since it was technically incorrect even though
the relevant cases are never hit in practice in the current
implementation.  This helps defend against code changes that could
easily inadvertently violate the non-obvious, and heretofore
undocumented, assumptions that avoided hitting the incorrect behavior.
---
 blockchain/utxocache.go      |   8 +-
 blockchain/utxocache_test.go | 216 +++++++++++++++++++++++++++++++----
 2 files changed, 195 insertions(+), 29 deletions(-)

diff --git a/blockchain/utxocache.go b/blockchain/utxocache.go
index e0d42b4cbe..6b6f0201fb 100644
--- a/blockchain/utxocache.go
+++ b/blockchain/utxocache.go
@@ -475,15 +475,15 @@ func (c *UtxoCache) Commit(view *UtxoViewpoint) error {
 			continue
 		}
 
-		// If the entry is not modified and not fresh, continue as there is
-		// nothing to do.
-		if !entry.isModified() && !entry.isFresh() {
+		// There is nothing to update in the cache nor the view when the view
+		// entry was not modified.
+		if !entry.isModified() {
 			continue
 		}
 
 		// If the entry is modified and spent, mark it as spent in the cache and
 		// then delete it from the view.
-		if entry.isModified() && entry.IsSpent() {
+		if entry.IsSpent() {
 			// Mark the entry as spent in the cache.
 			c.spendEntry(outpoint)
 
diff --git a/blockchain/utxocache_test.go b/blockchain/utxocache_test.go
index 695c1dada5..cb553db021 100644
--- a/blockchain/utxocache_test.go
+++ b/blockchain/utxocache_test.go
@@ -6,6 +6,7 @@ package blockchain
 
 import (
 	"context"
+	"errors"
 	"reflect"
 	"testing"
 	"time"
@@ -658,40 +659,196 @@ func TestCommit(t *testing.T) {
 	t.Parallel()
 
 	// Create test entries to be used throughout the tests.
-	outpoint299 := outpoint299()
+	outpoint299, entry299 := outpoint299(), makeEntryStates(entry299())
 	outpoint1100, entry1100 := outpoint1100(), makeEntryStates(entry1100())
 	outpoint1200, entry1200 := outpoint1200(), makeEntryStates(entry1200())
 	outpoint85314, entry85314 := outpoint85314(), makeEntryStates(entry85314())
 
 	tests := []struct {
-		name              string
-		viewEntries       map[wire.OutPoint]*UtxoEntry
-		cachedEntries     map[wire.OutPoint]*UtxoEntry
-		wantViewEntries   map[wire.OutPoint]*UtxoEntry
-		wantCachedEntries map[wire.OutPoint]*UtxoEntry
+		name              string                       // test description
+		viewEntries       map[wire.OutPoint]*UtxoEntry // view to commit
+		cachedEntries     map[wire.OutPoint]*UtxoEntry // existing cache entries
+		wantViewEntries   map[wire.OutPoint]*UtxoEntry // expected committed view
+		wantCachedEntries map[wire.OutPoint]*UtxoEntry // expected committed cache
+		err               error                        // expected error
 	}{{
-		name: "view contains nil, unmodified, spent, and modified entries",
+		name: "modified spent view entry w/ existing spent cache entry is noop",
+		viewEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint1200: entry1200.modifiedSpent,
+		},
+		cachedEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint1200: entry1200.modifiedSpent,
+		},
+		wantViewEntries: map[wire.OutPoint]*UtxoEntry{},
+		wantCachedEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint1200: entry1200.modifiedSpent,
+		},
+	}, {
+		name: "nil view entries are removed from view and do not affect cache",
 		viewEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint299:   nil,
+			outpoint299: nil,
+		},
+		cachedEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint299: entry299.unmodified,
+		},
+		wantViewEntries: map[wire.OutPoint]*UtxoEntry{},
+		wantCachedEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint299: entry299.unmodified,
+		},
+	}, {
+		// This covers the following scenarios:
+		//
+		// - unmodified spent view entries with no corresponding cache entry
+		//   are retained in the view and have no effect on the cache
+		// - unmodified spent view entries with a corresponding nil cache entry
+		//   are retained in the view and have no effect on the cache
+		// - unmodified spent view entries with a corresponding non-fresh
+		//   unmodified cache entry are removed from view and have no effect on
+		//   the cache
+		// - unmodified spent fresh view entries with a corresponding modified
+		//   unspent fresh cache entry are retained in the view and have no
+		//   effect on the cache
+		//
+		// NOTE: The case where there are unmodified spent view entries with
+		// corresponding unspent entries in the cache should really never happen
+		// in practice because any spending done in the view necessarily marks
+		// the entry spent, so the only realistic way for an unmodified spent
+		// view entry to exist would be for it to have come from the cache which
+		// means it would be spent in the cache too, but the cache will not have
+		// unmodified spent entries since those are pruned.
+		//
+		// Similarly, unmodified unspent fresh view entries never happen with
+		// the current implementation because the only way for a fresh view
+		// entry to exist is for it to have come from the cache, all fresh cache
+		// entries must necessarily also be marked modified, and cache entries
+		// are currently cloned directly into the view as is.
+		//
+		// However, they are tested here for completeness in order to ensure
+		// unmodified entries never affect the cache.
+		name: "several unmodified spent view entry combinations",
+		viewEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint299:   entry299.unmodifiedSpent,
+			outpoint1100:  entry1100.unmodifiedSpent,
+			outpoint1200:  entry1200.unmodifiedSpent,
+			outpoint85314: entry85314.unmodifiedSpentFresh,
+		},
+		cachedEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint1100:  nil,
+			outpoint1200:  entry1200.unmodified,
+			outpoint85314: entry85314.modifiedFresh,
+		},
+		wantViewEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint299:   entry299.unmodifiedSpent,
+			outpoint1100:  entry1100.unmodifiedSpent,
+			outpoint1200:  entry1200.unmodifiedSpent,
+			outpoint85314: entry85314.unmodifiedSpentFresh,
+		},
+		wantCachedEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint1100:  nil,
+			outpoint1200:  entry1200.unmodified,
+			outpoint85314: entry85314.modifiedFresh,
+		},
+	}, {
+		// This covers the following scenarios:
+		//
+		// - unmodified unspent view entries with no corresponding cache entry
+		//   are retained in the view and have no effect on the cache
+		// - unmodified unspent view entries with a corresponding nil cache
+		//   entry are retained in the view and have no effect on the cache
+		// - unmodified unspent view entries with a corresponding unmodified
+		//   cache entry are retained in the view and have no effect on the
+		//   cache
+		// - unmodified unspent fresh view entries with a corresponding modified
+		//   unspent fresh cache entry are retained in the view and have no
+		//   effect on the cache
+		//
+		// NOTE: Unmodified unspent fresh view entries never happen with the
+		// current implementation because the only way for a fresh view entry to
+		// exist is for it to have come from the cache, all fresh cache entries
+		// must necessarily also be marked modified, and cache entries are
+		// currently cloned directly into the view as is.  However, it is tested
+		// here for completeness.
+		name: "several unmodified unspent view entry combinations",
+		viewEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint299:   entry299.unmodified,
 			outpoint1100:  entry1100.unmodified,
-			outpoint1200:  entry1200.modifiedSpent,
-			outpoint85314: entry85314.modified,
+			outpoint1200:  entry1200.unmodified,
+			outpoint85314: entry85314.unmodifiedFresh,
 		},
 		cachedEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint1200: entry1200.unmodified,
+			outpoint1100:  nil,
+			outpoint1200:  entry1200.unmodified,
+			outpoint85314: entry85314.modifiedFresh,
 		},
-		// outpoint299 is removed from the view since the entry is nil.
-		// entry1100Unmodified remains in the view since it is unmodified.
-		// entry1200Spent is removed from the view since the entry is spent.
-		// entry85314Modified is removed from the view since it is modified and
-		// added to the cache.
 		wantViewEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint1100: entry1100.unmodified,
+			outpoint299:   entry299.unmodified,
+			outpoint1100:  entry1100.unmodified,
+			outpoint1200:  entry1200.unmodified,
+			outpoint85314: entry85314.unmodifiedFresh,
+		},
+		wantCachedEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint1100:  nil,
+			outpoint1200:  entry1200.unmodified,
+			outpoint85314: entry85314.modifiedFresh,
+		},
+	}, {
+		// This covers the following scenarios:
+		//
+		// - modified spent view entries with a corresponding nil cache entry
+		//   are removed from view and a noop to the cache
+		// - modified spent fresh view entries with a corresponding unspent
+		//   fresh cache entry makes the cache entry nil
+		// - modified spent view entries with a corresponding non-fresh
+		//   unmodified cache entry are removed from view and mark the cache
+		//   entry modified and spent
+		name: "several modified spent view entry combinations",
+		viewEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint299:  entry299.modifiedSpent,
+			outpoint1100: entry1100.modifiedSpentFresh,
+			outpoint1200: entry1200.modifiedSpent,
+		},
+		cachedEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint299:  nil,
+			outpoint1100: entry1100.modifiedFresh,
+			outpoint1200: entry1200.unmodified,
+		},
+		wantViewEntries: map[wire.OutPoint]*UtxoEntry{},
+		wantCachedEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint299:  nil,
+			outpoint1100: nil,
+			outpoint1200: entry1200.modifiedSpent,
 		},
-		// entry1200Spent remains in the cache but is now spent.
-		// entry85314Modified is added to the cache.
+	}, {
+		// This covers the following scenarios:
+		//
+		// - modified unspent view entries with no corresponding cache entry are
+		//   removed from view and added to the cache as fresh entries
+		// - modified unspent view entries with a corresponding unmodified cache
+		//   entry are removed from view and replace the cache entry making it
+		//   modified
+		// - modified unspent fresh view entries with a corresponding modified
+		//   fresh cache entry are removed from view and replace the cache entry
+		//   retaining its modified fresh status
+		// - modified unspent non-fresh view entries with a corresponding
+		//   modified non-fresh cache entry are removed from view and replace
+		//   the cache entry retaining its modified non-fresh status
+		name: "several modified unspent view entry combinations",
+		viewEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint299:   entry299.modified,
+			outpoint1100:  entry1100.modified,
+			outpoint1200:  entry1200.modifiedFresh,
+			outpoint85314: entry85314.modified,
+		},
+		cachedEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint1100:  entry1100.unmodified,
+			outpoint1200:  entry1200.modifiedFresh,
+			outpoint85314: entry85314.modified,
+		},
+		wantViewEntries: map[wire.OutPoint]*UtxoEntry{},
 		wantCachedEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint1200:  entry1200.modifiedSpent,
+			outpoint299:   entry299.modifiedFresh,
+			outpoint1100:  entry1100.modified,
+			outpoint1200:  entry1200.modifiedFresh,
 			outpoint85314: entry85314.modified,
 		},
 	}}
@@ -700,17 +857,26 @@ func TestCommit(t *testing.T) {
 		// Create a utxo cache with the cached entries specified by the test.
 		utxoCache := createTestUtxoCache(t, test.cachedEntries)
 
-		// Create a utxo cache with the view entries specified by the test.
+		// Create a utxo view with the entries specified by the test.  The view
+		// entries are cloned first in order to ensure the shared instances are
+		// not modified if the cache takes ownership of them.
+		viewEntries := make(map[wire.OutPoint]*UtxoEntry)
+		for k, v := range test.viewEntries {
+			viewEntries[k] = v.Clone()
+		}
 		view := &UtxoViewpoint{
 			cache:   utxoCache,
-			entries: test.viewEntries,
+			entries: viewEntries,
 		}
 
 		// Commit the view to the cache.
 		err := utxoCache.Commit(view)
-		if err != nil {
-			t.Fatalf("%q: unexpected error committing view to the cache: %v",
-				test.name, err)
+		if !errors.Is(err, test.err) {
+			t.Fatalf("%q: unexpected error committing view to the cache -- "+
+				"got %v (%[2]T), want %v (%[3]T)", test.name, err, test.err)
+		}
+		if test.err != nil {
+			continue
 		}
 
 		// Validate the cached entries after committing.

From a48f7a3bfc5db8fa80f9d1cc56849bf037e7205e Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Tue, 13 Sep 2022 05:24:17 -0500
Subject: [PATCH 13/17] [blockchain-v4] blockchain: Rework utxo cache add entry
 tests.

This reworks the utxo cache add entry tests to simplify them and make
them more flexible by moving the primary logic for all of the expected
results into the test definitions themselves as oppossed to special
casing depending on flags in the test body.
---
 blockchain/utxocache_test.go | 85 ++++++++++++++----------------------
 1 file changed, 33 insertions(+), 52 deletions(-)

diff --git a/blockchain/utxocache_test.go b/blockchain/utxocache_test.go
index cb553db021..caba097716 100644
--- a/blockchain/utxocache_test.go
+++ b/blockchain/utxocache_test.go
@@ -355,72 +355,53 @@ func TestAddEntry(t *testing.T) {
 	t.Parallel()
 
 	// Create test entries to be used throughout the tests.
-	outpoint, entry := outpoint299(), makeEntryStates(entry299())
-	entry.modified.amount++
+	outpoint, origEntry := outpoint299(), makeEntryStates(entry299())
+	mutatedEntry := makeEntryStates(entry299())
+	mutatedEntry.modified.amount++
+	mutatedEntry.modifiedFresh.amount++
 
+	type entriesMap map[wire.OutPoint]*UtxoEntry
 	tests := []struct {
-		name            string
-		existingEntries map[wire.OutPoint]*UtxoEntry
-		outpoint        wire.OutPoint
-		entry           *UtxoEntry
-		wantEntry       *UtxoEntry
+		name          string        // test description
+		cachedEntries entriesMap    // existing cache entires
+		outpoint      wire.OutPoint // outpoint for entry to add/update
+		entry         *UtxoEntry    // entry to add/update
+		wantEntry     *UtxoEntry    // expected entry in updated cache
+		wantCacheSize uint64        // expected size of updated cache
 	}{{
-		name:      "add an entry that does not already exist in the cache",
-		outpoint:  outpoint,
-		entry:     entry.unmodified,
-		wantEntry: entry.modifiedFresh,
+		name:          "entry not in cache adds modified fresh entry",
+		outpoint:      outpoint,
+		entry:         origEntry.unmodified,
+		wantEntry:     origEntry.modifiedFresh,
+		wantCacheSize: origEntry.modifiedFresh.size(),
 	}, {
-		name: "add an entry that overwrites an existing entry",
-		existingEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint: entry.unmodified,
-		},
-		outpoint:  outpoint,
-		entry:     entry.modified,
-		wantEntry: entry.modified,
+		name:          "existing cache entry is overwritten",
+		cachedEntries: entriesMap{outpoint: origEntry.unmodified},
+		outpoint:      outpoint,
+		entry:         mutatedEntry.modified,
+		wantEntry:     mutatedEntry.modified,
+		wantCacheSize: mutatedEntry.modified.size(),
 	}}
 
 	for _, test := range tests {
 		// Create a utxo cache with the existing entries specified by the test.
-		utxoCache := createTestUtxoCache(t, test.existingEntries)
-		wantTotalEntrySize := utxoCache.totalEntrySize
-
-		// Attempt to get an existing entry from the cache.  If it exists,
-		// subtract its size from the expected total entry size since it will be
-		// overwritten.
-		existingEntry := utxoCache.entries[test.outpoint]
-		if existingEntry != nil {
-			wantTotalEntrySize -= test.entry.size()
-		}
-
-		// Add the entry specified by the test.
-		utxoCache.addEntry(test.outpoint, test.entry)
-		wantTotalEntrySize += test.entry.size()
-
-		// Attempt to get the added entry from the cache.
-		cachedEntry := utxoCache.entries[test.outpoint]
-
-		// Validate that the added entry exists in the cache.
-		if cachedEntry == nil {
-			t.Fatalf("%q: expected entry for outpoint %v to exist in the cache",
-				test.name, test.outpoint)
-		}
+		utxoCache := createTestUtxoCache(t, test.cachedEntries)
 
-		// Validate that the entry is marked as modified.
-		if !cachedEntry.isModified() {
-			t.Fatalf("%q: unexpected modified flag -- got false, want true",
-				test.name)
-		}
+		// Add/update the entry specified by the test.
+		utxoCache.addEntry(test.outpoint, test.entry.Clone())
 
-		// Validate that the cached entry matches the expected entry.
-		if !reflect.DeepEqual(cachedEntry, test.wantEntry) {
-			t.Fatalf("%q: mismatched cached entry:\nwant: %+v\n got: %+v\n",
-				test.name, test.wantEntry, cachedEntry)
+		// Get the entry associated with the output from the cache and ensure it
+		// matches the expected one.
+		gotEntry := utxoCache.entries[test.outpoint]
+		if !reflect.DeepEqual(gotEntry, test.wantEntry) {
+			t.Fatalf("%q: unexpected entry after spend -- got: %+v, want %+v",
+				test.name, gotEntry, test.wantEntry)
 		}
 
 		// Validate that the total entry size was updated as expected.
-		if utxoCache.totalEntrySize != wantTotalEntrySize {
+		if utxoCache.totalEntrySize != test.wantCacheSize {
 			t.Fatalf("%q: unexpected total entry size -- got %v, want %v",
-				test.name, utxoCache.totalEntrySize, wantTotalEntrySize)
+				test.name, utxoCache.totalEntrySize, test.wantCacheSize)
 		}
 	}
 }

From b52d0cba9d0156ae27037d5ea8374582d4264c6e Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Tue, 13 Sep 2022 05:24:18 -0500
Subject: [PATCH 14/17] [blockchain-v4] blockchain: Separate utxo cache vs view
 state.

The code prior to this change clones entries from the cache directly
into a view which means any cache entries that are marked modified
and/or fresh end up in the view with the flags set.  This leads to
slightly less efficient code and several non-obvious assumptions that
are easy to violate when making updates to views or the cache.

For example, modified cache entries end up in the view marked as
modified which means that even if they aren't actually modified, once
the view is committed, those entries end up needlessly updating the
cache with the new entry that is actually the exact same as the existing
one.  Another example is that cache entries that are marked fresh have
to retain that flag in the view in order to ensure they remain fresh in
the cache once that entry is replaced per the previous point.

It is perhaps worth noting that, in practice, there is not typically a
lot extra work due to the modified entries since almost all utxos loaded
into a view from the cache are ultimately modified in some way which
necessarily marks them modified anyway.  However, it is still
technically incorrect for them to be marked modified before they are
actually modified and does lead to some extra work that is not
necessary.

To address the aforementioned, this modifies the utxo cache to ensure
the cache state and view states are separate by updating the addEntry
and fetchEntry methods to ensure the modified and spent flags are
cleared on cloned entries loaded from the cache and and always set
correctly when adding entries to the cache.

The result is slightly more efficient code and removal of potential
footguns due to non-obvious assumptions about the internals of the view
and cache logic.
---
 blockchain/utxocache.go      | 64 ++++++++++++++++++++++++++----------
 blockchain/utxocache_test.go | 47 ++++++++++++++++++++------
 2 files changed, 83 insertions(+), 28 deletions(-)

diff --git a/blockchain/utxocache.go b/blockchain/utxocache.go
index 6b6f0201fb..5aa6feacd5 100644
--- a/blockchain/utxocache.go
+++ b/blockchain/utxocache.go
@@ -274,26 +274,41 @@ func (c *UtxoCache) hitRatio() float64 {
 //
 // This function MUST be called with the cache lock held.
 func (c *UtxoCache) addEntry(outpoint wire.OutPoint, entry *UtxoEntry) {
+	// The entry will either be added to the cache when there is not already
+	// an existing entry for the specified outpoint or will overwrite the
+	// existing one when there is, but in either case the resulting cache
+	// entry is modified and thus needs to be marked as such.
+	entry.state |= utxoStateModified
+
 	// Attempt to get an existing entry from the cache.
 	cachedEntry := c.entries[outpoint]
 
-	// If an existing entry does not exist, the added entry should be marked as
-	// modified and fresh.
+	// Mark the entry as fresh, add it to the cache, and update the total
+	// cache size when there is not already an existing cache entry.
 	if cachedEntry == nil {
-		entry.state |= utxoStateModified | utxoStateFresh
+		entry.state |= utxoStateFresh
+		c.entries[outpoint] = entry
+		c.totalEntrySize += entry.size()
+		return
 	}
 
-	// Add the entry to the cache.  In the case that an entry already exists,
-	// the existing entry is overwritten.  All fields are overwritten because
-	// it's possible (although extremely unlikely) that the existing entry is
-	// being replaced by a different transaction with the same hash.  This is
-	// allowed so long as the previous transaction is fully spent.
-	c.entries[outpoint] = entry
-
-	// Update the total entry size of the cache.
-	if cachedEntry != nil {
-		c.totalEntrySize -= cachedEntry.size()
+	// There is an existing entry in the cache at this point.
+	//
+	// Ensure the state of whether or not the existing entry is fresh is
+	// maintained, overwrite the existing entry, and update the total cache size
+	// accordingly.
+	//
+	// All fields are overwritten because it's possible (although extremely
+	// unlikely) that the existing entry is being replaced by a different
+	// transaction with the same hash.  This is allowed so long as the previous
+	// transaction is fully spent.
+	if cachedEntry.isFresh() {
+		entry.state |= utxoStateFresh
+	} else {
+		entry.state &^= utxoStateFresh
 	}
+	c.entries[outpoint] = entry
+	c.totalEntrySize -= cachedEntry.size()
 	c.totalEntrySize += entry.size()
 }
 
@@ -353,20 +368,30 @@ func (c *UtxoCache) SpendEntry(outpoint wire.OutPoint) {
 // the output exists in the cache, it is returned immediately.  Otherwise, it
 // fetches the output from the backend, caches it, and returns it to the
 // caller.  A cloned copy of the entry is returned so it can safely be mutated
-// by the caller without invalidating the cache.
+// by the caller without invalidating the cache.  The returned entry will not
+// have the modified or fresh flags set to ensure the cache state is separate
+// from the caller state.
 //
 // When there is no entry for the provided output, nil will be returned for both
 // the entry and the error.
 //
 // This function MUST be called with the cache lock held.
 func (c *UtxoCache) fetchEntry(outpoint wire.OutPoint) (*UtxoEntry, error) {
-	// If the cache already has the entry, return it immediately.  A cloned copy
-	// of the entry is returned so it can safely be mutated by the caller
-	// without invalidating the cache.
+	// Return a cloned copy of the cache entry when one exists for the provided
+	// output.  A cloned copy of the entry is returned so it can safely be
+	// mutated by the caller without invalidating the cache.
+	//
+	// Also clear the modified and fresh flags for the returned entry to ensure
+	// the cache state is separate from the view state.
 	var entry *UtxoEntry
 	if entry, found := c.entries[outpoint]; found {
 		c.hits++
-		return entry.Clone(), nil
+
+		clonedEntry := entry.Clone()
+		if clonedEntry != nil {
+			clonedEntry.state &^= utxoStateModified | utxoStateFresh
+		}
+		return clonedEntry, nil
 	}
 
 	// Increment cache misses.
@@ -391,6 +416,9 @@ func (c *UtxoCache) fetchEntry(outpoint wire.OutPoint) (*UtxoEntry, error) {
 	// Add the entry to the cache and return it.  A cloned copy of the entry is
 	// returned so it can safely be mutated by the caller without invalidating
 	// the cache.
+	//
+	// Also note that all entries loaded from the backend will not have any
+	// state flags set since they are memory only flags.
 	c.entries[outpoint] = entry
 	return entry.Clone(), nil
 }
diff --git a/blockchain/utxocache_test.go b/blockchain/utxocache_test.go
index caba097716..57020f92f1 100644
--- a/blockchain/utxocache_test.go
+++ b/blockchain/utxocache_test.go
@@ -375,12 +375,33 @@ func TestAddEntry(t *testing.T) {
 		wantEntry:     origEntry.modifiedFresh,
 		wantCacheSize: origEntry.modifiedFresh.size(),
 	}, {
-		name:          "existing cache entry is overwritten",
+		name:          "existing !fresh cache entry w/ !fresh entry stays !fresh",
 		cachedEntries: entriesMap{outpoint: origEntry.unmodified},
 		outpoint:      outpoint,
 		entry:         mutatedEntry.modified,
 		wantEntry:     mutatedEntry.modified,
 		wantCacheSize: mutatedEntry.modified.size(),
+	}, {
+		name:          "existing !fresh cache entry with fresh entry stays !fresh",
+		cachedEntries: entriesMap{outpoint: origEntry.unmodified},
+		outpoint:      outpoint,
+		entry:         mutatedEntry.modifiedFresh,
+		wantEntry:     mutatedEntry.modified,
+		wantCacheSize: mutatedEntry.modified.size(),
+	}, {
+		name:          "existing fresh cache entry with !fresh entry stays fresh",
+		cachedEntries: entriesMap{outpoint: origEntry.modifiedFresh},
+		outpoint:      outpoint,
+		entry:         mutatedEntry.modified,
+		wantEntry:     mutatedEntry.modifiedFresh,
+		wantCacheSize: mutatedEntry.modifiedFresh.size(),
+	}, {
+		name:          "existing fresh cache entry with fresh entry stays fresh",
+		cachedEntries: entriesMap{outpoint: origEntry.modifiedFresh},
+		outpoint:      outpoint,
+		entry:         mutatedEntry.modifiedFresh,
+		wantEntry:     mutatedEntry.modifiedFresh,
+		wantCacheSize: mutatedEntry.modifiedFresh.size(),
 	}}
 
 	for _, test := range tests {
@@ -699,10 +720,8 @@ func TestCommit(t *testing.T) {
 		// unmodified spent entries since those are pruned.
 		//
 		// Similarly, unmodified unspent fresh view entries never happen with
-		// the current implementation because the only way for a fresh view
-		// entry to exist is for it to have come from the cache, all fresh cache
-		// entries must necessarily also be marked modified, and cache entries
-		// are currently cloned directly into the view as is.
+		// the current implementation because views never set the fresh flag and
+		// the flag is cleared on cloned entries that come from the cache.
 		//
 		// However, they are tested here for completeness in order to ensure
 		// unmodified entries never affect the cache.
@@ -744,11 +763,9 @@ func TestCommit(t *testing.T) {
 		//   effect on the cache
 		//
 		// NOTE: Unmodified unspent fresh view entries never happen with the
-		// current implementation because the only way for a fresh view entry to
-		// exist is for it to have come from the cache, all fresh cache entries
-		// must necessarily also be marked modified, and cache entries are
-		// currently cloned directly into the view as is.  However, it is tested
-		// here for completeness.
+		// current implementation because views never set the fresh flag and the
+		// flag is cleared on cloned entries that come from the cache.  However,
+		// it is tested here for completeness.
 		name: "several unmodified unspent view entry combinations",
 		viewEntries: map[wire.OutPoint]*UtxoEntry{
 			outpoint299:   entry299.unmodified,
@@ -782,6 +799,11 @@ func TestCommit(t *testing.T) {
 		// - modified spent view entries with a corresponding non-fresh
 		//   unmodified cache entry are removed from view and mark the cache
 		//   entry modified and spent
+		//
+		// NOTE: Modified spent fresh view entries never happen with the current
+		// implementation because views never set the fresh flag and the flag is
+		// cleared on cloned entries that come from the cache.  However, it is
+		// tested here for completeness.
 		name: "several modified spent view entry combinations",
 		viewEntries: map[wire.OutPoint]*UtxoEntry{
 			outpoint299:  entry299.modifiedSpent,
@@ -813,6 +835,11 @@ func TestCommit(t *testing.T) {
 		// - modified unspent non-fresh view entries with a corresponding
 		//   modified non-fresh cache entry are removed from view and replace
 		//   the cache entry retaining its modified non-fresh status
+		//
+		// NOTE: Modified unspent fresh view entries never happen with the
+		// current implementation because views never set the fresh flag and the
+		// flag is cleared on cloned entries that come from the cache.  However,
+		// it is tested here for completeness.
 		name: "several modified unspent view entry combinations",
 		viewEntries: map[wire.OutPoint]*UtxoEntry{
 			outpoint299:   entry299.modified,

From 4b7860884a919d45121289c705280c2a4c43aa59 Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Tue, 13 Sep 2022 05:24:19 -0500
Subject: [PATCH 15/17] [blockchain-v4] blockchain: Improve utxo cache spend
 robustness.

This modifies the utxo cache commit logic to ensure more robust handling
of spent outputs including the addition of a double spend assertion and
adds tests to ensure the new behavior works as intended.

In particular, it updates the spendEntry method to attempt to load any
entries that are not already in the cache from backend which is
necessary since lack of an entry does not always guarantee the entry is
not in the backend in the case of a hard to hit corner case involving a
mix of reorganizations and unclean shutdowns.  It also adds a new double
spend assertion while here since it's good practice to be paranoid and
double check assumptions when it comes to potential double spends.

While it is certainly possible to tackle the specific aforementioned
case in other ways, this approach was chosen because it is an overall
robust solution that ensures proper behavior regardless of other
independent assumptions that might be introduced in future code updates.
---
 blockchain/utxocache.go      | 147 ++++++++++++++++++++++++++++-------
 blockchain/utxocache_test.go |  84 ++++++++++++++++----
 2 files changed, 188 insertions(+), 43 deletions(-)

diff --git a/blockchain/utxocache.go b/blockchain/utxocache.go
index 5aa6feacd5..6394337c05 100644
--- a/blockchain/utxocache.go
+++ b/blockchain/utxocache.go
@@ -329,17 +329,91 @@ func (c *UtxoCache) AddEntry(outpoint wire.OutPoint, entry *UtxoEntry) error {
 
 // spendEntry marks the specified output as spent.
 //
+// When the provided output does not already have an associated entry in the
+// cache, the cache will be populated with the entry from the backend (if one
+// exists) and marked spent to ensure the utxo is later removed from the backend
+// on the next cache flush.
+//
+// An error will be returned in the case the output exists in the cache and is
+// already spent.
+//
 // This function MUST be called with the cache lock held.
-func (c *UtxoCache) spendEntry(outpoint wire.OutPoint) {
-	// Attempt to get an existing entry from the cache.
-	cachedEntry := c.entries[outpoint]
+func (c *UtxoCache) spendEntry(outpoint wire.OutPoint) error {
+	// Attempt to get the entry associated with the output from the cache.
+	//
+	// In the case there is a nil entry in the cache for the output, it does not
+	// exist in the backend.  This is because either an attempt to load the utxo
+	// from the backend was made and the nil entry was added to the cache to
+	// avoid further lookups against the backend when the output is known to not
+	// exist or the output was added to the cache and spent in between flushes
+	// to the backend and thus has been pruned.
+	//
+	// In either case, the cache does not need to be updated, so there is
+	// nothing more to do.
+	cachedEntry, found := c.entries[outpoint]
+	if found && cachedEntry == nil {
+		return nil
+	}
 
-	// If the entry is nil or already spent, return immediately.
-	if cachedEntry == nil || cachedEntry.IsSpent() {
-		return
+	// Sanity check double spends of existing entries.  This should be
+	// impossible to hit since a view should never be committed until after
+	// double spend checks have already been done.  However, it is good practice
+	// to be paranoid and double check assumptions when it comes to potential
+	// double spends.
+	if cachedEntry != nil && cachedEntry.IsSpent() {
+		return AssertError(fmt.Sprintf("attempt to double spend output %v in "+
+			"view commit", outpoint))
+	}
+
+	// Attempt to repopulate the cache from the backend when there is no entry
+	// for the output in the cache.  This is necessary because the associated
+	// utxo might exist in the backend but not have been loaded into the cache
+	// yet or have otherwise been evicted.
+	//
+	// Thus, in order to ensure the utxo is removed from the backend on the next
+	// cache flush in the case it exists, the associated cache entry from the
+	// backend is loaded and marked spent.
+	if !found {
+		// Account for the cache miss and attempt to load the entry for the utxo
+		// from the backend.
+		//
+		// Notice that this intentionally attempts to load the entry from the
+		// backend directly as opposed to using the normal cache entry fetching
+		// method since that involves querying the cache again which would be
+		// wasteful since it is already known to not exist and also employs
+		// slightly different logic regarding populating the cache.
+		//
+		// NOTE: Missing backend entries are not an error.
+		c.misses++
+		backendEntry, err := c.backend.FetchEntry(outpoint)
+		if err != nil {
+			return err
+		}
+
+		// The cache does not need to be updated when the associated utxo does
+		// not exist in the backend, so there is nothing more to do in that
+		// case.
+		//
+		// Note that a nil entry is not added to cache for the output here since
+		// there should realistically never be any future lookups for the spent
+		// output given that would be a double spend.
+		if backendEntry == nil {
+			return nil
+		}
+
+		// Add the entry to the cache and update its total entry size.
+		c.entries[outpoint] = backendEntry
+		c.totalEntrySize += backendEntry.size()
+
+		// Mark the output as spent and modified.
+		backendEntry.Spend()
+		return nil
 	}
 
-	// If the entry is fresh, and is now being spent, it can safely be removed.
+	// Remove the entry associated with the output from the cache when the
+	// backend does not need to be updated because the backend does not have an
+	// associated utxo (aka the cache entry is fresh).
+	//
 	// This is an optimization to skip writing to the backend for outputs that
 	// are added and spent in between flushes to the backend.
 	if cachedEntry.isFresh() {
@@ -348,11 +422,12 @@ func (c *UtxoCache) spendEntry(outpoint wire.OutPoint) {
 		// and avoid querying the backend.
 		c.entries[outpoint] = nil
 		c.totalEntrySize -= cachedEntry.size()
-		return
+		return nil
 	}
 
 	// Mark the output as spent and modified.
 	cachedEntry.Spend()
+	return nil
 }
 
 // SpendEntry marks the specified output as spent.
@@ -360,8 +435,11 @@ func (c *UtxoCache) spendEntry(outpoint wire.OutPoint) {
 // This function is safe for concurrent access.
 func (c *UtxoCache) SpendEntry(outpoint wire.OutPoint) {
 	c.cacheLock.Lock()
-	c.spendEntry(outpoint)
+	err := c.spendEntry(outpoint)
 	c.cacheLock.Unlock()
+	if err != nil {
+		panic(err)
+	}
 }
 
 // fetchEntry returns the specified transaction output from the utxo set.  If
@@ -495,9 +573,12 @@ func (c *UtxoCache) FetchStats(bestHash *chainhash.Hash, bestHeight uint32) (*Ut
 //
 // This function is safe for concurrent access.
 func (c *UtxoCache) Commit(view *UtxoViewpoint) error {
+	defer c.cacheLock.Unlock()
 	c.cacheLock.Lock()
+
 	for outpoint, entry := range view.entries {
-		// If the entry is nil, delete it from the view and continue.
+		// There is nothing to update in the cache when the view entry is nil,
+		// so remove it from the view and continue to the next entry.
 		if entry == nil {
 			delete(view.entries, outpoint)
 			continue
@@ -509,34 +590,42 @@ func (c *UtxoCache) Commit(view *UtxoViewpoint) error {
 			continue
 		}
 
-		// If the entry is modified and spent, mark it as spent in the cache and
-		// then delete it from the view.
+		// ---------------------------------------------------------------------
+		// NOTE: As an optimization in order to avoid an additional allocation,
+		// the cache takes ownership of all modified entries from the view.
+		//
+		// Thus, unless there is an error preventing the cache from taking
+		// ownership and ultimately causing the commit to fail, the entry will
+		// be removed from the view in all remaining paths to ensure that it is
+		// not mutated by the caller after being added to the cache.
+		//
+		// This does cause the view to refetch the entry if it is requested
+		// again after being removed.  However, this only really occurs during
+		// reorgs, whereas committing the view to the cache happens with every
+		// connected block, so this optimizes for the much more common case.
+		// ---------------------------------------------------------------------
+
+		// Mark the spent view entry as modified and spent in the cache and
+		// remove it from the view.
+		//
+		// This might entail populating the cache with the unspent output from
+		// the backend when there is not already a corresponding entry in the
+		// cache to ensure the utxo is later removed from the backend on the
+		// next cache flush.
 		if entry.IsSpent() {
-			// Mark the entry as spent in the cache.
-			c.spendEntry(outpoint)
+			if err := c.spendEntry(outpoint); err != nil {
+				return err
+			}
 
-			// Delete the entry from the view.
 			delete(view.entries, outpoint)
 			continue
 		}
 
-		// If we passed all of the conditions above, the entry is modified or
-		// fresh, but not spent, and should be added to the cache.
+		// Update the existing entry in the cache or add a new one whenever the
+		// view entry is both modified and unspent and remove it from the view.
 		c.addEntry(outpoint, entry)
-
-		// All entries that are added to the cache should be removed from the
-		// provided view.  This is an optimization to allow the cache to take
-		// ownership of the entry from the view and avoid an additional
-		// allocation.  It is removed from the view to ensure that it is not
-		// mutated by the caller after being added to the cache.
-		//
-		// This does cause the view to refetch the entry if it is requested
-		// again after being removed.  However, this only really occurs during
-		// reorgs, whereas committing the view to the cache happens with every
-		// connected block, so this optimizes for the much more common case.
 		delete(view.entries, outpoint)
 	}
-	c.cacheLock.Unlock()
 
 	return nil
 }
diff --git a/blockchain/utxocache_test.go b/blockchain/utxocache_test.go
index 57020f92f1..312274cfa2 100644
--- a/blockchain/utxocache_test.go
+++ b/blockchain/utxocache_test.go
@@ -432,6 +432,9 @@ func TestAddEntry(t *testing.T) {
 func TestSpendEntry(t *testing.T) {
 	t.Parallel()
 
+	// Create a test backend.
+	backend := createTestUtxoBackend(t)
+
 	// Create test entries to be used throughout the tests.
 	outpoint, entry := outpoint299(), makeEntryStates(entry299())
 
@@ -443,23 +446,29 @@ func TestSpendEntry(t *testing.T) {
 		outpoint       wire.OutPoint // outpoint for the entry to spend
 		wantEntry      *UtxoEntry    // expected entry in cache after spend
 		wantCacheSize  uint64        // expected size of the cache after spend
+		err            error         // expected error
 	}{{
+		name:          "spending cache entry already marked spent asserts",
+		cachedEntries: entriesMap{outpoint: entry.modifiedSpent},
+		outpoint:      outpoint,
+		err:           AssertError(""),
+	}, {
 		name:          "spending nil (aka pruned) cache entry is a noop",
 		cachedEntries: entriesMap{outpoint: nil},
 		outpoint:      outpoint,
 		wantEntry:     nil,
 		wantCacheSize: 0,
 	}, {
-		name:          "spending entry not in cache is a noop",
+		name:          "spending entry not in cache queries backend (no utxo)",
 		outpoint:      outpoint,
 		wantEntry:     nil,
 		wantCacheSize: 0,
 	}, {
-		name:          "spending cache entry already marked spent is a noop",
-		cachedEntries: entriesMap{outpoint: entry.modifiedSpent},
-		outpoint:      outpoint,
-		wantEntry:     entry.modifiedSpent,
-		wantCacheSize: entry.modifiedSpent.size(),
+		name:           "spending entry not in cache queries backend (with utxo)",
+		outpoint:       outpoint,
+		backendEntries: entriesMap{outpoint: entry.modified},
+		wantEntry:      entry.modifiedSpent,
+		wantCacheSize:  entry.modifiedSpent.size(),
 	}, {
 		name:          "spending cache entry marked fresh makes it nil",
 		cachedEntries: entriesMap{outpoint: entry.modifiedFresh},
@@ -477,9 +486,25 @@ func TestSpendEntry(t *testing.T) {
 	for _, test := range tests {
 		// Create a utxo cache with the existing entries specified by the test.
 		utxoCache := createTestUtxoCache(t, test.cachedEntries)
+		utxoCache.backend = backend
 
-		// Spend the entry specified by the test.
-		utxoCache.SpendEntry(test.outpoint)
+		// Add entries specified by the test to the test backend.
+		err := backend.PutUtxos(test.backendEntries, &UtxoSetState{})
+		if err != nil {
+			t.Fatalf("%q: unexpected error adding entries to test backend: %v",
+				test.name, err)
+		}
+
+		// Spend the entry specified by the test and ensure the error matches
+		// the expected one.
+		err = utxoCache.spendEntry(test.outpoint)
+		if !errors.Is(err, test.err) {
+			t.Fatalf("%q: unexpected error spending outpoint %v -- got %v, "+
+				"want %v (%[4]T)", test.name, test.outpoint, err, test.err)
+		}
+		if test.err != nil {
+			continue
+		}
 
 		// Get the entry associated with the output from the cache and ensure it
 		// matches the expected one.
@@ -660,6 +685,9 @@ func TestFetchEntries(t *testing.T) {
 func TestCommit(t *testing.T) {
 	t.Parallel()
 
+	// Create a test backend.
+	backend := createTestUtxoBackend(t)
+
 	// Create test entries to be used throughout the tests.
 	outpoint299, entry299 := outpoint299(), makeEntryStates(entry299())
 	outpoint1100, entry1100 := outpoint1100(), makeEntryStates(entry1100())
@@ -670,21 +698,19 @@ func TestCommit(t *testing.T) {
 		name              string                       // test description
 		viewEntries       map[wire.OutPoint]*UtxoEntry // view to commit
 		cachedEntries     map[wire.OutPoint]*UtxoEntry // existing cache entries
+		backendEntries    map[wire.OutPoint]*UtxoEntry // existing backend entries
 		wantViewEntries   map[wire.OutPoint]*UtxoEntry // expected committed view
 		wantCachedEntries map[wire.OutPoint]*UtxoEntry // expected committed cache
 		err               error                        // expected error
 	}{{
-		name: "modified spent view entry w/ existing spent cache entry is noop",
+		name: "modified spent view entry w/ existing spent cache entry asserts",
 		viewEntries: map[wire.OutPoint]*UtxoEntry{
 			outpoint1200: entry1200.modifiedSpent,
 		},
 		cachedEntries: map[wire.OutPoint]*UtxoEntry{
 			outpoint1200: entry1200.modifiedSpent,
 		},
-		wantViewEntries: map[wire.OutPoint]*UtxoEntry{},
-		wantCachedEntries: map[wire.OutPoint]*UtxoEntry{
-			outpoint1200: entry1200.modifiedSpent,
-		},
+		err: AssertError(""),
 	}, {
 		name: "nil view entries are removed from view and do not affect cache",
 		viewEntries: map[wire.OutPoint]*UtxoEntry{
@@ -821,6 +847,28 @@ func TestCommit(t *testing.T) {
 			outpoint1100: nil,
 			outpoint1200: entry1200.modifiedSpent,
 		},
+	}, {
+		// This covers the following scenarios:
+		//
+		// - modified spent view entries without a corresponding cache entry nor
+		//   corresponding backend entry are removed from the view and a noop to
+		//   the cache
+		// - modified spent view entries without a corresponding cache entry
+		//   but with a corresponding backend entry loads the entry from the
+		//   backend, spends it, and adds the spent entry to the cache
+		name: "modified spent view entries not in backend",
+		viewEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint299:  entry299.modifiedSpent,
+			outpoint1100: entry1100.modifiedSpent,
+		},
+		cachedEntries: map[wire.OutPoint]*UtxoEntry{},
+		backendEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint1100: entry1100.modified,
+		},
+		wantViewEntries: map[wire.OutPoint]*UtxoEntry{},
+		wantCachedEntries: map[wire.OutPoint]*UtxoEntry{
+			outpoint1100: entry1100.modifiedSpent,
+		},
 	}, {
 		// This covers the following scenarios:
 		//
@@ -864,6 +912,14 @@ func TestCommit(t *testing.T) {
 	for _, test := range tests {
 		// Create a utxo cache with the cached entries specified by the test.
 		utxoCache := createTestUtxoCache(t, test.cachedEntries)
+		utxoCache.backend = backend
+
+		// Add entries specified by the test to the test backend.
+		err := backend.PutUtxos(test.backendEntries, &UtxoSetState{})
+		if err != nil {
+			t.Fatalf("%q: unexpected error adding entries to test backend: %v",
+				test.name, err)
+		}
 
 		// Create a utxo view with the entries specified by the test.  The view
 		// entries are cloned first in order to ensure the shared instances are
@@ -878,7 +934,7 @@ func TestCommit(t *testing.T) {
 		}
 
 		// Commit the view to the cache.
-		err := utxoCache.Commit(view)
+		err = utxoCache.Commit(view)
 		if !errors.Is(err, test.err) {
 			t.Fatalf("%q: unexpected error committing view to the cache -- "+
 				"got %v (%[2]T), want %v (%[3]T)", test.name, err, test.err)

From 1bcc533276cebc69ea7654b4842db26c7d51b1d4 Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Tue, 13 Sep 2022 05:24:20 -0500
Subject: [PATCH 16/17] [blockchain-v4] blockchain: Split regular/stake view tx
 connect.

This splits the logic for handling the connection of transactions from
the regular tree and stake tree to a utxo view by introducing individual
methods for them and updating the callers accordingly.

The primary motivation for this change is that the regular and stake
transaction trees do not have identical logic in terms of the ability to
spend outputs earlier in the block and it is more efficient to avoid
continuously checking for special conditions across all transactions
when only certain types can possibly be in each respective tree.

Splitting this logic also paves the way for future commits to take
advantage of the aforementioned spending semantics to optimize the utxo
cache.
---
 blockchain/utxoviewpoint.go | 192 ++++++++++++++++++++++++++----------
 blockchain/validate.go      |   6 +-
 2 files changed, 144 insertions(+), 54 deletions(-)

diff --git a/blockchain/utxoviewpoint.go b/blockchain/utxoviewpoint.go
index c1ad46c791..ff6c31a0b7 100644
--- a/blockchain/utxoviewpoint.go
+++ b/blockchain/utxoviewpoint.go
@@ -215,27 +215,45 @@ func (view *UtxoViewpoint) PriorityInput(prevOut *wire.OutPoint) (int64, int64,
 	return 0, 0, false
 }
 
-// connectTransaction updates the view by adding all new utxos created by the
-// passed transaction 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.
-func (view *UtxoViewpoint) connectTransaction(tx *dcrutil.Tx, blockHeight int64,
-	blockIndex uint32, stxos *[]spentTxOut, isTreasuryEnabled,
-	isAutoRevocationsEnabled bool) error {
+// utxoEntryToSpentTxOut converts the provided utxo entry to a spent txout for
+// use in tracking all spent txouts for the spend journal.
+func utxoEntryToSpentTxOut(entry *UtxoEntry) spentTxOut {
+	// Populate the stxo details using the utxo entry.
+	return spentTxOut{
+		amount:        entry.Amount(),
+		pkScript:      entry.PkScript(),
+		ticketMinOuts: entry.ticketMinOuts,
+		blockHeight:   uint32(entry.BlockHeight()),
+		blockIndex:    entry.BlockIndex(),
+		scriptVersion: entry.ScriptVersion(),
+		packedFlags: encodeFlags(entry.IsCoinBase(), entry.HasExpiry(),
+			entry.TransactionType()),
+	}
+}
 
-	// Treasurybase transactions don't have any inputs to spend.
+// connectStakeTransaction updates the view by adding all new utxos created by
+// the passed transaction from the stake 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.
+func (view *UtxoViewpoint) connectStakeTransaction(tx *dcrutil.Tx,
+	blockHeight int64, blockIndex uint32, stxos *[]spentTxOut,
+	isTreasuryEnabled, isAutoRevocationsEnabled bool) error {
+
+	// Treasurybase transactions don't have any inputs to spend or outputs to
+	// add.
 	//
-	// NOTE: This check MUST come before the coinbase check because a
-	// treasurybase is identified as a coinbase as of the time this comment is
-	// being written.
+	// NOTE: This check relies on the code already having validated that the
+	// first transaction, and only the first transaction, in the stake tree is
+	// the required treasurybase when the treasury agenda is active.
 	msgTx := tx.MsgTx()
-	if isTreasuryEnabled && standalone.IsTreasuryBase(msgTx) {
+	if isTreasuryEnabled && blockIndex == 0 {
 		return nil
 	}
 
-	// Coinbase transactions don't have any inputs to spend.
-	if standalone.IsCoinBaseTx(msgTx, isTreasuryEnabled) {
+	// Treasury spends don't have any inputs to spend.
+	isTreasurySpend := isTreasuryEnabled && stake.IsTSpend(msgTx)
+	if isTreasurySpend {
 		// Add the transaction's outputs as available utxos.
 		view.AddTxOuts(tx, blockHeight, blockIndex, isTreasuryEnabled,
 			isAutoRevocationsEnabled)
@@ -245,41 +263,94 @@ func (view *UtxoViewpoint) connectTransaction(tx *dcrutil.Tx, blockHeight int64,
 	// Spend the referenced utxos by marking them spent in the view and, if a
 	// slice was provided for the spent txout details, append an entry to it.
 	isVote := stake.IsSSGen(msgTx, isTreasuryEnabled)
-	isTreasurySpend := isTreasuryEnabled && stake.IsTSpend(msgTx)
 	for txInIdx, txIn := range msgTx.TxIn {
 		// Ignore stakebase since it has no input.
 		if isVote && txInIdx == 0 {
 			continue
 		}
 
-		// Ignore treasury spends since they have no inputs.
-		if isTreasurySpend {
-			continue
+		// Ensure the referenced utxo exists in the view.  This should never
+		// happen unless a bug is introduced in the code.
+		prevOut := &txIn.PreviousOutPoint
+		entry := view.entries[*prevOut]
+		if entry == nil {
+			return AssertError(fmt.Sprintf("view missing input %v", *prevOut))
+		}
+
+		// Only create the stxo details if requested.
+		if stxos != nil {
+			// Populate the stxo details using the utxo entry.
+			*stxos = append(*stxos, utxoEntryToSpentTxOut(entry))
 		}
 
+		// Mark the entry as spent.  This is not done until after the relevant
+		// details have been accessed since spending it might clear the fields
+		// from memory in the future.
+		entry.Spend()
+	}
+
+	// Add the transaction's outputs as available utxos.
+	view.AddTxOuts(tx, blockHeight, blockIndex, isTreasuryEnabled,
+		isAutoRevocationsEnabled)
+
+	return nil
+}
+
+// connectStakeTransactions updates the view by adding all new utxos created by
+// the transactions in the stake 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.
+func (view *UtxoViewpoint) connectStakeTransactions(block *dcrutil.Block,
+	stxos *[]spentTxOut, isTreasuryEnabled, isAutoRevocationsEnabled bool) error {
+
+	// Connect all of the transactions in the stake transaction tree.
+	for i, tx := range block.STransactions() {
+		err := view.connectStakeTransaction(tx, block.Height(), uint32(i),
+			stxos, isTreasuryEnabled, isAutoRevocationsEnabled)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// 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.
+func (view *UtxoViewpoint) connectRegularTransaction(tx *dcrutil.Tx,
+	blockHeight int64, blockIndex uint32, stxos *[]spentTxOut,
+	isTreasuryEnabled, isAutoRevocationsEnabled 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,
+			isAutoRevocationsEnabled)
+		return nil
+	}
+
+	// Spend the referenced utxos by marking them spent in the view and, if a
+	// slice was provided for the spent txout details, append an entry to it.
+	for _, txIn := range msgTx.TxIn {
 		// Ensure the referenced utxo exists in the view.  This should never
-		// happen unless there is a bug is introduced in the code.
-		entry := view.entries[txIn.PreviousOutPoint]
+		// happen unless a bug is introduced in the code.
+		prevOut := &txIn.PreviousOutPoint
+		entry := view.entries[*prevOut]
 		if entry == nil {
-			return AssertError(fmt.Sprintf("view missing input %v",
-				txIn.PreviousOutPoint))
+			return AssertError(fmt.Sprintf("view missing input %v", *prevOut))
 		}
 
 		// Only create the stxo details if requested.
 		if stxos != nil {
 			// Populate the stxo details using the utxo entry.
-			var stxo = spentTxOut{
-				amount:        entry.Amount(),
-				pkScript:      entry.PkScript(),
-				ticketMinOuts: entry.ticketMinOuts,
-				blockHeight:   uint32(entry.BlockHeight()),
-				blockIndex:    entry.BlockIndex(),
-				scriptVersion: entry.ScriptVersion(),
-				packedFlags: encodeFlags(entry.IsCoinBase(), entry.HasExpiry(),
-					entry.TransactionType()),
-			}
-
-			*stxos = append(*stxos, stxo)
+			*stxos = append(*stxos, utxoEntryToSpentTxOut(entry))
 		}
 
 		// Mark the entry as spent.  This is not done until after the relevant
@@ -295,6 +366,27 @@ func (view *UtxoViewpoint) connectTransaction(tx *dcrutil.Tx, blockHeight int64,
 	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.
+func (view *UtxoViewpoint) connectRegularTransactions(block *dcrutil.Block,
+	stxos *[]spentTxOut, isTreasuryEnabled, isAutoRevocationsEnabled bool) error {
+
+	// Connect all of the transactions in the regular transaction tree.
+	for i, tx := range block.Transactions() {
+		err := view.connectRegularTransaction(tx, block.Height(), uint32(i),
+			stxos, isTreasuryEnabled, isAutoRevocationsEnabled)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // disconnectTransactions updates the view by removing all utxos created by the
 // transactions in either the regular or stake tree of the block, depending on
 // the flag, and unspending all of the txos spent by those same transactions by
@@ -544,19 +636,15 @@ func (view *UtxoViewpoint) connectBlock(db database.DB, block, parent *dcrutil.B
 	// of transactions created in the regular tree of the same block, which is
 	// important since the regular tree may be disapproved by the subsequent
 	// block while the stake tree must remain valid.
-	for i, stx := range block.STransactions() {
-		err := view.connectTransaction(stx, block.Height(), uint32(i),
-			stxos, isTreasuryEnabled, isAutoRevocationsEnabled)
-		if err != nil {
-			return err
-		}
+	err = view.connectStakeTransactions(block, stxos, isTreasuryEnabled,
+		isAutoRevocationsEnabled)
+	if err != nil {
+		return err
 	}
-	for i, tx := range block.Transactions() {
-		err := view.connectTransaction(tx, block.Height(), uint32(i),
-			stxos, isTreasuryEnabled, isAutoRevocationsEnabled)
-		if err != nil {
-			return err
-		}
+	err = view.connectRegularTransactions(block, stxos, isTreasuryEnabled,
+		isAutoRevocationsEnabled)
+	if err != nil {
+		return err
 	}
 
 	// Update the best hash for view to include this block since all of its
@@ -625,12 +713,10 @@ func (view *UtxoViewpoint) disconnectBlock(block, parent *dcrutil.Block,
 			return err
 		}
 
-		for i, tx := range parent.Transactions() {
-			err := view.connectTransaction(tx, parent.Height(),
-				uint32(i), nil, isTreasuryEnabled, isAutoRevocationsEnabled)
-			if err != nil {
-				return err
-			}
+		err = view.connectRegularTransactions(parent, nil, isTreasuryEnabled,
+			isAutoRevocationsEnabled)
+		if err != nil {
+			return err
 		}
 	}
 
diff --git a/blockchain/validate.go b/blockchain/validate.go
index 17b7666d1a..0d74e82d03 100644
--- a/blockchain/validate.go
+++ b/blockchain/validate.go
@@ -3575,7 +3575,11 @@ func (b *BlockChain) checkTransactionsAndConnect(inputFees dcrutil.Amount, node
 		//
 		// Also, update the passed spent txos slice to contain an entry for each
 		// output the transaction spends.
-		err = view.connectTransaction(tx, node.height, uint32(idx), stxos,
+		connectTransaction := view.connectRegularTransaction
+		if stakeTree {
+			connectTransaction = view.connectStakeTransaction
+		}
+		err = connectTransaction(tx, node.height, uint32(idx), stxos,
 			isTreasuryEnabled, isAutoRevocationsEnabled)
 		if err != nil {
 			return err

From 8208daf4c708454b408699d31ca923ef321637a5 Mon Sep 17 00:00:00 2001
From: Dave Collins <davec@conformal.com>
Date: Tue, 13 Sep 2022 05:24:20 -0500
Subject: [PATCH 17/17] [blockchain-v4] blockchain: Bypass utxo cache for zero
 conf spends.

This updates the utxo view and cache handling to detect zero
confirmation spends in the regular transaction tree (outputs that are
spent by the inputs of transactions later in the same tree) and bypass
the cache when committing the view.

This is desirable because such outputs never exist beyond the scope of
the view and therefore are guaranteed to never have any cache entries.
In other words, any attempts to spend them in cache will necessarily
result in a cache miss.

In order to implement the logic, a new state bit flag to track zero
confirmation spends is introduced along with code to appropriately set
and clear the flag when connecting and disconnection transactions.  The
bit flag is then used to bypass the cache spend when set.

Finally, cache commit tests are included to ensure proper functionality.
---
 blockchain/utxocache.go      | 22 +++++++++++
 blockchain/utxocache_test.go | 58 +++++++++++++++++++--------
 blockchain/utxoentry.go      | 36 +++++++++++------
 blockchain/utxoviewpoint.go  | 76 +++++++++++++++++++++++++++++-------
 blockchain/validate.go       | 29 ++++++++++----
 5 files changed, 171 insertions(+), 50 deletions(-)

diff --git a/blockchain/utxocache.go b/blockchain/utxocache.go
index 6394337c05..197a813adc 100644
--- a/blockchain/utxocache.go
+++ b/blockchain/utxocache.go
@@ -605,6 +605,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/blockchain/utxocache_test.go b/blockchain/utxocache_test.go
index 312274cfa2..475fd8cd02 100644
--- a/blockchain/utxocache_test.go
+++ b/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/blockchain/utxoentry.go b/blockchain/utxoentry.go
index 5c55d6eaee..b873adefa4 100644
--- a/blockchain/utxoentry.go
+++ b/blockchain/utxoentry.go
@@ -19,19 +19,25 @@ const (
 // utxoState defines the in-memory state of a utxo entry.
 //
 // 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.
@@ -42,9 +48,10 @@ const (
 // utxo entry.
 //
 // The bit representation is:
-//   bit  0    - containing transaction is a coinbase
-//   bit  1    - containing transaction has an expiry
-//   bits 2-5  - transaction type
+//
+//	bit  0    - containing transaction is a coinbase
+//	bit  1    - containing transaction has an expiry
+//	bits 2-5  - transaction type
 type utxoFlags uint8
 
 const (
@@ -147,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/blockchain/utxoviewpoint.go b/blockchain/utxoviewpoint.go
index ff6c31a0b7..77ec1d4d80 100644
--- a/blockchain/utxoviewpoint.go
+++ b/blockchain/utxoviewpoint.go
@@ -89,7 +89,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.  This is required since the tx out script is a
@@ -319,13 +319,16 @@ 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, isAutoRevocationsEnabled bool) error {
+	blockHeight int64, blockIndex uint32, inFlightTx map[chainhash.Hash]uint32,
+	stxos *[]spentTxOut, isTreasuryEnabled, isAutoRevocationsEnabled bool) error {
 
 	// Coinbase transactions don't have any inputs to spend.
 	msgTx := tx.MsgTx()
@@ -333,6 +336,10 @@ func (view *UtxoViewpoint) connectRegularTransaction(tx *dcrutil.Tx,
 		// Add the transaction's outputs as available utxos.
 		view.AddTxOuts(tx, blockHeight, blockIndex, isTreasuryEnabled,
 			isAutoRevocationsEnabled)
+
+		// 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
 	}
 
@@ -357,28 +364,43 @@ 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,
 		isAutoRevocationsEnabled)
 
+	// 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, isAutoRevocationsEnabled 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, isAutoRevocationsEnabled)
+			inFlightTx, stxos, isTreasuryEnabled, isAutoRevocationsEnabled)
 		if err != nil {
 			return err
 		}
@@ -402,11 +424,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()
@@ -482,6 +513,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
@@ -525,8 +564,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/blockchain/validate.go b/blockchain/validate.go
index 0d74e82d03..d0fe261025 100644
--- a/blockchain/validate.go
+++ b/blockchain/validate.go
@@ -3529,6 +3529,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
@@ -3573,16 +3577,25 @@ 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, isAutoRevocationsEnabled)
-		if err != nil {
-			return err
+		if !stakeTree {
+			err := view.connectRegularTransaction(tx, node.height, uint32(idx),
+				inFlightRegularTx, stxos, isTreasuryEnabled,
+				isAutoRevocationsEnabled)
+			if err != nil {
+				return err
+			}
+		} else {
+			err := view.connectStakeTransaction(tx, node.height, uint32(idx),
+				stxos, isTreasuryEnabled, isAutoRevocationsEnabled)
+			if err != nil {
+				return err
+			}
 		}
 	}