diff --git a/mempool/mempool_v2.go b/mempool/mempool_v2.go index 2a92457c63..112b9ad323 100644 --- a/mempool/mempool_v2.go +++ b/mempool/mempool_v2.go @@ -49,10 +49,16 @@ type OrphanTxManager interface { // AddOrphan adds an orphan transaction to the pool with the given tag. AddOrphan(tx *btcutil.Tx, tag Tag) error + // RemoveOrphan removes an orphan transaction from the pool. + RemoveOrphan(hash chainhash.Hash, cascade bool) error + + // RemoveOrphansByTag removes all orphans tagged with the given tag. + RemoveOrphansByTag(tag Tag) int + // ProcessOrphans processes orphans that may now be acceptable after a // parent transaction was added. Returns the list of promoted orphans. ProcessOrphans( - hash chainhash.Hash, + parentTx *btcutil.Tx, acceptFunc func(*btcutil.Tx) error, ) ([]*btcutil.Tx, error) } @@ -489,23 +495,27 @@ func (mp *TxMempoolV2) processOrphansLocked(acceptedTx *btcutil.Tx) []*TxDesc { acceptFunc := func(orphanTx *btcutil.Tx) error { // Try to accept the orphan. Don't reject if it's a duplicate // orphan since we're processing it from the orphan pool. - _, txDesc, err := mp.maybeAcceptTransactionLocked( + missingParents, txDesc, err := mp.maybeAcceptTransactionLocked( orphanTx, true, true, false, ) if err != nil { + // Orphan is invalid - return error so it gets removed. return err } - // If txDesc is nil, the orphan is still missing parents. - if txDesc != nil { - acceptedTxns = append(acceptedTxns, txDesc) + // If orphan still has missing parents, return error so + // OrphanManager keeps it in the pool for future processing. + if len(missingParents) > 0 { + return fmt.Errorf("orphan still has missing parents") } + // Successfully accepted - store the descriptor. + acceptedTxns = append(acceptedTxns, txDesc) return nil } // Process orphans that spend outputs from the accepted transaction. - promoted, err := mp.orphanMgr.ProcessOrphans(*acceptedTx.Hash(), acceptFunc) + promoted, err := mp.orphanMgr.ProcessOrphans(acceptedTx, acceptFunc) if err != nil { log.Warnf("Error processing orphans: %v", err) } @@ -589,18 +599,28 @@ func (mp *TxMempoolV2) RemoveDoubleSpends(tx *btcutil.Tx) { // RemoveOrphan removes the passed orphan transaction from the orphan pool. // -// STUB: Implementation pending in implement-orphan-ops task. +// This function is safe for concurrent access. func (mp *TxMempoolV2) RemoveOrphan(tx *btcutil.Tx) { - panic("RemoveOrphan: not implemented") + mp.mu.Lock() + defer mp.mu.Unlock() + + // Delegate to OrphanManager. Use cascade=false to only remove this + // specific orphan without affecting its descendants. + _ = mp.orphanMgr.RemoveOrphan(*tx.Hash(), false) } // RemoveOrphansByTag removes all orphan transactions tagged with the provided // identifier. This is useful when a peer disconnects to remove all orphans // that were received from that peer. // -// STUB: Implementation pending in implement-orphan-ops task. +// This function is safe for concurrent access. func (mp *TxMempoolV2) RemoveOrphansByTag(tag Tag) uint64 { - panic("RemoveOrphansByTag: not implemented") + mp.mu.Lock() + defer mp.mu.Unlock() + + // Delegate to OrphanManager and convert return type. + removed := mp.orphanMgr.RemoveOrphansByTag(tag) + return uint64(removed) } // ProcessOrphans determines if there are any orphans which depend on the passed @@ -610,59 +630,201 @@ func (mp *TxMempoolV2) RemoveOrphansByTag(tag Tag) uint64 { // It returns a slice of transactions added to the mempool as a result of // processing the orphans. // -// STUB: Implementation pending in implement-orphan-ops task. +// This function is safe for concurrent access. func (mp *TxMempoolV2) ProcessOrphans(acceptedTx *btcutil.Tx) []*TxDesc { - panic("ProcessOrphans: not implemented") + mp.mu.Lock() + defer mp.mu.Unlock() + + // Collect TxDesc results from successfully promoted orphans. + var acceptedDescs []*TxDesc + + // Create acceptance function that wraps maybeAcceptTransactionLocked. + acceptFunc := func(tx *btcutil.Tx) error { + // Try to accept the orphan into the mempool. + // isNew=true (orphan is new to main pool) + // rateLimit=false (already vetted when added as orphan) + // rejectDupOrphans=false (it's currently an orphan) + _, txDesc, err := mp.maybeAcceptTransactionLocked( + tx, true, false, false) + if err != nil { + return err + } + + // Successfully accepted - store the descriptor. + acceptedDescs = append(acceptedDescs, txDesc) + return nil + } + + // Delegate to OrphanManager to process orphans. + _, _ = mp.orphanMgr.ProcessOrphans(acceptedTx, acceptFunc) + + return acceptedDescs } // FetchTransaction returns the requested transaction from the transaction pool. // This only fetches from the main transaction pool and does not include orphans. // -// STUB: Implementation pending in implement-query-ops task. +// This function is safe for concurrent access. func (mp *TxMempoolV2) FetchTransaction(txHash *chainhash.Hash) (*btcutil.Tx, error) { - panic("FetchTransaction: not implemented") + mp.mu.RLock() + defer mp.mu.RUnlock() + + // Lookup transaction in graph. + node, exists := mp.graph.GetNode(*txHash) + if !exists { + return nil, fmt.Errorf("transaction is not in the pool") + } + + return node.Tx, nil } // TxHashes returns a slice of hashes for all of the transactions in the // memory pool. // -// STUB: Implementation pending in implement-query-ops task. +// This function is safe for concurrent access. func (mp *TxMempoolV2) TxHashes() []*chainhash.Hash { - panic("TxHashes: not implemented") + mp.mu.RLock() + defer mp.mu.RUnlock() + + // Pre-allocate slice with exact capacity. + count := mp.graph.GetNodeCount() + hashes := make([]*chainhash.Hash, 0, count) + + // Iterate all nodes and collect hashes. + for node := range mp.graph.Iterate() { + // Make a copy of the hash to avoid returning pointers to node internals. + hashCopy := node.TxHash + hashes = append(hashes, &hashCopy) + } + + return hashes } // TxDescs returns a slice of descriptors for all the transactions in the pool. // The descriptors are to be treated as read only. // -// STUB: Implementation pending in implement-query-ops task. +// This function is safe for concurrent access. func (mp *TxMempoolV2) TxDescs() []*TxDesc { - panic("TxDescs: not implemented") + mp.mu.RLock() + defer mp.mu.RUnlock() + + // Pre-allocate slice with exact capacity. + count := mp.graph.GetNodeCount() + descs := make([]*TxDesc, 0, count) + + // Iterate all nodes and build descriptors. + for node := range mp.graph.Iterate() { + // Convert graph TxDesc to mempool TxDesc format. + desc := &TxDesc{ + TxDesc: mining.TxDesc{ + Tx: node.Tx, + Added: node.TxDesc.Added, + Height: mp.cfg.BestHeight(), + Fee: node.TxDesc.Fee, + FeePerKB: node.TxDesc.FeePerKB, + }, + StartingPriority: 0, // Priority is deprecated. + } + descs = append(descs, desc) + } + + return descs } // MiningDescs returns a slice of mining descriptors for all the transactions // in the pool. The descriptors are specifically formatted for block template // generation. // -// STUB: Implementation pending in implement-query-ops task. +// This is part of the mining.TxSource interface implementation and is safe for +// concurrent access as required by the interface contract. func (mp *TxMempoolV2) MiningDescs() []*mining.TxDesc { - panic("MiningDescs: not implemented") + mp.mu.RLock() + defer mp.mu.RUnlock() + + // Pre-allocate slice with exact capacity. + count := mp.graph.GetNodeCount() + descs := make([]*mining.TxDesc, 0, count) + + // Iterate all nodes and build mining descriptors. + for node := range mp.graph.Iterate() { + desc := &mining.TxDesc{ + Tx: node.Tx, + Added: node.TxDesc.Added, + Height: mp.cfg.BestHeight(), + Fee: node.TxDesc.Fee, + FeePerKB: node.TxDesc.FeePerKB, + } + descs = append(descs, desc) + } + + return descs } // RawMempoolVerbose returns all the entries in the mempool as a fully // populated btcjson result for the getrawmempool RPC command. // -// STUB: Implementation pending in implement-query-ops task. +// This function is safe for concurrent access. func (mp *TxMempoolV2) RawMempoolVerbose() map[string]*btcjson.GetRawMempoolVerboseResult { - panic("RawMempoolVerbose: not implemented") + mp.mu.RLock() + defer mp.mu.RUnlock() + + count := mp.graph.GetNodeCount() + result := make(map[string]*btcjson.GetRawMempoolVerboseResult, count) + bestHeight := mp.cfg.BestHeight() + + // Iterate all nodes and build verbose results. + for node := range mp.graph.Iterate() { + tx := node.Tx + + // Calculate current priority based on the inputs. Use zero if we + // can't fetch the UTXO view for some reason. + var currentPriority float64 + utxos, err := mp.cfg.FetchUtxoView(tx) + if err == nil { + currentPriority = mining.CalcPriority(tx.MsgTx(), utxos, + bestHeight+1) + } + + mpd := &btcjson.GetRawMempoolVerboseResult{ + Size: int32(tx.MsgTx().SerializeSize()), + Vsize: int32(GetTxVirtualSize(tx)), + Weight: int32(blockchain.GetTransactionWeight(tx)), + Fee: btcutil.Amount(node.TxDesc.Fee).ToBTC(), + Time: node.TxDesc.Added.Unix(), + Height: int64(bestHeight), + StartingPriority: 0, // Priority is deprecated. + CurrentPriority: currentPriority, + Depends: make([]string, 0), + } + + // Build dependency list (parents that are also in the mempool). + for _, txIn := range tx.MsgTx().TxIn { + parentHash := txIn.PreviousOutPoint.Hash + if mp.graph.HasTransaction(parentHash) { + mpd.Depends = append(mpd.Depends, parentHash.String()) + } + } + + result[tx.Hash().String()] = mpd + } + + return result } // CheckSpend checks whether the passed outpoint is already spent by a // transaction in the mempool. If that's the case, the spending transaction // will be returned, otherwise nil will be returned. -// -// STUB: Implementation pending in implement-query-ops task. func (mp *TxMempoolV2) CheckSpend(op wire.OutPoint) *btcutil.Tx { - panic("CheckSpend: not implemented") + mp.mu.RLock() + defer mp.mu.RUnlock() + + // Use graph's spentBy index for O(1) lookup. + spender, exists := mp.graph.GetSpendingTx(op) + if !exists { + return nil + } + + return spender.Tx } // CheckMempoolAcceptance behaves similarly to bitcoind's `testmempoolaccept` @@ -670,9 +832,16 @@ func (mp *TxMempoolV2) CheckSpend(op wire.OutPoint) *btcutil.Tx { // transaction can be accepted to the mempool. If not, the specific error is // returned and the caller needs to take actions based on it. // -// STUB: Implementation pending in implement-rbf-validation task. +// This function is safe for concurrent access. func (mp *TxMempoolV2) CheckMempoolAcceptance(tx *btcutil.Tx) (*MempoolAcceptResult, error) { - panic("CheckMempoolAcceptance: not implemented") + mp.mu.RLock() + defer mp.mu.RUnlock() + + // Call internal validation with testmempoolaccept-RPC parameters: + // - isNew=true (treat as new transaction) + // - rateLimit=false (no rate limiting for RPC checks) + // - rejectDupOrphans=true (reject if already in orphan pool) + return mp.checkMempoolAcceptance(tx, true, false, true) } // Ensure TxMempoolV2 implements the TxMempool interface. diff --git a/mempool/mempool_v2_ops_test.go b/mempool/mempool_v2_ops_test.go index 88b4808b87..4409db8340 100644 --- a/mempool/mempool_v2_ops_test.go +++ b/mempool/mempool_v2_ops_test.go @@ -201,12 +201,38 @@ func (h *testHarness) expectTxAccepted(tx *btcutil.Tx, view *blockchain.UtxoView // expectTxOrphan sets up mocks for orphan detection. func (h *testHarness) expectTxOrphan(tx *btcutil.Tx, view *blockchain.UtxoViewpoint) { - h.mockPolicy.On("ValidateSegWitDeployment", tx).Return(nil) - h.mockValidator.On("ValidateSanity", tx).Return(nil) + h.mockPolicy.On("ValidateSegWitDeployment", tx).Return(nil).Once() + h.mockValidator.On("ValidateSanity", tx).Return(nil).Once() // Return a missing parent to indicate this is an orphan. + // Use Once() so subsequent calls (during promotion) can have different behavior. missingParent := chainhash.Hash{0x01} - h.mockValidator.On("ValidateUtxoAvailability", tx, mock.Anything).Return([]*chainhash.Hash{&missingParent}, nil) - h.mockUtxoView.On("FetchUtxoView", tx).Return(view, nil) + h.mockValidator.On("ValidateUtxoAvailability", tx, mock.Anything).Return([]*chainhash.Hash{&missingParent}, nil).Once() + h.mockUtxoView.On("FetchUtxoView", tx).Return(view, nil).Once() +} + +// expectEarlyValidation sets up mocks for just the early validation steps +// (SegWit deployment check, sanity check, and UTXO view fetch). This is useful +// for tests that expect validation to fail at a later stage or for duplicate +// transaction detection. +func (h *testHarness) expectEarlyValidation(tx *btcutil.Tx, view *blockchain.UtxoViewpoint) { + h.mockPolicy.On("ValidateSegWitDeployment", tx).Return(nil).Once() + h.mockValidator.On("ValidateSanity", tx).Return(nil).Once() + h.mockUtxoView.On("FetchUtxoView", tx).Return(view, nil).Once() +} + +// expectOrphanPromotion sets up mocks for promoting an orphan transaction. +// This includes setting up a UTXO view that contains the parent's output and +// all the standard transaction acceptance checks. +func (h *testHarness) expectOrphanPromotion(childTx *btcutil.Tx, parentTx *btcutil.Tx, parentOutputIdx uint32) { + // Create UTXO view with parent's output. + promotionView := blockchain.NewUtxoViewpoint() + parentOut := wire.OutPoint{Hash: *parentTx.Hash(), Index: parentOutputIdx} + promotionView.Entries()[parentOut] = blockchain.NewUtxoEntry( + parentTx.MsgTx().TxOut[parentOutputIdx], 100, false) + + // Set up mocks for child promotion. + h.mockUtxoView.On("FetchUtxoView", childTx).Return(promotionView, nil).Once() + h.expectTxAccepted(childTx, nil) // Don't set FetchUtxoView again - already done above } // expectValidationFailure sets up mocks for a validation failure at a specific stage. @@ -487,10 +513,26 @@ func TestProcessTransactionWithOrphans(t *testing.T) { require.True(t, h.mempool.IsOrphanInPool(childTx.Hash())) require.Equal(t, 0, h.mempool.Count()) // Not in main pool - // TODO: Fix orphan promotion test. - // The orphan promotion is not working correctly in the test. - // This may require better mock setup or a different testing approach. - // For now, we're testing that orphans are correctly detected and stored. + // Now add the parent, which should trigger orphan promotion. + parentView := createDefaultUtxoView(parentTx) + h.expectTxAccepted(parentTx, parentView) + + // Set up mocks for orphan promotion (called during ProcessOrphans). + h.expectOrphanPromotion(childTx, parentTx, 0) + + // Process parent transaction. + acceptedTxs, err = h.mempool.ProcessTransaction(parentTx, true, false, 0) + require.NoError(t, err) + require.NotNil(t, acceptedTxs) + require.Len(t, acceptedTxs, 2) // Parent + promoted child + + // Verify both parent and child are in main pool. + require.True(t, h.mempool.IsTransactionInPool(parentTx.Hash())) + require.True(t, h.mempool.IsTransactionInPool(childTx.Hash())) + require.Equal(t, 2, h.mempool.Count()) + + // Verify child is no longer in orphan pool. + require.False(t, h.mempool.IsOrphanInPool(childTx.Hash())) } // TestRemoveTransaction tests transaction removal. @@ -843,6 +885,9 @@ func TestDuplicateOrphan(t *testing.T) { // Try to add the same orphan again. When rejectDupOrphans=true (which // ProcessTransaction sets internally), it should reject duplicates. + // Set up mocks for the early validation steps (before duplicate check). + h.expectEarlyValidation(tx, view) + _, err = h.mempool.ProcessTransaction(tx, true, false, 0) require.Error(t, err) require.Contains(t, err.Error(), "already have transaction") diff --git a/mempool/mempool_v2_query_test.go b/mempool/mempool_v2_query_test.go new file mode 100644 index 0000000000..05a84b8e85 --- /dev/null +++ b/mempool/mempool_v2_query_test.go @@ -0,0 +1,290 @@ +// Copyright (c) 2013-2025 The btcsuite developers +// Use of this source code is governed by an ISC +// license that can be found in the LICENSE file. + +package mempool + +import ( + "testing" + + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/wire" + "github.com/stretchr/testify/require" +) + +// TestFetchTransaction verifies that FetchTransaction correctly retrieves +// transactions from the mempool. +func TestFetchTransaction(t *testing.T) { + t.Parallel() + + h := createTestMempoolV2() + + // Test fetching non-existent transaction. + randomHash := chainhash.Hash{0x01} + tx, err := h.mempool.FetchTransaction(&randomHash) + require.Error(t, err, "should error for non-existent transaction") + require.Nil(t, tx, "should return nil for non-existent transaction") + require.Contains(t, err.Error(), "not in the pool") + + // Add a transaction to the mempool. + testTx := createTestTx() + view := createDefaultUtxoView(testTx) + h.expectTxAccepted(testTx, view) + + _, _, err = h.mempool.MaybeAcceptTransaction(testTx, true, false) + require.NoError(t, err) + + // Fetch the transaction and verify it matches. + fetchedTx, err := h.mempool.FetchTransaction(testTx.Hash()) + require.NoError(t, err) + require.NotNil(t, fetchedTx) + require.Equal(t, testTx.Hash(), fetchedTx.Hash()) +} + +// TestTxHashes verifies that TxHashes returns correct hash list. +func TestTxHashes(t *testing.T) { + t.Parallel() + + h := createTestMempoolV2() + + // Empty mempool should return empty slice. + hashes := h.mempool.TxHashes() + require.NotNil(t, hashes, "should return non-nil slice") + require.Empty(t, hashes, "empty mempool should have no hashes") + + // Add several unique transactions. + // Create a parent and two children to ensure unique hashes. + parent := createTestTx() + child1 := createChildTx(parent, 0) + child2 := createChildTx(parent, 1) + + // Add parent first. + parentView := createDefaultUtxoView(parent) + h.expectTxAccepted(parent, parentView) + _, _, err := h.mempool.MaybeAcceptTransaction(parent, true, false) + require.NoError(t, err) + + // Add child1. + child1View := createDefaultUtxoView(child1) + h.expectTxAccepted(child1, child1View) + _, _, err = h.mempool.MaybeAcceptTransaction(child1, true, false) + require.NoError(t, err) + + // Add child2. + child2View := createDefaultUtxoView(child2) + h.expectTxAccepted(child2, child2View) + _, _, err = h.mempool.MaybeAcceptTransaction(child2, true, false) + require.NoError(t, err) + + // Get hashes and verify. + hashes = h.mempool.TxHashes() + require.Len(t, hashes, 3) + + // Verify all three transaction hashes are present. + hashMap := make(map[chainhash.Hash]bool) + for _, hash := range hashes { + hashMap[*hash] = true + } + require.True(t, hashMap[*parent.Hash()]) + require.True(t, hashMap[*child1.Hash()]) + require.True(t, hashMap[*child2.Hash()]) +} + +// TestTxDescs verifies that TxDescs returns correct descriptors. +func TestTxDescs(t *testing.T) { + t.Parallel() + + h := createTestMempoolV2() + + // Empty mempool should return empty slice. + descs := h.mempool.TxDescs() + require.NotNil(t, descs, "should return non-nil slice") + require.Empty(t, descs, "empty mempool should have no descriptors") + + // Add a transaction. + tx := createTestTx() + view := createDefaultUtxoView(tx) + h.expectTxAccepted(tx, view) + + _, txDesc, err := h.mempool.MaybeAcceptTransaction(tx, true, false) + require.NoError(t, err) + require.NotNil(t, txDesc) + + // Get descriptors and verify. + descs = h.mempool.TxDescs() + require.Len(t, descs, 1) + + // Verify descriptor fields. + desc := descs[0] + require.Equal(t, tx.Hash(), desc.Tx.Hash()) + require.Equal(t, txDesc.Height, desc.Height) + require.Equal(t, txDesc.Fee, desc.Fee) + require.Equal(t, txDesc.FeePerKB, desc.FeePerKB) +} + +// TestMiningDescs verifies that MiningDescs returns correct mining descriptors. +func TestMiningDescs(t *testing.T) { + t.Parallel() + + h := createTestMempoolV2() + + // Empty mempool should return empty slice. + descs := h.mempool.MiningDescs() + require.NotNil(t, descs, "should return non-nil slice") + require.Empty(t, descs, "empty mempool should have no mining descriptors") + + // Add parent and child transactions to test topological ordering. + parent := createTestTx() + child := createChildTx(parent, 0) + + // Add parent first. + parentView := createDefaultUtxoView(parent) + h.expectTxAccepted(parent, parentView) + _, _, err := h.mempool.MaybeAcceptTransaction(parent, true, false) + require.NoError(t, err) + + // Add child. + childView := createDefaultUtxoView(child) + h.expectTxAccepted(child, childView) + _, _, err = h.mempool.MaybeAcceptTransaction(child, true, false) + require.NoError(t, err) + + // Get mining descriptors. + descs = h.mempool.MiningDescs() + require.Len(t, descs, 2) + + // Verify both parent and child are present. + hashes := make(map[chainhash.Hash]bool) + for _, desc := range descs { + hashes[*desc.Tx.Hash()] = true + } + require.True(t, hashes[*parent.Hash()], "parent should be in results") + require.True(t, hashes[*child.Hash()], "child should be in results") + + // TODO: Add more sophisticated topological ordering verification + // once we have a more complex transaction graph test case. +} + +// TestRawMempoolVerbose verifies that RawMempoolVerbose returns correct verbose info. +func TestRawMempoolVerbose(t *testing.T) { + t.Parallel() + + h := createTestMempoolV2() + + // Empty mempool should return empty map. + verbose := h.mempool.RawMempoolVerbose() + require.NotNil(t, verbose, "should return non-nil map") + require.Empty(t, verbose, "empty mempool should have empty verbose map") + + // Add parent and child to test dependency tracking. + parent := createTestTx() + child := createChildTx(parent, 0) + + parentView := createDefaultUtxoView(parent) + h.expectTxAccepted(parent, parentView) + _, _, err := h.mempool.MaybeAcceptTransaction(parent, true, false) + require.NoError(t, err) + + childView := createDefaultUtxoView(child) + h.expectTxAccepted(child, childView) + _, _, err = h.mempool.MaybeAcceptTransaction(child, true, false) + require.NoError(t, err) + + // Get verbose info. + verbose = h.mempool.RawMempoolVerbose() + require.Len(t, verbose, 2) + + // Verify child has parent in depends list. + childVerbose, ok := verbose[child.Hash().String()] + require.True(t, ok) + require.Contains(t, childVerbose.Depends, parent.Hash().String()) + + // Verify parent has no dependencies. + parentVerbose, ok := verbose[parent.Hash().String()] + require.True(t, ok) + require.Empty(t, parentVerbose.Depends) +} + +// TestMempoolV2CheckSpend verifies that CheckSpend correctly detects spending transactions. +func TestMempoolV2CheckSpend(t *testing.T) { + t.Parallel() + + h := createTestMempoolV2() + + // Test checking spend for non-existent outpoint. + op := wire.OutPoint{ + Hash: chainhash.Hash{0x01}, + Index: 0, + } + spender := h.mempool.CheckSpend(op) + require.Nil(t, spender, "should return nil for unspent outpoint") + + // Add parent and child transactions. + parent := createTestTx() + child := createChildTx(parent, 0) + + parentView := createDefaultUtxoView(parent) + h.expectTxAccepted(parent, parentView) + _, _, err := h.mempool.MaybeAcceptTransaction(parent, true, false) + require.NoError(t, err) + + childView := createDefaultUtxoView(child) + h.expectTxAccepted(child, childView) + _, _, err = h.mempool.MaybeAcceptTransaction(child, true, false) + require.NoError(t, err) + + // Check if child spends parent's output. + parentOutpoint := wire.OutPoint{ + Hash: *parent.Hash(), + Index: 0, + } + spender = h.mempool.CheckSpend(parentOutpoint) + require.NotNil(t, spender) + require.Equal(t, child.Hash(), spender.Hash()) + + // Check an unspent output from parent. + unspentOutpoint := wire.OutPoint{ + Hash: *parent.Hash(), + Index: 1, + } + spender = h.mempool.CheckSpend(unspentOutpoint) + require.Nil(t, spender) +} + +// TestQueryMethodsConcurrentAccess verifies that query methods are safe for +// concurrent access. +func TestQueryMethodsConcurrentAccess(t *testing.T) { + t.Parallel() + + mp := newTestMempoolV2(t) + + // Launch multiple goroutines performing concurrent query operations. + const numReaders = 10 + const numReads = 100 + + done := make(chan bool) + + for i := 0; i < numReaders; i++ { + go func() { + for j := 0; j < numReads; j++ { + // Perform various query operations concurrently. + _ = mp.TxHashes() + _ = mp.TxDescs() + _ = mp.MiningDescs() + _ = mp.RawMempoolVerbose() + + randomHash := chainhash.Hash{byte(j)} + _, _ = mp.FetchTransaction(&randomHash) + _ = mp.CheckSpend(wire.OutPoint{Hash: randomHash}) + } + done <- true + }() + } + + // Wait for all goroutines to complete. + for i := 0; i < numReaders; i++ { + <-done + } + + // No panics or races should occur (verified by race detector). +} diff --git a/mempool/mempool_v2_test.go b/mempool/mempool_v2_test.go index fe85038cf3..e2f0dd0b8a 100644 --- a/mempool/mempool_v2_test.go +++ b/mempool/mempool_v2_test.go @@ -12,7 +12,6 @@ import ( "github.com/btcsuite/btcd/blockchain" "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg/chainhash" - "github.com/btcsuite/btcd/wire" "github.com/stretchr/testify/require" ) @@ -159,13 +158,34 @@ func TestMempoolMissingDependenciesError(t *testing.T) { func TestMempoolCount(t *testing.T) { t.Parallel() - mp := newTestMempoolV2(t) + h := createTestMempoolV2() // Initial count should be zero. - require.Equal(t, 0, mp.Count(), "initial count should be 0") + require.Equal(t, 0, h.mempool.Count(), "initial count should be 0") + + // Add first transaction (parent1). + parent1 := createTestTx() + parent1View := createDefaultUtxoView(parent1) + h.expectTxAccepted(parent1, parent1View) + _, _, err := h.mempool.MaybeAcceptTransaction(parent1, true, false) + require.NoError(t, err) + require.Equal(t, 1, h.mempool.Count(), "count should be 1 after adding first tx") + + // Add second transaction (child of parent1) to ensure unique hash. + child1 := createChildTx(parent1, 0) + child1View := createDefaultUtxoView(child1) + h.expectTxAccepted(child1, child1View) + _, _, err = h.mempool.MaybeAcceptTransaction(child1, true, false) + require.NoError(t, err) + require.Equal(t, 2, h.mempool.Count(), "count should be 2 after adding second tx") - // TODO: Add transactions and verify count increases. - // This will be tested properly in implement-tx-operations task. + // Add third transaction (child of child1) to avoid conflicts. + child2 := createChildTx(child1, 0) + child2View := createDefaultUtxoView(child2) + h.expectTxAccepted(child2, child2View) + _, _, err = h.mempool.MaybeAcceptTransaction(child2, true, false) + require.NoError(t, err) + require.Equal(t, 3, h.mempool.Count(), "count should be 3 after adding third tx") } // TestMempoolHaveTransaction verifies HaveTransaction checks both main pool @@ -173,15 +193,40 @@ func TestMempoolCount(t *testing.T) { func TestMempoolHaveTransaction(t *testing.T) { t.Parallel() - mp := newTestMempoolV2(t) + h := createTestMempoolV2() // Random hash should not exist. randomHash := chainhash.Hash{0x01, 0x02, 0x03} - require.False(t, mp.HaveTransaction(&randomHash), + require.False(t, h.mempool.HaveTransaction(&randomHash), "random hash should not exist") - // TODO: Add transaction and verify it's found. - // This will be tested properly in implement-tx-operations task. + // Add transaction to main pool. + tx := createTestTx() + view := createDefaultUtxoView(tx) + h.expectTxAccepted(tx, view) + _, _, err := h.mempool.MaybeAcceptTransaction(tx, true, false) + require.NoError(t, err) + + // Verify transaction is found. + require.True(t, h.mempool.HaveTransaction(tx.Hash()), + "should find transaction in mempool") + + // Add an orphan transaction (create with unique signature). + orphan := createTestTx() + // Make unique by modifying signature. + orphan.MsgTx().TxIn[0].SignatureScript[0] = 0xFF + orphanView := createOrphanUtxoView(orphan) + h.expectTxOrphan(orphan, orphanView) + + // Use ProcessTransaction to actually add orphan to orphan pool. + acceptedTxs, err := h.mempool.ProcessTransaction(orphan, true, false, 0) + require.NoError(t, err) + // Orphan returns nil (not accepted to main pool). + require.Nil(t, acceptedTxs) + + // Verify orphan is also found by HaveTransaction (checks both pools). + require.True(t, h.mempool.HaveTransaction(orphan.Hash()), + "should find orphan transaction in orphan pool") } // TestMempoolIsTransactionInPool verifies IsTransactionInPool checks only @@ -189,14 +234,42 @@ func TestMempoolHaveTransaction(t *testing.T) { func TestMempoolIsTransactionInPool(t *testing.T) { t.Parallel() - mp := newTestMempoolV2(t) + h := createTestMempoolV2() randomHash := chainhash.Hash{0x01, 0x02, 0x03} - require.False(t, mp.IsTransactionInPool(&randomHash), + require.False(t, h.mempool.IsTransactionInPool(&randomHash), "random hash should not exist in main pool") - // TODO: Add transaction and verify it's found in main pool but not as - // orphan. This will be tested properly in implement-tx-operations task. + // Add transaction to main pool. + tx := createTestTx() + view := createDefaultUtxoView(tx) + h.expectTxAccepted(tx, view) + _, _, err := h.mempool.MaybeAcceptTransaction(tx, true, false) + require.NoError(t, err) + + // Verify transaction is found in main pool. + require.True(t, h.mempool.IsTransactionInPool(tx.Hash()), + "should find transaction in main pool") + + // Add an orphan transaction (create with unique signature). + orphan := createTestTx() + // Make unique by modifying signature. + orphan.MsgTx().TxIn[0].SignatureScript[0] = 0xFF + orphanView := createOrphanUtxoView(orphan) + h.expectTxOrphan(orphan, orphanView) + + // Use ProcessTransaction to actually add orphan to orphan pool. + acceptedTxs, err := h.mempool.ProcessTransaction(orphan, true, false, 0) + require.NoError(t, err) + // Orphan returns nil (not accepted to main pool). + require.Nil(t, acceptedTxs) + + // Verify orphan is NOT found by IsTransactionInPool (checks only main pool). + require.False(t, h.mempool.IsTransactionInPool(orphan.Hash()), + "should not find orphan in main pool") + // But verify it's in the orphan pool. + require.True(t, h.mempool.IsOrphanInPool(orphan.Hash()), + "should find orphan in orphan pool") } // TestMempoolIsOrphanInPool verifies IsOrphanInPool checks only the orphan @@ -204,14 +277,45 @@ func TestMempoolIsTransactionInPool(t *testing.T) { func TestMempoolIsOrphanInPool(t *testing.T) { t.Parallel() - mp := newTestMempoolV2(t) + h := createTestMempoolV2() randomHash := chainhash.Hash{0x01, 0x02, 0x03} - require.False(t, mp.IsOrphanInPool(&randomHash), + require.False(t, h.mempool.IsOrphanInPool(&randomHash), "random hash should not exist in orphan pool") - // TODO: Add orphan and verify it's found in orphan pool but not main - // pool. This will be tested properly in implement-orphan-ops task. + // Add an orphan transaction (create with unique signature). + orphan := createTestTx() + // Make unique by modifying signature. + orphan.MsgTx().TxIn[0].SignatureScript[0] = 0xFF + orphanView := createOrphanUtxoView(orphan) + h.expectTxOrphan(orphan, orphanView) + + // Use ProcessTransaction to actually add orphan to orphan pool. + acceptedTxs, err := h.mempool.ProcessTransaction(orphan, true, false, 0) + require.NoError(t, err) + // Orphan returns nil (not accepted to main pool). + require.Nil(t, acceptedTxs) + + // Verify orphan is found in orphan pool. + require.True(t, h.mempool.IsOrphanInPool(orphan.Hash()), + "should find orphan in orphan pool") + // But NOT found in main pool. + require.False(t, h.mempool.IsTransactionInPool(orphan.Hash()), + "should not find orphan in main pool") + + // Add a regular transaction to main pool (create as child to make unique). + parent := createTestTx() + parentView := createDefaultUtxoView(parent) + h.expectTxAccepted(parent, parentView) + _, _, err = h.mempool.MaybeAcceptTransaction(parent, true, false) + require.NoError(t, err) + + // Verify main pool transaction is NOT in orphan pool. + require.False(t, h.mempool.IsOrphanInPool(parent.Hash()), + "should not find main pool tx in orphan pool") + // But IS found in main pool. + require.True(t, h.mempool.IsTransactionInPool(parent.Hash()), + "should find main pool tx in main pool") } // TestMempoolLastUpdated verifies that LastUpdated returns a valid timestamp. @@ -264,98 +368,6 @@ func TestMempoolConcurrentAccess(t *testing.T) { wg.Wait() } -// TestMempoolStubbedMethodsPanic verifies that unimplemented methods panic -// with "not implemented" message as expected. -func TestMempoolStubbedMethodsPanic(t *testing.T) { - t.Parallel() - - mp := newTestMempoolV2(t) - - // Test that stubbed methods panic (only for methods not yet implemented). - tests := []struct { - name string - fn func() - }{ - { - name: "RemoveOrphan", - fn: func() { - mp.RemoveOrphan(nil) - }, - }, - { - name: "RemoveOrphansByTag", - fn: func() { - _ = mp.RemoveOrphansByTag(0) - }, - }, - { - name: "ProcessOrphans", - fn: func() { - _ = mp.ProcessOrphans(nil) - }, - }, - { - name: "FetchTransaction", - fn: func() { - _, _ = mp.FetchTransaction(nil) - }, - }, - { - name: "TxHashes", - fn: func() { - _ = mp.TxHashes() - }, - }, - { - name: "TxDescs", - fn: func() { - _ = mp.TxDescs() - }, - }, - { - name: "MiningDescs", - fn: func() { - _ = mp.MiningDescs() - }, - }, - { - name: "RawMempoolVerbose", - fn: func() { - _ = mp.RawMempoolVerbose() - }, - }, - { - name: "CheckSpend", - fn: func() { - _ = mp.CheckSpend(wire.OutPoint{}) - }, - }, - { - name: "CheckMempoolAcceptance", - fn: func() { - _, _ = mp.CheckMempoolAcceptance(nil) - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Verify that calling the stubbed method panics. - require.Panics(t, tt.fn, - "stubbed method %s should panic", tt.name) - }) - } -} - -// TestMempoolInterfaceCompliance verifies that TxMempoolV2 implements the -// TxMempool interface. This is a compile-time check via the var declaration -// in mempool_v2.go, but we add an explicit test for documentation. -func TestMempoolInterfaceCompliance(t *testing.T) { - t.Parallel() - - var _ TxMempool = (*TxMempoolV2)(nil) -} - // TestMempoolConfigCustomization verifies that custom dependencies can be // injected via configuration. func TestMempoolConfigCustomization(t *testing.T) { diff --git a/mempool/orphan.go b/mempool/orphan.go index 70607b942e..8be5656309 100644 --- a/mempool/orphan.go +++ b/mempool/orphan.go @@ -9,6 +9,7 @@ import ( "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/mempool/txgraph" + "github.com/btcsuite/btcd/wire" ) // Orphan-specific errors. @@ -368,28 +369,36 @@ func (om *OrphanManager) Count() int { // // Returns the list of successfully promoted transactions. func (om *OrphanManager) ProcessOrphans( - parentHash chainhash.Hash, + parentTx *btcutil.Tx, acceptFunc func(*btcutil.Tx) error, ) ([]*btcutil.Tx, error) { om.mu.Lock() defer om.mu.Unlock() - // Find orphans that spend from this parent. The graph structure makes - // this efficient: we just look at the children of the parent node. - parentNode, exists := om.graph.GetNode(parentHash) - if !exists { - // Parent isn't in orphan graph, so no orphans spend from it. - return nil, nil - } - - // Collect all orphan descendants of this parent. + // Find orphans that spend from this parent's outputs using the spentBy + // index. The parent itself is not in the orphan graph (it's in the main + // mempool), but orphans that spend its outputs are indexed by outpoint. var promoted []*btcutil.Tx toProcess := txgraph.NewQueue[*txgraph.TxGraphNode]() visited := make(map[chainhash.Hash]bool) - // Start with direct children. - for _, child := range parentNode.Children { + // Check each output of the parent to see if it's spent by an orphan. + // Iterate through actual outputs of the transaction. + parentHash := parentTx.Hash() + for txOutIdx := range parentTx.MsgTx().TxOut { + outpoint := wire.OutPoint{ + Hash: *parentHash, + Index: uint32(txOutIdx), + } + + child, exists := om.graph.GetSpendingTx(outpoint) + if !exists { + // No orphan spends this output. + continue + } + + // Found an orphan that spends this parent output. if !visited[child.TxHash] { toProcess.Enqueue(child) visited[child.TxHash] = true diff --git a/mempool/orphan_test.go b/mempool/orphan_test.go index 971ea353cd..d5de5d631b 100644 --- a/mempool/orphan_test.go +++ b/mempool/orphan_test.go @@ -305,16 +305,9 @@ func TestOrphanManagerProcessOrphans(t *testing.T) { } // Process orphans when parent arrives (parent is not in orphan pool). - result, err := om.ProcessOrphans(*parent.Hash(), acceptFunc) - require.NoError(t, err) - require.Empty(t, result) - require.Empty(t, promoted) - - // Now add parent to orphan pool. - require.NoError(t, om.AddOrphan(parent, Tag(1))) - - // Process orphans again. - result, err = om.ProcessOrphans(*parent.Hash(), acceptFunc) + // This is the common case: parent is accepted to main mempool, and we + // promote its orphan children. + result, err := om.ProcessOrphans(parent, acceptFunc) require.NoError(t, err) // Should have promoted child1, child2, and grandchild (in some order). @@ -330,9 +323,9 @@ func TestOrphanManagerProcessOrphans(t *testing.T) { require.Contains(t, promotedMap, *child2.Hash()) require.Contains(t, promotedMap, *grandchild.Hash()) - // Orphan pool should now only contain parent and unrelated. - require.Equal(t, 2, om.Count()) - require.True(t, om.IsOrphan(*parent.Hash())) + // Orphan pool should now only contain unrelated. + // (The 3 promoted orphans have been removed.) + require.Equal(t, 1, om.Count()) require.True(t, om.IsOrphan(*unrelated.Hash())) require.False(t, om.IsOrphan(*child1.Hash())) require.False(t, om.IsOrphan(*child2.Hash())) @@ -368,7 +361,7 @@ func TestOrphanManagerProcessOrphansPartialFailure(t *testing.T) { } // Process orphans. - result, err := om.ProcessOrphans(*parent.Hash(), acceptFunc) + result, err := om.ProcessOrphans(parent, acceptFunc) require.NoError(t, err) // Should only have promoted child2. @@ -429,7 +422,7 @@ func TestOrphanManagerPackageTracking(t *testing.T) { return nil } - result, err := om.ProcessOrphans(*root.Hash(), acceptFunc) + result, err := om.ProcessOrphans(root, acceptFunc) require.NoError(t, err) // Should have promoted all descendants: A, B, C. diff --git a/mempool/txgraph/graph.go b/mempool/txgraph/graph.go index 731c028470..73fa9efdd1 100644 --- a/mempool/txgraph/graph.go +++ b/mempool/txgraph/graph.go @@ -427,6 +427,19 @@ func (g *TxGraph) HasTransaction(hash chainhash.Hash) bool { return exists } +// GetSpendingTx returns the transaction that spends the given outpoint, if any. +// This provides O(1) access to the spentBy index for mempool query operations. +// +// Returns the spending transaction node and true if the outpoint is spent by a +// mempool transaction, otherwise returns nil and false. +func (g *TxGraph) GetSpendingTx(op wire.OutPoint) (*TxGraphNode, bool) { + g.mu.RLock() + defer g.mu.RUnlock() + + spender, exists := g.indexes.spentBy[op] + return spender, exists +} + // GetAncestors returns all ancestors of a transaction up to maxDepth. func (g *TxGraph) GetAncestors(hash chainhash.Hash, maxDepth int) map[chainhash.Hash]*TxGraphNode {