From aa826c64cf34173eb9fcbac3dd367a001f798d9d Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 8 Jun 2018 18:59:57 -0700 Subject: [PATCH] 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 } }