From 0066eee3ea8c06cb4f6efe5b971735b068a0f5c6 Mon Sep 17 00:00:00 2001 From: Bjarne Magnussen Date: Fri, 28 May 2021 14:29:48 +0200 Subject: [PATCH 1/2] wtxmgr: add `InsertTxCheckIfExsists` to check if a tx already recorded Adds new error `ErrDuplicateTx` and method `InserTxCheckIfExists` to check if a transaction to be inserted was already recorded. --- wtxmgr/tx.go | 31 +++++++++++++++++++++++++++---- wtxmgr/tx_test.go | 15 +++++++++++++++ wtxmgr/unconfirmed.go | 2 +- 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/wtxmgr/tx.go b/wtxmgr/tx.go index 19c5fc2..7af646f 100644 --- a/wtxmgr/tx.go +++ b/wtxmgr/tx.go @@ -55,6 +55,10 @@ var ( // ErrOutputUnlockNotAllowed is an error returned when an output unlock // is attempted with a different ID than the one which locked it. ErrOutputUnlockNotAllowed = errors.New("output unlock not alowed") + + // ErrDuplicateTx is returned when attempting to record a mined or + // unmined transaction that is already recorded. + ErrDuplicateTx = errors.New("transaction already exists") ) // Block contains the minimum amount of data to uniquely identify any block on @@ -355,11 +359,30 @@ func (s *Store) deleteUnminedTx(ns walletdb.ReadWriteBucket, rec *TxRecord) erro // 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 { +func (s *Store) InsertTx(ns walletdb.ReadWriteBucket, rec *TxRecord, + block *BlockMeta) error { + _, err := s.InsertTxCheckIfExists(ns, rec, block) + return err +} + +// InsertTxCheckIfExists 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. It will return true if the +// transaction was already recorded prior to the call. +func (s *Store) InsertTxCheckIfExists(ns walletdb.ReadWriteBucket, + rec *TxRecord, block *BlockMeta) (bool, error) { + + var err error if block == nil { - return s.insertMemPoolTx(ns, rec) + if err = s.insertMemPoolTx(ns, rec); err == ErrDuplicateTx { + return true, nil + } + return false, err } - return s.insertMinedTx(ns, rec, block) + if err = s.insertMinedTx(ns, rec, block); err == ErrDuplicateTx { + return true, nil + } + return false, err } // RemoveUnminedTx attempts to remove an unmined transaction from the @@ -386,7 +409,7 @@ func (s *Store) insertMinedTx(ns walletdb.ReadWriteBucket, rec *TxRecord, // 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 + return ErrDuplicateTx } // If a block record does not yet exist for any transactions from this diff --git a/wtxmgr/tx_test.go b/wtxmgr/tx_test.go index 855dc3b..c172b7b 100644 --- a/wtxmgr/tx_test.go +++ b/wtxmgr/tx_test.go @@ -7,6 +7,7 @@ package wtxmgr import ( "bytes" "encoding/hex" + "fmt" "io/ioutil" "os" "path/filepath" @@ -192,6 +193,13 @@ func TestInsertsCreditsDebitsRollbacks(t *testing.T) { return nil, err } + // Check that the duplicate transaction is found. + if exists, _ := s.InsertTxCheckIfExists(ns, rec, nil); !exists { + return nil, fmt.Errorf( + "duplicate transaction was not found as already recorded", + ) + } + err = s.AddCredit(ns, rec, nil, 0, false) return s, err }, @@ -244,6 +252,13 @@ func TestInsertsCreditsDebitsRollbacks(t *testing.T) { return nil, err } + // Make sure the duplicate transaction is found. + if exists, _ := s.InsertTxCheckIfExists(ns, rec, TstRecvTxBlockDetails); !exists { + return nil, fmt.Errorf( + "duplicate transaction was not found as already recorded", + ) + } + err = s.AddCredit(ns, rec, TstRecvTxBlockDetails, 0, false) return s, err }, diff --git a/wtxmgr/unconfirmed.go b/wtxmgr/unconfirmed.go index fc0d13b..e02b4c0 100644 --- a/wtxmgr/unconfirmed.go +++ b/wtxmgr/unconfirmed.go @@ -22,7 +22,7 @@ func (s *Store) insertMemPoolTx(ns walletdb.ReadWriteBucket, rec *TxRecord) erro // TODO: compare serialized txs to ensure this isn't a hash // collision? if txDetails, _ := s.TxDetails(ns, &rec.Hash); txDetails != nil { - return nil + return ErrDuplicateTx } // Since transaction records within the store are keyed by their From 419381b7499b02d88af1f758b52653aed07f5bd0 Mon Sep 17 00:00:00 2001 From: Bjarne Magnussen Date: Fri, 4 Jun 2021 08:07:24 +0200 Subject: [PATCH 2/2] wallet: call `InsertTxCheckIfExists` to add a relevant tx Let the method `addRelevantTx` use `InsertTxCheckIfExists` to insert a relevant transaction. If the transaction has already been recorded, the method call `addRelevantTx` will return early and not proceed with duplicating work of checking every output and duplicating the tx notification to the notification server. --- wallet/chainntfns.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/wallet/chainntfns.go b/wallet/chainntfns.go index 990d8bc..4182ad2 100644 --- a/wallet/chainntfns.go +++ b/wallet/chainntfns.go @@ -280,11 +280,19 @@ func (w *Wallet) addRelevantTx(dbtx walletdb.ReadWriteTx, rec *wtxmgr.TxRecord, // relevant. This assumption will not hold true when SPV support is // added, but until then, simply insert the transaction because there // should either be one or more relevant inputs or outputs. - err := w.TxStore.InsertTx(txmgrNs, rec, block) + exists, err := w.TxStore.InsertTxCheckIfExists(txmgrNs, rec, block) if err != nil { return err } + // If the transaction has already been recorded, we can return early. + // Note: Returning here is safe as we're within the context of an atomic + // database transaction, so we don't need to worry about the MarkUsed + // calls below. + if exists { + return nil + } + // Check every output to determine whether it is controlled by a wallet // key. If so, mark the output as a credit. for i, output := range rec.MsgTx.TxOut {