Extend EthereumBlockNotification with reorg info to allow compliance with Ethereum specs#1787
Conversation
|
This is (kind of) a follow-up PR to #1781 |
|
@RomarQ concerning the comments above, the fact that we populate the reorg info at Specifically this: let reorg_info = if let Some(ref tree_route) = notification.tree_route {
log::debug!(
target: "frontier-sql",
"🔀 Re-org happened at new best {}, proceeding to canonicalize db",
notification.hash
);
let retracted = tree_route
.retracted()
.iter()
.map(|hash_and_number| hash_and_number.hash)
.collect::<Vec<_>>();
let enacted = tree_route
.enacted()
.iter()
.map(|hash_and_number| hash_and_number.hash)
.collect::<Vec<_>>();
let common = tree_route.common_block().hash;
tx.send(WorkerCommand::Canonicalize {
common,
enacted: enacted.clone(),
retracted: retracted.clone(),
}).await.ok();
Some(ReorgInfo {
common_ancestor: common,
retracted,
enacted,
})
} else {
None
}; |
There was a problem hiding this comment.
I am a bit concerned that the notification mechanism for key-value and sql backends are so different. Could we unify the notification mechanism in a single place? maybe reusing the WorkerCommand approach.
We also need to run the test for each storage backend sql and key-value.
…eads-comply-with-ethereum-spec
Possibly addressed the comment in dbd0537 and 4582a9a (It might need more work.) |
There was a problem hiding this comment.
Currently, the implementation of kv and sql backends seems to different. This makes the maintenance more difficult and can cause inconsistencies. I think we should try to align the mapping-sync implementation on both backends, by making it more generic.
I am fine to approve this once the remarks have been addressed, and do the alignment of the mapping-sync implementation in a follow-up PR.
…e with Ethereum specs (polkadot-evm#1787) * feat: ✨ extend EthereumBlockNotification with reorg info * style: 🎨 fmt * fix: 🐛 emit reorg information from Canonicalize * test: ✅ add basic integration tests * fix: 🐛 fix newHead missing enacted block * test: ✅ test deeper forks * style: 🎨 fmt * fix: 🐛 sort enacted blocks * fix: 🐛 guarantee async sends ordering * style: 🎨 fmt * Revert "style: 🎨 fmt" This reverts commit 6d644a6. * Revert "fix: 🐛 guarantee async sends ordering" This reverts commit 2945dda. * Revert "fix: 🐛 sort enacted blocks" This reverts commit 44e7517. * ci: 👷 test sql backend in CI * refactor: ♻️ dedupliacte common logic between sql and kv backends * style: 🎨 fmt * refactor: ♻️ uniform notification process across backends * docs: 📝 remove comments * refactor: ♻️ add defensive check * fix: 🐛 properly distinguish enacted blocks from new best * refactor: ♻️ reduce code duplication
Merge upstream frontier master (polkadot-sdk stable2512 compatible). This brings 43 commits including: - Update to polkadot-sdk stable2512 (polkadot-evm#1796) - Add support for EIP-7825, EIP-7823, EIP-7883, EIP-7939 (Osaka fork) - Validate max block range eth_getLogs RPC (polkadot-evm#1794) - Extend EthereumBlockNotification with reorg info (polkadot-evm#1787) Bifrost customizations preserved: - view_call: Read-only EVM calls without nonce modification - call_as_internal_call: Internal EVM calls for fee payment operations - FeelessCallFilter: Gas-free EVM call support - ERC20 fee payment (NODE-194) - Debug/trace RPC extensions (NODE-190, NODE-193) - Custom web3_clientVersion format (NODE-84) - logs_request_timeout parameter Conflicts resolved: - frame/evm/src/runner/mod.rs: Added both create_force_address and Bifrost methods - frame/evm/src/runner/stack.rs: Added both create_force_address and Bifrost methods - client/rpc/src/eth/filter.rs: Kept both logs_request_timeout and max_block_range Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Description
Modify the
EthereumBlockNotificationstruct infc-mapping-syncto include reorg information. The mapping sync layer already tracks the canonical chain and could provide this data.Implementation:
This allows us to comply with Ethereum specs.