pallet-revive-eth-rpc: Introduce SubstrateClientT trait abstraction and Asset Hub embedded RPC integration#11297
pallet-revive-eth-rpc: Introduce SubstrateClientT trait abstraction and Asset Hub embedded RPC integration#11297Nathy-bajo wants to merge 35 commits intoparitytech:masterfrom
Conversation
|
@Wizdave97 @seunlanlege Ready for review |
pgherveou
left a comment
pgherveou
left a comment
There was a problem hiding this comment.
Thanks for putting that together.
Before we can move forward, we need to check with the Node team
if they are ok adding these dependencies to the omninode cc @skunert
One of the reason we didn't do it initially was:
- 1 so we could move fast and deploy new version without touching the client.
- 2 so that it's not tied to a client implementation. But I guess since most collator should run the omni -ode and there are no other collator implementation, that doesn't really matter.
The eth server is more stable now so it make sense to remove the requirement for a second binary / host url, and your design allow to keep both the binary inside the client and to build it as a separate binary with subxt.
There was a problem hiding this comment.
To simplify the review and not scare the reviewer, I would move the dev-node deletion to a follow up
|
Also tried to build it locally but seems not to compile yet. |
Thanks for the review, fixing ci |
| }) | ||
| } | ||
|
|
||
| async fn system_health(&self) -> Result<NodeHealth, ClientError> { |
There was a problem hiding this comment.
Could the network service be passed in so we get real peer count and sync status?
|
@marian-radu What's the right way to regenerate |
I propose migrating revive-dev-runtime off polkadot-sdk umbrella to individual crate dependencies to break the circular dependency With that done, revive-dev-runtime can stay as a build dependency (gated behind the subxt feature), and polkadot-omni-node-lib can depend on pallet-revive-eth-rpc with default-features = false — so the native path doesn't need subxt or the .scale file at all. I pushed a gist with this approach as a reference: (generated with Claude Code, so treat it as a starting point) The key changes:
|
|
/cmd prdoc --audience node_dev --bump patch |
All done and ready for review, thank you! |
5 participants
closes #11221
Removes the
revive-dev-nodestandalone binary and integrates thepallet-reviveETH RPC server directly into the Asset Hub (Omni) node. Introduces aSubstrateClientTtrait that abstracts all Substrate node interactions, allowingEthRpcServerImplto be generic over its backend. A newNativeSubstrateClientimplementation backed bysc_client_apitraits provides a direct in-process path with nosubxtdependency on the hot path. TheSubxtClientpath is preserved unchanged for the standaloneeth-rpcbinary. On the node side,BuildAssetHubRpcExtensionsis added topolkadot-omni-node-libbehind an opt-inrevive-rpcfeature, registering the fulleth_*/debug_*/net_*/web3_*surface on the standard RPC port alongside existing parachain RPCs.