diff --git a/wtxmgr/query_test.go b/wtxmgr/query_test.go index f5a2f50..bd2acf1 100644 --- a/wtxmgr/query_test.go +++ b/wtxmgr/query_test.go @@ -7,6 +7,7 @@ package wtxmgr_test import ( "bytes" "encoding/binary" + "errors" "fmt" "testing" "time" @@ -64,12 +65,8 @@ func deepCopyTxDetails(d *TxDetails) *TxDetails { return &cpy } -func (q *queryState) compare(t *testing.T, s *Store, ns walletdb.ReadBucket, changeDesc string) { - defer func() { - if t.Failed() { - t.Fatalf("Store state queries failed after '%s'", changeDesc) - } - }() +func (q *queryState) compare(s *Store, ns walletdb.ReadBucket, + changeDesc string) error { fwdBlocks := q.blocks revBlocks := make([][]TxDetails, len(q.blocks)) @@ -80,17 +77,22 @@ func (q *queryState) compare(t *testing.T, s *Store, ns walletdb.ReadBucket, cha checkBlock := func(blocks [][]TxDetails) func([]TxDetails) (bool, error) { return func(got []TxDetails) (bool, error) { if len(fwdBlocks) == 0 { - return false, fmt.Errorf("entered range when no more details expected") + return false, errors.New("entered range " + + "when no more details expected") } exp := blocks[0] if len(got) != len(exp) { - return false, fmt.Errorf("got len(details)=%d in transaction range, expected %d", len(got), len(exp)) + return false, fmt.Errorf("got len(details)=%d "+ + "in transaction range, expected %d", + len(got), len(exp)) } for i := range got { - equalTxDetails(t, &got[i], &exp[i]) - } - if t.Failed() { - return false, fmt.Errorf("Failed comparing range of transaction details") + err := equalTxDetails(&got[i], &exp[i]) + if err != nil { + return false, fmt.Errorf("failed "+ + "comparing range of "+ + "transaction details: %v", err) + } } blocks = blocks[1:] return false, nil @@ -98,11 +100,13 @@ func (q *queryState) compare(t *testing.T, s *Store, ns walletdb.ReadBucket, cha } err := s.RangeTransactions(ns, 0, -1, checkBlock(fwdBlocks)) if err != nil { - t.Fatalf("Failed in RangeTransactions (forwards iteration): %v", err) + return fmt.Errorf("%s: failed in RangeTransactions (forwards "+ + "iteration): %v", changeDesc, err) } err = s.RangeTransactions(ns, -1, 0, checkBlock(revBlocks)) if err != nil { - t.Fatalf("Failed in RangeTransactions (reverse iteration): %v", err) + return fmt.Errorf("%s: failed in RangeTransactions (reverse "+ + "iteration): %v", changeDesc, err) } for txHash, details := range q.txDetails { @@ -113,16 +117,18 @@ func (q *queryState) compare(t *testing.T, s *Store, ns walletdb.ReadBucket, cha } d, err := s.UniqueTxDetails(ns, &txHash, blk) if err != nil { - t.Fatal(err) + return err } if d == nil { - t.Errorf("Found no matching transaction at height %d", detail.Block.Height) - continue + return fmt.Errorf("found no matching "+ + "transaction at height %d", + detail.Block.Height) + } + if err := equalTxDetails(d, &detail); err != nil { + return fmt.Errorf("%s: failed querying latest "+ + "details regarding transaction %v", + changeDesc, txHash) } - equalTxDetails(t, d, &detail) - } - if t.Failed() { - t.Fatalf("Failed querying unique details regarding transaction %v", txHash) } // For the most recent tx with this hash, check that @@ -131,79 +137,80 @@ func (q *queryState) compare(t *testing.T, s *Store, ns walletdb.ReadBucket, cha detail := &details[len(details)-1] d, err := s.TxDetails(ns, &txHash) if err != nil { - t.Fatal(err) + return err } - equalTxDetails(t, d, detail) - if t.Failed() { - t.Fatalf("Failed querying latest details regarding transaction %v", txHash) + if err := equalTxDetails(d, detail); err != nil { + return fmt.Errorf("%s: failed querying latest details "+ + "regarding transaction %v", changeDesc, txHash) } } + + return nil } -func equalTxDetails(t *testing.T, got, exp *TxDetails) { +func equalTxDetails(got, exp *TxDetails) error { // Need to avoid using reflect.DeepEqual against slices, since it // returns false for nil vs non-nil zero length slices. + if err := equalTxs(&got.MsgTx, &exp.MsgTx); err != nil { + return err + } - equalTxs(t, &got.MsgTx, &exp.MsgTx) if got.Hash != exp.Hash { - t.Errorf("Found mismatched hashes") - t.Errorf("Got: %v", got.Hash) - t.Errorf("Expected: %v", exp.Hash) + return fmt.Errorf("found mismatched hashes: got %v, expected %v", + got.Hash, exp.Hash) } if got.Received != exp.Received { - t.Errorf("Found mismatched receive time") - t.Errorf("Got: %v", got.Received) - t.Errorf("Expected: %v", exp.Received) + return fmt.Errorf("found mismatched receive time: got %v, "+ + "expected %v", got.Received, exp.Received) } if !bytes.Equal(got.SerializedTx, exp.SerializedTx) { - t.Errorf("Found mismatched serialized txs") - t.Errorf("Got: %x", got.SerializedTx) - t.Errorf("Expected: %x", exp.SerializedTx) + return fmt.Errorf("found mismatched serialized txs: got %v, "+ + "expected %v", got.SerializedTx, exp.SerializedTx) } if got.Block != exp.Block { - t.Errorf("Found mismatched block meta") - t.Errorf("Got: %v", got.Block) - t.Errorf("Expected: %v", exp.Block) + return fmt.Errorf("found mismatched block meta: got %v, "+ + "expected %v", got.Block, exp.Block) } if len(got.Credits) != len(exp.Credits) { - t.Errorf("Credit slice lengths differ: Got %d Expected %d", len(got.Credits), len(exp.Credits)) - } else { - for i := range got.Credits { - if got.Credits[i] != exp.Credits[i] { - t.Errorf("Found mismatched Credit[%d]", i) - t.Errorf("Got: %v", got.Credits[i]) - t.Errorf("Expected: %v", exp.Credits[i]) - } + return fmt.Errorf("credit slice lengths differ: got %d, "+ + "expected %d", len(got.Credits), len(exp.Credits)) + } + for i := range got.Credits { + if got.Credits[i] != exp.Credits[i] { + return fmt.Errorf("found mismatched credit[%d]: got %v, "+ + "expected %v", i, got.Credits[i], exp.Credits[i]) } } if len(got.Debits) != len(exp.Debits) { - t.Errorf("Debit slice lengths differ: Got %d Expected %d", len(got.Debits), len(exp.Debits)) - } else { - for i := range got.Debits { - if got.Debits[i] != exp.Debits[i] { - t.Errorf("Found mismatched Debit[%d]", i) - t.Errorf("Got: %v", got.Debits[i]) - t.Errorf("Expected: %v", exp.Debits[i]) - } + return fmt.Errorf("debit slice lengths differ: got %d, "+ + "expected %d", len(got.Debits), len(exp.Debits)) + } + for i := range got.Debits { + if got.Debits[i] != exp.Debits[i] { + return fmt.Errorf("found mismatched debit[%d]: got %v, "+ + "expected %v", i, got.Debits[i], exp.Debits[i]) } } + + return nil } -func equalTxs(t *testing.T, got, exp *wire.MsgTx) { +func equalTxs(got, exp *wire.MsgTx) error { var bufGot, bufExp bytes.Buffer err := got.Serialize(&bufGot) if err != nil { - t.Fatal(err) + return err } err = exp.Serialize(&bufExp) if err != nil { - t.Fatal(err) + return err } if !bytes.Equal(bufGot.Bytes(), bufExp.Bytes()) { - t.Errorf("Found unexpected wire.MsgTx:") - t.Errorf("Got: %v", got) - t.Errorf("Expected: %v", exp) + return fmt.Errorf("found unexpected wire.MsgTx: got: %v, "+ + "expected %v", got, exp) } + + return nil } // Returns time.Now() with seconds resolution, this is what Store saves. @@ -238,7 +245,7 @@ func TestStoreQueries(t *testing.T) { type queryTest struct { desc string - updates func(ns walletdb.ReadWriteBucket) // Unwinds from t.Fatal if the update errors. + updates func(ns walletdb.ReadWriteBucket) error state *queryState } var tests []queryTest @@ -252,40 +259,16 @@ func TestStoreQueries(t *testing.T) { lastState := newQueryState() tests = append(tests, queryTest{ desc: "initial store", - updates: func(walletdb.ReadWriteBucket) {}, + updates: func(walletdb.ReadWriteBucket) error { return nil }, state: lastState, }) - // simplify error handling - insertTx := func(ns walletdb.ReadWriteBucket, rec *TxRecord, block *BlockMeta) { - err := s.InsertTx(ns, rec, block) - if err != nil { - t.Fatal(err) - } - } - addCredit := func(s *Store, ns walletdb.ReadWriteBucket, rec *TxRecord, block *BlockMeta, index uint32, change bool) { - err := s.AddCredit(ns, rec, block, index, change) - if err != nil { - t.Fatal(err) - } - } - newTxRecordFromMsgTx := func(tx *wire.MsgTx, received time.Time) *TxRecord { - rec, err := NewTxRecordFromMsgTx(tx, received) - if err != nil { - t.Fatal(err) - } - return rec - } - rollback := func(ns walletdb.ReadWriteBucket, height int32) { - err := s.Rollback(ns, height) - if err != nil { - t.Fatal(err) - } - } - // Insert an unmined transaction. Mark no credits yet. txA := spendOutput(&chainhash.Hash{}, 0, 100e8) - recA := newTxRecordFromMsgTx(txA, timeNow()) + recA, err := NewTxRecordFromMsgTx(txA, timeNow()) + if err != nil { + t.Fatal(err) + } newState := lastState.deepCopy() newState.blocks = [][]TxDetails{ { @@ -300,9 +283,11 @@ func TestStoreQueries(t *testing.T) { } lastState = newState tests = append(tests, queryTest{ - desc: "insert tx A unmined", - updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recA, nil) }, - state: newState, + desc: "insert tx A unmined", + updates: func(ns walletdb.ReadWriteBucket) error { + return s.InsertTx(ns, recA, nil) + }, + state: newState, }) // Add txA:0 as a change credit. @@ -318,15 +303,20 @@ func TestStoreQueries(t *testing.T) { newState.txDetails[recA.Hash][0].Credits = newState.blocks[0][0].Credits lastState = newState tests = append(tests, queryTest{ - desc: "mark unconfirmed txA:0 as credit", - updates: func(ns walletdb.ReadWriteBucket) { addCredit(s, ns, recA, nil, 0, true) }, - state: newState, + desc: "mark unconfirmed txA:0 as credit", + updates: func(ns walletdb.ReadWriteBucket) error { + return s.AddCredit(ns, recA, nil, 0, true) + }, + state: newState, }) // Insert another unmined transaction which spends txA:0, splitting the // amount into outputs of 40 and 60 BTC. txB := spendOutput(&recA.Hash, 0, 40e8, 60e8) - recB := newTxRecordFromMsgTx(txB, timeNow()) + recB, err := NewTxRecordFromMsgTx(txB, timeNow()) + if err != nil { + t.Fatal(err) + } newState = lastState.deepCopy() newState.blocks[0][0].Credits[0].Spent = true newState.blocks[0] = append(newState.blocks[0], TxDetails{ @@ -343,9 +333,11 @@ func TestStoreQueries(t *testing.T) { newState.txDetails[recB.Hash] = []TxDetails{newState.blocks[0][1]} lastState = newState tests = append(tests, queryTest{ - desc: "insert tx B unmined", - updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recB, nil) }, - state: newState, + desc: "insert tx B unmined", + updates: func(ns walletdb.ReadWriteBucket) error { + return s.InsertTx(ns, recB, nil) + }, + state: newState, }) newState = lastState.deepCopy() newState.blocks[0][1].Credits = []CreditRecord{ @@ -359,9 +351,11 @@ func TestStoreQueries(t *testing.T) { newState.txDetails[recB.Hash][0].Credits = newState.blocks[0][1].Credits lastState = newState tests = append(tests, queryTest{ - desc: "mark txB:0 as non-change credit", - updates: func(ns walletdb.ReadWriteBucket) { addCredit(s, ns, recB, nil, 0, false) }, - state: newState, + desc: "mark txB:0 as non-change credit", + updates: func(ns walletdb.ReadWriteBucket) error { + return s.AddCredit(ns, recB, nil, 0, false) + }, + state: newState, }) // Mine tx A at block 100. Leave tx B unmined. @@ -373,9 +367,11 @@ func TestStoreQueries(t *testing.T) { newState.txDetails[recA.Hash][0].Block = b100 lastState = newState tests = append(tests, queryTest{ - desc: "mine tx A", - updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recA, &b100) }, - state: newState, + desc: "mine tx A", + updates: func(ns walletdb.ReadWriteBucket) error { + return s.InsertTx(ns, recA, &b100) + }, + state: newState, }) // Mine tx B at block 101. @@ -385,17 +381,20 @@ func TestStoreQueries(t *testing.T) { newState.txDetails[recB.Hash][0].Block = b101 lastState = newState tests = append(tests, queryTest{ - desc: "mine tx B", - updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recB, &b101) }, - state: newState, + desc: "mine tx B", + updates: func(ns walletdb.ReadWriteBucket) error { + return s.InsertTx(ns, recB, &b101) + }, + state: newState, }) for _, tst := range tests { err := walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { ns := tx.ReadWriteBucket(namespaceKey) - tst.updates(ns) - tst.state.compare(t, s, ns, tst.desc) - return nil + if err := tst.updates(ns); err != nil { + return err + } + return tst.state.compare(s, ns, tst.desc) }) if err != nil { t.Fatal(err) @@ -414,14 +413,18 @@ func TestStoreQueries(t *testing.T) { ns := tx.ReadWriteBucket(namespaceKey) missingTx := spendOutput(&recB.Hash, 0, 40e8) - missingRec := newTxRecordFromMsgTx(missingTx, timeNow()) + missingRec, err := NewTxRecordFromMsgTx(missingTx, timeNow()) + if err != nil { + return err + } missingBlock := makeBlockMeta(102) missingDetails, err := s.TxDetails(ns, &missingRec.Hash) if err != nil { - t.Fatal(err) + return err } if missingDetails != nil { - t.Errorf("Expected no details, found details for tx %v", missingDetails.Hash) + return fmt.Errorf("Expected no details, found details "+ + "for tx %v", missingDetails.Hash) } missingUniqueTests := []struct { hash *chainhash.Hash @@ -461,7 +464,9 @@ func TestStoreQueries(t *testing.T) { t.Errorf("RangeTransactions (reverse) ran func %d times", iterations) } // Make sure it also breaks early after one iteration through unmined transactions. - rollback(ns, b101.Height) + if err := s.Rollback(ns, b101.Height); err != nil { + return err + } iterations = 0 err = s.RangeTransactions(ns, -1, 0, func([]TxDetails) (bool, error) { iterations++ @@ -487,9 +492,11 @@ func TestStoreQueries(t *testing.T) { newState.txDetails[recB.Hash][0].Block = b100 lastState = newState tests = append(tests[:0:0], queryTest{ - desc: "move tx B to block 100", - updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recB, &b100) }, - state: newState, + desc: "move tx B to block 100", + updates: func(ns walletdb.ReadWriteBucket) error { + return s.InsertTx(ns, recB, &b100) + }, + state: newState, }) newState = lastState.deepCopy() newState.blocks[0][0].Block = makeBlockMeta(-1) @@ -498,58 +505,20 @@ func TestStoreQueries(t *testing.T) { newState.txDetails[recB.Hash][0].Block = makeBlockMeta(-1) lastState = newState tests = append(tests, queryTest{ - desc: "rollback block 100", - updates: func(ns walletdb.ReadWriteBucket) { rollback(ns, b100.Height) }, - state: newState, - }) - - // None of the above tests have tested transactions with colliding - // hashes, so mine tx A in block 100, and then insert tx A again - // unmined. Also mine tx A in block 101 (this moves it from unmined). - // This is a valid test because the store does not perform signature - // validation or keep a full utxo set, and duplicated transaction hashes - // from different blocks are allowed so long as all previous outputs are - // spent. - newState = lastState.deepCopy() - newState.blocks = append(newState.blocks, newState.blocks[0][1:]) - newState.blocks[0] = newState.blocks[0][:1:1] - newState.blocks[0][0].Block = b100 - newState.blocks[1] = []TxDetails{ - { - TxRecord: *stripSerializedTx(recA), - Block: makeBlockMeta(-1), + desc: "rollback block 100", + updates: func(ns walletdb.ReadWriteBucket) error { + return s.Rollback(ns, b100.Height) }, - newState.blocks[1][0], - } - newState.txDetails[recA.Hash][0].Block = b100 - newState.txDetails[recA.Hash] = append(newState.txDetails[recA.Hash], newState.blocks[1][0]) - lastState = newState - tests = append(tests, queryTest{ - desc: "insert duplicate tx A", - updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recA, &b100); insertTx(ns, recA, nil) }, - state: newState, - }) - newState = lastState.deepCopy() - newState.blocks = [][]TxDetails{ - newState.blocks[0], - {newState.blocks[1][0]}, - {newState.blocks[1][1]}, - } - newState.blocks[1][0].Block = b101 - newState.txDetails[recA.Hash][1].Block = b101 - lastState = newState - tests = append(tests, queryTest{ - desc: "mine duplicate tx A", - updates: func(ns walletdb.ReadWriteBucket) { insertTx(ns, recA, &b101) }, - state: newState, + state: newState, }) for _, tst := range tests { err := walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { ns := tx.ReadWriteBucket(namespaceKey) - tst.updates(ns) - tst.state.compare(t, s, ns, tst.desc) - return nil + if err := tst.updates(ns); err != nil { + return err + } + return tst.state.compare(s, ns, tst.desc) }) if err != nil { t.Fatal(err) diff --git a/wtxmgr/tx.go b/wtxmgr/tx.go index a9b9651..0b232ea 100644 --- a/wtxmgr/tx.go +++ b/wtxmgr/tx.go @@ -405,10 +405,16 @@ func (s *Store) AddCredit(ns walletdb.ReadWriteBucket, rec *TxRecord, block *Blo // duplicate (false). func (s *Store) addCredit(ns walletdb.ReadWriteBucket, rec *TxRecord, block *BlockMeta, index uint32, change bool) (bool, error) { if block == nil { + // If the outpoint that we should mark as credit already exists + // within the store, either as unconfirmed or confirmed, then we + // have nothing left to do and can exit. k := canonicalOutPoint(&rec.Hash, index) if existsRawUnminedCredit(ns, k) != nil { return false, nil } + if existsRawUnspent(ns, k) != nil { + return false, nil + } v := valueUnminedCredit(btcutil.Amount(rec.MsgTx.TxOut[index].Value), change) return true, putRawUnminedCredit(ns, k, v) } diff --git a/wtxmgr/tx_test.go b/wtxmgr/tx_test.go index 56873fc..4125849 100644 --- a/wtxmgr/tx_test.go +++ b/wtxmgr/tx_test.go @@ -64,17 +64,18 @@ func testStore() (*Store, walletdb.DB, func(), error) { if err != nil { return nil, nil, func() {}, err } + db, err := walletdb.Create("bdb", filepath.Join(tmpDir, "db")) if err != nil { - teardown := func() { - os.RemoveAll(tmpDir) - } - return nil, nil, teardown, err + os.RemoveAll(tmpDir) + return nil, nil, nil, err } + teardown := func() { db.Close() os.RemoveAll(tmpDir) } + var s *Store err = walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { ns, err := tx.CreateTopLevelBucket(namespaceKey) @@ -88,6 +89,7 @@ func testStore() (*Store, walletdb.DB, func(), error) { s, err = Open(ns, &chaincfg.TestNet3Params) return err }) + return s, db, teardown, err } @@ -489,10 +491,10 @@ func TestInsertsCreditsDebitsRollbacks(t *testing.T) { } s, db, teardown, err := testStore() - defer teardown() if err != nil { t.Fatal(err) } + defer teardown() for _, test := range tests { err := walletdb.Update(db, func(tx walletdb.ReadWriteTx) error { @@ -560,10 +562,10 @@ func TestFindingSpentCredits(t *testing.T) { t.Parallel() s, db, teardown, err := testStore() - defer teardown() if err != nil { t.Fatal(err) } + defer teardown() dbtx, err := db.BeginReadWriteTx() if err != nil { @@ -667,10 +669,10 @@ func TestCoinbases(t *testing.T) { t.Parallel() s, db, teardown, err := testStore() - defer teardown() if err != nil { t.Fatal(err) } + defer teardown() dbtx, err := db.BeginReadWriteTx() if err != nil { @@ -1073,10 +1075,10 @@ func TestMoveMultipleToSameBlock(t *testing.T) { t.Parallel() s, db, teardown, err := testStore() - defer teardown() if err != nil { t.Fatal(err) } + defer teardown() dbtx, err := db.BeginReadWriteTx() if err != nil { @@ -1250,10 +1252,10 @@ func TestInsertUnserializedTx(t *testing.T) { t.Parallel() s, db, teardown, err := testStore() - defer teardown() if err != nil { t.Fatal(err) } + defer teardown() dbtx, err := db.BeginReadWriteTx() if err != nil { @@ -1317,10 +1319,10 @@ func TestRemoveUnminedTx(t *testing.T) { t.Parallel() store, db, teardown, err := testStore() - defer teardown() if err != nil { t.Fatal(err) } + defer teardown() dbtx, err := db.BeginReadWriteTx() if err != nil { @@ -1382,6 +1384,8 @@ func TestRemoveUnminedTx(t *testing.T) { func commitDBTx(t *testing.T, store *Store, db walletdb.DB, f func(walletdb.ReadWriteBucket)) { + t.Helper() + dbTx, err := db.BeginReadWriteTx() if err != nil { t.Fatal(err) @@ -1401,10 +1405,10 @@ func commitDBTx(t *testing.T, store *Store, db walletdb.DB, // removed from the wallet's store. func testInsertMempoolDoubleSpendTx(t *testing.T, first bool) { store, db, teardown, err := testStore() - defer teardown() if err != nil { t.Fatal(err) } + defer teardown() // In order to reproduce real-world scenarios, we'll use a new database // transaction for each interaction with the wallet. @@ -1559,10 +1563,10 @@ func TestInsertConfirmedDoubleSpendTx(t *testing.T) { t.Parallel() store, db, teardown, err := testStore() - defer teardown() if err != nil { t.Fatal(err) } + defer teardown() // In order to reproduce real-world scenarios, we'll use a new database // transaction for each interaction with the wallet. @@ -1724,3 +1728,137 @@ func TestInsertConfirmedDoubleSpendTx(t *testing.T) { } }) } + +// TestAddDuplicateCreditAfterConfirm aims to test the case where a duplicate +// unconfirmed credit is added to the store after the intial credit has already +// confirmed. This can lead to outputs being duplicated in the store, which can +// lead to creating double spends when querying the wallet's UTXO set. +func TestAddDuplicateCreditAfterConfirm(t *testing.T) { + t.Parallel() + + store, db, teardown, err := testStore() + if err != nil { + t.Fatal(err) + } + defer teardown() + + // In order to reproduce real-world scenarios, we'll use a new database + // transaction for each interaction with the wallet. + // + // We'll start off the test by creating a new coinbase output at height + // 100 and inserting it into the store. + b100 := &BlockMeta{ + Block: Block{Height: 100}, + Time: time.Now(), + } + cb := newCoinBase(1e8) + cbRec, err := NewTxRecordFromMsgTx(cb, b100.Time) + if err != nil { + t.Fatal(err) + } + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + if err := store.InsertTx(ns, cbRec, b100); err != nil { + t.Fatal(err) + } + err := store.AddCredit(ns, cbRec, b100, 0, false) + if err != nil { + t.Fatal(err) + } + }) + + // We'll confirm that there is one unspent output in the store, which + // should be the coinbase output created above. + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + minedTxs, err := store.UnspentOutputs(ns) + if err != nil { + t.Fatal(err) + } + if len(minedTxs) != 1 { + t.Fatalf("expected 1 mined tx, got %v", len(minedTxs)) + } + if !minedTxs[0].Hash.IsEqual(&cbRec.Hash) { + t.Fatalf("expected tx hash %v, got %v", cbRec.Hash, + minedTxs[0].Hash) + } + }) + + // Then, we'll create an unconfirmed spend for the coinbase output. + b101 := &BlockMeta{ + Block: Block{Height: 101}, + Time: time.Now(), + } + spendTx := spendOutput(&cbRec.Hash, 0, 5e7, 4e7) + spendTxRec, err := NewTxRecordFromMsgTx(spendTx, b101.Time) + if err != nil { + t.Fatal(err) + } + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + if err := store.InsertTx(ns, spendTxRec, nil); err != nil { + t.Fatal(err) + } + err := store.AddCredit(ns, spendTxRec, nil, 1, true) + if err != nil { + t.Fatal(err) + } + }) + + // Confirm the spending transaction at the next height. + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + if err := store.InsertTx(ns, spendTxRec, b101); err != nil { + t.Fatal(err) + } + err := store.AddCredit(ns, spendTxRec, b101, 1, true) + if err != nil { + t.Fatal(err) + } + }) + + // We should see one unspent output within the store once again, this + // time being the change output of the spending transaction. + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + minedTxs, err := store.UnspentOutputs(ns) + if err != nil { + t.Fatal(err) + } + if len(minedTxs) != 1 { + t.Fatalf("expected 1 mined txs, got %v", len(minedTxs)) + } + if !minedTxs[0].Hash.IsEqual(&spendTxRec.Hash) { + t.Fatalf("expected tx hash %v, got %v", spendTxRec.Hash, + minedTxs[0].Hash) + } + }) + + // Now, we'll insert the spending transaction once again, this time as + // unconfirmed. This can happen if the backend happens to forward an + // unconfirmed chain.RelevantTx notification to the client even after it + // has confirmed, which results in us adding it to the store once again. + // + // TODO(wilmer): ideally this shouldn't happen, so we should identify + // the real reason for this. + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + if err := store.InsertTx(ns, spendTxRec, nil); err != nil { + t.Fatal(err) + } + err := store.AddCredit(ns, spendTxRec, nil, 1, true) + if err != nil { + t.Fatal(err) + } + }) + + // Finally, we'll ensure the change output is still the only unspent + // output within the store. + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + minedTxs, err := store.UnspentOutputs(ns) + if err != nil { + t.Fatal(err) + } + if len(minedTxs) != 1 { + t.Fatalf("expected 1 mined txs, got %v", len(minedTxs)) + } + if !minedTxs[0].Hash.IsEqual(&spendTxRec.Hash) { + t.Fatalf("expected tx hash %v, got %v", spendTxRec.Hash, + minedTxs[0].Hash) + } + }) +} diff --git a/wtxmgr/unconfirmed.go b/wtxmgr/unconfirmed.go index 812d2d2..37bf7e1 100644 --- a/wtxmgr/unconfirmed.go +++ b/wtxmgr/unconfirmed.go @@ -14,12 +14,25 @@ import ( // insertMemPoolTx inserts the unmined transaction record. It also marks // previous outputs referenced by the inputs as spent. func (s *Store) insertMemPoolTx(ns walletdb.ReadWriteBucket, rec *TxRecord) error { - v := existsRawUnmined(ns, rec.Hash[:]) - if v != nil { - // TODO: compare serialized txs to ensure this isn't a hash collision? + // Check whether the transaction has already been added to the + // unconfirmed bucket. + if existsRawUnmined(ns, rec.Hash[:]) != nil { + // TODO: compare serialized txs to ensure this isn't a hash + // collision? return nil } + // Since transaction records within the store are keyed by their + // transaction _and_ block confirmation, we'll iterate through the + // transaction's outputs to determine if we've already seen them to + // prevent from adding this transaction to the unconfirmed bucket. + for i := range rec.MsgTx.TxOut { + k := canonicalOutPoint(&rec.Hash, uint32(i)) + if existsRawUnspent(ns, k) != nil { + return nil + } + } + log.Infof("Inserting unconfirmed transaction %v", rec.Hash) v, err := valueTxRecord(rec) if err != nil {