-
Notifications
You must be signed in to change notification settings - Fork 10
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
[Indexer] Implement log providers and filters #33
[Indexer] Implement log providers and filters #33
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 🚀 Just a couple minor comments.
[Indexer] Decoding of event values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🪄
gethTypes "github.com/ethereum/go-ethereum/core/types" | ||
"github.com/onflow/flow-evm-gateway/storage" | ||
"math/big" | ||
"slices" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"slices" | |
"golang.org/x/exp/slices" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good. mostly small comments.
"fmt" | ||
"github.com/ethereum/go-ethereum/common" | ||
"github.com/onflow/cadence" | ||
cdcCommon "github.com/onflow/cadence/runtime/common" | ||
"github.com/onflow/flow-go/fvm/evm/types" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: split stdlib and external imports (here and elsewhere)
"fmt" | |
"github.com/ethereum/go-ethereum/common" | |
"github.com/onflow/cadence" | |
cdcCommon "github.com/onflow/cadence/runtime/common" | |
"github.com/onflow/flow-go/fvm/evm/types" | |
"fmt" | |
"github.com/ethereum/go-ethereum/common" | |
"github.com/onflow/cadence" | |
cdcCommon "github.com/onflow/cadence/runtime/common" | |
"github.com/onflow/flow-go/fvm/evm/types" |
} | ||
|
||
receipt := &gethTypes.Receipt{ | ||
Type: 0, // todo check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
were you planning to address these todo
s in this PR or a future one? is there an issue tracking them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
services/logs/filter.go
Outdated
} | ||
} | ||
|
||
func (s *StreamFilter) Match() (chan *gethTypes.Log, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
best to only return the channel receiver, otherwise it could be written to or closed
func (s *StreamFilter) Match() (chan *gethTypes.Log, error) { | |
func (s *StreamFilter) Match() (<-chan *gethTypes.Log, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point
services/logs/filter.go
Outdated
defer close(logs) | ||
|
||
for { | ||
select { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need this select since there's only one condition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol habit
|
||
// exactMatch checks the topic and address values of the log match the filer exactly. | ||
func exactMatch(log *gethTypes.Log, criteria FilterCriteria) bool { | ||
if len(criteria.Topics) > len(log.Topics) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be?
if len(criteria.Topics) > len(log.Topics) { | |
if len(criteria.Topics) != len(log.Topics) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It shouldn't because the criteria can have less topics than the log, in case where topics are wildcards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a comment
[Indexer] Decoding of event values
…ndexer-dec Revert "[Indexer] Decoding of event values"
Closes: #28
Description
This PR implements logs filters. This will be used to implement log filtering based on addresses and topics and will allow for multiple sources either logs indexed in the storage and being queried by block height range, or by subscribing to new stream of logs directly from the network.
There are three log filters:
All the filters reuse the same filter criteria which defines the Addresses and Topics.
The filters are fetched from two different sources:
Providers use bloom filters to determine if a log is relevant or not in order to optimize loading all the data.
The implementation follows the design:
For contributor use:
master
branchFiles changed
in the Github PR explorer