[rpc] add limitation on GetBlocks and GetLogs. Also some other small changes (#3830)

rpc/blockchain.go

@@ -42,7 +42,8 @@ type PublicBlockchainService struct {
 
 const (
 	DefaultRateLimiterWaitTimeout = 5 * time.Second
-	blockCacheLimit               = 256
+	blockCacheLimit               = 2048
+	rpcGetBlocksLimit             = 1024
 )
 
 // NewPublicBlockchainAPI creates a new API for the RPC interface
@@ -68,7 +69,6 @@ func NewPublicBlockchainAPI(hmy *hmy.Harmony, version Version, limiterEnable boo
 			Public:    true,
 		}
 	}
-
 }
 
 // ChainId returns the chain id of the chain - required by MetaMask
@@ -188,7 +188,6 @@ func (s *PublicBlockchainService) GetBlockByNumber(
 			return nil, err
 		}
 	}
-	blockArgs.InclTx = true
 
 	blockNum := blockNumber.EthBlockNumber()
 	if blockNum != rpc.PendingBlockNumber {
@@ -218,53 +217,52 @@ func (s *PublicBlockchainService) GetBlockByNumber(
 	}
 
 	blk, err := s.hmy.BlockByNumber(ctx, blockNum)
-	if blk != nil && err == nil {
+	if blk == nil || err != nil {
-		if blockArgs.WithSigners {
+		return nil, err
-			blockArgs.Signers, err = s.GetBlockSigners(ctx, blockNumber)
+	}
-			if err != nil {
+	if blockArgs.WithSigners {
-				DoMetricRPCQueryInfo(GetBlockByNumber, FailedNumber)
+		blockArgs.Signers, err = s.GetBlockSigners(ctx, blockNumber)
-				return nil, err
-			}
-		}
-
-		// Format the response according to version
-		leader := s.hmy.GetLeaderAddress(blk.Header().Coinbase(), blk.Header().Epoch())
-		var rpcBlock interface{}
-		switch s.version {
-		case V1:
-			rpcBlock, err = v1.NewBlock(blk, blockArgs, leader)
-		case V2:
-			rpcBlock, err = v2.NewBlock(blk, blockArgs, leader)
-		case Eth:
-			rpcBlock, err = eth.NewBlock(blk, blockArgs, leader)
-		default:
-			return nil, ErrUnknownRPCVersion
-		}
 		if err != nil {
 			DoMetricRPCQueryInfo(GetBlockByNumber, FailedNumber)
 			return nil, err
 		}
+	}
 
-		response, err = NewStructuredResponse(rpcBlock)
+	// Format the response according to version
-		if err != nil {
+	leader := s.hmy.GetLeaderAddress(blk.Header().Coinbase(), blk.Header().Epoch())
-			return nil, err
+	var rpcBlock interface{}
-		}
+	switch s.version {
+	case V1:
+		rpcBlock, err = v1.NewBlock(blk, blockArgs, leader)
+	case V2:
+		rpcBlock, err = v2.NewBlock(blk, blockArgs, leader)
+	case Eth:
+		rpcBlock, err = eth.NewBlock(blk, blockArgs, leader)
+	default:
+		return nil, ErrUnknownRPCVersion
+	}
+	if err != nil {
+		DoMetricRPCQueryInfo(GetBlockByNumber, FailedNumber)
+		return nil, err
+	}
 
-		// Pending blocks need to nil out a few fields
+	response, err = NewStructuredResponse(rpcBlock)
-		if blockNum == rpc.PendingBlockNumber {
+	if err != nil {
-			for _, field := range []string{"hash", "nonce", "miner"} {
+		return nil, err
-				response[field] = nil
+	}
-			}
-		}
 
-		if blockNum != rpc.PendingBlockNumber {
+	// Pending blocks need to nil out a few fields
-			cacheKey := combineCacheKey(blk.NumberU64(), s.version, blockArgs)
+	if blockNum == rpc.PendingBlockNumber {
-			s.blockCache.Add(cacheKey, response)
+		for _, field := range []string{"hash", "nonce", "miner"} {
+			response[field] = nil
 		}
-		return response, err
 	}
-	DoMetricRPCQueryInfo(GetBlockByNumber, FailedNumber)
+
-	return nil, err
+	if blockNum != rpc.PendingBlockNumber {
+		cacheKey := combineCacheKey(blk.NumberU64(), s.version, blockArgs)
+		s.blockCache.Add(cacheKey, response)
+	}
+	return response, err
 }
 
 // GetBlockByHash returns the requested block. When fullTx is true all transactions in the block are returned in full
@@ -292,7 +290,6 @@ func (s *PublicBlockchainService) GetBlockByHash(
 			return nil, err
 		}
 	}
-	blockArgs.InclTx = true
 
 	// Fetch the block
 	blk, err := s.hmy.GetBlock(ctx, blockHash)
@@ -366,10 +363,22 @@ func (s *PublicBlockchainService) GetBlocks(
 
 	blockStart := blockNumberStart.Int64()
 	blockEnd := blockNumberEnd.Int64()
+	if blockNumberEnd.EthBlockNumber() == rpc.LatestBlockNumber {
+		blockEnd = s.hmy.BlockChain.CurrentHeader().Number().Int64()
+	}
+	if blockEnd >= blockStart && blockEnd-blockStart > rpcGetBlocksLimit {
+		return nil, fmt.Errorf("GetBlocks query must be smaller than size %v", rpcGetBlocksLimit)
+	}
 
 	// Fetch blocks within given range
 	result := []StructuredResponse{}
 	for i := blockStart; i <= blockEnd; i++ {
+		select {
+		case <-ctx.Done():
+			return nil, ctx.Err()
+		default:
+		}
+
 		blockNum := BlockNumber(i)
 		if blockNum.Int64() > s.hmy.CurrentBlock().Number().Int64() {
 			break
@@ -916,5 +925,5 @@ func (s *PublicBlockchainService) SetNodeToBackupMode(ctx context.Context, isBac
 func combineCacheKey(number uint64, version Version, blockArgs *rpc_common.BlockArgs) string {
 	// no need format blockArgs.Signers[] as a part of cache key
 	// because it's not input from rpc caller, it's caculate with blockArgs.WithSigners
-	return strconv.FormatUint(number, 10) + strconv.FormatInt(int64(version), 10) + strconv.FormatBool(blockArgs.WithSigners) + strconv.FormatBool(blockArgs.InclTx) + strconv.FormatBool(blockArgs.FullTx) + strconv.FormatBool(blockArgs.InclStaking)
+	return strconv.FormatUint(number, 10) + strconv.FormatInt(int64(version), 10) + strconv.FormatBool(blockArgs.WithSigners) + strconv.FormatBool(blockArgs.FullTx) + strconv.FormatBool(blockArgs.InclStaking)
 }

rpc/common/types.go

@@ -13,9 +13,8 @@ import (
 // BlockArgs is struct to include optional block formatting params.
 type BlockArgs struct {
 	WithSigners bool     `json:"withSigners"`
-	InclTx      bool     `json:"inclTx"`
 	FullTx      bool     `json:"fullTx"`
-	Signers     []string `json:"signers"`
+	Signers     []string `json:"-"`
 	InclStaking bool     `json:"inclStaking"`
 }
 

rpc/filters/api.go

@@ -18,6 +18,10 @@ var (
 	deadline = 5 * time.Minute // consider a filter inactive if it has not been polled for within deadline
 )
 
+const (
+	rpcGetLogsLimit = 1024
+)
+
 // filter is a helper struct that holds meta information over the filter type
 // and associated subscription in the event system.
 type filter struct {
@@ -378,6 +382,10 @@ func (api *PublicFilterAPI) GetLogs(ctx context.Context, crit FilterCriteria) ([
 		if crit.ToBlock != nil {
 			end = crit.ToBlock.Int64()
 		}
+		if end >= begin && end-begin > rpcGetLogsLimit {
+			return nil, fmt.Errorf("GetLogs query must be smaller than size %v", rpcGetLogsLimit)
+		}
+
 		// Construct the range filter
 		filter = NewRangeFilter(api.backend, begin, end, crit.Addresses, crit.Topics, api.isEth())
 	}

rpc/filters/filter.go

@@ -220,6 +220,11 @@ func (f *Filter) unindexedLogs(ctx context.Context, end uint64) ([]*types.Log, e
 	var logs []*types.Log
 
 	for ; f.begin <= int64(end); f.begin++ {
+		select {
+		case <-ctx.Done():
+			return nil, ctx.Err()
+		default:
+		}
 		header, err := f.backend.HeaderByNumber(ctx, rpc.BlockNumber(f.begin))
 		if header == nil || err != nil {
 			return logs, err

rpc/v2/types.go

@@ -704,7 +704,7 @@ func NewBlockWithFullTx(
 	}
 
 	for _, tx := range b.Transactions() {
-		fmtTx, err := NewTransactionFromBlockHash(b, tx.Hash())
+		fmtTx, err := NewTransactionFromHash(b, tx.Hash())
 		if err != nil {
 			return nil, err
 		}
@@ -727,8 +727,8 @@ func NewBlockWithFullTx(
 	return blk, nil
 }
 
-// NewTransactionFromBlockHash returns a transaction that will serialize to the RPC representation.
+// NewTransactionFromHash returns a transaction that will serialize to the RPC representation.
-func NewTransactionFromBlockHash(b *types.Block, hash common.Hash) (*Transaction, error) {
+func NewTransactionFromHash(b *types.Block, hash common.Hash) (*Transaction, error) {
 	for idx, tx := range b.Transactions() {
 		if tx.Hash() == hash {
 			return NewTransactionFromBlockIndex(b, uint64(idx))