-
Notifications
You must be signed in to change notification settings - Fork 668
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
[parachain-template] runtime API Implementations into mod apis
#3817
Merged
bkchr
merged 7 commits into
paritytech:master
from
asiniscalchi:refactor/parachain-runtime-apis
Mar 28, 2024
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
d4aa573
fix compilation for all targets
asiniscalchi ba72c77
fix node compilation
asiniscalchi d7c06e9
Merge branch 'master' into refactor/parachain-runtime-apis
asiniscalchi bf650ac
added license header
asiniscalchi 80e4ef2
Merge branch 'master' into refactor/parachain-runtime-apis
asiniscalchi 3aef97f
added prdoc
asiniscalchi 0bee19a
Merge branch 'master' into refactor/parachain-runtime-apis
asiniscalchi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 | ||
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json | ||
|
||
title: Parachain Runtime API Implementations into mod apis Refactoring | ||
|
||
doc: | ||
- audience: Runtime Dev | ||
description: | | ||
This PR introduces a refactoring to the runtime API implementations within the parachain template project. The primary changes include enhancing the visibility of `RUNTIME_API_VERSIONS` to `pub` in `impl_runtime_apis.rs`, centralizing API implementations in a new `apis.rs` file, and streamlining `lib.rs`. These changes aim to improve project structure, maintainability, and readability. | ||
|
||
Key Changes: | ||
- `RUNTIME_API_VERSIONS` is now publicly accessible, enhancing module-wide visibility. | ||
- Introduction of `apis.rs` centralizes runtime API implementations, promoting a cleaner and more navigable project structure. | ||
- The main runtime library file, `lib.rs`, has been updated to reflect these structural changes, removing redundant API implementations and simplifying runtime configuration by pointing `VERSION` to the newly exposed `RUNTIME_API_VERSIONS` from `apis.rs`. | ||
|
||
Motivations: | ||
- **Improved Project Structure**: Centralizing API implementations offers a more organized and understandable project layout. | ||
- **Enhanced Readability**: The refactoring efforts aim to declutter `lib.rs`, facilitating easier comprehension for new contributors. | ||
|
||
crates: | ||
- name: sp-api-proc-macro | ||
- name: parachain-template-node | ||
- name: parachain-template-runtime |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,275 @@ | ||
// This is free and unencumbered software released into the public domain. | ||
// | ||
// Anyone is free to copy, modify, publish, use, compile, sell, or | ||
// distribute this software, either in source code form or as a compiled | ||
// binary, for any purpose, commercial or non-commercial, and by any | ||
// means. | ||
// | ||
// In jurisdictions that recognize copyright laws, the author or authors | ||
// of this software dedicate any and all copyright interest in the | ||
// software to the public domain. We make this dedication for the benefit | ||
// of the public at large and to the detriment of our heirs and | ||
// successors. We intend this dedication to be an overt act of | ||
// relinquishment in perpetuity of all present and future rights to this | ||
// software under copyright law. | ||
// | ||
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, | ||
// EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF | ||
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. | ||
// IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES OR | ||
// OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, | ||
// ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR | ||
// OTHER DEALINGS IN THE SOFTWARE. | ||
// | ||
// For more information, please refer to <http://unlicense.org> | ||
|
||
// External crates imports | ||
ggwpez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
use frame_support::{ | ||
genesis_builder_helper::{build_config, create_default_config}, | ||
weights::Weight, | ||
}; | ||
use pallet_aura::Authorities; | ||
use sp_api::impl_runtime_apis; | ||
use sp_consensus_aura::sr25519::AuthorityId as AuraId; | ||
use sp_core::{crypto::KeyTypeId, OpaqueMetadata}; | ||
use sp_runtime::{ | ||
traits::Block as BlockT, | ||
transaction_validity::{TransactionSource, TransactionValidity}, | ||
ApplyExtrinsicResult, | ||
}; | ||
use sp_std::prelude::Vec; | ||
use sp_version::RuntimeVersion; | ||
|
||
// Local module imports | ||
use super::{ | ||
AccountId, Aura, Balance, Block, Executive, InherentDataExt, Nonce, ParachainSystem, Runtime, | ||
RuntimeCall, RuntimeGenesisConfig, SessionKeys, System, TransactionPayment, VERSION, | ||
}; | ||
|
||
impl_runtime_apis! { | ||
impl sp_consensus_aura::AuraApi<Block, AuraId> for Runtime { | ||
fn slot_duration() -> sp_consensus_aura::SlotDuration { | ||
sp_consensus_aura::SlotDuration::from_millis(Aura::slot_duration()) | ||
} | ||
|
||
fn authorities() -> Vec<AuraId> { | ||
Authorities::<Runtime>::get().into_inner() | ||
} | ||
} | ||
|
||
impl sp_api::Core<Block> for Runtime { | ||
fn version() -> RuntimeVersion { | ||
VERSION | ||
} | ||
|
||
fn execute_block(block: Block) { | ||
Executive::execute_block(block) | ||
} | ||
|
||
fn initialize_block(header: &<Block as BlockT>::Header) -> sp_runtime::ExtrinsicInclusionMode { | ||
Executive::initialize_block(header) | ||
} | ||
} | ||
|
||
impl sp_api::Metadata<Block> for Runtime { | ||
fn metadata() -> OpaqueMetadata { | ||
OpaqueMetadata::new(Runtime::metadata().into()) | ||
} | ||
|
||
fn metadata_at_version(version: u32) -> Option<OpaqueMetadata> { | ||
Runtime::metadata_at_version(version) | ||
} | ||
|
||
fn metadata_versions() -> sp_std::vec::Vec<u32> { | ||
Runtime::metadata_versions() | ||
} | ||
} | ||
|
||
impl sp_block_builder::BlockBuilder<Block> for Runtime { | ||
fn apply_extrinsic(extrinsic: <Block as BlockT>::Extrinsic) -> ApplyExtrinsicResult { | ||
Executive::apply_extrinsic(extrinsic) | ||
} | ||
|
||
fn finalize_block() -> <Block as BlockT>::Header { | ||
Executive::finalize_block() | ||
} | ||
|
||
fn inherent_extrinsics(data: sp_inherents::InherentData) -> Vec<<Block as BlockT>::Extrinsic> { | ||
data.create_extrinsics() | ||
} | ||
|
||
fn check_inherents( | ||
block: Block, | ||
data: sp_inherents::InherentData, | ||
) -> sp_inherents::CheckInherentsResult { | ||
data.check_extrinsics(&block) | ||
} | ||
} | ||
|
||
impl sp_transaction_pool::runtime_api::TaggedTransactionQueue<Block> for Runtime { | ||
fn validate_transaction( | ||
source: TransactionSource, | ||
tx: <Block as BlockT>::Extrinsic, | ||
block_hash: <Block as BlockT>::Hash, | ||
) -> TransactionValidity { | ||
Executive::validate_transaction(source, tx, block_hash) | ||
} | ||
} | ||
|
||
impl sp_offchain::OffchainWorkerApi<Block> for Runtime { | ||
fn offchain_worker(header: &<Block as BlockT>::Header) { | ||
Executive::offchain_worker(header) | ||
} | ||
} | ||
|
||
impl sp_session::SessionKeys<Block> for Runtime { | ||
fn generate_session_keys(seed: Option<Vec<u8>>) -> Vec<u8> { | ||
SessionKeys::generate(seed) | ||
} | ||
|
||
fn decode_session_keys( | ||
encoded: Vec<u8>, | ||
) -> Option<Vec<(Vec<u8>, KeyTypeId)>> { | ||
SessionKeys::decode_into_raw_public_keys(&encoded) | ||
} | ||
} | ||
|
||
impl frame_system_rpc_runtime_api::AccountNonceApi<Block, AccountId, Nonce> for Runtime { | ||
fn account_nonce(account: AccountId) -> Nonce { | ||
System::account_nonce(account) | ||
} | ||
} | ||
|
||
impl pallet_transaction_payment_rpc_runtime_api::TransactionPaymentApi<Block, Balance> for Runtime { | ||
fn query_info( | ||
uxt: <Block as BlockT>::Extrinsic, | ||
len: u32, | ||
) -> pallet_transaction_payment_rpc_runtime_api::RuntimeDispatchInfo<Balance> { | ||
TransactionPayment::query_info(uxt, len) | ||
} | ||
fn query_fee_details( | ||
uxt: <Block as BlockT>::Extrinsic, | ||
len: u32, | ||
) -> pallet_transaction_payment::FeeDetails<Balance> { | ||
TransactionPayment::query_fee_details(uxt, len) | ||
} | ||
fn query_weight_to_fee(weight: Weight) -> Balance { | ||
TransactionPayment::weight_to_fee(weight) | ||
} | ||
fn query_length_to_fee(length: u32) -> Balance { | ||
TransactionPayment::length_to_fee(length) | ||
} | ||
} | ||
|
||
impl pallet_transaction_payment_rpc_runtime_api::TransactionPaymentCallApi<Block, Balance, RuntimeCall> | ||
for Runtime | ||
{ | ||
fn query_call_info( | ||
call: RuntimeCall, | ||
len: u32, | ||
) -> pallet_transaction_payment::RuntimeDispatchInfo<Balance> { | ||
TransactionPayment::query_call_info(call, len) | ||
} | ||
fn query_call_fee_details( | ||
call: RuntimeCall, | ||
len: u32, | ||
) -> pallet_transaction_payment::FeeDetails<Balance> { | ||
TransactionPayment::query_call_fee_details(call, len) | ||
} | ||
fn query_weight_to_fee(weight: Weight) -> Balance { | ||
TransactionPayment::weight_to_fee(weight) | ||
} | ||
fn query_length_to_fee(length: u32) -> Balance { | ||
TransactionPayment::length_to_fee(length) | ||
} | ||
} | ||
|
||
impl cumulus_primitives_core::CollectCollationInfo<Block> for Runtime { | ||
fn collect_collation_info(header: &<Block as BlockT>::Header) -> cumulus_primitives_core::CollationInfo { | ||
ParachainSystem::collect_collation_info(header) | ||
} | ||
} | ||
|
||
#[cfg(feature = "try-runtime")] | ||
impl frame_try_runtime::TryRuntime<Block> for Runtime { | ||
fn on_runtime_upgrade(checks: frame_try_runtime::UpgradeCheckSelect) -> (Weight, Weight) { | ||
use super::RuntimeBlockWeights; | ||
|
||
let weight = Executive::try_runtime_upgrade(checks).unwrap(); | ||
(weight, RuntimeBlockWeights::get().max_block) | ||
} | ||
|
||
fn execute_block( | ||
block: Block, | ||
state_root_check: bool, | ||
signature_check: bool, | ||
select: frame_try_runtime::TryStateSelect, | ||
) -> Weight { | ||
// NOTE: intentional unwrap: we don't want to propagate the error backwards, and want to | ||
// have a backtrace here. | ||
Executive::try_execute_block(block, state_root_check, signature_check, select).unwrap() | ||
} | ||
} | ||
|
||
#[cfg(feature = "runtime-benchmarks")] | ||
impl frame_benchmarking::Benchmark<Block> for Runtime { | ||
fn benchmark_metadata(extra: bool) -> ( | ||
Vec<frame_benchmarking::BenchmarkList>, | ||
Vec<frame_support::traits::StorageInfo>, | ||
) { | ||
use frame_benchmarking::{Benchmarking, BenchmarkList}; | ||
use frame_support::traits::StorageInfoTrait; | ||
use frame_system_benchmarking::Pallet as SystemBench; | ||
use cumulus_pallet_session_benchmarking::Pallet as SessionBench; | ||
use super::*; | ||
|
||
let mut list = Vec::<BenchmarkList>::new(); | ||
list_benchmarks!(list, extra); | ||
|
||
let storage_info = AllPalletsWithSystem::storage_info(); | ||
(list, storage_info) | ||
} | ||
|
||
fn dispatch_benchmark( | ||
config: frame_benchmarking::BenchmarkConfig | ||
) -> Result<Vec<frame_benchmarking::BenchmarkBatch>, sp_runtime::RuntimeString> { | ||
use frame_benchmarking::{BenchmarkError, Benchmarking, BenchmarkBatch}; | ||
use super::*; | ||
|
||
use frame_system_benchmarking::Pallet as SystemBench; | ||
impl frame_system_benchmarking::Config for Runtime { | ||
fn setup_set_code_requirements(code: &sp_std::vec::Vec<u8>) -> Result<(), BenchmarkError> { | ||
ParachainSystem::initialize_for_set_code_benchmark(code.len() as u32); | ||
Ok(()) | ||
} | ||
|
||
fn verify_set_code() { | ||
System::assert_last_event(cumulus_pallet_parachain_system::Event::<Runtime>::ValidationFunctionStored.into()); | ||
} | ||
} | ||
|
||
use cumulus_pallet_session_benchmarking::Pallet as SessionBench; | ||
impl cumulus_pallet_session_benchmarking::Config for Runtime {} | ||
|
||
use frame_support::traits::WhitelistedStorageKeys; | ||
let whitelist = AllPalletsWithSystem::whitelisted_storage_keys(); | ||
|
||
let mut batches = Vec::<BenchmarkBatch>::new(); | ||
let params = (&config, &whitelist); | ||
add_benchmarks!(params, batches); | ||
|
||
if batches.is_empty() { return Err("Benchmark not found for this pallet.".into()) } | ||
Ok(batches) | ||
} | ||
} | ||
|
||
impl sp_genesis_builder::GenesisBuilder<Block> for Runtime { | ||
fn create_default_config() -> Vec<u8> { | ||
create_default_config::<RuntimeGenesisConfig>() | ||
} | ||
|
||
fn build_config(config: Vec<u8>) -> sp_genesis_builder::Result { | ||
build_config::<RuntimeGenesisConfig>(config) | ||
} | ||
} | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This refactoring also reveals a potential issue with the visibility of the
RUNTIME_API_VERSIONS
constant. It necessitates placing theVERSION: RuntimeVersion
definition on the same level as theimpl_runtime_apis
macro call.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I guess making that thing
pub
is valid. But that is a one-line change...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line change looks useful. I'm also wondering how to separate
api!
fromlib.rs
. because I don't like a single file too big (we have more than 2k lines...)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, while the change to make the
RUNTIME_API_VERSIONS
constant public might seem minor, it aligns with a broader goal.The presence of a "god file" exceeding 700 lines in Substrate's parachain template node, leading to runtimes spanning over 2000 lines throughout the ecosystem, highlights a structural inefficiency.
My proposed refactoring, although not functionally essential, is designed with the parachain template node's users in mind, especially considering the template's role as both an example and a starting point for new parachains.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The proposed refactor beneficially exposes SDK challenges that obstruct the creation of organized runtimes.