From 01fd2de07837ed44c8b24c77323e90ff0f45f6f1 Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Fri, 26 Jun 2020 19:12:13 -0500 Subject: [PATCH] blockchain: Correct mempool view construction. This corrects the construction of the view used by the mempool when dealing with disapproved blocks and includes new tests to ensure the various disapproval combinations work as intended. --- blockchain/utxoviewpoint.go | 21 ++- blockchain/utxoviewpoint_test.go | 234 +++++++++++++++++++++++++++++++ 2 files changed, 244 insertions(+), 11 deletions(-) create mode 100644 blockchain/utxoviewpoint_test.go diff --git a/blockchain/utxoviewpoint.go b/blockchain/utxoviewpoint.go index ac6d8cc6d6..f7e3acf1be 100644 --- a/blockchain/utxoviewpoint.go +++ b/blockchain/utxoviewpoint.go @@ -911,13 +911,13 @@ func NewUtxoViewpoint() *UtxoViewpoint { // FetchUtxoView loads utxo details about the input transactions referenced by // the passed transaction from the point of view of the end of the main chain // while taking into account whether or not the transactions in the regular tree -// of the block just prior should be included or not depending on the provided +// of the current tip block should be included or not depending on the provided // flag. It also attempts to fetch the utxo details for the transaction itself // so the returned view can be examined for duplicate unspent transaction // outputs. // // This function is safe for concurrent access however the returned view is NOT. -func (b *BlockChain) FetchUtxoView(tx *dcrutil.Tx, includePrevRegularTxns bool) (*UtxoViewpoint, error) { +func (b *BlockChain) FetchUtxoView(tx *dcrutil.Tx, includeRegularTxns bool) (*UtxoViewpoint, error) { b.chainLock.RLock() defer b.chainLock.RUnlock() @@ -932,24 +932,23 @@ func (b *BlockChain) FetchUtxoView(tx *dcrutil.Tx, includePrevRegularTxns bool) return view, nil } - // Disconnect the transactions in the regular tree of the parent block if - // the caller requests it. In order to avoid the overhead of repeated - // lookups, only create a view with the changes once and cache it. - if !includePrevRegularTxns { + // Disconnect the transactions in the regular tree of the tip block if the + // caller requests it. In order to avoid the overhead of repeated lookups, + // only create a view with the changes once and cache it. + if !includeRegularTxns { b.disapprovedViewLock.Lock() if b.disapprovedView == nil || *b.disapprovedView.BestHash() != tip.hash { - // Grab the parent of the current block. - parent, err := b.fetchMainChainBlockByNode(tip.parent) + // Grab the current tip block. + tipBlock, err := b.fetchMainChainBlockByNode(tip) if err != nil { b.disapprovedViewLock.Unlock() return nil, err } - // Disconnect the transactions in the regular tree of the parent - // block. - err = view.disconnectDisapprovedBlock(b.db, parent) + // Disconnect the transactions in the regular tree of the tip block. + err = view.disconnectDisapprovedBlock(b.db, tipBlock) if err != nil { b.disapprovedViewLock.Unlock() return nil, err diff --git a/blockchain/utxoviewpoint_test.go b/blockchain/utxoviewpoint_test.go new file mode 100644 index 0000000000..97351311b9 --- /dev/null +++ b/blockchain/utxoviewpoint_test.go @@ -0,0 +1,234 @@ +// Copyright (c) 2020 The Decred developers +// Use of this source code is governed by an ISC +// license that can be found in the LICENSE file. + +package blockchain + +import ( + "testing" + + "github.com/decred/dcrd/blockchain/v3/chaingen" + "github.com/decred/dcrd/chaincfg/v3" + "github.com/decred/dcrd/dcrutil/v3" + "github.com/decred/dcrd/wire" +) + +// TestFetchUtxoView ensures fetching a utxo viewpoint for the current tip block +// works as intended for various combinations of disapproval states for the tip +// block itself as well as when the tip block disapproves its parent. +func TestFetchUtxoView(t *testing.T) { + const ( + // voteBitNo and voteBitYes represent no and yes votes, respectively, on + // whether or not to approve the previous block. + voteBitNo = 0x0000 + voteBitYes = 0x0001 + ) + + // Create a test harness initialized with the genesis block as the tip. + params := chaincfg.RegNetParams() + g, teardownFunc := newChaingenHarness(t, params, "fetchutxoviewtest") + defer teardownFunc() + + // --------------------------------------------------------------------- + // Create some convenience functions to improve test readability. + // --------------------------------------------------------------------- + + // fetchUtxoViewTipApproved calls FetchUtxoView method with the flag set to + // indicate that the transactions in the regular transaction tree of the tip + // should be included. + fetchUtxoViewTipApproved := func(tx *dcrutil.Tx) (*UtxoViewpoint, error) { + return g.chain.FetchUtxoView(tx, true) + } + + // fetchUtxoViewTipDisapproved calls FetchUtxoView method with the flag set + // to indicate that the transactions in the regular transaction tree of the + // tip should NOT be included. + fetchUtxoViewTipDisapproved := func(tx *dcrutil.Tx) (*UtxoViewpoint, error) { + return g.chain.FetchUtxoView(tx, false) + } + + // testInputsSpent checks the provided view considers all of the transaction + // outputs referenced by the inputs of the provided transaction spent + // according to the given flag. + testInputsSpent := func(view *UtxoViewpoint, tx *dcrutil.Tx, spent bool) { + t.Helper() + + for txInIdx, txIn := range tx.MsgTx().TxIn { + prevOut := &txIn.PreviousOutPoint + entry := view.LookupEntry(&prevOut.Hash) + gotSpent := entry == nil || entry.IsOutputSpent(prevOut.Index) + if gotSpent != spent { + t.Fatalf("unexpected spent state for txo %s referenced by "+ + "input %d -- got %v, want %v", prevOut, txInIdx, + gotSpent, spent) + } + } + } + + // --------------------------------------------------------------------- + // Generate and accept enough blocks to reach stake validation height. + // --------------------------------------------------------------------- + + g.AdvanceToStakeValidationHeight() + + // --------------------------------------------------------------------- + // Create block that has a transaction available in its regular tx tree + // to use as a base for the tests below. + // + // ... -> b0 + // --------------------------------------------------------------------- + + outs := g.OldestCoinbaseOuts() + b0 := g.NextBlock("b0", &outs[0], outs[1:]) + g.AcceptTipBlock() + + // Create a transaction that spends from an output in the regular tx tree of + // the tip block. + fee := dcrutil.Amount(1) + b0Tx1Out0 := chaingen.MakeSpendableOut(b0, 1, 0) + spendB0Tx1Out0 := dcrutil.NewTx(g.CreateSpendTx(&b0Tx1Out0, fee)) + + // Ensure that fetching a view for a transaction that spends an output in + // the regular tx tree of the tip block does NOT consider the referenced + // output spent when the regular tx tree of the tip block is treated as + // approved. + view, err := fetchUtxoViewTipApproved(spendB0Tx1Out0) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + testInputsSpent(view, spendB0Tx1Out0, false) + + // Ensure that fetching a view for a transaction that spends an output in + // the regular tx tree of the tip block considers the referenced output + // spent when the regular tx tree of the tip block is treated as disapproved + // since they should be treated as if they do NOT exist. + view, err = fetchUtxoViewTipDisapproved(spendB0Tx1Out0) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + testInputsSpent(view, spendB0Tx1Out0, true) + + // --------------------------------------------------------------------- + // Create block that disapproves the regular tx tree of the prev block + // and does NOT include any disapproved regular transactions from it. + // + // ... -> b0 -> b1 (disapproves b0) + // --------------------------------------------------------------------- + + outs = g.OldestCoinbaseOuts() + g.NextBlock("b1", &outs[0], outs[1:], func(b *wire.MsgBlock) { + b.Header.VoteBits &^= voteBitYes + for i := 0; i < 5; i++ { + g.ReplaceVoteBitsN(i, voteBitNo)(b) + } + }) + g.AssertTipDisapprovesPrevious() + g.AcceptTipBlock() + + // Ensure that fetching a view for a transaction that is in the regular tx + // tree of a disapproved block prior to the tip block when the tip block + // does NOT otherwise spend those outputs does NOT consider the referenced + // outputs spent (because the spend was undone by the disapproval) + // regardless of whether or not the regular tx tree of the tip block is + // treated as approved or disapproved. + b0Tx1 := dcrutil.NewTx(b0.Transactions[1]) + view, err = fetchUtxoViewTipApproved(b0Tx1) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + testInputsSpent(view, b0Tx1, false) + + view, err = fetchUtxoViewTipDisapproved(b0Tx1) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + testInputsSpent(view, b0Tx1, false) + + // Ensure that fetching a view for a transaction that spends an output in + // the regular tx tree of a disapproved block prior to the tip block when + // the tip block does NOT otherwise spend the output considers the output + // spent (since it was undone by the disapproval and therefore does NOT + // exist) regardless of whether or not the regular tx tree of the tip block + // is treated as approved or disapproved. + view, err = fetchUtxoViewTipApproved(spendB0Tx1Out0) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + testInputsSpent(view, spendB0Tx1Out0, true) + + view, err = fetchUtxoViewTipDisapproved(spendB0Tx1Out0) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + testInputsSpent(view, spendB0Tx1Out0, true) + + // --------------------------------------------------------------------- + // Create block that disapproves the regular tx tree of the prev block + // and also includes one of those disapproved regular transactions and + // then reorganize the chain to it. + // + // ... -> b0 -> b1a (disapproves b0) + // \-> b1 (disapproves b0) + // --------------------------------------------------------------------- + + g.SetTip("b0") + b1a := g.NextBlock("b1a", &outs[0], outs[1:], func(b *wire.MsgBlock) { + b.Header.VoteBits &^= voteBitYes + for i := 0; i < 5; i++ { + g.ReplaceVoteBitsN(i, voteBitNo)(b) + } + + b.Transactions[1] = b0Tx1.MsgTx() + }) + g.AssertTipDisapprovesPrevious() + g.AcceptedToSideChainWithExpectedTip("b1") + g.ForceTipReorg("b1", "b1a") + + // Ensure that fetching a view for a transaction that is in the regular tx + // tree of a disapproved block prior to the tip block and is also included + // in the tip block itself considers the referenced outputs spent when the + // regular tx tree of the tip block is treated as approved. + view, err = fetchUtxoViewTipApproved(b0Tx1) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + testInputsSpent(view, b0Tx1, true) + + // Ensure that fetching a view for a transaction that is in the regular tx + // tree of a disapproved block prior to the tip block and is also included + // in the tip block itself does NOT consider the referenced outputs spent + // when the regular tx tree of the tip block is treated as disapproved since + // both instances of the transaction are disapproved. + view, err = fetchUtxoViewTipDisapproved(b0Tx1) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + testInputsSpent(view, b0Tx1, false) + + // Create a transaction that spends from an output in the regular tx tree of + // the tip block. Note that this is spending from the same transaction that + // was disapproved in the block prior to the current tip and included in the + // tip again. + b1aTx1Out0 := chaingen.MakeSpendableOut(b1a, 1, 0) + spendB1aTx1Out0 := dcrutil.NewTx(g.CreateSpendTx(&b1aTx1Out0, fee)) + + // Ensure that fetching a view for a transaction that spends an output in + // the regular tx tree of the tip block that was also included in the block + // the tip disapproves does NOT consider the referenced output spent when + // the regular tx tree of the tip block is treated as approved. + view, err = fetchUtxoViewTipApproved(spendB1aTx1Out0) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + testInputsSpent(view, spendB1aTx1Out0, false) + + // Ensure that fetching a view for a transaction that spends an output in + // the regular tx tree of the tip block that was also included in the block + // the tip disapproves consider the referenced output spent when the regular + // tx tree of the tip block is treated as disapproved. + view, err = fetchUtxoViewTipDisapproved(spendB1aTx1Out0) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + testInputsSpent(view, spendB1aTx1Out0, true) +}