Fix/config fix small bug and set default maxBlockRange to -1 to not u…

…se it (ethereum#529) * feat: configure max block range for eth_getLogs (ethereum#521) * configure max block range for eth_getLogs * bump version * address comments, add test * set default value * address comments * fix golint * Update cmd/utils/flags.go Co-authored-by: colin <102356659+colinlyguo@users.noreply.github.com> --------- Co-authored-by: colinlyguo <colinlyguo@scroll.io> Co-authored-by: colin <102356659+colinlyguo@users.noreply.github.com> * set dafault maxblockrange to -1 and fix nil pointer bug * update version * change error logic * check for range in filter * Update eth/ethconfig/config.go Co-authored-by: colin <102356659+colinlyguo@users.noreply.github.com> * adjust error message * fix duplicate version --------- Co-authored-by: colinlyguo <colinlyguo@scroll.io> Co-authored-by: colin <102356659+colinlyguo@users.noreply.github.com> Co-authored-by: Péter Garamvölgyi <peter@scroll.io>
tanishqjasoria · Oct 4, 2023 · fea7d94 · fea7d94
1 parent d430133
commit fea7d94
Show file tree

Hide file tree

Showing 10 changed files with 127 additions and 28 deletions.
diff --git a/cmd/geth/main.go b/cmd/geth/main.go
@@ -190,6 +190,7 @@ var (
 		utils.RPCGlobalEVMTimeoutFlag,
 		utils.RPCGlobalTxFeeCapFlag,
 		utils.AllowUnprotectedTxs,
+		utils.MaxBlockRangeFlag,
 	}
 
 	metricsFlags = []cli.Flag{

diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go
@@ -831,6 +831,12 @@ var (
 		Name:  "ccc",
 		Usage: "Enable circuit capacity check during block validation",
 	}
+
+	// Max block range for `eth_getLogs` method
+	MaxBlockRangeFlag = cli.Int64Flag{
+		Name:  "rpc.getlogs.maxrange",
+		Usage: "Limit max fetched block range for `eth_getLogs` method",
+	}
 )
 
 // MakeDataDir retrieves the currently requested data directory, terminating
@@ -1529,6 +1535,14 @@ func setCircuitCapacityCheck(ctx *cli.Context, cfg *ethconfig.Config) {
 	}
 }
 
+func setMaxBlockRange(ctx *cli.Context, cfg *ethconfig.Config) {
+	if ctx.GlobalIsSet(MaxBlockRangeFlag.Name) {
+		cfg.MaxBlockRange = ctx.GlobalInt64(MaxBlockRangeFlag.Name)
+	} else {
+		cfg.MaxBlockRange = -1
+	}
+}
+
 // CheckExclusive verifies that only a single instance of the provided flags was
 // set by the user. Each flag might optionally be followed by a string type to
 // specialize it further.
@@ -1595,6 +1609,7 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) {
 	setWhitelist(ctx, cfg)
 	setLes(ctx, cfg)
 	setCircuitCapacityCheck(ctx, cfg)
+	setMaxBlockRange(ctx, cfg)
 
 	// Cap the cache allowance and tune the garbage collector
 	mem, err := gopsutil.VirtualMemory()

diff --git a/eth/backend.go b/eth/backend.go
@@ -333,7 +333,7 @@ func (s *Ethereum) APIs() []rpc.API {
 		}, {
 			Namespace: "eth",
 			Version:   "1.0",
-			Service:   filters.NewPublicFilterAPI(s.APIBackend, false, 5*time.Minute),
+			Service:   filters.NewPublicFilterAPI(s.APIBackend, false, 5*time.Minute, s.config.MaxBlockRange),
 			Public:    true,
 		}, {
 			Namespace: "admin",

diff --git a/eth/ethconfig/config.go b/eth/ethconfig/config.go
@@ -91,7 +91,8 @@ var Defaults = Config{
 	RPCGasCap:     50000000,
 	RPCEVMTimeout: 5 * time.Second,
 	GPO:           FullNodeGPO,
-	RPCTxFeeCap:   1, // 1 ether
+	RPCTxFeeCap:   1,  // 1 ether
+	MaxBlockRange: -1, // Default unconfigured value: no block range limit for backward compatibility
 }
 
 func init() {
@@ -210,6 +211,9 @@ type Config struct {
 
 	// Check circuit capacity in block validator
 	CheckCircuitCapacity bool
+
+	// Max block range for eth_getLogs api method
+	MaxBlockRange int64
 }
 
 // CreateConsensusEngine creates a consensus engine for the given chain configuration.

diff --git a/eth/ethconfig/gen_config.go b/eth/ethconfig/gen_config.go
diff --git a/eth/filters/api.go b/eth/filters/api.go
@@ -48,24 +48,26 @@ type filter struct {
 // PublicFilterAPI offers support to create and manage filters. This will allow external clients to retrieve various
 // information related to the Ethereum protocol such als blocks, transactions and logs.
 type PublicFilterAPI struct {
-	backend   Backend
-	mux       *event.TypeMux
-	quit      chan struct{}
-	chainDb   ethdb.Database
-	events    *EventSystem
-	filtersMu sync.Mutex
-	filters   map[rpc.ID]*filter
-	timeout   time.Duration
+	backend       Backend
+	mux           *event.TypeMux
+	quit          chan struct{}
+	chainDb       ethdb.Database
+	events        *EventSystem
+	filtersMu     sync.Mutex
+	filters       map[rpc.ID]*filter
+	timeout       time.Duration
+	maxBlockRange int64
 }
 
 // NewPublicFilterAPI returns a new PublicFilterAPI instance.
-func NewPublicFilterAPI(backend Backend, lightMode bool, timeout time.Duration) *PublicFilterAPI {
+func NewPublicFilterAPI(backend Backend, lightMode bool, timeout time.Duration, maxBlockRange int64) *PublicFilterAPI {
 	api := &PublicFilterAPI{
-		backend: backend,
-		chainDb: backend.ChainDb(),
-		events:  NewEventSystem(backend, lightMode),
-		filters: make(map[rpc.ID]*filter),
-		timeout: timeout,
+		backend:       backend,
+		chainDb:       backend.ChainDb(),
+		events:        NewEventSystem(backend, lightMode),
+		filters:       make(map[rpc.ID]*filter),
+		timeout:       timeout,
+		maxBlockRange: maxBlockRange,
 	}
 	go api.timeoutLoop(timeout)
 
@@ -346,7 +348,7 @@ func (api *PublicFilterAPI) GetLogs(ctx context.Context, crit FilterCriteria) ([
 			end = crit.ToBlock.Int64()
 		}
 		// Construct the range filter
-		filter = NewRangeFilter(api.backend, begin, end, crit.Addresses, crit.Topics)
+		filter = NewRangeFilter(api.backend, begin, end, crit.Addresses, crit.Topics, api.maxBlockRange)
 	}
 	// Run the filter and return all the logs
 	logs, err := filter.Logs(ctx)

diff --git a/eth/filters/filter.go b/eth/filters/filter.go
@@ -19,6 +19,7 @@ package filters
 import (
 	"context"
 	"errors"
+	"fmt"
 	"math/big"
 
 	"github.com/scroll-tech/go-ethereum/common"
@@ -59,11 +60,13 @@ type Filter struct {
 	begin, end int64       // Range interval if filtering multiple blocks
 
 	matcher *bloombits.Matcher
+
+	maxBlockRange int64
 }
 
 // NewRangeFilter creates a new filter which uses a bloom filter on blocks to
 // figure out whether a particular block is interesting or not.
-func NewRangeFilter(backend Backend, begin, end int64, addresses []common.Address, topics [][]common.Hash) *Filter {
+func NewRangeFilter(backend Backend, begin, end int64, addresses []common.Address, topics [][]common.Hash, maxBlockRange ...int64) *Filter {
 	// Flatten the address and topic filter clauses into a single bloombits filter
 	// system. Since the bloombits are not positional, nil topics are permitted,
 	// which get flattened into a nil byte slice.
@@ -91,6 +94,11 @@ func NewRangeFilter(backend Backend, begin, end int64, addresses []common.Addres
 	filter.begin = begin
 	filter.end = end
 
+	if len(maxBlockRange) > 0 {
+		filter.maxBlockRange = maxBlockRange[0]
+	} else {
+		filter.maxBlockRange = -1
+	}
 	return filter
 }
 
@@ -142,6 +150,11 @@ func (f *Filter) Logs(ctx context.Context) ([]*types.Log, error) {
 	if f.end == -1 {
 		end = head
 	}
+
+	// if maxBlockRange configured then check for
+	if f.maxBlockRange != -1 && int64(end)-f.begin+1 > f.maxBlockRange {
+		return nil, fmt.Errorf("block range is larger than max block range, block range = %d, max block range = %d", int64(end)-f.begin+1, f.maxBlockRange)
+	}
 	// Gather all indexed logs, and finish with non indexed ones
 	var (
 		logs []*types.Log

diff --git a/eth/filters/filter_system_test.go b/eth/filters/filter_system_test.go
@@ -33,6 +33,8 @@ import (
 	"github.com/scroll-tech/go-ethereum/core/bloombits"
 	"github.com/scroll-tech/go-ethereum/core/rawdb"
 	"github.com/scroll-tech/go-ethereum/core/types"
+	"github.com/scroll-tech/go-ethereum/core/vm"
+	"github.com/scroll-tech/go-ethereum/eth/ethconfig"
 	"github.com/scroll-tech/go-ethereum/ethdb"
 	"github.com/scroll-tech/go-ethereum/event"
 	"github.com/scroll-tech/go-ethereum/params"
@@ -168,7 +170,7 @@ func TestBlockSubscription(t *testing.T) {
 	var (
 		db          = rawdb.NewMemoryDatabase()
 		backend     = &testBackend{db: db}
-		api         = NewPublicFilterAPI(backend, false, deadline)
+		api         = NewPublicFilterAPI(backend, false, deadline, ethconfig.Defaults.MaxBlockRange)
 		genesis     = (&core.Genesis{BaseFee: big.NewInt(params.InitialBaseFee)}).MustCommit(db)
 		chain, _    = core.GenerateChain(params.TestChainConfig, genesis, ethash.NewFaker(), db, 10, func(i int, gen *core.BlockGen) {})
 		chainEvents = []core.ChainEvent{}
@@ -220,7 +222,7 @@ func TestPendingTxFilter(t *testing.T) {
 	var (
 		db      = rawdb.NewMemoryDatabase()
 		backend = &testBackend{db: db}
-		api     = NewPublicFilterAPI(backend, false, deadline)
+		api     = NewPublicFilterAPI(backend, false, deadline, ethconfig.Defaults.MaxBlockRange)
 
 		transactions = []*types.Transaction{
 			types.NewTransaction(0, common.HexToAddress("0xb794f5ea0ba39494ce83a213fffba74279579268"), new(big.Int), 0, new(big.Int), nil),
@@ -275,7 +277,7 @@ func TestLogFilterCreation(t *testing.T) {
 	var (
 		db      = rawdb.NewMemoryDatabase()
 		backend = &testBackend{db: db}
-		api     = NewPublicFilterAPI(backend, false, deadline)
+		api     = NewPublicFilterAPI(backend, false, deadline, ethconfig.Defaults.MaxBlockRange)
 
 		testCases = []struct {
 			crit    FilterCriteria
@@ -319,7 +321,7 @@ func TestInvalidLogFilterCreation(t *testing.T) {
 	var (
 		db      = rawdb.NewMemoryDatabase()
 		backend = &testBackend{db: db}
-		api     = NewPublicFilterAPI(backend, false, deadline)
+		api     = NewPublicFilterAPI(backend, false, deadline, ethconfig.Defaults.MaxBlockRange)
 	)
 
 	// different situations where log filter creation should fail.
@@ -341,7 +343,7 @@ func TestInvalidGetLogsRequest(t *testing.T) {
 	var (
 		db        = rawdb.NewMemoryDatabase()
 		backend   = &testBackend{db: db}
-		api       = NewPublicFilterAPI(backend, false, deadline)
+		api       = NewPublicFilterAPI(backend, false, deadline, ethconfig.Defaults.MaxBlockRange)
 		blockHash = common.HexToHash("0x1111111111111111111111111111111111111111111111111111111111111111")
 	)
 
@@ -359,14 +361,58 @@ func TestInvalidGetLogsRequest(t *testing.T) {
 	}
 }
 
+func TestGetLogsRange(t *testing.T) {
+	var (
+		db      = rawdb.NewMemoryDatabase()
+		backend = &testBackend{db: db}
+		api     = NewPublicFilterAPI(backend, false, deadline, 2)
+	)
+	(&core.Genesis{
+		Config: params.TestChainConfig,
+	}).MustCommit(db)
+	chain, _ := core.NewBlockChain(db, nil, params.TestChainConfig, ethash.NewFaker(), vm.Config{}, nil, nil, false)
+	bs, _ := core.GenerateChain(params.TestChainConfig, chain.Genesis(), ethash.NewFaker(), db, 10, nil)
+	if _, err := chain.InsertChain(bs); err != nil {
+		panic(err)
+	}
+	// those test cases should fail because block range is greater then limit
+	failTestCases := []FilterCriteria{
+		// from 0 to 2 block
+		0: {FromBlock: big.NewInt(0), ToBlock: big.NewInt(2)},
+		// from 8 to latest block (10)
+		1: {FromBlock: big.NewInt(8)},
+		// from 0 to latest block (10)
+		2: {FromBlock: big.NewInt(0)},
+	}
+	for i, test := range failTestCases {
+		if _, err := api.GetLogs(context.Background(), test); err == nil {
+			t.Errorf("Expected Logs for failing case #%d to fail", i)
+		}
+	}
+
+	okTestCases := []FilterCriteria{
+		// from latest to latest block
+		0: {},
+		// from 9 to last block (10)
+		1: {FromBlock: big.NewInt(9)},
+		// from 3 to 4 block
+		2: {FromBlock: big.NewInt(3), ToBlock: big.NewInt(4)},
+	}
+	for i, test := range okTestCases {
+		if _, err := api.GetLogs(context.Background(), test); err != nil {
+			t.Errorf("Expected Logs for ok case #%d not to fail", i)
+		}
+	}
+}
+
 // TestLogFilter tests whether log filters match the correct logs that are posted to the event feed.
 func TestLogFilter(t *testing.T) {
 	t.Parallel()
 
 	var (
 		db      = rawdb.NewMemoryDatabase()
 		backend = &testBackend{db: db}
-		api     = NewPublicFilterAPI(backend, false, deadline)
+		api     = NewPublicFilterAPI(backend, false, deadline, ethconfig.Defaults.MaxBlockRange)
 
 		firstAddr      = common.HexToAddress("0x1111111111111111111111111111111111111111")
 		secondAddr     = common.HexToAddress("0x2222222222222222222222222222222222222222")
@@ -480,7 +526,7 @@ func TestPendingLogsSubscription(t *testing.T) {
 	var (
 		db      = rawdb.NewMemoryDatabase()
 		backend = &testBackend{db: db}
-		api     = NewPublicFilterAPI(backend, false, deadline)
+		api     = NewPublicFilterAPI(backend, false, deadline, ethconfig.Defaults.MaxBlockRange)
 
 		firstAddr      = common.HexToAddress("0x1111111111111111111111111111111111111111")
 		secondAddr     = common.HexToAddress("0x2222222222222222222222222222222222222222")
@@ -664,7 +710,7 @@ func TestPendingTxFilterDeadlock(t *testing.T) {
 	var (
 		db      = rawdb.NewMemoryDatabase()
 		backend = &testBackend{db: db}
-		api     = NewPublicFilterAPI(backend, false, timeout)
+		api     = NewPublicFilterAPI(backend, false, timeout, ethconfig.Defaults.MaxBlockRange)
 		done    = make(chan struct{})
 	)
 

diff --git a/les/client.go b/les/client.go
@@ -301,7 +301,7 @@ func (s *LightEthereum) APIs() []rpc.API {
 		}, {
 			Namespace: "eth",
 			Version:   "1.0",
-			Service:   filters.NewPublicFilterAPI(s.ApiBackend, true, 5*time.Minute),
+			Service:   filters.NewPublicFilterAPI(s.ApiBackend, true, 5*time.Minute, s.config.MaxBlockRange),
 			Public:    true,
 		}, {
 			Namespace: "net",

diff --git a/params/version.go b/params/version.go
@@ -24,7 +24,7 @@ import (
 const (
 	VersionMajor = 4         // Major version component of the current release
 	VersionMinor = 4         // Minor version component of the current release
-	VersionPatch = 18        // Patch version component of the current release
+	VersionPatch = 19        // Patch version component of the current release
 	VersionMeta  = "sepolia" // Version metadata to append to the version string
 )