Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix log range filter #445

Merged
merged 26 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
e22301b
fix how bloom is matched
sideninja Aug 16, 2024
16c936e
test improvement variable
sideninja Aug 16, 2024
0c08329
add randomness to bloom
sideninja Aug 16, 2024
38a35bf
remove todo, not relevant anymore
sideninja Aug 16, 2024
1ca6ee3
change non-standard declaration
sideninja Aug 16, 2024
726d42e
increase total range
sideninja Aug 16, 2024
156fc86
improve range test and add specific height in range test
sideninja Aug 16, 2024
73dc218
change storage interface to use uint64 as height values
sideninja Aug 16, 2024
cdb08dc
use uint64 for heights
sideninja Aug 16, 2024
120ef9a
change to uint64 for height in bloom height type
sideninja Aug 16, 2024
fd0869e
use uint64 for height in filter
sideninja Aug 16, 2024
abd02a2
update usage of uint64 for height
sideninja Aug 16, 2024
5e6b280
don't use big int bytes
sideninja Aug 16, 2024
06168b5
update filter test with uint64 types
sideninja Aug 16, 2024
ef0c378
update filter test with uint64 types
sideninja Aug 16, 2024
9516618
update type change to uint64
sideninja Aug 16, 2024
5498a03
make sure response is correctly serialized
sideninja Aug 16, 2024
cd5b182
fix filter test with uint64 type change
sideninja Aug 16, 2024
12b6822
use nil return type
sideninja Aug 16, 2024
8443c37
change bloom match logic
sideninja Aug 16, 2024
6a8f471
Add test for single topic and single
sideninja Aug 16, 2024
40d2a43
fix type changes
sideninja Aug 16, 2024
d6d0b66
Merge branch 'main' into gregor/log-filter-fix
sideninja Aug 16, 2024
bc23583
improve test
sideninja Aug 16, 2024
c99767a
Merge remote-tracking branch 'origin/gregor/log-filter-fix' into greg…
sideninja Aug 16, 2024
41ff74b
test typo
sideninja Aug 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ func (b *BlockChainAPI) GetLogs(
to = latest
}

f, err := logs.NewRangeFilter(*from, *to, filter, b.receipts)
f, err := logs.NewRangeFilter(from.Uint64(), to.Uint64(), filter, b.receipts)
if err != nil {
return handleError[[]*types.Log](err, l, b.collector)
}
Expand All @@ -659,6 +659,11 @@ func (b *BlockChainAPI) GetLogs(
return handleError[[]*types.Log](err, l, b.collector)
}

// makes sure the response is correctly serialized
if res == nil {
return []*types.Log{}, nil
}

return res, nil
}

Expand Down
24 changes: 9 additions & 15 deletions api/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"errors"
"fmt"
"math/big"
"sync"
"time"

Expand Down Expand Up @@ -478,36 +477,31 @@ func (api *PullAPI) getLogs(latestHeight uint64, filter *logsFilter) (any, error

to := filter.criteria.ToBlock
// we use latest as default for end range
end := big.NewInt(int64(latestHeight))
end := latestHeight
// unless "to" is defined in the criteria
if to != nil && to.Int64() >= 0 {
end = to
end = to.Uint64()
// if latest height is bigger than range "to" then we don't return anything
if latestHeight > to.Uint64() {
if latestHeight > end {
return []*gethTypes.Log{}, nil
}
}

start := big.NewInt(int64(nextHeight))
start := nextHeight
// we fetched all available data since start is now bigger than end value
if start.Cmp(end) > 0 {
if start > end {
return []*gethTypes.Log{}, nil
}

f, err := logs.NewRangeFilter(*start, *end, criteria, api.receipts)
f, err := logs.NewRangeFilter(start, end, criteria, api.receipts)
if err != nil {
return nil, fmt.Errorf(
"could not create range filter from %d to %d: %w",
start.Int64(),
end.Int64(),
err,
)
return nil, fmt.Errorf("could not create range filter from %d to %d: %w", start, end, err)
}

api.logger.Debug().
Uint64("latest", latestHeight).
Int64("from", start.Int64()).
Int64("to", end.Int64()).
Uint64("from", start).
Uint64("to", end).
Msg("get filter logs")

return f.Match()
Expand Down
11 changes: 5 additions & 6 deletions models/receipt.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@ import (
// geth node has the data locally, but we don't in evm gateway, so we can not reproduce those values
// and we need to store them
type StorageReceipt struct {
Type uint8 `json:"type,omitempty"`
PostState []byte `json:"root"`
Status uint64 `json:"status"`
CumulativeGasUsed uint64 `json:"cumulativeGasUsed"`
// todo we could skip bloom to optimize storage and dynamically recalculate it
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this todo is not really true since we store blooms in storage

Type uint8 `json:"type,omitempty"`
PostState []byte `json:"root"`
Status uint64 `json:"status"`
CumulativeGasUsed uint64 `json:"cumulativeGasUsed"`
Bloom gethTypes.Bloom `json:"logsBloom"`
Logs []*gethTypes.Log `json:"logs"`
TxHash common.Hash `json:"transactionHash"`
Expand Down Expand Up @@ -134,5 +133,5 @@ func MarshalReceipt(

type BloomsHeight struct {
Blooms []*gethTypes.Bloom
Height *big.Int
Height uint64
}
36 changes: 22 additions & 14 deletions services/logs/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package logs
import (
"errors"
"fmt"
"math/big"

"github.com/onflow/go-ethereum/common"
gethTypes "github.com/onflow/go-ethereum/core/types"
Expand Down Expand Up @@ -49,32 +48,31 @@ func NewFilterCriteria(addresses []common.Address, topics [][]common.Hash) (*Fil
// RangeFilter matches all the indexed logs within the range defined as
// start and end block height. The start must be strictly smaller or equal than end value.
type RangeFilter struct {
start, end *big.Int
start, end uint64
criteria *FilterCriteria
receipts storage.ReceiptIndexer
}

func NewRangeFilter(
start, end big.Int,
start, end uint64,
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 {
// make sure that beginning number is not bigger than end
if start > end {
return nil, errors.Join(
errs.ErrInvalid,
fmt.Errorf("start block number must be smaller or equal to end block number"),
)
}

return &RangeFilter{
start: &start,
end: &end,
start: start,
end: end,
criteria: &criteria,
receipts: receipts,
}, nil
Expand All @@ -86,16 +84,26 @@ func (r *RangeFilter) Match() ([]*gethTypes.Log, error) {
return nil, err
}

logs := make([]*gethTypes.Log, 0)
var bloomHeightMatches []uint64
var logs []*gethTypes.Log

// first filter all the logs based on whether a bloom matches,
// if bloom matches we fetch only that height later and do exact match
for _, bloomHeight := range bloomsHeight {
for _, bloom := range bloomHeight.Blooms {
if !bloomMatch(*bloom, r.criteria) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was broken, probably got broken after changes, because it did nothing, so it didn't optimize next step.

continue
if bloomMatch(*bloom, r.criteria) {
bloomHeightMatches = append(bloomHeightMatches, bloomHeight.Height)
// if there's a match we add the height and skip to next height
// even if there would be multiple matches for height we just want to have unique heights
break
}
}
}

// todo do this concurrently
receipts, err := r.receipts.GetByBlockHeight(bloomHeight.Height)
// do exact matches only on subset of heights in the range that matched the bloom
for _, height := range bloomHeightMatches {
// todo do this concurrently but make sure order is correct
receipts, err := r.receipts.GetByBlockHeight(height)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -147,7 +155,7 @@ func (i *IDFilter) Match() ([]*gethTypes.Log, error) {
return nil, err
}

receipts, err := i.receipts.GetByBlockHeight(big.NewInt(int64(blk.Height)))
receipts, err := i.receipts.GetByBlockHeight(blk.Height)
if err != nil {
return nil, err
}
Expand Down
55 changes: 32 additions & 23 deletions services/logs/filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,11 @@ func receiptStorage() storage.ReceiptIndexer {

receiptStorage := &mocks.ReceiptIndexer{}
receiptStorage.
On("GetByBlockHeight", mock.AnythingOfType("*big.Int")).
Return(func(height *big.Int) ([]*models.StorageReceipt, error) {
On("GetByBlockHeight", mock.AnythingOfType("uint64")).
Return(func(height uint64) ([]*models.StorageReceipt, error) {
rcps := make([]*models.StorageReceipt, 0)
for _, r := range receipts {
if r.BlockNumber.Cmp(height) == 0 {
if r.BlockNumber.Uint64() == height {
rcps = append(rcps, r)
}
}
Expand All @@ -135,17 +135,17 @@ func receiptStorage() storage.ReceiptIndexer {
})

receiptStorage.
On("BloomsForBlockRange", mock.AnythingOfType("*big.Int"), mock.AnythingOfType("*big.Int")).
Return(func(start, end *big.Int) ([]*models.BloomsHeight, error) {
On("BloomsForBlockRange", mock.AnythingOfType("uint64"), mock.AnythingOfType("uint64")).
Return(func(start, end uint64) ([]*models.BloomsHeight, error) {
blooms := make([]*gethTypes.Bloom, 0)
bloomsHeight := make([]*models.BloomsHeight, 0)

for _, r := range receipts {
if r.BlockNumber.Cmp(start) >= 0 && r.BlockNumber.Cmp(end) <= 0 {
if r.BlockNumber.Uint64() >= start && r.BlockNumber.Uint64() <= end {
blooms = append(blooms, &r.Bloom)
bloomsHeight = append(bloomsHeight, &models.BloomsHeight{
Blooms: blooms,
Height: r.BlockNumber,
Height: r.BlockNumber.Uint64(),
})
}
}
Expand Down Expand Up @@ -285,56 +285,65 @@ func TestRangeFilter(t *testing.T) {

tests := []struct {
desc string
start, end *big.Int
start, end uint64
expectLogs []*gethTypes.Log
criteria FilterCriteria
}{{
desc: "single topic, single address, single block match single log",
start: big.NewInt(0),
end: big.NewInt(1),
start: 0,
end: 1,
criteria: FilterCriteria{
Addresses: []common.Address{logs[0][0].Address},
Topics: [][]common.Hash{logs[0][0].Topics[:1]},
},
expectLogs: logs[0][:1],
}, {
desc: "single topic, single address, all blocks match multiple logs",
start: big.NewInt(0),
end: big.NewInt(4),
start: 0,
end: 4,
criteria: FilterCriteria{
Addresses: []common.Address{logs[0][0].Address},
Topics: [][]common.Hash{logs[0][0].Topics[:1]},
},
expectLogs: []*gethTypes.Log{logs[0][0], logs[3][1]},
}, {
desc: "single topic, single address, subset of blocks match single log",
start: 2,
end: 4,
criteria: FilterCriteria{
Addresses: []common.Address{logs[0][0].Address},
Topics: [][]common.Hash{logs[0][0].Topics[:1]},
},
expectLogs: []*gethTypes.Log{logs[3][1]},
}, {
desc: "single address, all blocks match multiple logs",
start: big.NewInt(0),
end: big.NewInt(4),
start: 0,
end: 4,
criteria: FilterCriteria{
Addresses: []common.Address{logs[0][0].Address},
},
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),
end: big.NewInt(4),
start: 0,
end: 4,
criteria: FilterCriteria{
Addresses: []common.Address{common.HexToAddress("0x123")},
},
expectLogs: []*gethTypes.Log{},
expectLogs: nil,
}, {
desc: "single address, non-existing range no match",
start: big.NewInt(5),
end: big.NewInt(10),
start: 5,
end: 10,
criteria: FilterCriteria{
Addresses: []common.Address{logs[0][0].Address},
},
expectLogs: []*gethTypes.Log{},
expectLogs: nil,
}}

for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
filter, err := NewRangeFilter(*tt.start, *tt.end, tt.criteria, receiptStorage())
filter, err := NewRangeFilter(tt.start, tt.end, tt.criteria, receiptStorage())
require.NoError(t, err)

matchedLogs, err := filter.Match()
Expand All @@ -346,8 +355,8 @@ func TestRangeFilter(t *testing.T) {

t.Run("with topics count exceeding limit", func(t *testing.T) {
_, err := NewRangeFilter(
*big.NewInt(0),
*big.NewInt(4),
0,
4,
FilterCriteria{
Topics: [][]common.Hash{
{common.HexToHash("123")},
Expand Down
12 changes: 6 additions & 6 deletions storage/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,28 +45,28 @@ type BlockIndexer interface {

// SetLatestCadenceHeight sets the latest Cadence height.
// Batch is required to batch multiple indexer operations, skipped if nil.
SetLatestCadenceHeight(height uint64, batch *pebble.Batch) error
SetLatestCadenceHeight(cadenceHeight uint64, batch *pebble.Batch) error

// GetCadenceHeight returns the Cadence height that matches the
// provided EVM height. Each EVM block indexed contains a link
// to the Cadence height.
// - errors.NotFound if the height is not found
GetCadenceHeight(evmHeight uint64) (uint64, error)
GetCadenceHeight(height uint64) (uint64, error)

// GetCadenceID returns the Cadence block ID that matches the
// provided EVM height. Each EVM block indexed contains a link to the
// Cadence block ID. Multiple EVM heights can point to the same
// Cadence block ID.
// - errors.NotFound if the height is not found
GetCadenceID(evmHeight uint64) (flow.Identifier, error)
GetCadenceID(height uint64) (flow.Identifier, error)
}

type ReceiptIndexer interface {
// Store provided receipt.
// Batch is required to batch multiple indexer operations, skipped if nil.
// Expected errors:
// - errors.Duplicate if the block already exists.
Store(receipts []*models.StorageReceipt, evmHeight uint64, batch *pebble.Batch) error
Store(receipts []*models.StorageReceipt, height uint64, batch *pebble.Batch) error

// GetByTransactionID returns the receipt for the transaction ID.
// Expected errors:
Expand All @@ -76,14 +76,14 @@ type ReceiptIndexer interface {
// GetByBlockHeight returns the receipt for the block height.
// Expected errors:
// - errors.NotFound if the receipt is not found
GetByBlockHeight(height *big.Int) ([]*models.StorageReceipt, error)
GetByBlockHeight(height uint64) ([]*models.StorageReceipt, error)

// BloomsForBlockRange returns slice of bloom values and a slice of block heights
// corresponding to each item in the bloom slice. It only matches the blooms between
// inclusive start and end block height.
// Expected errors:
// - errors.InvalidRange if the block by the height was not indexed or if the end and start values are invalid.
BloomsForBlockRange(start, end *big.Int) ([]*models.BloomsHeight, error)
BloomsForBlockRange(start, end uint64) ([]*models.BloomsHeight, error)
}

type TransactionIndexer interface {
Expand Down
Loading
Loading