Repository: decred/dcrd
Pull request #2291 (2020-07-23 02:58:46)

rpcserver: Add handleExistsAddress(es) tests.
    rpcserver: Add handleExistsAddress test.
    This removes the ExistsAddrIndex method from the SyncManager interface
    and replaces it with a new interface ExistsAddresser that supplies the
    two methods used by the rpcserver, ExistsAddress and ExistsAddresses.

    rpcserver: Add handleExistsAddresses test.

    rpcserver: Add exists upper bounds TODOs.
    Although permissioned, the RPC server will allow for checking any number
    of values for its various exists commands. It would be reasonable to
    limit unreasonable requests that could exhaust dcrd's available memory
    and cause node failure. Changes will come with a major RPC version bump.




rstaudt2: Since all that rpcserver really cares about is whether the address(es) exist or not, what do you think about just exposing `ExistsAddress` and `ExistsAddresses` methods on the `SyncManager` interface?

This way, the handler could just call `exists, err := s.cfg.SyncMgr.ExistsAddress(addr)`, and the adaptor could handle checking if the index is enabled and returning the appropriate err if its not.
JoeGruffins: rebased

Patch:
diff --git a/internal/rpcserver/interface.go b/internal/rpcserver/interface.go
index f79851fc..e0f16a1e 100644
--- a/internal/rpcserver/interface.go
+++ b/internal/rpcserver/interface.go
@@ -13,7 +13,6 @@ import (
 	"github.com/decred/dcrd/addrmgr"
 	"github.com/decred/dcrd/blockchain/stake/v3"
 	"github.com/decred/dcrd/blockchain/v3"
-	"github.com/decred/dcrd/blockchain/v3/indexers"
 	"github.com/decred/dcrd/chaincfg/chainhash"
 	"github.com/decred/dcrd/dcrutil/v3"
 	"github.com/decred/dcrd/gcs/v2"
@@ -156,9 +155,6 @@ type SyncManager interface {
 	LocateBlocks(locator blockchain.BlockLocator, hashStop *chainhash.Hash,
 		maxHashes uint32) []chainhash.Hash
 
-	// ExistsAddrIndex returns the address index.
-	ExistsAddrIndex() *indexers.ExistsAddrIndex
-
 	// TipGeneration returns the entire generation of blocks stemming from the
 	// parent of the current tip.
 	TipGeneration() ([]chainhash.Hash, error)
@@ -538,3 +534,21 @@ type FiltererV2 interface {
 	// for the given block hash does not exist.
 	FilterByBlockHash(hash *chainhash.Hash) (*gcs.FilterV2, error)
 }
+
+// ExistsAddresser represents a source of exists address methods for the RPC
+// server. These methods return whether or not an address or addresses have
+// been seen on the blockchain.
+//
+// The interface contract requires that all of these methods are safe for
+// concurrent access.
+//
+// ExistsAddresser may be nil. The RPC server must check for the presence of an
+// ExistsAddresser before calling methods associated with it.
+type ExistsAddresser interface {
+	// ExistsAddress returns whether or not an address has been seen before.
+	ExistsAddress(addr dcrutil.Address) (bool, error)
+
+	// ExistsAddresses returns whether or not each address in a slice of
+	// addresses has been seen before.
+	ExistsAddresses(addrs []dcrutil.Address) ([]bool, error)
+}
diff --git a/internal/rpcserver/rpcserver.go b/internal/rpcserver/rpcserver.go
index 29039819..41cc5eef 100644
--- a/internal/rpcserver/rpcserver.go
+++ b/internal/rpcserver/rpcserver.go
@@ -1434,8 +1434,7 @@ func handleEstimateStakeDiff(_ context.Context, s *Server, cmd interface{}) (int
 
 // handleExistsAddress implements the existsaddress command.
 func handleExistsAddress(_ context.Context, s *Server, cmd interface{}) (interface{}, error) {
-	existsAddrIndex := s.cfg.SyncMgr.ExistsAddrIndex()
-	if existsAddrIndex == nil {
+	if s.cfg.ExistsAddresser == nil {
 		return nil, rpcInternalError("Exists address index disabled",
 			"Configuration")
 	}
@@ -1450,7 +1449,7 @@ func handleExistsAddress(_ context.Context, s *Server, cmd interface{}) (interfa
 			err)
 	}
 
-	exists, err := existsAddrIndex.ExistsAddress(addr)
+	exists, err := s.cfg.ExistsAddresser.ExistsAddress(addr)
 	if err != nil {
 		return nil, rpcInvalidError("Could not query address: %v", err)
 	}
@@ -1459,9 +1458,11 @@ func handleExistsAddress(_ context.Context, s *Server, cmd interface{}) (interfa
 }
 
 // handleExistsAddresses implements the existsaddresses command.
+//
+// TODO: Add an upper bound to the number of addresses that can be checked.
+// This will come with a major RPC version bump.
 func handleExistsAddresses(_ context.Context, s *Server, cmd interface{}) (interface{}, error) {
-	existsAddrIndex := s.cfg.SyncMgr.ExistsAddrIndex()
-	if existsAddrIndex == nil {
+	if s.cfg.ExistsAddresser == nil {
 		return nil, rpcInternalError("Exists address index disabled",
 			"Configuration")
 	}
@@ -1478,7 +1479,7 @@ func handleExistsAddresses(_ context.Context, s *Server, cmd interface{}) (inter
 		addresses[i] = addr
 	}
 
-	exists, err := existsAddrIndex.ExistsAddresses(addresses)
+	exists, err := s.cfg.ExistsAddresser.ExistsAddresses(addresses)
 	if err != nil {
 		return nil, rpcInvalidError("Could not query address: %v", err)
 	}
@@ -1525,6 +1526,9 @@ func decodeHashPointers(strs []string) ([]*chainhash.Hash, error) {
 }
 
 // handleExistsMissedTickets implements the existsmissedtickets command.
+//
+// TODO: Add an upper bound to the number of hashes that can be checked. This
+// will come with a major RPC version bump.
 func handleExistsMissedTickets(_ context.Context, s *Server, cmd interface{}) (interface{}, error) {
 	c := cmd.(*types.ExistsMissedTicketsCmd)
 
@@ -1551,6 +1555,9 @@ func handleExistsMissedTickets(_ context.Context, s *Server, cmd interface{}) (i
 }
 
 // handleExistsExpiredTickets implements the existsexpiredtickets command.
+//
+// TODO: Add an upper bound to the number of hashes that can be checked. This
+// will come with a major RPC version bump.
 func handleExistsExpiredTickets(_ context.Context, s *Server, cmd interface{}) (interface{}, error) {
 	c := cmd.(*types.ExistsExpiredTicketsCmd)
 
@@ -1589,6 +1596,9 @@ func handleExistsLiveTicket(_ context.Context, s *Server, cmd interface{}) (inte
 }
 
 // handleExistsLiveTickets implements the existslivetickets command.
+//
+// TODO: Add an upper bound to the number of hashes that can be checked. This
+// will come with a major RPC version bump.
 func handleExistsLiveTickets(_ context.Context, s *Server, cmd interface{}) (interface{}, error) {
 	c := cmd.(*types.ExistsLiveTicketsCmd)
 
@@ -1615,6 +1625,9 @@ func handleExistsLiveTickets(_ context.Context, s *Server, cmd interface{}) (int
 }
 
 // handleExistsMempoolTxs implements the existsmempooltxs command.
+//
+// TODO: Add an upper bound to the number of hashes that can be checked. This
+// will come with a major RPC version bump.
 func handleExistsMempoolTxs(_ context.Context, s *Server, cmd interface{}) (interface{}, error) {
 	c := cmd.(*types.ExistsMempoolTxsCmd)
 
@@ -5594,6 +5607,10 @@ type Config struct {
 	// SyncMgr defines the sync manager for the RPC server to use.
 	SyncMgr SyncManager
 
+	// ExistsAddresser defines the exist addresser for the RPC server to
+	// use.
+	ExistsAddresser ExistsAddresser
+
 	// These fields allow the RPC server to interface with the local block
 	// chain data and state.
 	TimeSource    blockchain.MedianTimeSource
diff --git a/rpcadaptors.go b/rpcadaptors.go
index c7e97b1a..0f041151 100644
--- a/rpcadaptors.go
+++ b/rpcadaptors.go
@@ -363,14 +363,6 @@ func (b *rpcSyncMgr) LocateBlocks(locator blockchain.BlockLocator, hashStop *cha
 	return b.server.chain.LocateBlocks(locator, hashStop, maxHashes)
 }
 
-// ExistsAddrIndex returns the address index.
-//
-// This function is safe for concurrent access and is part of the
-// rpcserver.SyncManager interface implementation.
-func (b *rpcSyncMgr) ExistsAddrIndex() *indexers.ExistsAddrIndex {
-	return b.server.existsAddrIndex
-}
-
 // TipGeneration returns the entire generation of blocks stemming from the
 // parent of the current tip.
 func (b *rpcSyncMgr) TipGeneration() ([]chainhash.Hash, error) {
@@ -597,3 +589,35 @@ 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
+}
+
+// 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
+// returned rpcserver.ExistsAddresser.
+func newRPCExistsAddresser(e *indexers.ExistsAddrIndex) rpcserver.ExistsAddresser {
+	if e == nil {
+		return nil
+	}
+	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)
+}
+
+// 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)
+}
diff --git a/server.go b/server.go
index ce726750..e9783fc5 100644
--- a/server.go
+++ b/server.go
@@ -3265,20 +3265,21 @@ func newServer(ctx context.Context, listenAddrs []string, db database.DB, chainP
 		}
 
 		s.rpcServer, err = rpcserver.New(&rpcserver.Config{
-			Listeners:     rpcListeners,
-			ConnMgr:       &rpcConnManager{&s},
-			SyncMgr:       &rpcSyncMgr{&s, s.blockManager},
-			FeeEstimator:  &rpcFeeEstimator{s.feeEstimator},
-			TimeSource:    s.timeSource,
-			Services:      s.services,
-			AddrManager:   &rpcAddrManager{s.addrManager},
-			Clock:         &rpcClock{},
-			SubsidyCache:  s.subsidyCache,
-			Chain:         &rpcChain{s.chain},
-			ChainParams:   chainParams,
-			SanityChecker: &rpcSanityChecker{s.timeSource, chainParams},
-			DB:            db,
-			TxMemPool:     s.txMemPool,
+			Listeners:       rpcListeners,
+			ConnMgr:         &rpcConnManager{&s},
+			SyncMgr:         &rpcSyncMgr{server: &s, blockMgr: s.blockManager},
+			ExistsAddresser: newRPCExistsAddresser(s.existsAddrIndex),
+			FeeEstimator:    &rpcFeeEstimator{s.feeEstimator},
+			TimeSource:      s.timeSource,
+			Services:        s.services,
+			AddrManager:     &rpcAddrManager{s.addrManager},
+			Clock:           &rpcClock{},
+			SubsidyCache:    s.subsidyCache,
+			Chain:           &rpcChain{s.chain},
+			ChainParams:     chainParams,
+			SanityChecker:   &rpcSanityChecker{s.timeSource, chainParams},
+			DB:              db,
+			TxMemPool:       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 6cf8f250..4a1ea554 100644
--- a/internal/rpcserver/rpcserverhandlers_test.go
+++ b/internal/rpcserver/rpcserverhandlers_test.go
@@ -26,7 +26,6 @@ import (
 	"github.com/decred/dcrd/blockchain/stake/v3"
 	"github.com/decred/dcrd/blockchain/standalone/v2"
 	"github.com/decred/dcrd/blockchain/v3"
-	"github.com/decred/dcrd/blockchain/v3/indexers"
 	"github.com/decred/dcrd/chaincfg/chainhash"
 	"github.com/decred/dcrd/chaincfg/v3"
 	"github.com/decred/dcrd/dcrjson/v3"
@@ -443,7 +442,6 @@ type testSyncManager struct {
 	submitBlockErr     error
 	syncPeerID         int32
 	locateBlocks       []chainhash.Hash
-	existsAddrIndex    *indexers.ExistsAddrIndex
 	tipGeneration      []chainhash.Hash
 	syncHeight         int64
 	processTransaction []*dcrutil.Tx
@@ -473,11 +471,6 @@ func (s *testSyncManager) LocateBlocks(locator blockchain.BlockLocator, hashStop
 	return s.locateBlocks
 }
 
-// ExistsAddrIndex returns a mocked address index.
-func (s *testSyncManager) ExistsAddrIndex() *indexers.ExistsAddrIndex {
-	return s.existsAddrIndex
-}
-
 // TipGeneration returns a mocked entire generation of blocks stemming from the
 // parent of the current tip.
 func (s *testSyncManager) TipGeneration() ([]chainhash.Hash, error) {
@@ -496,6 +489,27 @@ func (s *testSyncManager) ProcessTransaction(tx *dcrutil.Tx, allowOrphans bool,
 	return s.processTransaction, nil
 }
 
+// testExistsAddresser provides a mock exists addresser by implementing the
+// ExistsAddresser interface.
+type testExistsAddresser struct {
+	existsAddress      bool
+	existsAddressErr   error
+	existsAddresses    []bool
+	existsAddressesErr error
+}
+
+// ExistsAddress returns a mocked bool representing whether or not an address
+// has been seen before.
+func (e *testExistsAddresser) ExistsAddress(addr dcrutil.Address) (bool, error) {
+	return e.existsAddress, e.existsAddressErr
+}
+
+// ExistsAddresses returns a mocked bool slice representing whether or not each
+// address in a slice of addresses has been seen before.
+func (e *testExistsAddresser) ExistsAddresses(addrs []dcrutil.Address) ([]bool, error) {
+	return e.existsAddresses, e.existsAddressesErr
+}
+
 // testConnManager provides a mock connection manager by implementing the
 // ConnManager interface.
 type testConnManager struct {
@@ -752,23 +766,25 @@ var block432100 = func() wire.MsgBlock {
 }()
 
 type rpcTest struct {
-	name              string
-	handler           commandHandler
-	cmd               interface{}
-	mockChainParams   *chaincfg.Params
-	mockChain         *testRPCChain
-	mockSanityChecker *testSanityChecker
-	mockAddrManager   *testAddrManager
-	mockFeeEstimator  *testFeeEstimator
-	mockSyncManager   *testSyncManager
-	mockConnManager   *testConnManager
-	mockClock         *testClock
-	mockLogManager    *testLogManager
-	mockFilterer      *testFilterer
-	mockFiltererV2    *testFiltererV2
-	result            interface{}
-	wantErr           bool
-	errCode           dcrjson.RPCErrorCode
+	name                  string
+	handler               commandHandler
+	cmd                   interface{}
+	mockChainParams       *chaincfg.Params
+	mockChain             *testRPCChain
+	mockSanityChecker     *testSanityChecker
+	mockAddrManager       *testAddrManager
+	mockFeeEstimator      *testFeeEstimator
+	mockSyncManager       *testSyncManager
+	mockExistsAddresser   *testExistsAddresser
+	setExistsAddresserNil bool
+	mockConnManager       *testConnManager
+	mockClock             *testClock
+	mockLogManager        *testLogManager
+	mockFilterer          *testFilterer
+	mockFiltererV2        *testFiltererV2
+	result                interface{}
+	wantErr               bool
+	errCode               dcrjson.RPCErrorCode
 }
 
 // defaultChainParams provides a default chaincfg.Params to be used throughout
@@ -938,6 +954,15 @@ func defaultMockAddrManager() *testAddrManager {
 	}
 }
 
+// defaultMockExistsAddresser provides a default mock exists addresser to be
+// used throughout the tests. Tests can override these defaults by calling
+// defaultMockExistsAddresser, updating fields as necessary on the returned
+// *testExistsAddresser, and then setting rpcTest.mockExistsAddresser as that
+// *testExistsAddresser.
+func defaultMockExistsAddresser() *testExistsAddresser {
+	return &testExistsAddresser{}
+}
+
 // defaultMockSyncManager provides a default mock sync manager to be used
 // throughout the tests. Tests can override these defaults by calling
 // defaultMockSyncManager, updating fields as necessary on the returned
@@ -1049,19 +1074,20 @@ func defaultMockFiltererV2() *testFiltererV2 {
 // the tests.  Defaults can be overridden by tests through the rpcTest struct.
 func defaultMockConfig(chainParams *chaincfg.Params) *Config {
 	return &Config{
-		ChainParams:   chainParams,
-		Chain:         defaultMockRPCChain(),
-		SanityChecker: defaultMockSanityChecker(),
-		AddrManager:   defaultMockAddrManager(),
-		FeeEstimator:  defaultMockFeeEstimator(),
-		SyncMgr:       defaultMockSyncManager(),
-		ConnMgr:       defaultMockConnManager(),
-		Clock:         &testClock{},
-		LogManager:    defaultMockLogManager(),
-		FiltererV2:    defaultMockFiltererV2(),
-		TimeSource:    blockchain.NewMedianTime(),
-		Services:      wire.SFNodeNetwork | wire.SFNodeCF,
-		SubsidyCache:  standalone.NewSubsidyCache(chainParams),
+		ChainParams:     chainParams,
+		Chain:           defaultMockRPCChain(),
+		SanityChecker:   defaultMockSanityChecker(),
+		AddrManager:     defaultMockAddrManager(),
+		FeeEstimator:    defaultMockFeeEstimator(),
+		SyncMgr:         defaultMockSyncManager(),
+		ExistsAddresser: defaultMockExistsAddresser(),
+		ConnMgr:         defaultMockConnManager(),
+		Clock:           &testClock{},
+		LogManager:      defaultMockLogManager(),
+		FiltererV2:      defaultMockFiltererV2(),
+		TimeSource:      blockchain.NewMedianTime(),
+		Services:        wire.SFNodeNetwork | wire.SFNodeCF,
+		SubsidyCache:    standalone.NewSubsidyCache(chainParams),
 		NetInfo: []types.NetworksResult{{
 			Name:                      "IPV4",
 			Limited:                   false,
@@ -2202,6 +2228,103 @@ func TestHandleEstimateStakeDiff(t *testing.T) {
 	}})
 }
 
+func TestHandleExistsAddress(t *testing.T) {
+	t.Parallel()
+
+	validAddr := "DcurAwesomeAddressmqDctW5wJCW1Cn2MF"
+	testRPCServerHandler(t, []rpcTest{{
+		name:    "handleExistsAddress: ok",
+		handler: handleExistsAddress,
+		cmd: &types.ExistsAddressCmd{
+			Address: validAddr,
+		},
+		result: false,
+	}, {
+		name:    "handleExistsAddress: exist address indexing not enabled",
+		handler: handleExistsAddress,
+		cmd: &types.ExistsAddressCmd{
+			Address: validAddr,
+		},
+		setExistsAddresserNil: true,
+		wantErr:               true,
+		errCode:               dcrjson.ErrRPCInternal.Code,
+	}, {
+		name:    "handleExistsAddress: bad address",
+		handler: handleExistsAddress,
+		cmd: &types.ExistsAddressCmd{
+			Address: "bad address",
+		},
+		wantErr: true,
+		errCode: dcrjson.ErrRPCInvalidAddressOrKey,
+	}, {
+		name:    "handleExistsAddress: ExistsAddress error",
+		handler: handleExistsAddress,
+		cmd: &types.ExistsAddressCmd{
+			Address: validAddr,
+		},
+		mockExistsAddresser: func() *testExistsAddresser {
+			existsAddrIndexer := defaultMockExistsAddresser()
+			existsAddrIndexer.existsAddressErr = errors.New("")
+			return existsAddrIndexer
+		}(),
+		wantErr: true,
+		errCode: dcrjson.ErrRPCInvalidParameter,
+	}})
+}
+
+func TestHandleExistsAddresses(t *testing.T) {
+	t.Parallel()
+
+	validAddr := "DcurAwesomeAddressmqDctW5wJCW1Cn2MF"
+	validAddrs := []string{validAddr, validAddr, validAddr}
+	existsSlice := []bool{false, true, true}
+	// existsSlice as a bitset is 110₂ which is 6₁₆ in hex.
+	existsStr := "06"
+	testRPCServerHandler(t, []rpcTest{{
+		name:    "handleExistsAddresses: ok",
+		handler: handleExistsAddresses,
+		cmd: &types.ExistsAddressesCmd{
+			Addresses: validAddrs,
+		},
+		mockExistsAddresser: func() *testExistsAddresser {
+			existsAddrIndexer := defaultMockExistsAddresser()
+			existsAddrIndexer.existsAddresses = existsSlice
+			return existsAddrIndexer
+		}(),
+		result: existsStr,
+	}, {
+		name:    "handleExistsAddresses: exist address indexing not enabled",
+		handler: handleExistsAddresses,
+		cmd: &types.ExistsAddressesCmd{
+			Addresses: validAddrs,
+		},
+		setExistsAddresserNil: true,
+		wantErr:               true,
+		errCode:               dcrjson.ErrRPCInternal.Code,
+	}, {
+		name:    "handleExistsAddresses: bad address",
+		handler: handleExistsAddresses,
+		cmd: &types.ExistsAddressesCmd{
+			Addresses: append(validAddrs, "bad address"),
+		},
+		wantErr: true,
+		errCode: dcrjson.ErrRPCInvalidAddressOrKey,
+	}, {
+		name:    "handleExistsAddresses: ExistsAddresses error",
+		handler: handleExistsAddresses,
+		cmd: &types.ExistsAddressesCmd{
+			Addresses: validAddrs,
+		},
+		mockExistsAddresser: func() *testExistsAddresser {
+			existsAddrIndexer := defaultMockExistsAddresser()
+			existsAddrIndexer.existsAddressesErr = errors.New("")
+			return existsAddrIndexer
+		}(),
+		wantErr: true,
+		errCode: dcrjson.ErrRPCInvalidParameter,
+	}})
+}
+
 func TestHandleExistsExpiredTickets(t *testing.T) {
 	t.Parallel()
 
@@ -4289,6 +4412,12 @@ func testRPCServerHandler(t *testing.T, tests []rpcTest) {
 			if test.mockSyncManager != nil {
 				rpcserverConfig.SyncMgr = test.mockSyncManager
 			}
+			if test.mockExistsAddresser != nil {
+				rpcserverConfig.ExistsAddresser = test.mockExistsAddresser
+			}
+			if test.setExistsAddresserNil {
+				rpcserverConfig.ExistsAddresser = nil
+			}
 			if test.mockConnManager != nil {
 				rpcserverConfig.ConnMgr = test.mockConnManager
 			}