-
Notifications
You must be signed in to change notification settings - Fork 385
Guarantee spec compliance of eth_subscribe's newHeads
#3597
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
Guarantee spec compliance of eth_subscribe's newHeads
#3597
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
WASM runtime size check:Compared to target branchMoonbase runtime: 2108 KB (no changes) ✅ Moonbeam runtime: 2232 KB (no changes) ✅ Moonriver runtime: 2232 KB (no changes) ✅ Compared to latest release (runtime-4100)Moonbase runtime: 2108 KB (+172 KB compared to latest release) Moonbeam runtime: 2232 KB (+200 KB compared to latest release) Moonriver runtime: 2232 KB (+200 KB compared to latest release) |
Coverage Report@@ Coverage Diff @@
## master manuel/test-for-spec-compliance-of-eth-subscribe-new-heads +/- ##
=============================================================================================
Coverage 76.65% 76.65% 0.00%
Files 389 389
Lines 76440 76440
=============================================================================================
Hits 58595 58595
Misses 17845 17845
|
eth_subzscribe's newHeadseth_subscribe's newHeads
eth_subscribe's newHeadseth_subscribe's newHeads
…c-compliance-of-eth-subscribe-new-heads
The previous implementation was not compliant with the Ethereum specs, thus the new (compliant) behavior might be considered a breaking change with regard to previous assumptions.
In particular, while the current implementation in case of a reorg just returns the new head with a parent hash that was never mentioned within the subscription, the new behavior according to the specs is as follows:
"When a chain reorganization occurs, this subscription will emit an event containing all new headers (blocks) for the new chain. This means that you may see multiple headers emitted with the same height (block number), and when this happens the later (highest) block number should be taken as the correct one after a reorganization."
What does it do?
Guarantees spec compliance of
eth_subscribe'snewHeads.More information here:https://github.com/ethereum/go-ethereum/wiki/RPC-PUB-SUB/7a0c1704737ece636aabda1d08d43f96dd1a619f#newheads
Relevant PRs
EthereumBlockNotificationwith reorg info to allow compliance with Ethereum specs polkadot-evm/frontier#1787