From 038ff766ff9f9d89c1593c75edeb101c0801a0a4 Mon Sep 17 00:00:00 2001 From: Ceyhun Onur Date: Fri, 1 Aug 2025 18:14:30 +0300 Subject: [PATCH] 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. --- eth/filters/api.go | 8 ++++ eth/filters/filter.go | 2 +- eth/filters/filter_system_test.go | 67 +++++++++++++++++++++++++------ 3 files changed, 63 insertions(+), 14 deletions(-) diff --git a/eth/filters/api.go b/eth/filters/api.go index 232af23957..c929810a12 100644 --- a/eth/filters/api.go +++ b/eth/filters/api.go @@ -38,6 +38,8 @@ var ( errInvalidTopic = errors.New("invalid topic(s)") errFilterNotFound = errors.New("filter not found") errInvalidBlockRange = errors.New("invalid block range params") + errUnknownBlock = errors.New("unknown block") + errBlockHashWithRange = errors.New("can't specify fromBlock/toBlock with blockHash") errPendingLogsUnsupported = errors.New("pending logs are not supported") errExceedMaxTopics = errors.New("exceed max topics") errExceedMaxAddresses = errors.New("exceed max addresses") @@ -348,8 +350,13 @@ func (api *FilterAPI) GetLogs(ctx context.Context, crit FilterCriteria) ([]*type if len(crit.Addresses) > maxAddresses { return nil, errExceedMaxAddresses } + var filter *Filter if crit.BlockHash != nil { + if crit.FromBlock != nil || crit.ToBlock != nil { + return nil, errBlockHashWithRange + } + // Block filter requested, construct a single-shot filter filter = api.sys.NewBlockFilter(*crit.BlockHash, crit.Addresses, crit.Topics) } else { @@ -372,6 +379,7 @@ func (api *FilterAPI) GetLogs(ctx context.Context, crit FilterCriteria) ([]*type // Construct the range filter filter = api.sys.NewRangeFilter(begin, end, crit.Addresses, crit.Topics) } + // Run the filter and return all the logs logs, err := filter.Logs(ctx) if err != nil { diff --git a/eth/filters/filter.go b/eth/filters/filter.go index c4d787eb22..1a9918d0ee 100644 --- a/eth/filters/filter.go +++ b/eth/filters/filter.go @@ -85,7 +85,7 @@ func (f *Filter) Logs(ctx context.Context) ([]*types.Log, error) { return nil, err } if header == nil { - return nil, errors.New("unknown block") + return nil, errUnknownBlock } if header.Number.Uint64() < f.sys.backend.HistoryPruningCutoff() { return nil, &history.PrunedHistoryError{} diff --git a/eth/filters/filter_system_test.go b/eth/filters/filter_system_test.go index 3e686ca2eb..013c1ae527 100644 --- a/eth/filters/filter_system_test.go +++ b/eth/filters/filter_system_test.go @@ -450,24 +450,65 @@ func TestInvalidGetLogsRequest(t *testing.T) { t.Parallel() var ( - db = rawdb.NewMemoryDatabase() - _, sys = newTestFilterSystem(db, Config{}) - api = NewFilterAPI(sys) - blockHash = common.HexToHash("0x1111111111111111111111111111111111111111111111111111111111111111") + genesis = &core.Genesis{ + Config: params.TestChainConfig, + BaseFee: big.NewInt(params.InitialBaseFee), + } + db, blocks, _ = core.GenerateChainWithGenesis(genesis, ethash.NewFaker(), 10, func(i int, gen *core.BlockGen) {}) + _, sys = newTestFilterSystem(db, Config{}) + api = NewFilterAPI(sys) + blockHash = blocks[0].Hash() + unknownBlockHash = common.HexToHash("0x1111111111111111111111111111111111111111111111111111111111111111") ) - // Reason: Cannot specify both BlockHash and FromBlock/ToBlock) - testCases := []FilterCriteria{ - 0: {BlockHash: &blockHash, FromBlock: big.NewInt(100)}, - 1: {BlockHash: &blockHash, ToBlock: big.NewInt(500)}, - 2: {BlockHash: &blockHash, FromBlock: big.NewInt(rpc.LatestBlockNumber.Int64())}, - 3: {BlockHash: &blockHash, Topics: [][]common.Hash{{}, {}, {}, {}, {}}}, - 4: {BlockHash: &blockHash, Addresses: make([]common.Address, maxAddresses+1)}, + // Insert the blocks into the chain so filter can look them up + blockchain, err := core.NewBlockChain(db, genesis, ethash.NewFaker(), nil) + if err != nil { + t.Fatalf("failed to create tester chain: %v", err) + } + if n, err := blockchain.InsertChain(blocks); err != nil { + t.Fatalf("block %d: failed to insert into chain: %v", n, err) + } + + type testcase struct { + f FilterCriteria + err error + } + testCases := []testcase{ + { + f: FilterCriteria{BlockHash: &blockHash, FromBlock: big.NewInt(100)}, + err: errBlockHashWithRange, + }, + { + f: FilterCriteria{BlockHash: &blockHash, ToBlock: big.NewInt(500)}, + err: errBlockHashWithRange, + }, + { + f: FilterCriteria{BlockHash: &blockHash, FromBlock: big.NewInt(rpc.LatestBlockNumber.Int64())}, + err: errBlockHashWithRange, + }, + { + f: FilterCriteria{BlockHash: &unknownBlockHash}, + err: errUnknownBlock, + }, + { + f: FilterCriteria{BlockHash: &blockHash, Topics: [][]common.Hash{{}, {}, {}, {}, {}}}, + err: errExceedMaxTopics, + }, + { + f: FilterCriteria{BlockHash: &blockHash, Topics: [][]common.Hash{{}, {}, {}, {}, {}}}, + err: errExceedMaxTopics, + }, + { + f: FilterCriteria{BlockHash: &blockHash, Addresses: make([]common.Address, maxAddresses+1)}, + err: errExceedMaxAddresses, + }, } for i, test := range testCases { - if _, err := api.GetLogs(context.Background(), test); err == nil { - t.Errorf("Expected Logs for case #%d to fail", i) + _, err := api.GetLogs(context.Background(), test.f) + if !errors.Is(err, test.err) { + t.Errorf("case %d: wrong error: %q\nwant: %q", i, err, test.err) } } }