From 15cec7d90d2cc3b93ec435f7804c6ad7795cecb5 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 8 Jun 2018 18:52:25 -0700 Subject: [PATCH 1/4] wtxmgr: add transaction replacement and double spend tests In this commit, we add a set of double spend tests to ensure that we can properly detect and handle them. At this point, we do not do this, but a follow up commit will address this. --- wtxmgr/tx_test.go | 360 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 360 insertions(+) diff --git a/wtxmgr/tx_test.go b/wtxmgr/tx_test.go index 388436c..56873fc 100644 --- a/wtxmgr/tx_test.go +++ b/wtxmgr/tx_test.go @@ -651,6 +651,18 @@ func spendOutput(txHash *chainhash.Hash, index uint32, outputValues ...int64) *w return &tx } +func spendOutputs(outputs []wire.OutPoint, outputValues ...int64) *wire.MsgTx { + tx := &wire.MsgTx{} + for _, output := range outputs { + tx.TxIn = append(tx.TxIn, &wire.TxIn{PreviousOutPoint: output}) + } + for _, value := range outputValues { + tx.TxOut = append(tx.TxOut, &wire.TxOut{Value: value}) + } + + return tx +} + func TestCoinbases(t *testing.T) { t.Parallel() @@ -1364,3 +1376,351 @@ func TestRemoveUnminedTx(t *testing.T) { len(unminedTxns)) } } + +// commitDBTx is a helper function that allows us to perform multiple operations +// on a specific database's bucket as a single atomic operation. +func commitDBTx(t *testing.T, store *Store, db walletdb.DB, + f func(walletdb.ReadWriteBucket)) { + + dbTx, err := db.BeginReadWriteTx() + if err != nil { + t.Fatal(err) + } + defer dbTx.Commit() + + ns := dbTx.ReadWriteBucket(namespaceKey) + + f(ns) +} + +// testInsertDoubleSpendTx is a helper test which double spends an output. The +// boolean parameter indicates whether the first spending transaction should be +// the one confirmed. This test ensures that when a double spend occurs and both +// spending transactions are present in the mempool, if one of them confirms, +// then the remaining conflicting transactions within the mempool should be +// 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) + } + + // 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) + } + }) + + // Then, we'll create two spends from the same coinbase output, in order + // to replicate a double spend scenario. + firstSpend := spendOutput(&cbRec.Hash, 0, 5e7, 5e7) + firstSpendRec, err := NewTxRecordFromMsgTx(firstSpend, time.Now()) + if err != nil { + t.Fatal(err) + } + secondSpend := spendOutput(&cbRec.Hash, 0, 4e7, 6e7) + secondSpendRec, err := NewTxRecordFromMsgTx(secondSpend, time.Now()) + if err != nil { + t.Fatal(err) + } + + // We'll insert both of them into the store without confirming them. + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + if err := store.InsertTx(ns, firstSpendRec, nil); err != nil { + t.Fatal(err) + } + err := store.AddCredit(ns, firstSpendRec, nil, 0, false) + if err != nil { + t.Fatal(err) + } + }) + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + if err := store.InsertTx(ns, secondSpendRec, nil); err != nil { + t.Fatal(err) + } + err := store.AddCredit(ns, secondSpendRec, nil, 0, false) + if err != nil { + t.Fatal(err) + } + }) + + // Ensure that both spends are found within the unconfirmed transactions + // in the wallet's store. + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + unminedTxs, err := store.UnminedTxs(ns) + if err != nil { + t.Fatal(err) + } + if len(unminedTxs) != 2 { + t.Fatalf("expected 2 unmined txs, got %v", + len(unminedTxs)) + } + }) + + // Then, we'll confirm either the first or second spend, depending on + // the boolean passed, with a height deep enough that allows us to + // succesfully spend the coinbase output. + coinbaseMaturity := int32(chaincfg.TestNet3Params.CoinbaseMaturity) + bMaturity := BlockMeta{ + Block: Block{Height: b100.Height + coinbaseMaturity}, + Time: time.Now(), + } + + var confirmedSpendRec *wtxmgr.TxRecord + if first { + confirmedSpendRec = firstSpendRec + } else { + confirmedSpendRec = secondSpendRec + } + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + err := store.InsertTx(ns, confirmedSpendRec, &bMaturity) + if err != nil { + t.Fatal(err) + } + err = store.AddCredit( + ns, confirmedSpendRec, &bMaturity, 0, false, + ) + if err != nil { + t.Fatal(err) + } + }) + + // This should now trigger the store to remove any other pending double + // spends for this coinbase output, as we've already seen the confirmed + // one. Therefore, we shouldn't see any other unconfirmed transactions + // within it. We also ensure that the transaction that confirmed and is + // now listed as a UTXO within the wallet is the correct one. + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + unminedTxs, err := store.UnminedTxs(ns) + if err != nil { + t.Fatal(err) + } + if len(unminedTxs) != 0 { + t.Fatalf("expected 0 unmined txs, got %v", + len(unminedTxs)) + } + + 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(&confirmedSpendRec.Hash) { + t.Fatalf("expected confirmed tx hash %v, got %v", + confirmedSpendRec.Hash, minedTxs[0].Hash) + } + }) +} + +// TestInsertMempoolDoubleSpendConfirmedFirstTx ensures that when a double spend +// occurs and both spending transactions are present in the mempool, if the +// first spend seen is confirmed, then the second spend transaction within the +// mempool should be removed from the wallet's store. +func TestInsertMempoolDoubleSpendConfirmedFirstTx(t *testing.T) { + t.Parallel() + testInsertMempoolDoubleSpendTx(t, true) +} + +// TestInsertMempoolDoubleSpendConfirmedFirstTx ensures that when a double spend +// occurs and both spending transactions are present in the mempool, if the +// second spend seen is confirmed, then the first spend transaction within the +// mempool should be removed from the wallet's store. +func TestInsertMempoolDoubleSpendConfirmSecondTx(t *testing.T) { + t.Parallel() + testInsertMempoolDoubleSpendTx(t, false) +} + +// TestInsertConfirmedDoubleSpendTx tests that when one or more double spends +// occur and a spending transaction confirms that was not known to the wallet, +// then the unconfirmed double spends within the mempool should be removed from +// the wallet's store. +func TestInsertConfirmedDoubleSpendTx(t *testing.T) { + t.Parallel() + + store, db, teardown, err := testStore() + defer teardown() + if err != nil { + t.Fatal(err) + } + + // 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(), + } + cb1 := newCoinBase(1e8) + cbRec1, err := NewTxRecordFromMsgTx(cb1, b100.Time) + if err != nil { + t.Fatal(err) + } + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + if err := store.InsertTx(ns, cbRec1, &b100); err != nil { + t.Fatal(err) + } + err := store.AddCredit(ns, cbRec1, &b100, 0, false) + if err != nil { + t.Fatal(err) + } + }) + + // Then, we'll create three spends from the same coinbase output. The + // first two will remain unconfirmed, while the last should confirm and + // remove the remaining unconfirmed from the wallet's store. + firstSpend1 := spendOutput(&cbRec1.Hash, 0, 5e7) + firstSpendRec1, err := NewTxRecordFromMsgTx(firstSpend1, time.Now()) + if err != nil { + t.Fatal(err) + } + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + if err := store.InsertTx(ns, firstSpendRec1, nil); err != nil { + t.Fatal(err) + } + err := store.AddCredit(ns, firstSpendRec1, nil, 0, false) + if err != nil { + t.Fatal(err) + } + }) + + secondSpend1 := spendOutput(&cbRec1.Hash, 0, 4e7) + secondSpendRec1, err := NewTxRecordFromMsgTx(secondSpend1, time.Now()) + if err != nil { + t.Fatal(err) + } + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + if err := store.InsertTx(ns, secondSpendRec1, nil); err != nil { + t.Fatal(err) + } + err := store.AddCredit(ns, secondSpendRec1, nil, 0, false) + if err != nil { + t.Fatal(err) + } + }) + + // We'll also create another output and have one unconfirmed and one + // confirmed spending transaction also spend it. + cb2 := newCoinBase(2e8) + cbRec2, err := NewTxRecordFromMsgTx(cb2, b100.Time) + if err != nil { + t.Fatal(err) + } + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + if err := store.InsertTx(ns, cbRec2, &b100); err != nil { + t.Fatal(err) + } + err := store.AddCredit(ns, cbRec2, &b100, 0, false) + if err != nil { + t.Fatal(err) + } + }) + + firstSpend2 := spendOutput(&cbRec2.Hash, 0, 5e7) + firstSpendRec2, err := NewTxRecordFromMsgTx(firstSpend2, time.Now()) + if err != nil { + t.Fatal(err) + } + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + if err := store.InsertTx(ns, firstSpendRec2, nil); err != nil { + t.Fatal(err) + } + err := store.AddCredit(ns, firstSpendRec2, nil, 0, false) + if err != nil { + t.Fatal(err) + } + }) + + // At this point, we should see all unconfirmed transactions within the + // store. + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + unminedTxs, err := store.UnminedTxs(ns) + if err != nil { + t.Fatal(err) + } + if len(unminedTxs) != 3 { + t.Fatalf("expected 3 unmined txs, got %d", + len(unminedTxs)) + } + }) + + // Then, we'll insert the confirmed spend at a height deep enough that + // allows us to successfully spend the coinbase outputs. + coinbaseMaturity := int32(chaincfg.TestNet3Params.CoinbaseMaturity) + bMaturity := BlockMeta{ + Block: Block{Height: b100.Height + coinbaseMaturity}, + Time: time.Now(), + } + outputsToSpend := []wire.OutPoint{ + {Hash: cbRec1.Hash, Index: 0}, + {Hash: cbRec2.Hash, Index: 0}, + } + confirmedSpend := spendOutputs(outputsToSpend, 3e7) + confirmedSpendRec, err := NewTxRecordFromMsgTx( + confirmedSpend, bMaturity.Time, + ) + if err != nil { + t.Fatal(err) + } + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + err := store.InsertTx(ns, confirmedSpendRec, &bMaturity) + if err != nil { + t.Fatal(err) + } + err = store.AddCredit( + ns, confirmedSpendRec, &bMaturity, 0, false, + ) + if err != nil { + t.Fatal(err) + } + }) + + // Now that the confirmed spend exists within the store, we should no + // longer see the unconfirmed spends within it. We also ensure that the + // transaction that confirmed and is now listed as a UTXO within the + // wallet is the correct one. + commitDBTx(t, store, db, func(ns walletdb.ReadWriteBucket) { + unminedTxs, err := store.UnminedTxs(ns) + if err != nil { + t.Fatal(err) + } + if len(unminedTxs) != 0 { + t.Fatalf("expected 0 unmined txs, got %v", + len(unminedTxs)) + } + + 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(&confirmedSpendRec.Hash) { + t.Fatalf("expected confirmed tx hash %v, got %v", + confirmedSpend, minedTxs[0].Hash) + } + }) +} From 6340c65d141ff460e92d18c45a604cc5ae2211ff Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 8 Jun 2018 18:57:53 -0700 Subject: [PATCH 2/4] wtxmgr: refactor common code within insertMinedTx In this commit, we remove most of the common code between insertMinedTx and moveMinedTx. Now, all the common logic is handled within insertMinedTx, and moveMinedTx only contains its unique logic. --- wtxmgr/tx.go | 217 +++++++++++++++++++-------------------------------- 1 file changed, 79 insertions(+), 138 deletions(-) diff --git a/wtxmgr/tx.go b/wtxmgr/tx.go index 1690fa3..be67677 100644 --- a/wtxmgr/tx.go +++ b/wtxmgr/tx.go @@ -169,72 +169,18 @@ func Create(ns walletdb.ReadWriteBucket) error { // moveMinedTx moves a transaction record from the unmined buckets to block // buckets. -func (s *Store) moveMinedTx(ns walletdb.ReadWriteBucket, rec *TxRecord, recKey, recVal []byte, block *BlockMeta) error { +func (s *Store) moveMinedTx(ns walletdb.ReadWriteBucket, rec *TxRecord, + block *BlockMeta) error { + log.Infof("Marking unconfirmed transaction %v mined in block %d", &rec.Hash, block.Height) - // Insert block record as needed. - blockKey, blockVal := existsBlockRecord(ns, block.Height) - var err error - if blockVal == nil { - blockVal = valueBlockRecord(block, &rec.Hash) - } else { - blockVal, err = appendRawBlockRecord(blockVal, &rec.Hash) - if err != nil { - return err - } - } - err = putRawBlockRecord(ns, blockKey, blockVal) - if err != nil { - return err - } - - err = putRawTxRecord(ns, recKey, recVal) - if err != nil { - return err - } + // Fetch the mined balance in case we need to update it. minedBalance, err := fetchMinedBalance(ns) if err != nil { return err } - // For all transaction inputs, remove the previous output marker from the - // unmined inputs bucket. For any mined transactions with unspent credits - // spent by this transaction, mark each spent, remove from the unspents map, - // and insert a debit record for the spent credit. - debitIncidence := indexedIncidence{ - incidence: incidence{txHash: rec.Hash, block: block.Block}, - // index set for each rec input below. - } - for i, input := range rec.MsgTx.TxIn { - unspentKey, credKey := existsUnspent(ns, &input.PreviousOutPoint) - - err = deleteRawUnminedInput(ns, unspentKey) - if err != nil { - return err - } - - if credKey == nil { - continue - } - - debitIncidence.index = uint32(i) - amt, err := spendCredit(ns, credKey, &debitIncidence) - if err != nil { - return err - } - minedBalance -= amt - err = deleteRawUnspent(ns, unspentKey) - if err != nil { - return err - } - - err = putDebit(ns, &rec.Hash, uint32(i), amt, &block.Block, credKey) - if err != nil { - return err - } - } - // For each output of the record that is marked as a credit, if the // output is marked as a credit by the unconfirmed store, remove the // marker and mark the output as a credit in the db. @@ -246,6 +192,8 @@ func (s *Store) moveMinedTx(ns walletdb.ReadWriteBucket, rec *TxRecord, recKey, block: block.Block, spentBy: indexedIncidence{index: ^uint32(0)}, } + + newMinedBalance := minedBalance it := makeUnminedCreditIterator(ns, &rec.Hash) for it.next() { // TODO: This should use the raw apis. The credit value (it.cv) @@ -260,12 +208,12 @@ func (s *Store) moveMinedTx(ns walletdb.ReadWriteBucket, rec *TxRecord, recKey, if err != nil { return err } + cred.outPoint.Index = index cred.amount = amount cred.change = change - err = putUnspentCredit(ns, &cred) - if err != nil { + if err := putUnspentCredit(ns, &cred); err != nil { return err } err = putUnspent(ns, &cred.outPoint, &block.Block) @@ -273,37 +221,30 @@ func (s *Store) moveMinedTx(ns walletdb.ReadWriteBucket, rec *TxRecord, recKey, return err } - // reposition cursor before deleting, since the above puts have - // invalidated the cursor. - it.reposition(&rec.Hash, index) - - // Avoid cursor deletion until bolt issue #620 is resolved. - // err = it.delete() - // if err != nil { - // return err - // } - - minedBalance += amount + newMinedBalance += amount } if it.err != nil { return it.err } - // Delete all possible credits outside of the iteration since the cursor - // deletion is broken. - for i := 0; i < len(rec.MsgTx.TxOut); i++ { - k := canonicalOutPoint(&rec.Hash, uint32(i)) - err = deleteRawUnminedCredit(ns, k) - if err != nil { + // Update the balance if it has changed. + if newMinedBalance != minedBalance { + if err := putMinedBalance(ns, newMinedBalance); err != nil { return err } } - err = putMinedBalance(ns, minedBalance) - if err != nil { - return err + // Delete all the *now* confirmed credits from the unmined credits + // bucket. We do this outside of the iteration since the cursor deletion + // is broken within boltdb. + for i := range rec.MsgTx.TxOut { + k := canonicalOutPoint(&rec.Hash, uint32(i)) + if err := deleteRawUnminedCredit(ns, k); err != nil { + return err + } } + // Finally, we'll remove the transaction from the unconfirmed bucket. return deleteRawUnmined(ns, rec.Hash[:]) } @@ -331,24 +272,57 @@ func (s *Store) RemoveUnminedTx(ns walletdb.ReadWriteBucket, rec *TxRecord) erro } // insertMinedTx inserts a new transaction record for a mined transaction into -// the database. It is expected that the exact transation does not already -// exist in the unmined buckets, but unmined double spends (including mutations) -// are removed. -func (s *Store) insertMinedTx(ns walletdb.ReadWriteBucket, rec *TxRecord, block *BlockMeta) error { +// the database under the confirmed bucket. It guarantees that, if the +// tranasction was previously unconfirmed, then it will take care of cleaning up +// the unconfirmed state. All other unconfirmed double spend attempts will be +// removed as well. +func (s *Store) insertMinedTx(ns walletdb.ReadWriteBucket, rec *TxRecord, + block *BlockMeta) error { + + // If a transaction record for this hash and block already exists, we + // can exit early. + if _, v := existsTxRecord(ns, &rec.Hash, &block.Block); v != nil { + return nil + } + + // If a block record does not yet exist for any transactions from this + // block, insert a block record first. Otherwise, update it by adding + // the transaction hash to the set of transactions from this block. + var err error + blockKey, blockValue := existsBlockRecord(ns, block.Height) + if blockValue == nil { + err = putBlockRecord(ns, block, &rec.Hash) + } else { + blockValue, err = appendRawBlockRecord(blockValue, &rec.Hash) + if err != nil { + return err + } + err = putRawBlockRecord(ns, blockKey, blockValue) + } + if err != nil { + return err + } + + if err := putTxRecord(ns, rec, &block.Block); err != nil { + return err + } + // Fetch the mined balance in case we need to update it. minedBalance, err := fetchMinedBalance(ns) if err != nil { return err } - // Add a debit record for each unspent credit spent by this tx. + // Add a debit record for each unspent credit spent by this transaction. + // The index is set in each iteration below. spender := indexedIncidence{ incidence: incidence{ txHash: rec.Hash, block: block.Block, }, - // index set for each iteration below } + + newMinedBalance := minedBalance for i, input := range rec.MsgTx.TxIn { unspentKey, credKey := existsUnspent(ns, &input.PreviousOutPoint) if credKey == nil { @@ -370,44 +344,38 @@ func (s *Store) insertMinedTx(ns walletdb.ReadWriteBucket, rec *TxRecord, block // implementation is currently used. continue } + + // If this output is relevant to us, we'll mark the it as spent + // and remove its amount from the store. spender.index = uint32(i) amt, err := spendCredit(ns, credKey, &spender) if err != nil { return err } - err = putDebit(ns, &rec.Hash, uint32(i), amt, &block.Block, - credKey) + err = putDebit( + ns, &rec.Hash, uint32(i), amt, &block.Block, credKey, + ) if err != nil { return err } + if err := deleteRawUnspent(ns, unspentKey); err != nil { + return err + } - minedBalance -= amt + newMinedBalance -= amt + } - err = deleteRawUnspent(ns, unspentKey) - if err != nil { + // Update the balance if it has changed. + if newMinedBalance != minedBalance { + if err := putMinedBalance(ns, newMinedBalance); err != nil { return err } } - // TODO only update if we actually modified the - // mined balance. - err = putMinedBalance(ns, minedBalance) - if err != nil { - return nil - } - - // If a transaction record for this tx hash and block already exist, - // there is nothing left to do. - k, v := existsTxRecord(ns, &rec.Hash, &block.Block) - if v != nil { - return nil - } - - // If the exact tx (not a double spend) is already included but - // unconfirmed, move it to a block. - v = existsRawUnmined(ns, rec.Hash[:]) - if v != nil { - return s.moveMinedTx(ns, rec, k, v, block) + // If this transaction previously existed within the store as + // unconfirmed, we'll need to move it to the confirmed bucket. + if v := existsRawUnmined(ns, rec.Hash[:]); v != nil { + return s.moveMinedTx(ns, rec, block) } // As there may be unconfirmed transactions that are invalidated by this @@ -415,34 +383,7 @@ func (s *Store) insertMinedTx(ns walletdb.ReadWriteBucket, rec *TxRecord, block // from the unconfirmed set. This also handles removing unconfirmed // transaction spend chains if any other unconfirmed transactions spend // outputs of the removed double spend. - err = s.removeDoubleSpends(ns, rec) - if err != nil { - return err - } - - // If a block record does not yet exist for any transactions from this - // block, insert the record. Otherwise, update it by adding the - // transaction hash to the set of transactions from this block. - blockKey, blockValue := existsBlockRecord(ns, block.Height) - if blockValue == nil { - err = putBlockRecord(ns, block, &rec.Hash) - } else { - blockValue, err = appendRawBlockRecord(blockValue, &rec.Hash) - if err != nil { - return err - } - err = putRawBlockRecord(ns, blockKey, blockValue) - } - if err != nil { - return err - } - - err = putTxRecord(ns, rec, &block.Block) - if err != nil { - return err - } - - return nil + return s.removeDoubleSpends(ns, rec) } // AddCredit marks a transaction record as containing a transaction output From ff646086b7ef61ff10f10d7b58c5e5b989e5f64c Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 13 Jul 2018 14:42:43 -0700 Subject: [PATCH 3/4] wtxmgr: split moveMinedTx into deleteUnminedTx and updateMinedBalance In this commit, we slightly refactor the existing moveMinedTx method and split it into two: deleteUnminedTx and updateMinedBalance. We do this as moveMinedTx is no longer moving the transaction from the unmined bucket to the mined, instead it just removes it from the unmined bucket. --- wtxmgr/tx.go | 278 +++++++++++++++++++++++++-------------------------- 1 file changed, 136 insertions(+), 142 deletions(-) diff --git a/wtxmgr/tx.go b/wtxmgr/tx.go index be67677..34cc98d 100644 --- a/wtxmgr/tx.go +++ b/wtxmgr/tx.go @@ -167,146 +167,11 @@ func Create(ns walletdb.ReadWriteBucket) error { return createStore(ns) } -// moveMinedTx moves a transaction record from the unmined buckets to block -// buckets. -func (s *Store) moveMinedTx(ns walletdb.ReadWriteBucket, rec *TxRecord, +// updateMinedBalance updates the mined balance within the store, if changed, +// after processing the given transaction record. +func (s *Store) updateMinedBalance(ns walletdb.ReadWriteBucket, rec *TxRecord, block *BlockMeta) error { - log.Infof("Marking unconfirmed transaction %v mined in block %d", - &rec.Hash, block.Height) - - // Fetch the mined balance in case we need to update it. - minedBalance, err := fetchMinedBalance(ns) - if err != nil { - return err - } - - // For each output of the record that is marked as a credit, if the - // output is marked as a credit by the unconfirmed store, remove the - // marker and mark the output as a credit in the db. - // - // Moved credits are added as unspents, even if there is another - // unconfirmed transaction which spends them. - cred := credit{ - outPoint: wire.OutPoint{Hash: rec.Hash}, - block: block.Block, - spentBy: indexedIncidence{index: ^uint32(0)}, - } - - newMinedBalance := minedBalance - it := makeUnminedCreditIterator(ns, &rec.Hash) - for it.next() { - // TODO: This should use the raw apis. The credit value (it.cv) - // can be moved from unmined directly to the credits bucket. - // The key needs a modification to include the block - // height/hash. - index, err := fetchRawUnminedCreditIndex(it.ck) - if err != nil { - return err - } - amount, change, err := fetchRawUnminedCreditAmountChange(it.cv) - if err != nil { - return err - } - - cred.outPoint.Index = index - cred.amount = amount - cred.change = change - - if err := putUnspentCredit(ns, &cred); err != nil { - return err - } - err = putUnspent(ns, &cred.outPoint, &block.Block) - if err != nil { - return err - } - - newMinedBalance += amount - } - if it.err != nil { - return it.err - } - - // Update the balance if it has changed. - if newMinedBalance != minedBalance { - if err := putMinedBalance(ns, newMinedBalance); err != nil { - return err - } - } - - // Delete all the *now* confirmed credits from the unmined credits - // bucket. We do this outside of the iteration since the cursor deletion - // is broken within boltdb. - for i := range rec.MsgTx.TxOut { - k := canonicalOutPoint(&rec.Hash, uint32(i)) - if err := deleteRawUnminedCredit(ns, k); err != nil { - return err - } - } - - // Finally, we'll remove the transaction from the unconfirmed bucket. - return deleteRawUnmined(ns, rec.Hash[:]) -} - -// InsertTx records a transaction as belonging to a wallet's transaction -// history. If block is nil, the transaction is considered unspent, and the -// transaction's index must be unset. -func (s *Store) InsertTx(ns walletdb.ReadWriteBucket, rec *TxRecord, block *BlockMeta) error { - if block == nil { - return s.insertMemPoolTx(ns, rec) - } - return s.insertMinedTx(ns, rec, block) -} - -// RemoveUnminedTx attempts to remove an unmined transaction from the -// transaction store. This is to be used in the scenario that a transaction -// that we attempt to rebroadcast, turns out to double spend one of our -// existing inputs. This function we remove the conflicting transaction -// identified by the tx record, and also recursively remove all transactions -// that depend on it. -func (s *Store) RemoveUnminedTx(ns walletdb.ReadWriteBucket, rec *TxRecord) error { - // As we already have a tx record, we can directly call the - // removeConflict method. This will do the job of recursively removing - // this unmined transaction, and any transactions that depend on it. - return s.removeConflict(ns, rec) -} - -// insertMinedTx inserts a new transaction record for a mined transaction into -// the database under the confirmed bucket. It guarantees that, if the -// tranasction was previously unconfirmed, then it will take care of cleaning up -// the unconfirmed state. All other unconfirmed double spend attempts will be -// removed as well. -func (s *Store) insertMinedTx(ns walletdb.ReadWriteBucket, rec *TxRecord, - block *BlockMeta) error { - - // If a transaction record for this hash and block already exists, we - // can exit early. - if _, v := existsTxRecord(ns, &rec.Hash, &block.Block); v != nil { - return nil - } - - // If a block record does not yet exist for any transactions from this - // block, insert a block record first. Otherwise, update it by adding - // the transaction hash to the set of transactions from this block. - var err error - blockKey, blockValue := existsBlockRecord(ns, block.Height) - if blockValue == nil { - err = putBlockRecord(ns, block, &rec.Hash) - } else { - blockValue, err = appendRawBlockRecord(blockValue, &rec.Hash) - if err != nil { - return err - } - err = putRawBlockRecord(ns, blockKey, blockValue) - } - if err != nil { - return err - } - - if err := putTxRecord(ns, rec, &block.Block); err != nil { - return err - } - // Fetch the mined balance in case we need to update it. minedBalance, err := fetchMinedBalance(ns) if err != nil { @@ -365,17 +230,146 @@ func (s *Store) insertMinedTx(ns walletdb.ReadWriteBucket, rec *TxRecord, newMinedBalance -= amt } + // For each output of the record that is marked as a credit, if the + // output is marked as a credit by the unconfirmed store, remove the + // marker and mark the output as a credit in the db. + // + // Moved credits are added as unspents, even if there is another + // unconfirmed transaction which spends them. + cred := credit{ + outPoint: wire.OutPoint{Hash: rec.Hash}, + block: block.Block, + spentBy: indexedIncidence{index: ^uint32(0)}, + } + + it := makeUnminedCreditIterator(ns, &rec.Hash) + for it.next() { + // TODO: This should use the raw apis. The credit value (it.cv) + // can be moved from unmined directly to the credits bucket. + // The key needs a modification to include the block + // height/hash. + index, err := fetchRawUnminedCreditIndex(it.ck) + if err != nil { + return err + } + amount, change, err := fetchRawUnminedCreditAmountChange(it.cv) + if err != nil { + return err + } + + cred.outPoint.Index = index + cred.amount = amount + cred.change = change + + if err := putUnspentCredit(ns, &cred); err != nil { + return err + } + err = putUnspent(ns, &cred.outPoint, &block.Block) + if err != nil { + return err + } + + newMinedBalance += amount + } + if it.err != nil { + return it.err + } + // Update the balance if it has changed. if newMinedBalance != minedBalance { - if err := putMinedBalance(ns, newMinedBalance); err != nil { + return putMinedBalance(ns, newMinedBalance) + } + + return nil +} + +// deleteUnminedTx deletes an unmined transaction from the store. +// +// NOTE: This should only be used once the transaction has been mined. +func (s *Store) deleteUnminedTx(ns walletdb.ReadWriteBucket, rec *TxRecord) error { + for i := range rec.MsgTx.TxOut { + k := canonicalOutPoint(&rec.Hash, uint32(i)) + if err := deleteRawUnminedCredit(ns, k); err != nil { return err } } - // If this transaction previously existed within the store as - // unconfirmed, we'll need to move it to the confirmed bucket. + return deleteRawUnmined(ns, rec.Hash[:]) +} + +// InsertTx records a transaction as belonging to a wallet's transaction +// history. If block is nil, the transaction is considered unspent, and the +// transaction's index must be unset. +func (s *Store) InsertTx(ns walletdb.ReadWriteBucket, rec *TxRecord, block *BlockMeta) error { + if block == nil { + return s.insertMemPoolTx(ns, rec) + } + return s.insertMinedTx(ns, rec, block) +} + +// RemoveUnminedTx attempts to remove an unmined transaction from the +// transaction store. This is to be used in the scenario that a transaction +// that we attempt to rebroadcast, turns out to double spend one of our +// existing inputs. This function we remove the conflicting transaction +// identified by the tx record, and also recursively remove all transactions +// that depend on it. +func (s *Store) RemoveUnminedTx(ns walletdb.ReadWriteBucket, rec *TxRecord) error { + // As we already have a tx record, we can directly call the + // removeConflict method. This will do the job of recursively removing + // this unmined transaction, and any transactions that depend on it. + return s.removeConflict(ns, rec) +} + +// insertMinedTx inserts a new transaction record for a mined transaction into +// the database under the confirmed bucket. It guarantees that, if the +// tranasction was previously unconfirmed, then it will take care of cleaning up +// the unconfirmed state. All other unconfirmed double spend attempts will be +// removed as well. +func (s *Store) insertMinedTx(ns walletdb.ReadWriteBucket, rec *TxRecord, + block *BlockMeta) error { + + // If a transaction record for this hash and block already exists, we + // can exit early. + if _, v := existsTxRecord(ns, &rec.Hash, &block.Block); v != nil { + return nil + } + + // If a block record does not yet exist for any transactions from this + // block, insert a block record first. Otherwise, update it by adding + // the transaction hash to the set of transactions from this block. + var err error + blockKey, blockValue := existsBlockRecord(ns, block.Height) + if blockValue == nil { + err = putBlockRecord(ns, block, &rec.Hash) + } else { + blockValue, err = appendRawBlockRecord(blockValue, &rec.Hash) + if err != nil { + return err + } + err = putRawBlockRecord(ns, blockKey, blockValue) + } + if err != nil { + return err + } + if err := putTxRecord(ns, rec, &block.Block); err != nil { + return err + } + + // Determine if this transaction has affected our balance, and if so, + // update it. + if err := s.updateMinedBalance(ns, rec, block); err != nil { + return err + } + + // If this transaction previously existed within the store as unmined, + // we'll need to remove it from the unmined bucket. if v := existsRawUnmined(ns, rec.Hash[:]); v != nil { - return s.moveMinedTx(ns, rec, block) + log.Infof("Marking unconfirmed transaction %v mined in block %d", + &rec.Hash, block.Height) + + if err := s.deleteUnminedTx(ns, rec); err != nil { + return err + } } // As there may be unconfirmed transactions that are invalidated by this From aa826c64cf34173eb9fcbac3dd367a001f798d9d Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 8 Jun 2018 18:59:57 -0700 Subject: [PATCH 4/4] wtxmgr: store multiple spending tx hashes for outpoints In this commit, we modify the way we store spending transaction hashes for unconfirmed spends. Now, rather than only keeping track of one possible unconfirmed spend, we track multiple in order to ensure we properly handle transaction replacements, like in the case of RBF, double spends, etc. With this in, the double spent tests recently added should now pass. --- wtxmgr/db.go | 28 +++++++++++++++++++- wtxmgr/tx.go | 18 ++++++++++--- wtxmgr/unconfirmed.go | 60 ++++++++++++++++++++++++++++--------------- 3 files changed, 80 insertions(+), 26 deletions(-) diff --git a/wtxmgr/db.go b/wtxmgr/db.go index 88dc7a6..0c53b2a 100644 --- a/wtxmgr/db.go +++ b/wtxmgr/db.go @@ -1214,12 +1214,18 @@ func (it *unminedCreditIterator) reposition(txHash *chainhash.Hash, index uint32 // // [0:32] Transaction hash (32 bytes) +// putRawUnminedInput maintains a list of unmined transaction hashes that have +// spent an outpoint. Each entry in the bucket is keyed by the outpoint being +// spent. func putRawUnminedInput(ns walletdb.ReadWriteBucket, k, v []byte) error { - err := ns.NestedReadWriteBucket(bucketUnminedInputs).Put(k, v) + spendTxHashes := ns.NestedReadBucket(bucketUnminedInputs).Get(k) + spendTxHashes = append(spendTxHashes, v...) + err := ns.NestedReadWriteBucket(bucketUnminedInputs).Put(k, spendTxHashes) if err != nil { str := "failed to put unmined input" return storeError(ErrDatabase, str, err) } + return nil } @@ -1227,6 +1233,26 @@ func existsRawUnminedInput(ns walletdb.ReadBucket, k []byte) (v []byte) { return ns.NestedReadBucket(bucketUnminedInputs).Get(k) } +// fetchUnminedInputSpendTxHashes fetches the list of unmined transactions that +// spend the serialized outpoint. +func fetchUnminedInputSpendTxHashes(ns walletdb.ReadBucket, k []byte) []chainhash.Hash { + rawSpendTxHashes := ns.NestedReadBucket(bucketUnminedInputs).Get(k) + if rawSpendTxHashes == nil { + return nil + } + + // Each transaction hash is 32 bytes. + spendTxHashes := make([]chainhash.Hash, 0, len(rawSpendTxHashes)/32) + for len(rawSpendTxHashes) > 0 { + var spendTxHash chainhash.Hash + copy(spendTxHash[:], rawSpendTxHashes[:32]) + spendTxHashes = append(spendTxHashes, spendTxHash) + rawSpendTxHashes = rawSpendTxHashes[32:] + } + + return spendTxHashes +} + func deleteRawUnminedInput(ns walletdb.ReadWriteBucket, k []byte) error { err := ns.NestedReadWriteBucket(bucketUnminedInputs).Delete(k) if err != nil { diff --git a/wtxmgr/tx.go b/wtxmgr/tx.go index 34cc98d..a9b9651 100644 --- a/wtxmgr/tx.go +++ b/wtxmgr/tx.go @@ -664,11 +664,21 @@ func (s *Store) rollback(ns walletdb.ReadWriteBucket, height int32) error { for _, op := range coinBaseCredits { opKey := canonicalOutPoint(&op.Hash, op.Index) - unminedKey := existsRawUnminedInput(ns, opKey) - if unminedKey != nil { - unminedVal := existsRawUnmined(ns, unminedKey) + unminedSpendTxHashKeys := fetchUnminedInputSpendTxHashes(ns, opKey) + for _, unminedSpendTxHashKey := range unminedSpendTxHashKeys { + unminedVal := existsRawUnmined(ns, unminedSpendTxHashKey[:]) + + // If the spending transaction spends multiple outputs + // from the same transaction, we'll find duplicate + // entries within the store, so it's possible we're + // unable to find it if the conflicts have already been + // removed in a previous iteration. + if unminedVal == nil { + continue + } + var unminedRec TxRecord - copy(unminedRec.Hash[:], unminedKey) // Silly but need an array + unminedRec.Hash = unminedSpendTxHashKey err = readRawTxRecord(&unminedRec.Hash, unminedVal, &unminedRec) if err != nil { return err diff --git a/wtxmgr/unconfirmed.go b/wtxmgr/unconfirmed.go index be68dba..812d2d2 100644 --- a/wtxmgr/unconfirmed.go +++ b/wtxmgr/unconfirmed.go @@ -53,25 +53,37 @@ func (s *Store) removeDoubleSpends(ns walletdb.ReadWriteBucket, rec *TxRecord) e for _, input := range rec.MsgTx.TxIn { prevOut := &input.PreviousOutPoint prevOutKey := canonicalOutPoint(&prevOut.Hash, prevOut.Index) - doubleSpendHash := existsRawUnminedInput(ns, prevOutKey) - if doubleSpendHash != nil { + + doubleSpendHashes := fetchUnminedInputSpendTxHashes(ns, prevOutKey) + for _, doubleSpendHash := range doubleSpendHashes { + doubleSpendVal := existsRawUnmined(ns, doubleSpendHash[:]) + + // If the spending transaction spends multiple outputs + // from the same transaction, we'll find duplicate + // entries within the store, so it's possible we're + // unable to find it if the conflicts have already been + // removed in a previous iteration. + if doubleSpendVal == nil { + continue + } + var doubleSpend TxRecord - doubleSpendVal := existsRawUnmined(ns, doubleSpendHash) - copy(doubleSpend.Hash[:], doubleSpendHash) // Silly but need an array - err := readRawTxRecord(&doubleSpend.Hash, doubleSpendVal, - &doubleSpend) + doubleSpend.Hash = doubleSpendHash + err := readRawTxRecord( + &doubleSpend.Hash, doubleSpendVal, &doubleSpend, + ) if err != nil { return err } log.Debugf("Removing double spending transaction %v", doubleSpend.Hash) - err = s.removeConflict(ns, &doubleSpend) - if err != nil { + if err := s.removeConflict(ns, &doubleSpend); err != nil { return err } } } + return nil } @@ -83,14 +95,23 @@ func (s *Store) removeConflict(ns walletdb.ReadWriteBucket, rec *TxRecord) error // For each potential credit for this record, each spender (if any) must // be recursively removed as well. Once the spenders are removed, the // credit is deleted. - numOuts := uint32(len(rec.MsgTx.TxOut)) - for i := uint32(0); i < numOuts; i++ { - k := canonicalOutPoint(&rec.Hash, i) - spenderHash := existsRawUnminedInput(ns, k) - if spenderHash != nil { + for i := range rec.MsgTx.TxOut { + k := canonicalOutPoint(&rec.Hash, uint32(i)) + spenderHashes := fetchUnminedInputSpendTxHashes(ns, k) + for _, spenderHash := range spenderHashes { + spenderVal := existsRawUnmined(ns, spenderHash[:]) + + // If the spending transaction spends multiple outputs + // from the same transaction, we'll find duplicate + // entries within the store, so it's possible we're + // unable to find it if the conflicts have already been + // removed in a previous iteration. + if spenderVal == nil { + continue + } + var spender TxRecord - spenderVal := existsRawUnmined(ns, spenderHash) - copy(spender.Hash[:], spenderHash) // Silly but need an array + spender.Hash = spenderHash err := readRawTxRecord(&spender.Hash, spenderVal, &spender) if err != nil { return err @@ -98,13 +119,11 @@ func (s *Store) removeConflict(ns walletdb.ReadWriteBucket, rec *TxRecord) error log.Debugf("Transaction %v is part of a removed conflict "+ "chain -- removing as well", spender.Hash) - err = s.removeConflict(ns, &spender) - if err != nil { + if err := s.removeConflict(ns, &spender); err != nil { return err } } - err := deleteRawUnminedCredit(ns, k) - if err != nil { + if err := deleteRawUnminedCredit(ns, k); err != nil { return err } } @@ -115,8 +134,7 @@ func (s *Store) removeConflict(ns walletdb.ReadWriteBucket, rec *TxRecord) error for _, input := range rec.MsgTx.TxIn { prevOut := &input.PreviousOutPoint k := canonicalOutPoint(&prevOut.Hash, prevOut.Index) - err := deleteRawUnminedInput(ns, k) - if err != nil { + if err := deleteRawUnminedInput(ns, k); err != nil { return err } }