Repository: decred/dcrd
Pull request #2324 (2020-08-05 03:38:06)

rpcserver: Add TxMempooler interface and TestHandleExistsMempoolTxs.
    rpcserver: Add TxMempooler interface.
    
    In order to allow tests to use mocked mempool transaction data, replace
    the RPC server's TxMemPool with an interface that satisfies what is
    needed for the server.

    rpcserver: Add handleExistsMempoolTxs test.

davecgh: This is good to go now, but I see there is a conflict due to merging #2306.  Would  you rebase this to resolve that?
JoeGruffins: rebased

Patch:
diff --git a/internal/rpcserver/interface.go b/internal/rpcserver/interface.go
index 908530ae..93bda8d6 100644
--- a/internal/rpcserver/interface.go
+++ b/internal/rpcserver/interface.go
@@ -566,3 +566,32 @@ type ExistsAddresser interface {
 	// addresses has been seen before.
 	ExistsAddresses(addrs []dcrutil.Address) ([]bool, error)
 }
+
+// TxMempooler represents a source of mempool transaction data for the RPC
+// server. Methods assume the existence of a main pool and an orphans pool.
+//
+// The interface contract requires that all of these methods are safe for
+// concurrent access.
+type TxMempooler interface {
+	// HaveTransactions returns whether or not the passed transactions
+	// already exist in the main pool or in the orphan pool.
+	HaveTransactions(hashes []*chainhash.Hash) []bool
+
+	// TxDescs returns a slice of descriptors for all the transactions in
+	// the pool. The descriptors must be treated as read only.
+	TxDescs() []*mempool.TxDesc
+
+	// VerboseTxDescs returns a slice of verbose descriptors for all the
+	// transactions in the pool. The descriptors must be treated as read
+	// only.
+	VerboseTxDescs() []*mempool.VerboseTxDesc
+
+	// Count returns the number of transactions in the main pool. It does
+	// not include the orphan pool.
+	Count() int
+
+	// FetchTransaction returns the requested transaction from the
+	// transaction pool. This only fetches from the main transaction pool
+	// and does not include orphans.
+	FetchTransaction(txHash *chainhash.Hash) (*dcrutil.Tx, error)
+}
diff --git a/internal/rpcserver/rpcserver.go b/internal/rpcserver/rpcserver.go
index 109fe45e..19fd9296 100644
--- a/internal/rpcserver/rpcserver.go
+++ b/internal/rpcserver/rpcserver.go
@@ -1636,7 +1636,7 @@ func handleExistsMempoolTxs(_ context.Context, s *Server, cmd interface{}) (inte
 		return nil, err
 	}
 
-	exists := s.cfg.TxMemPool.HaveTransactions(hashes)
+	exists := s.cfg.TxMempooler.HaveTransactions(hashes)
 	if len(exists) != len(hashes) {
 		return nil, rpcInternalError(fmt.Sprintf("got %v, want %v",
 			len(exists), len(hashes)),
@@ -2407,7 +2407,7 @@ func handleGetInfo(_ context.Context, s *Server, cmd interface{}) (interface{},
 
 // handleGetMempoolInfo implements the getmempoolinfo command.
 func handleGetMempoolInfo(_ context.Context, s *Server, cmd interface{}) (interface{}, error) {
-	mempoolTxns := s.cfg.TxMemPool.TxDescs()
+	mempoolTxns := s.cfg.TxMempooler.TxDescs()
 
 	var numBytes int64
 	for _, txD := range mempoolTxns {
@@ -2456,7 +2456,7 @@ func handleGetMiningInfo(ctx context.Context, s *Server, cmd interface{}) (inter
 		GenProcLimit:     s.cfg.CPUMiner.NumWorkers(),
 		HashesPerSec:     int64(s.cfg.CPUMiner.HashesPerSecond()),
 		NetworkHashPS:    networkHashesPerSec,
-		PooledTx:         uint64(s.cfg.TxMemPool.Count()),
+		PooledTx:         uint64(s.cfg.TxMempooler.Count()),
 		TestNet:          s.cfg.TestNet,
 	}
 	return &result, nil
@@ -2672,9 +2672,8 @@ func handleGetRawMempool(_ context.Context, s *Server, cmd interface{}) (interfa
 	}
 
 	// Return verbose results if requested.
-	mp := s.cfg.TxMemPool
 	if c.Verbose != nil && *c.Verbose {
-		descs := mp.VerboseTxDescs()
+		descs := s.cfg.TxMempooler.VerboseTxDescs()
 		result := make(map[string]*types.GetRawMempoolVerboseResult, len(descs))
 		for i := range descs {
 			desc := descs[i]
@@ -2704,7 +2703,7 @@ func handleGetRawMempool(_ context.Context, s *Server, cmd interface{}) (interfa
 
 	// The response is simply an array of the transaction hashes if the
 	// verbose flag is not set.
-	descs := mp.TxDescs()
+	descs := s.cfg.TxMempooler.TxDescs()
 	hashStrings := make([]string, 0, len(descs))
 	for i := range descs {
 		if filterType != nil && descs[i].Type != *filterType {
@@ -2736,7 +2735,7 @@ func handleGetRawTransaction(_ context.Context, s *Server, cmd interface{}) (int
 	var blkHash *chainhash.Hash
 	var blkHeight int64
 	var blkIndex uint32
-	tx, err := s.cfg.TxMemPool.FetchTransaction(txHash)
+	tx, err := s.cfg.TxMempooler.FetchTransaction(txHash)
 	if err != nil {
 		txIndex := s.cfg.TxIndex
 		if txIndex == nil {
@@ -3166,7 +3165,7 @@ func handleGetTxOut(_ context.Context, s *Server, cmd interface{}) (interface{},
 	}
 	var txFromMempool *dcrutil.Tx
 	if includeMempool {
-		txFromMempool, _ = s.cfg.TxMemPool.FetchTransaction(txHash)
+		txFromMempool, _ = s.cfg.TxMempooler.FetchTransaction(txHash)
 	}
 	if txFromMempool != nil {
 		mtx := txFromMempool.MsgTx()
@@ -3657,7 +3656,6 @@ type retrievedTx struct {
 // inputs to the passed transaction by checking the transaction mempool first
 // then the transaction index for those already mined into blocks.
 func fetchInputTxos(s *Server, tx *wire.MsgTx) (map[wire.OutPoint]wire.TxOut, error) {
-	mp := s.cfg.TxMemPool
 	originOutputs := make(map[wire.OutPoint]wire.TxOut)
 	voteTx := stake.IsSSGen(tx)
 	for txInIndex, txIn := range tx.TxIn {
@@ -3669,7 +3667,7 @@ func fetchInputTxos(s *Server, tx *wire.MsgTx) (map[wire.OutPoint]wire.TxOut, er
 		// Attempt to fetch and use the referenced transaction from the
 		// memory pool.
 		origin := &txIn.PreviousOutPoint
-		originTx, err := mp.FetchTransaction(&origin.Hash)
+		originTx, err := s.cfg.TxMempooler.FetchTransaction(&origin.Hash)
 		if err == nil {
 			txOuts := originTx.MsgTx().TxOut
 			if origin.Index >= uint32(len(txOuts)) {
@@ -4381,7 +4379,7 @@ func stdDev(s []dcrutil.Amount) dcrutil.Amount {
 // feeInfoForMempool returns the fee information for the passed tx type in the
 // memory pool.
 func feeInfoForMempool(s *Server, txType stake.TxType) *types.FeeInfoMempool {
-	txDs := s.cfg.TxMemPool.TxDescs()
+	txDs := s.cfg.TxMempooler.TxDescs()
 	ticketFees := make([]dcrutil.Amount, 0, len(txDs))
 	for _, txD := range txDs {
 		if txD.Type == txType {
@@ -5632,8 +5630,8 @@ type Config struct {
 	// Clock defines the clock for the RPC server to use.
 	Clock Clock
 
-	// TxMemPool defines the transaction memory pool to interact with.
-	TxMemPool *mempool.TxPool
+	// TxMempooler defines the transaction memory pool to interact with.
+	TxMempooler TxMempooler
 
 	// These fields allow the RPC server to interface with mining.
 	//
diff --git a/rpcadaptors.go b/rpcadaptors.go
index 5a8b0f54..9afca691 100644
--- a/rpcadaptors.go
+++ b/rpcadaptors.go
@@ -597,16 +597,16 @@ type rpcFiltererV2 struct {
 // Ensure rpcFiltererV2 implements the rpcserver.FiltererV2 interface.
 var _ rpcserver.FiltererV2 = (*rpcFiltererV2)(nil)
 
-// Ensure rpcExistsAddresser implements the rpcserver.ExistsAddresser interface.
-var _ rpcserver.ExistsAddresser = (*rpcExistsAddresser)(nil)
-
 // rpcExistsAddresser provides exists address methods for use with the RPC
 // server and implements the rpcserver.ExistsAddresser interface. It should be
 // constructed by calling newRPCExistsAddresser.
 type rpcExistsAddresser struct {
-	existsAddrIndex *indexers.ExistsAddrIndex
+	*indexers.ExistsAddrIndex
 }
 
+// Ensure rpcExistsAddresser implements the rpcserver.ExistsAddresser interface.
+var _ rpcserver.ExistsAddresser = (*rpcExistsAddresser)(nil)
+
 // newRPCExistsAddresser is a constructor for a new rpcserver.ExistsAddresser
 // that wraps *rpcExistsAddresser. If the server's existsAddrIndex is nil, nil
 // is returned. Consumers must check for nil before calling methods on the
@@ -618,13 +618,11 @@ func newRPCExistsAddresser(e *indexers.ExistsAddrIndex) rpcserver.ExistsAddresse
 	return &rpcExistsAddresser{e}
 }
 
-// ExistsAddress returns whether or not an address has been seen before.
-func (e *rpcExistsAddresser) ExistsAddress(addr dcrutil.Address) (bool, error) {
-	return e.existsAddrIndex.ExistsAddress(addr)
+// rpcTxMempooler provides a mempool transaction data source for use with the
+// RPC server and implements the rpcserver.TxMempooler interface.
+type rpcTxMempooler struct {
+	*mempool.TxPool
 }
 
-// ExistsAddresses returns whether or not each address in a slice of addresses
-// has been seen before.
-func (e *rpcExistsAddresser) ExistsAddresses(addrs []dcrutil.Address) ([]bool, error) {
-	return e.existsAddrIndex.ExistsAddresses(addrs)
-}
+// Ensure rpcTxMempooler implements the rpcserver.TxMempooler interface.
+var _ rpcserver.TxMempooler = (*rpcTxMempooler)(nil)
diff --git a/server.go b/server.go
index c0d821f5..d7b5e69f 100644
--- a/server.go
+++ b/server.go
@@ -3279,7 +3279,7 @@ func newServer(ctx context.Context, listenAddrs []string, db database.DB, chainP
 			ChainParams:     chainParams,
 			SanityChecker:   &rpcSanityChecker{s.timeSource, chainParams},
 			DB:              db,
-			TxMemPool:       s.txMemPool,
+			TxMempooler:     &rpcTxMempooler{s.txMemPool},
 			BlockTemplater: func() rpcserver.BlockTemplater {
 				if s.bg == nil {
 					return nil


Test patch:
diff --git a/internal/rpcserver/rpcserverhandlers_test.go b/internal/rpcserver/rpcserverhandlers_test.go
index 3e7b1d02..7ce70a65 100644
--- a/internal/rpcserver/rpcserverhandlers_test.go
+++ b/internal/rpcserver/rpcserverhandlers_test.go
@@ -828,6 +828,46 @@ func (b *testBlockTemplater) UpdateBlockTime(header *wire.BlockHeader) error {
 	return b.updateBlockTimeErr
 }
 
+// testTxMempooler provides a mock mempool transaction data source by
+// implementing the TxMempooler interface.
+type testTxMempooler struct {
+	haveTransactions    []bool
+	txDescs             []*mempool.TxDesc
+	verboseTxDescs      []*mempool.VerboseTxDesc
+	count               int
+	fetchTransaction    *dcrutil.Tx
+	fetchTransactionErr error
+}
+
+// HaveTransactions returns a mocked bool slice representing whether or not the
+// passed transactions already exist.
+func (mp *testTxMempooler) HaveTransactions(hashes []*chainhash.Hash) []bool {
+	return mp.haveTransactions
+}
+
+// TxDescs returns a mock slice of descriptors for all the transactions in
+// the pool.
+func (mp *testTxMempooler) TxDescs() []*mempool.TxDesc {
+	return mp.txDescs
+}
+
+// VerboseTxDescs returns a mock slice of verbose descriptors for all the
+// transactions in the pool.
+func (mp *testTxMempooler) VerboseTxDescs() []*mempool.VerboseTxDesc {
+	return mp.verboseTxDescs
+}
+
+// Count returns a mock number of transactions in the main pool.
+func (mp *testTxMempooler) Count() int {
+	return mp.count
+}
+
+// FetchTransaction returns the mocked requested transaction from the
+// transaction pool.
+func (mp *testTxMempooler) FetchTransaction(txHash *chainhash.Hash) (*dcrutil.Tx, error) {
+	return mp.fetchTransaction, mp.fetchTransactionErr
+}
+
 // mustParseHash converts the passed big-endian hex string into a
 // chainhash.Hash and will panic if there is an error.  It only differs from the
 // one available in chainhash in that it will panic so errors in the source code
@@ -907,6 +947,7 @@ type rpcTest struct {
 	mockLogManager        *testLogManager
 	mockFilterer          *testFilterer
 	mockFiltererV2        *testFiltererV2
+	mockTxMempooler       *testTxMempooler
 	result                interface{}
 	wantErr               bool
 	errCode               dcrjson.RPCErrorCode
@@ -1233,6 +1274,15 @@ func defaultMockCPUMiner() *testCPUMiner {
 	return &testCPUMiner{}
 }
 
+// defaultMockTxMempooler provides a default mock mempool transaction source
+// to be used throughout the tests. Tests can override these defaults by
+// calling defaultMockTxMempooler, updating fields as necessary on the returned
+// *testTxMempooler, and then setting rpcTest.mockTxMempooler as that
+// *testTxMempooler.
+func defaultMockTxMempooler() *testTxMempooler {
+	return &testTxMempooler{}
+}
+
 // defaultMockConfig provides a default Config that is used throughout
 // the tests.  Defaults can be overridden by tests through the rpcTest struct.
 func defaultMockConfig(chainParams *chaincfg.Params) *Config {
@@ -1247,6 +1297,7 @@ func defaultMockConfig(chainParams *chaincfg.Params) *Config {
 		ExistsAddresser: defaultMockExistsAddresser(),
 		ConnMgr:         defaultMockConnManager(),
 		CPUMiner:        defaultMockCPUMiner(),
+		TxMempooler:     defaultMockTxMempooler(),
 		Clock:           &testClock{},
 		LogManager:      defaultMockLogManager(),
 		FiltererV2:      defaultMockFiltererV2(),
@@ -2694,6 +2745,51 @@ func TestHandleExistsLiveTickets(t *testing.T) {
 	}})
 }
 
+func TestHandleExistsMempoolTxs(t *testing.T) {
+	t.Parallel()
+
+	defaultCmdTxHashes := []string{
+		"1189cbe656c2ef1e0fcb91f107624d9aa8f0db7b28e6a86f694a4cf49abc5e39",
+		"2189cbe656c2ef1e0fcb91f107624d9aa8f0db7b28e6a86f694a4cf49abc5e39",
+	}
+	testRPCServerHandler(t, []rpcTest{{
+		name:    "handleExistsMempoolTxs: ok",
+		handler: handleExistsMempoolTxs,
+		cmd: &types.ExistsMempoolTxsCmd{
+			TxHashes: defaultCmdTxHashes,
+		},
+		mockTxMempooler: func() *testTxMempooler {
+			mp := defaultMockTxMempooler()
+			mp.haveTransactions = []bool{false, true}
+			return mp
+		}(),
+		result: "02",
+	}, {
+		name:    "handleExistsMempoolTxs: invalid hash",
+		handler: handleExistsMempoolTxs,
+		cmd: &types.ExistsMempoolTxsCmd{
+			TxHashes: []string{
+				"g189cbe656c2ef1e0fcb91f107624d9aa8f0db7b28e6a86f694a4cf49abc5e39",
+			},
+		},
+		wantErr: true,
+		errCode: dcrjson.ErrRPCDecodeHexString,
+	}, {
+		name:    "handleExistsMempoolTxs: have transaction count different than number of args",
+		handler: handleExistsMempoolTxs,
+		cmd: &types.ExistsMempoolTxsCmd{
+			TxHashes: defaultCmdTxHashes,
+		},
+		mockTxMempooler: func() *testTxMempooler {
+			mp := defaultMockTxMempooler()
+			mp.haveTransactions = []bool{false}
+			return mp
+		}(),
+		wantErr: true,
+		errCode: dcrjson.ErrRPCInternal.Code,
+	}})
+}
+
 func TestHandleExistsMissedTickets(t *testing.T) {
 	t.Parallel()
 
@@ -4928,6 +5024,9 @@ func testRPCServerHandler(t *testing.T, tests []rpcTest) {
 			if test.setBlockTemplaterNil {
 				rpcserverConfig.BlockTemplater = nil
 			}
+			if test.mockTxMempooler != nil {
+				rpcserverConfig.TxMempooler = test.mockTxMempooler
+			}
 
 			testServer := &Server{cfg: *rpcserverConfig, workState: workState}
 			result, err := test.handler(nil, testServer, test.cmd)