Skip to content

Commit 038ff76

Browse files
authored
eth/filters: fix error when blockHash is used with fromBlock/toBlock (#31877)
This introduces an error when the filter has both `blockHash` and `fromBlock`/`toBlock`, since these are mutually exclusive. Seems the tests were actually returning `not found` error, which went undetected since there was no check on the actual returned error in the test.
1 parent 9c58810 commit 038ff76

File tree

3 files changed

+63
-14
lines changed

3 files changed

+63
-14
lines changed

eth/filters/api.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ var (
3838
errInvalidTopic = errors.New("invalid topic(s)")
3939
errFilterNotFound = errors.New("filter not found")
4040
errInvalidBlockRange = errors.New("invalid block range params")
41+
errUnknownBlock = errors.New("unknown block")
42+
errBlockHashWithRange = errors.New("can't specify fromBlock/toBlock with blockHash")
4143
errPendingLogsUnsupported = errors.New("pending logs are not supported")
4244
errExceedMaxTopics = errors.New("exceed max topics")
4345
errExceedMaxAddresses = errors.New("exceed max addresses")
@@ -348,8 +350,13 @@ func (api *FilterAPI) GetLogs(ctx context.Context, crit FilterCriteria) ([]*type
348350
if len(crit.Addresses) > maxAddresses {
349351
return nil, errExceedMaxAddresses
350352
}
353+
351354
var filter *Filter
352355
if crit.BlockHash != nil {
356+
if crit.FromBlock != nil || crit.ToBlock != nil {
357+
return nil, errBlockHashWithRange
358+
}
359+
353360
// Block filter requested, construct a single-shot filter
354361
filter = api.sys.NewBlockFilter(*crit.BlockHash, crit.Addresses, crit.Topics)
355362
} else {
@@ -372,6 +379,7 @@ func (api *FilterAPI) GetLogs(ctx context.Context, crit FilterCriteria) ([]*type
372379
// Construct the range filter
373380
filter = api.sys.NewRangeFilter(begin, end, crit.Addresses, crit.Topics)
374381
}
382+
375383
// Run the filter and return all the logs
376384
logs, err := filter.Logs(ctx)
377385
if err != nil {

eth/filters/filter.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ func (f *Filter) Logs(ctx context.Context) ([]*types.Log, error) {
8585
return nil, err
8686
}
8787
if header == nil {
88-
return nil, errors.New("unknown block")
88+
return nil, errUnknownBlock
8989
}
9090
if header.Number.Uint64() < f.sys.backend.HistoryPruningCutoff() {
9191
return nil, &history.PrunedHistoryError{}

eth/filters/filter_system_test.go

Lines changed: 54 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -450,24 +450,65 @@ func TestInvalidGetLogsRequest(t *testing.T) {
450450
t.Parallel()
451451

452452
var (
453-
db = rawdb.NewMemoryDatabase()
454-
_, sys = newTestFilterSystem(db, Config{})
455-
api = NewFilterAPI(sys)
456-
blockHash = common.HexToHash("0x1111111111111111111111111111111111111111111111111111111111111111")
453+
genesis = &core.Genesis{
454+
Config: params.TestChainConfig,
455+
BaseFee: big.NewInt(params.InitialBaseFee),
456+
}
457+
db, blocks, _ = core.GenerateChainWithGenesis(genesis, ethash.NewFaker(), 10, func(i int, gen *core.BlockGen) {})
458+
_, sys = newTestFilterSystem(db, Config{})
459+
api = NewFilterAPI(sys)
460+
blockHash = blocks[0].Hash()
461+
unknownBlockHash = common.HexToHash("0x1111111111111111111111111111111111111111111111111111111111111111")
457462
)
458463

459-
// Reason: Cannot specify both BlockHash and FromBlock/ToBlock)
460-
testCases := []FilterCriteria{
461-
0: {BlockHash: &blockHash, FromBlock: big.NewInt(100)},
462-
1: {BlockHash: &blockHash, ToBlock: big.NewInt(500)},
463-
2: {BlockHash: &blockHash, FromBlock: big.NewInt(rpc.LatestBlockNumber.Int64())},
464-
3: {BlockHash: &blockHash, Topics: [][]common.Hash{{}, {}, {}, {}, {}}},
465-
4: {BlockHash: &blockHash, Addresses: make([]common.Address, maxAddresses+1)},
464+
// Insert the blocks into the chain so filter can look them up
465+
blockchain, err := core.NewBlockChain(db, genesis, ethash.NewFaker(), nil)
466+
if err != nil {
467+
t.Fatalf("failed to create tester chain: %v", err)
468+
}
469+
if n, err := blockchain.InsertChain(blocks); err != nil {
470+
t.Fatalf("block %d: failed to insert into chain: %v", n, err)
471+
}
472+
473+
type testcase struct {
474+
f FilterCriteria
475+
err error
476+
}
477+
testCases := []testcase{
478+
{
479+
f: FilterCriteria{BlockHash: &blockHash, FromBlock: big.NewInt(100)},
480+
err: errBlockHashWithRange,
481+
},
482+
{
483+
f: FilterCriteria{BlockHash: &blockHash, ToBlock: big.NewInt(500)},
484+
err: errBlockHashWithRange,
485+
},
486+
{
487+
f: FilterCriteria{BlockHash: &blockHash, FromBlock: big.NewInt(rpc.LatestBlockNumber.Int64())},
488+
err: errBlockHashWithRange,
489+
},
490+
{
491+
f: FilterCriteria{BlockHash: &unknownBlockHash},
492+
err: errUnknownBlock,
493+
},
494+
{
495+
f: FilterCriteria{BlockHash: &blockHash, Topics: [][]common.Hash{{}, {}, {}, {}, {}}},
496+
err: errExceedMaxTopics,
497+
},
498+
{
499+
f: FilterCriteria{BlockHash: &blockHash, Topics: [][]common.Hash{{}, {}, {}, {}, {}}},
500+
err: errExceedMaxTopics,
501+
},
502+
{
503+
f: FilterCriteria{BlockHash: &blockHash, Addresses: make([]common.Address, maxAddresses+1)},
504+
err: errExceedMaxAddresses,
505+
},
466506
}
467507

468508
for i, test := range testCases {
469-
if _, err := api.GetLogs(context.Background(), test); err == nil {
470-
t.Errorf("Expected Logs for case #%d to fail", i)
509+
_, err := api.GetLogs(context.Background(), test.f)
510+
if !errors.Is(err, test.err) {
511+
t.Errorf("case %d: wrong error: %q\nwant: %q", i, err, test.err)
471512
}
472513
}
473514
}

0 commit comments

Comments
 (0)