From 6307a35ebb0a64ca5f5e5f7f2bcfaaa4b9619673 Mon Sep 17 00:00:00 2001 From: JoeGruff Date: Thu, 23 Jul 2020 11:23:49 +0900 Subject: [PATCH] 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. --- internal/rpcserver/interface.go | 22 ++- internal/rpcserver/rpcserver.go | 14 +- internal/rpcserver/rpcserverhandlers_test.go | 150 ++++++++++++++----- rpcadaptors.go | 40 ++++- server.go | 29 ++-- 5 files changed, 186 insertions(+), 69 deletions(-) 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..07acd8bc 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) } @@ -1460,8 +1459,7 @@ func handleExistsAddress(_ context.Context, s *Server, cmd interface{}) (interfa // handleExistsAddresses implements the existsaddresses command. 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 +1476,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) } @@ -5594,6 +5592,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/internal/rpcserver/rpcserverhandlers_test.go b/internal/rpcserver/rpcserverhandlers_test.go index 6cf8f250..71b30b0c 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,50 @@ 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 TestHandleExistsExpiredTickets(t *testing.T) { t.Parallel() @@ -4289,6 +4359,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 } 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