Skip to content

eth/filter, ethclient/gethclient: added fullTx-flag to NewPendingTransactions #2239

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

Merged
merged 1 commit into from
Jan 31, 2024

Conversation

diwu1989
Copy link
Contributor

@diwu1989 diwu1989 commented Jan 27, 2024

Porting https://github.com/ethereum/go-ethereum/pull/25186/files to Celo.
There were some trivial merge conflicts in eth/filters/api.go that needed to be resolved before this could apply.

Would like this to be merged in and deployed to Forno so that websocket streams from the pub RPC TX pool includes the body, otherwise, there's just extra load doing round trip to fetch via pending tx header.

@diwu1989 diwu1989 force-pushed the port-full-pending-txs branch from 88c49e6 to 2dc3c78 Compare January 27, 2024 05:45
Copy link

codecov bot commented Jan 29, 2024

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (755b653) 55.06% compared to head (776aed1) 55.17%.
Report is 5 commits behind head on master.

Files Patch % Lines
eth/filters/api.go 42.10% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2239      +/-   ##
==========================================
+ Coverage   55.06%   55.17%   +0.10%     
==========================================
  Files         683      683              
  Lines      114533   114538       +5     
==========================================
+ Hits        63073    63191     +118     
+ Misses      47562    47468      -94     
+ Partials     3898     3879      -19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@diwu1989 diwu1989 force-pushed the port-full-pending-txs branch from 2dc3c78 to 776aed1 Compare January 31, 2024 08:28
@palango palango self-requested a review January 31, 2024 14:18
Copy link
Collaborator

@palango palango left a comment

Choose a reason for hiding this comment

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

Great, thanks for creating this PR ⭐
Code looks good and the remaining CI failures are due to missing permissions.

@palango palango merged commit 69dbdae into celo-org:master Jan 31, 2024
Copy link

Coverage from tests in ./e2e_test/... for ./consensus/istanbul/... at commit 69dbdae

coverage: 50.9% of statements across all listed packages
coverage:  63.2% of statements in consensus/istanbul
coverage:  42.8% of statements in consensus/istanbul/announce
coverage:  56.0% of statements in consensus/istanbul/backend
coverage:   0.0% of statements in consensus/istanbul/backend/backendtest
coverage:  24.3% of statements in consensus/istanbul/backend/internal/replica
coverage:  65.1% of statements in consensus/istanbul/core
coverage:  50.0% of statements in consensus/istanbul/db
coverage:   0.0% of statements in consensus/istanbul/proxy
coverage:  64.2% of statements in consensus/istanbul/uptime
coverage:  51.8% of statements in consensus/istanbul/validator
coverage:  79.2% of statements in consensus/istanbul/validator/random

Copy link

5882 passed, 45 skipped

@diwu1989 diwu1989 deleted the port-full-pending-txs branch February 7, 2024 19:20
@diwu1989
Copy link
Contributor Author

diwu1989 commented Feb 7, 2024

Thanks for the merge, there's a followup bug fix in ethereum/go-ethereum#26126 that I'll also port over so that the pending RPC transaction JSON is consistent across the different APIs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants