Respond to getdata requests for transactions.

This commit adds code to properly respond to getdata requests for
transactions by fetching them from the transaction pool.  Previously, we
advertised newly available transactions, but the code to respond with the
actual transaction was not written yet.

Also, fix a couple of comments and make the pushTxMsg and pushBlockMsg
functions consistent.

blockmanager.go

@@ -385,7 +385,7 @@ func (b *blockManager) haveInventory(invVect *btcwire.InvVect) bool {
 	case btcwire.InvVect_Tx:
 		// Ask the transaction memory pool if the transaction is known
 		// to it in any form (main pool or orphan).
-		if b.server.txMemPool.IsTransactionInPool(&invVect.Hash) {
+		if b.server.txMemPool.HaveTransaction(&invVect.Hash) {
 			return true
 		}
 

mempool.go

@@ -264,7 +264,7 @@ func checkTransactionStandard(tx *btcwire.MsgTx, height int64) error {
 	return nil
 }
 
-// checkInputsStandard performs a series of checks on a transactions inputs
+// checkInputsStandard performs a series of checks on a transaction's inputs
 // to ensure they are "standard".  A standard transaction input is one that
 // that consumes the same number of outputs from the stack as the output script
 // pushes.  This help prevent resource exhaustion attacks by "creative" use of
@@ -408,7 +408,7 @@ func (mp *txMemPool) maybeAddOrphan(tx *btcwire.MsgTx, txHash *btcwire.ShaHash)
 }
 
 // IsTransactionInPool returns whether or not the passed transaction already
-// exists in the memory pool.
+// exists in the main pool.
 func (mp *txMemPool) IsTransactionInPool(hash *btcwire.ShaHash) bool {
 	mp.lock.RLock()
 	defer mp.lock.RUnlock()
@@ -417,13 +417,28 @@ func (mp *txMemPool) IsTransactionInPool(hash *btcwire.ShaHash) bool {
 		return true
 	}
 
-	if _, exists := mp.orphans[*hash]; exists {
+	return false
+}
+
+// IsOrphanInPool returns whether or not the passed transaction already exists
+// in the orphan pool.
+func (mp *txMemPool) IsOrphanInPool(hash *btcwire.ShaHash) bool {
+	mp.lock.RLock()
+	defer mp.lock.RUnlock()
+
+	if _, exists := mp.pool[*hash]; exists {
 		return true
 	}
 
 	return false
 }
 
+// HaveTransaction returns whether or not the passed transaction already exists
+// in the main pool or in the orphan pool.
+func (mp *txMemPool) HaveTransaction(hash *btcwire.ShaHash) bool {
+	return mp.IsTransactionInPool(hash) || mp.IsOrphanInPool(hash)
+}
+
 // removeTransaction removes the passed transaction from the memory pool.
 func (mp *txMemPool) removeTransaction(tx *btcwire.MsgTx) {
 	mp.lock.Lock()
@@ -513,6 +528,20 @@ func (mp *txMemPool) fetchInputTransactions(tx *btcwire.MsgTx) (btcchain.TxStore
 	return txStore, nil
 }
 
+// FetchTransaction returns the requested transaction from the transaction pool.
+// This only fetches from the main transaction pool and does not include
+// orphans.
+func (mp *txMemPool) FetchTransaction(txHash *btcwire.ShaHash) (*btcwire.MsgTx, error) {
+	mp.lock.RLock()
+	defer mp.lock.RUnlock()
+
+	if tx, exists := mp.pool[*txHash]; exists {
+		return tx, nil
+	}
+
+	return nil, fmt.Errorf("transaction is not in the pool")
+}
+
 // maybeAcceptTransaction is the main workhorse for handling insertion of new
 // free-standing transactions into a memory pool.  It includes functionality
 // such as rejecting duplicate transactions, ensuring transactions follow all
@@ -526,10 +555,8 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcwire.MsgTx, isOrphan *bool) e
 
 	// Don't accept the transaction if it already exists in the pool.  This
 	// applies to orphan transactions as well.  This check is intended to
-	// be a quick check to weed out duplicates.  It is more expensive to
-	// detect a duplicate transaction in the main chain, so that is done
-	// later.
-	if mp.IsTransactionInPool(&txHash) {
+	// be a quick check to weed out duplicates.
+	if mp.HaveTransaction(&txHash) {
 		str := fmt.Sprintf("already have transaction %v", txHash)
 		return TxRuleError(str)
 	}

peer.go

@@ -7,7 +7,6 @@ package main
 import (
 	"bytes"
 	"container/list"
-	"errors"
 	"fmt"
 	"github.com/conformal/btcchain"
 	"github.com/conformal/btcdb"
@@ -303,15 +302,25 @@ func (p *peer) handleVersionMsg(msg *btcwire.MsgVersion) {
 }
 
 // pushTxMsg sends a tx message for the provided transaction hash to the
-// connected peer.  An error is returned if the transaction sha is not known.
-func (p *peer) pushTxMsg(sha btcwire.ShaHash) error {
-	// We dont deal with these for now.
-	return errors.New("Tx fetching not implemented")
+// connected peer.  An error is returned if the transaction hash is not known.
+func (p *peer) pushTxMsg(sha *btcwire.ShaHash) error {
+	// Attempt to fetch the requested transaction from the pool.  A
+	// call could be made to check for existence first, but simply trying
+	// to fetch a missing transaction results in the same behavior.
+	tx, err := p.server.txMemPool.FetchTransaction(sha)
+	if err != nil {
+		log.Tracef("PEER: Unable to fetch tx %v from transaction "+
+			"pool: %v", sha, err)
+		return err
+	}
+	p.QueueMessage(tx)
+
+	return nil
 }
 
 // pushBlockMsg sends a block message for the provided block hash to the
 // connected peer.  An error is returned if the block hash is not known.
-func (p *peer) pushBlockMsg(sha btcwire.ShaHash) error {
+func (p *peer) pushBlockMsg(sha *btcwire.ShaHash) error {
 	// What should this function do about the rate limiting the
 	// number of blocks queued for this peer?
 	// Current thought is have a counting mutex in the peer
@@ -326,10 +335,10 @@ func (p *peer) pushBlockMsg(sha btcwire.ShaHash) error {
 	// outstanding objects.
 	// Should the tx complete api be a mutex or channel?
 
-	blk, err := p.server.db.FetchBlockBySha(&sha)
+	blk, err := p.server.db.FetchBlockBySha(sha)
 	if err != nil {
 		log.Tracef("PEER: Unable to fetch requested block sha %v: %v",
-			&sha, err)
+			sha, err)
 		return err
 	}
 	p.QueueMessage(blk.MsgBlock())
@@ -339,7 +348,7 @@ func (p *peer) pushBlockMsg(sha btcwire.ShaHash) error {
 	// would fit into a single message, send it a new inventory message
 	// to trigger it to issue another getblocks message for the next
 	// batch of inventory.
-	if p.continueHash != nil && p.continueHash.IsEqual(&sha) {
+	if p.continueHash != nil && p.continueHash.IsEqual(sha) {
 		hash, _, err := p.server.db.NewestSha()
 		if err == nil {
 			invMsg := btcwire.NewMsgInv()
@@ -489,9 +498,9 @@ out:
 		var err error
 		switch iv.Type {
 		case btcwire.InvTypeTx:
-			err = p.pushTxMsg(iv.Hash)
+			err = p.pushTxMsg(&iv.Hash)
 		case btcwire.InvTypeBlock:
-			err = p.pushBlockMsg(iv.Hash)
+			err = p.pushBlockMsg(&iv.Hash)
 		default:
 			log.Warnf("PEER: Unknown type in inventory request %d",
 				iv.Type)