-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[BCI-3863] - Use filtered logs in eventBinding GetLatestValue instead of manual filtering #14096
[BCI-3863] - Use filtered logs in eventBinding GetLatestValue instead of manual filtering #14096
Conversation
logToUse = &tmp | ||
} | ||
// Gets the latest log that matches the filter and limiter. | ||
logs, err := e.lp.FilteredLogs(ctx, filter, limiter, e.contractName+"-"+e.eventName) |
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 I add address to the queryName
too?
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.
don't see why not , CC @EasterTheBunny
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.
Using queryName
here might be helpful. The idea is that the string passed will show in slow query logs and help identify problems in the log file.
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.
One small non-functional change and then adding the queryName
, but otherwise it looks good.
logToUse = &tmp | ||
} | ||
// Gets the latest log that matches the filter and limiter. | ||
logs, err := e.lp.FilteredLogs(ctx, filter, limiter, e.contractName+"-"+e.eventName) |
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.
Using queryName
here might be helpful. The idea is that the string passed will show in slow query logs and help identify problems in the log file.
// Create limiter and filter for the query. | ||
limiter := query.NewLimitAndSort(query.CountLimit(1), query.NewSortBySequence(query.Desc)) | ||
filter, err := query.Where( | ||
e.address.String(), |
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.
Passing the address here isn't necessary and can be left empty. Since you're within the evm
relay package here, you're able to use the logpoller
filters directly and no key -> filter mappings are necessary. Putting an actual value here might just cause confusion later on.
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.
addressed in fa8056c
(#14096) and 48aad52
(#14096)
Quality Gate passedIssues Measures |
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.
LGTM
Task Description:
Use log poller Filtered logs instead of manual post filtering.
This PR:
Replaces the use of IndexedLogs and manual post filtering with log poller Filtered logs and query filters.
Ticket: