diff --git a/api/api.go b/api/api.go index 7251f43e..a3807953 100644 --- a/api/api.go +++ b/api/api.go @@ -623,9 +623,17 @@ func (b *BlockChainAPI) GetLogs( // if filter provided specific block ID if criteria.BlockHash != nil { - return logs. - NewIDFilter(*criteria.BlockHash, filter, b.blocks, b.receipts). - Match() + f, err := logs.NewIDFilter(*criteria.BlockHash, filter, b.blocks, b.receipts) + if err != nil { + return handleError[[]*types.Log](err, l, b.collector) + } + + res, err := f.Match() + if err != nil { + return handleError[[]*types.Log](err, l, b.collector) + } + + return res, nil } // otherwise we use the block range as the filter diff --git a/services/logs/filter.go b/services/logs/filter.go index 3d7fee46..ce6fb778 100644 --- a/services/logs/filter.go +++ b/services/logs/filter.go @@ -59,6 +59,10 @@ func NewRangeFilter( criteria FilterCriteria, receipts storage.ReceiptIndexer, ) (*RangeFilter, error) { + if len(criteria.Topics) > maxTopics { + return nil, fmt.Errorf("max topics exceeded, only %d allowed", maxTopics) + } + // check if both start and end don't have special values (negative values representing last block etc.) // if so, make sure that beginning number is not bigger than end if start.Cmp(big.NewInt(0)) > 0 && end.Cmp(big.NewInt(0)) > 0 && start.Cmp(&end) > 0 { @@ -124,13 +128,17 @@ func NewIDFilter( criteria FilterCriteria, blocks storage.BlockIndexer, receipts storage.ReceiptIndexer, -) *IDFilter { +) (*IDFilter, error) { + if len(criteria.Topics) > maxTopics { + return nil, fmt.Errorf("max topics exceeded, only %d allowed", maxTopics) + } + return &IDFilter{ id: id, criteria: &criteria, blocks: blocks, receipts: receipts, - } + }, nil } func (i *IDFilter) Match() ([]*gethTypes.Log, error) { diff --git a/services/logs/filter_test.go b/services/logs/filter_test.go index 1ba51795..477bfea7 100644 --- a/services/logs/filter_test.go +++ b/services/logs/filter_test.go @@ -7,6 +7,7 @@ import ( "github.com/onflow/flow-go/fvm/evm/types" "github.com/onflow/go-ethereum/common" gethTypes "github.com/onflow/go-ethereum/core/types" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -155,10 +156,9 @@ func receiptStorage() storage.ReceiptIndexer { return receiptStorage } -// todo check if empty address with only topics provided is a valid query - func TestIDFilter(t *testing.T) { - lgs := receipts[0].Logs + logs := receipts[0].Logs + tests := []struct { desc string id common.Hash @@ -168,33 +168,72 @@ func TestIDFilter(t *testing.T) { desc: "wildcard all logs", id: mustHash(blocks[0]), criteria: FilterCriteria{ - Addresses: []common.Address{lgs[0].Address}, + Addresses: []common.Address{}, Topics: [][]common.Hash{}, }, - expectLogs: lgs[:2], // first two are all logs from the address + expectLogs: logs[:], // block 0 has 3 logs in total + }, { + desc: "single topic no address match single log", + id: mustHash(blocks[0]), + criteria: FilterCriteria{ + Addresses: []common.Address{}, + Topics: [][]common.Hash{logs[0].Topics[:1]}, + }, + expectLogs: logs[:1], + }, { + desc: "single out of order topic no address match no logs", + id: mustHash(blocks[0]), + criteria: FilterCriteria{ + Addresses: []common.Address{}, + Topics: [][]common.Hash{logs[0].Topics[1:]}, + }, + expectLogs: []*gethTypes.Log{}, + }, { + desc: "single topic with first position wildcard match single log", + id: mustHash(blocks[0]), + criteria: FilterCriteria{ + Addresses: []common.Address{}, + Topics: [][]common.Hash{ + {}, + {logs[0].Topics[1]}, + }, + }, + expectLogs: logs[:1], + }, { + desc: "single topic with second position wildcard match single log", + id: mustHash(blocks[0]), + criteria: FilterCriteria{ + Addresses: []common.Address{}, + Topics: [][]common.Hash{ + {logs[0].Topics[0]}, + {}, + }, + }, + expectLogs: logs[:1], }, { desc: "single topic, single address match single log", id: mustHash(blocks[0]), criteria: FilterCriteria{ - Addresses: []common.Address{lgs[0].Address}, - Topics: [][]common.Hash{lgs[0].Topics[:1]}, + Addresses: []common.Address{logs[0].Address}, + Topics: [][]common.Hash{logs[0].Topics[:1]}, }, - expectLogs: lgs[:1], + expectLogs: logs[:1], }, { desc: "single address no topic match two logs", id: mustHash(blocks[0]), criteria: FilterCriteria{ - Addresses: []common.Address{lgs[0].Address}, + Addresses: []common.Address{logs[0].Address}, + Topics: [][]common.Hash{}, }, - expectLogs: lgs[:2], + expectLogs: logs[:2], }, { desc: "single address, both topics match single log", id: mustHash(blocks[0]), criteria: FilterCriteria{ - Addresses: []common.Address{lgs[0].Address}, - Topics: [][]common.Hash{lgs[0].Topics}, + Addresses: []common.Address{logs[0].Address}, + Topics: [][]common.Hash{logs[0].Topics}, }, - expectLogs: lgs[:1], + expectLogs: logs[:1], }, { desc: "invalid topic match no logs", id: mustHash(blocks[0]), @@ -206,17 +245,43 @@ func TestIDFilter(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - filter := NewIDFilter(tt.id, tt.criteria, blockStorage(), receiptStorage()) - logs, err := filter.Match() + filter, err := NewIDFilter(tt.id, tt.criteria, blockStorage(), receiptStorage()) + require.NoError(t, err) + + matchedLogs, err := filter.Match() require.NoError(t, err) - require.Equal(t, tt.expectLogs, logs) + require.Equal(t, tt.expectLogs, matchedLogs) }) } + + t.Run("with topics count exceeding limit", func(t *testing.T) { + _, err := NewIDFilter( + common.HexToHash("123"), + FilterCriteria{ + Topics: [][]common.Hash{ + {common.HexToHash("123")}, + {common.HexToHash("456")}, + {common.HexToHash("789")}, + {common.HexToHash("101")}, + {common.HexToHash("120")}, + }, + }, + blockStorage(), + receiptStorage(), + ) + + require.Error(t, err) + assert.ErrorContains( + t, + err, + "max topics exceeded, only 4 allowed", + ) + }) } func TestRangeFilter(t *testing.T) { - lgs := [][]*gethTypes.Log{receipts[0].Logs, receipts[1].Logs, receipts[2].Logs, receipts[3].Logs, receipts[4].Logs} + logs := [][]*gethTypes.Log{receipts[0].Logs, receipts[1].Logs, receipts[2].Logs, receipts[3].Logs, receipts[4].Logs} tests := []struct { desc string @@ -228,27 +293,27 @@ func TestRangeFilter(t *testing.T) { start: big.NewInt(0), end: big.NewInt(1), criteria: FilterCriteria{ - Addresses: []common.Address{lgs[0][0].Address}, - Topics: [][]common.Hash{lgs[0][0].Topics[:1]}, + Addresses: []common.Address{logs[0][0].Address}, + Topics: [][]common.Hash{logs[0][0].Topics[:1]}, }, - expectLogs: lgs[0][:1], + expectLogs: logs[0][:1], }, { desc: "single topic, single address, all blocks match multiple logs", start: big.NewInt(0), end: big.NewInt(4), criteria: FilterCriteria{ - Addresses: []common.Address{lgs[0][0].Address}, - Topics: [][]common.Hash{lgs[0][0].Topics[:1]}, + Addresses: []common.Address{logs[0][0].Address}, + Topics: [][]common.Hash{logs[0][0].Topics[:1]}, }, - expectLogs: []*gethTypes.Log{lgs[0][0], lgs[3][1]}, + expectLogs: []*gethTypes.Log{logs[0][0], logs[3][1]}, }, { desc: "single address, all blocks match multiple logs", start: big.NewInt(0), end: big.NewInt(4), criteria: FilterCriteria{ - Addresses: []common.Address{lgs[0][0].Address}, + Addresses: []common.Address{logs[0][0].Address}, }, - expectLogs: []*gethTypes.Log{lgs[0][0], lgs[0][1], lgs[1][0], lgs[3][1]}, + expectLogs: []*gethTypes.Log{logs[0][0], logs[0][1], logs[1][0], logs[3][1]}, }, { desc: "invalid address, all blocks no match", start: big.NewInt(0), @@ -262,7 +327,7 @@ func TestRangeFilter(t *testing.T) { start: big.NewInt(5), end: big.NewInt(10), criteria: FilterCriteria{ - Addresses: []common.Address{lgs[0][0].Address}, + Addresses: []common.Address{logs[0][0].Address}, }, expectLogs: []*gethTypes.Log{}, }} @@ -272,12 +337,36 @@ func TestRangeFilter(t *testing.T) { filter, err := NewRangeFilter(*tt.start, *tt.end, tt.criteria, receiptStorage()) require.NoError(t, err) - logs, err := filter.Match() + matchedLogs, err := filter.Match() require.NoError(t, err) - require.Equal(t, tt.expectLogs, logs) + require.Equal(t, tt.expectLogs, matchedLogs) }) } + + t.Run("with topics count exceeding limit", func(t *testing.T) { + _, err := NewRangeFilter( + *big.NewInt(0), + *big.NewInt(4), + FilterCriteria{ + Topics: [][]common.Hash{ + {common.HexToHash("123")}, + {common.HexToHash("456")}, + {common.HexToHash("789")}, + {common.HexToHash("101")}, + {common.HexToHash("120")}, + }, + }, + receiptStorage(), + ) + + require.Error(t, err) + assert.ErrorContains( + t, + err, + "max topics exceeded, only 4 allowed", + ) + }) } func TestStreamFilter(t *testing.T) { diff --git a/tests/web3js/eth_logs_filtering_test.js b/tests/web3js/eth_logs_filtering_test.js index 616aa2ec..bd58e4e4 100644 --- a/tests/web3js/eth_logs_filtering_test.js +++ b/tests/web3js/eth_logs_filtering_test.js @@ -3,8 +3,8 @@ const conf = require('./config') const helpers = require('./helpers') const web3 = conf.web3 -it('emit logs and retrieve them using different filters', async() => { - setTimeout(() => process.exit(1), 19*1000) // hack if the ws connection is not closed +it('emit logs and retrieve them using different filters', async () => { + setTimeout(() => process.exit(1), 19 * 1000) // hack if the ws connection is not closed let deployed = await helpers.deployContract("storage") let contractAddress = deployed.receipt.contractAddress @@ -28,13 +28,11 @@ it('emit logs and retrieve them using different filters', async() => { }) assert.equal(res.receipt.status, conf.successStatus) - let latest = await web3.eth.getBlockNumber() - // filter each event just emitted by both A and B matching the exact event const events = await deployed.contract.getPastEvents('Calculated', { filter: { numA: A, numB: B }, fromBlock: conf.startBlockHeight, - toBlock: latest, // todo 'latest' special value doesn't seem to work + toBlock: 'latest' }) // Assert that the event is found and the result is correct @@ -42,16 +40,15 @@ it('emit logs and retrieve them using different filters', async() => { assert.equal(events[0].returnValues.sum, (A + B).toString()) } - let latest = await web3.eth.getBlockNumber() - // filter events by A value equal to 10 which should equal to 3 events with different B values let events = await deployed.contract.getPastEvents('Calculated', { filter: { numA: repeatA }, - fromBlock: conf.startBlockHeight, - toBlock: latest, + fromBlock: 'earliest', + toBlock: 'latest', }) assert.lengthOf(events, 3) + // this filters the test values by A = 10 and makes sure the response logs are expected testValues .filter(v => v.A == repeatA) @@ -64,8 +61,8 @@ it('emit logs and retrieve them using different filters', async() => { // make sure all events are returned events = await deployed.contract.getPastEvents({ - fromBlock: conf.startBlockHeight, - toBlock: latest, + fromBlock: 'earliest', + toBlock: 'latest', }) assert.lengthOf(events, testValues.length) @@ -79,8 +76,8 @@ it('emit logs and retrieve them using different filters', async() => { // filter by value A being 1 or -1 events = await deployed.contract.getPastEvents('Calculated', { filter: { numA: [-1, 1] }, - fromBlock: conf.startBlockHeight, - toBlock: latest, + fromBlock: 'earliest', + toBlock: 'latest', }) assert.lengthOf(events, 2) assert.equal(events[0].returnValues.numB, 2)