-
Notifications
You must be signed in to change notification settings - Fork 77
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
feat: calculate logsBloom
for synthetic transactions
#2785
Conversation
Signed-off-by: nikolay <[email protected]>
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
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, I left a few suggestions
Signed-off-by: nikolay <[email protected]>
Signed-off-by: nikolay <[email protected]>
Signed-off-by: nikolay <[email protected]>
Signed-off-by: nikolay <[email protected]>
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, few last suggestions:
Signed-off-by: nikolay <[email protected]>
Signed-off-by: nikolay <[email protected]>
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2785 +/- ##
==========================================
+ Coverage 77.80% 80.07% +2.27%
==========================================
Files 43 47 +4
Lines 3329 3478 +149
Branches 706 736 +30
==========================================
+ Hits 2590 2785 +195
+ Misses 493 429 -64
- Partials 246 264 +18
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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
Description:
Currently with our synthetic transactions, which are constructed on the fly using events from the operations, we don't return adequate and real logsBloom. This might be a problem for some tools, which requires logsBloom for their operation.
Solution:
We can easily calculate logsBloom for each receipt, this can be done, by taking a look at how it's implemented in the javascript execution client for Ethereum here
Related issue(s):
Fixes #2664
Notes for reviewer:
Checklist