[pallet-revive]: move eth-rpc-only types out of pallet-revive#12037
[pallet-revive]: move eth-rpc-only types out of pallet-revive#120370xOmarA wants to merge 12 commits into
Conversation
We had some block specifier types defined in the codebase such as `BlockTag`, `BlockNumberOrTag`, and `BlockNumberOrTagOrHash`. These types were defined in pallet-revive and were never used there in any way. The primary consumer of these types was the eth-rpc. These types were removed in favor of types which are available through alloy. Additionally, the `Filter`, `AddressOrAddresses` types were also removed as we can use their alloy equivalent types in the eth-rpc.
This commit moves the types `SubscriptionKind`, `SubscriptionOptions`, `LogsSubscriptionFilter`, `SubscriptionParameters`, `SubscriptionItem`, and `BoundedOneOrMany` from pallet-revive and into the eth-rpc. In a similar way to the previous commit, these types were not used in any capacity by pallet-revive. They were defined in there but exclusively used by the eth-rpc.
The `Log`, `FilterResults`, and `ReceiptInfo` types have been moved from pallet-revive to the eth-rpc since they were not being used in revive and only in the eth-rpc.
| mod fee_history; | ||
| mod log; | ||
| mod receipt; | ||
| pub mod subscriptions; |
There was a problem hiding this comment.
nit: is there a reason this one module needs to be pub?
|
|
||
| /// Build pallet transaction information from an RPC receipt and signed transaction. | ||
| #[must_use] | ||
| pub(crate) fn transaction_info_from_receipt( |
There was a problem hiding this comment.
nit: any reason this is a free function instead of ReceiptInfo::to_transaction_info?
| /// A helper type used when a type can be serialized and deserialized as either being one or as an | ||
| /// array. | ||
| #[derive( | ||
| Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Encode, Decode, TypeInfo, |
There was a problem hiding this comment.
nit: are Encode, Decode, and TypeInfo still required for BoundedOneOrMany?
| match (from_block, to_block, filter.block_hash) { | ||
| match (from_block, to_block, block_hash) { | ||
| (Some(_), _, Some(_)) | (_, Some(_), Some(_)) => { | ||
| anyhow::bail!("block number and block hash cannot be used together"); |
There was a problem hiding this comment.
nit: is this arm still reachable? Looks like the let above makes it impossible.
marian-radu
left a comment
marian-radu
left a comment
There was a problem hiding this comment.
LGTM!
Worth pinging the anvil-polkadot team. After this PR is merged, revive_conversions.rs will stop compiling on the next polkadot-sdk bump.
Note: CI needs a look.
athei
left a comment
athei
left a comment
There was a problem hiding this comment.
The reason for this moving of types is that a lot of types were defined in
pallet-revivebut were not being used bypallet-revivein any way. They were defined there and were being used exclusively in the eth-rpc. Therefore, them being defined inpallet-revivedid not make sense.
What about downstream users of those types? They now need to import the rpc. Which is std dependency. Defining all types inside pallet-revive was on purpose for this reason.
Requesting changes and waiting for a rationale why those need to be moved.
3 participants
Description
This PR is part of the versioning effort #11923 and closes #12045. This PR moves a number of types which used to be defined in
pallet-reviveover to the eth-rpc. A small set of byte wrapper types are now used from thepallet-revive-typescrate.The reason for this moving of types is that a lot of types were defined in
pallet-revivebut were not being used bypallet-revivein any way. They were defined there and were being used exclusively in the eth-rpc. Therefore, them being defined inpallet-revivedid not make sense.The public types which were moved to the eth-rpc are as follows:
pallet_revive_eth_rpc::types::fee_history:
FeeHistoryResultpallet_revive_eth_rpc::types::log:
FilterResultsLogpallet_revive_eth_rpc::types::receipt:
ReceiptInfopallet_revive_eth_rpc::types::subscriptions:
BlockHeaderBoundedOneOrManyLogsSubscriptionFilterSubscriptionItemSubscriptionKindSubscriptionOptionsSubscriptionParameterspallet_revive_eth_rpc::types::sync:
SyncingProgressSyncingStatuspallet_revive_eth_rpc::types::trace:
TraceCallConfigTracerConfigTransactionTraceThe byte wrapper types which were removed from
pallet-reviveand are now used frompallet-revive-typesare:ByteBytesBytes8Bytes32Bytes256The following pallet-owned types are still defined in
pallet-revive, but the generatedrpc_types_gen.rscatch-all file was removed and these types were split into focused modules:pallet_revive::evm::api::block:
BlockWithdrawalpallet_revive::evm::api::transaction:
AccessListAccessListEntryAuthorizationListEntryGenericTransactionHashesOrTransactionInfosInputOrDataTransaction1559SignedTransaction1559UnsignedTransaction2930SignedTransaction2930UnsignedTransaction4844SignedTransaction4844UnsignedTransaction7702SignedTransaction7702UnsignedTransactionInfoTransactionLegacySignedTransactionLegacyUnsignedTransactionSignedTransactionUnsignedpallet_revive::evm::api::state_override:
StateOverrideStateOverrideSetStorageOverrideThe old pallet-local block specifier and filter request types were removed instead of being moved. The eth-rpc now uses the corresponding alloy types:
BlockIdBlockNumberOrTagFilterIntegration
Downstream code which imported eth-rpc-only response or request helper types from
pallet_revive::evmshould import them frompallet-revive-eth-rpcinstead. The eth-rpc crate re-exports these through its crate root and also groups them underpallet_revive_eth_rpc::types.Code using Ethereum block specifiers or log filters in the eth-rpc should use the alloy request types re-exported by
pallet-revive-eth-rpc:The core pallet transaction, block, and state override types remain available through the existing
pallet_revive::evm::*public re-exports. Their backing modules changed, but users of the public re-export path should not need source changes for those types.The byte wrappers are now the canonical definitions from
pallet_revive_types::common.pallet_revive::evm::*continues to re-export them for compatibility.Review Notes
The main structure change is that
src/evm/api/rpc_types_gen.rswas removed. Types which are still used by pallet execution/storage were moved intosrc/evm/api/block.rs,src/evm/api/transaction.rs, andsrc/evm/api/state_override.rs. This is intended to be a definition and impl move, while keeping the publicpallet_revive::evm::*re-export surface stable for the pallet-owned types.The eth-rpc-only types now live in
rpc/src/types/, grouped by use: fee history, logs, receipts, subscriptions, sync status, and debug tracing. The eth-rpc crate re-exportstypes::*, so RPC users can continue importing these types from the RPC crate root.The
TracerConfigdeserializer was kept in the eth-rpc because the debug RPC accepts multiple request shapes. It handles both explicit tracer objects such as{ "tracer": "callTracer", "tracerConfig": { ... } }and the inline execution tracer shape where execution tracer options are placed directly at the top level.The duplicate byte wrapper definitions in
pallet-revivewere deleted.pallet-revivenow re-exportsByte,Bytes,Bytes8,Bytes32, andBytes256frompallet-revive-types::common. The missingDecodeWithMemTrackingderive support and the old byte serialization tests were added to the types crate.