Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Add after_inherents #14414

Draft
wants to merge 47 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
7f6a396
Add after_inherents
ggwpez Jun 19, 2023
03f6b4e
Fix template runtime
ggwpez Jun 19, 2023
6d04b2a
Fix test
ggwpez Jun 19, 2023
76fb1bc
Fix test
ggwpez Jun 19, 2023
2f62b6a
Move executive tests to own file
ggwpez Jun 19, 2023
5740d37
fmt
ggwpez Jun 19, 2023
911111d
Add test
ggwpez Jun 19, 2023
93d2171
Fix try-runtime
ggwpez Jun 19, 2023
b1b4b43
Merge branch 'master' into oty-after-inherents
ggwpez Jun 19, 2023
b12af3a
Fix test?
ggwpez Jun 19, 2023
cead998
Cleanup
ggwpez Jun 19, 2023
94e4658
Cleanup
ggwpez Jun 20, 2023
4ac155d
Consume weight instead of return
ggwpez Jun 21, 2023
627b57a
Review
ggwpez Jun 21, 2023
99d37ee
Check runtime API version
ggwpez Jun 21, 2023
3aca2c3
Cleanup
ggwpez Jun 21, 2023
85d6eb2
Merge remote-tracking branch 'origin/master' into oty-after-inherents
ggwpez Jun 21, 2023
0aa9112
Revert "Consume weight instead of return"
ggwpez Jun 23, 2023
f29b46f
Return ExecutiveMode from initialize_block
ggwpez Jul 4, 2023
e9b15c7
Fix tests
ggwpez Jul 4, 2023
975b8c1
Merge remote-tracking branch 'origin/master' into oty-after-inherents
ggwpez Jul 4, 2023
b9bfdd7
Cleanup
ggwpez Jul 4, 2023
ce2b00c
Fixup
ggwpez Jul 5, 2023
a54f5fd
Move initialize_block to the block builder
ggwpez Jul 5, 2023
a01f247
Fix test
ggwpez Jul 5, 2023
b50a78b
Fix test
ggwpez Jul 5, 2023
8f7751c
Fix imports
ggwpez Jul 5, 2023
baa9a39
Update UI tests
ggwpez Jul 5, 2023
2c6f764
Check runtime API
ggwpez Jul 5, 2023
706bcd9
Revert "Update UI tests"
ggwpez Jul 5, 2023
835941c
Fix doc tests
ggwpez Jul 5, 2023
708ede6
Fix UI tests
ggwpez Jul 6, 2023
b7869da
Also version initialize_block
ggwpez Jul 6, 2023
2ce81eb
Cleanup
ggwpez Jul 6, 2023
83bbe6f
Update client/block-builder/src/lib.rs
ggwpez Jul 11, 2023
66b1091
Review fixes
ggwpez Jul 11, 2023
1c1f71c
Remove unused dep
ggwpez Jul 11, 2023
0e589e0
Merge remote-tracking branch 'origin/master' into oty-after-inherents
ggwpez Jul 11, 2023
e9e60ef
Adapt to set_call_context
ggwpez Jul 11, 2023
a36bcc6
Review fixes and renames
ggwpez Jul 20, 2023
469a103
Merge remote-tracking branch 'origin/master' into oty-after-inherents
ggwpez Jul 20, 2023
da4c138
Review
ggwpez Jul 22, 2023
d04928c
Comment
ggwpez Jul 22, 2023
7369e42
Fix docs
ggwpez Jul 25, 2023
585373a
Remove after_inherents
ggwpez Aug 7, 2023
cb59e4f
Merge remote-tracking branch 'origin/master' into oty-after-inherents
ggwpez Aug 7, 2023
0929252
Fixup
ggwpez Aug 7, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion bin/node-template/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ impl_runtime_apis! {
Executive::execute_block(block);
}

fn initialize_block(header: &<Block as BlockT>::Header) {
fn initialize_block(header: &<Block as BlockT>::Header) -> sp_runtime::RuntimeExecutiveMode {
Executive::initialize_block(header)
}
}
Expand Down Expand Up @@ -392,6 +392,10 @@ impl_runtime_apis! {
) -> sp_inherents::CheckInherentsResult {
data.check_extrinsics(&block)
}

fn after_inherents(mode: sp_runtime::RuntimeExecutiveMode) {
Executive::after_inherents(mode)
}
}

impl sp_transaction_pool::runtime_api::TaggedTransactionQueue<Block> for Runtime {
Expand Down
6 changes: 5 additions & 1 deletion bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2085,7 +2085,7 @@ impl_runtime_apis! {
Executive::execute_block(block);
}

fn initialize_block(header: &<Block as BlockT>::Header) {
fn initialize_block(header: &<Block as BlockT>::Header) -> sp_runtime::RuntimeExecutiveMode {
Executive::initialize_block(header)
}
}
Expand Down Expand Up @@ -2120,6 +2120,10 @@ impl_runtime_apis! {
fn check_inherents(block: Block, data: InherentData) -> CheckInherentsResult {
data.check_extrinsics(&block)
}

fn after_inherents(mode: sp_runtime::RuntimeExecutiveMode) {
Executive::after_inherents(mode)
}
}

impl sp_transaction_pool::runtime_api::TaggedTransactionQueue<Block> for Runtime {
Expand Down
18 changes: 10 additions & 8 deletions client/basic-authorship/src/basic_authorship.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use sp_core::traits::SpawnNamed;
use sp_inherents::InherentData;
use sp_runtime::{
traits::{BlakeTwo256, Block as BlockT, Hash as HashT, Header as HeaderT},
Digest, Percent, SaturatedConversion,
Digest, Percent, RuntimeExecutiveMode, SaturatedConversion,
};
use std::{marker::PhantomData, pin::Pin, sync::Arc, time};

Expand Down Expand Up @@ -347,12 +347,14 @@ where

self.apply_inherents(&mut block_builder, inherent_data)?;

// TODO call `after_inherents` and check if we should apply extrinsincs here
// <https://github.com/paritytech/substrate/pull/14275/>

let block_timer = time::Instant::now();
let end_reason =
self.apply_extrinsics(&mut block_builder, deadline, block_size_limit).await?;
block_builder.after_inherents(block_builder.executive_mode)?;
let end_reason = match block_builder.executive_mode {
RuntimeExecutiveMode::Normal =>
self.apply_transactions(&mut block_builder, deadline, block_size_limit).await?,
RuntimeExecutiveMode::Minimal => EndProposingReason::TransactionsForbidden,
};

let (block, storage_changes, proof) = block_builder.build()?.into_inner();
let block_took = block_timer.elapsed();

Expand Down Expand Up @@ -401,8 +403,8 @@ where
Ok(())
}

/// Apply as many extrinsics as possible to the block.
async fn apply_extrinsics(
/// Apply as many transactions as possible to the block.
async fn apply_transactions(
&self,
block_builder: &mut sc_block_builder::BlockBuilder<'_, Block, C, B>,
deadline: time::Instant,
Expand Down
47 changes: 37 additions & 10 deletions client/block-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use sp_core::ExecutionContext;
use sp_runtime::{
legacy,
traits::{Block as BlockT, Hash, HashFor, Header as HeaderT, NumberFor, One},
Digest,
Digest, RuntimeExecutiveMode,
};

use sc_client_api::backend;
Expand Down Expand Up @@ -139,6 +139,8 @@ pub struct BlockBuilder<'a, Block: BlockT, A: ProvideRuntimeApi<Block>, B> {
backend: &'a B,
/// The estimated size of the block header.
estimated_header_size: usize,
/// The executive mode of the block that is currently being built.
pub executive_mode: RuntimeExecutiveMode,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub executive_mode: RuntimeExecutiveMode,
executive_mode: RuntimeExecutiveMode,

Copy link
Member Author

@ggwpez ggwpez Jul 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am actually using this in the authorship here

let end_reason = match block_builder.executive_mode {

Do you think it should rather be returned by the new function?

}

impl<'a, Block, A, B> BlockBuilder<'a, Block, A, B>
Expand Down Expand Up @@ -173,20 +175,29 @@ where
let estimated_header_size = header.encoded_size();

let mut api = api.runtime_api();
let version = api
.api_version::<dyn BlockBuilderApi<Block>>(parent_hash)?
.ok_or_else(|| Error::VersionInvalid("BlockBuilderApi".to_string()))?;

if record_proof.yes() {
api.record_proof();
}

api.initialize_block_with_context(
parent_hash,
ExecutionContext::BlockConstruction,
&header,
)?;

let version = api
.api_version::<dyn BlockBuilderApi<Block>>(parent_hash)?
.ok_or_else(|| Error::VersionInvalid("BlockBuilderApi".to_string()))?;
let executive_mode = if version >= 5 {
api.initialize_block_with_context(
parent_hash,
ExecutionContext::BlockConstruction,
&header,
)?
} else {
#[allow(deprecated)]
api.initialize_block_before_version_5_with_context(
parent_hash,
ExecutionContext::BlockConstruction,
&header,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

runtime_api.initialize_block(block_hash, &sp_runtime::traits::Header::new(
probably not that important anymore, but we also should do there the versioning.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay calling initialize_block_before_version_5 now since it is in a branch with version < 3.

)?;
RuntimeExecutiveMode::Normal
};

Ok(Self {
parent_hash,
Expand All @@ -195,9 +206,25 @@ where
version,
backend,
estimated_header_size,
executive_mode,
})
}

/// Called after inherents but before extrinsics have been applied.
pub fn after_inherents(&self, mode: RuntimeExecutiveMode) -> Result<(), Error> {
if self.version >= 7 {
self.api
.after_inherents_with_context(
self.parent_hash,
ExecutionContext::BlockConstruction,
mode,
)
.map_err(Into::into)
} else {
Ok(())
}
}

/// Push onto the block's list of extrinsics.
///
/// This will ensure the extrinsic can be validly executed (by executing it).
Expand Down
3 changes: 3 additions & 0 deletions client/proposer-metrics/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ pub enum EndProposingReason {
HitDeadline,
HitBlockSizeLimit,
HitBlockWeightLimit,
/// No transactions are allowed in the block.
TransactionsForbidden,
}

/// Authorship metrics.
Expand Down Expand Up @@ -112,6 +114,7 @@ impl Metrics {
EndProposingReason::NoMoreTransactions => "no_more_transactions",
EndProposingReason::HitBlockSizeLimit => "hit_block_size_limit",
EndProposingReason::HitBlockWeightLimit => "hit_block_weight_limit",
EndProposingReason::TransactionsForbidden => "transactions_forbidden",
};

self.end_proposing_reason.with_label_values(&[reason]).inc();
Expand Down
4 changes: 2 additions & 2 deletions client/rpc-spec-v2/src/chain_head/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,8 @@ async fn follow_with_runtime() {

// it is basically json-encoded substrate_test_runtime_client::runtime::VERSION
let runtime_str = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1,\
\"specVersion\":2,\"implVersion\":2,\"apis\":[[\"0xdf6acb689907609b\",4],\
[\"0x37e397fc7c91f5e4\",2],[\"0xd2bc9897eed08f15\",3],[\"0x40fe3ad401f8959a\",6],\
\"specVersion\":2,\"implVersion\":2,\"apis\":[[\"0xdf6acb689907609b\",5],\
[\"0x37e397fc7c91f5e4\",2],[\"0xd2bc9897eed08f15\",3],[\"0x40fe3ad401f8959a\",7],\
[\"0xbc9d89904f5b923f\",1],[\"0xc6e9a76309f39b09\",2],[\"0xdd718d5cc53262d4\",1],\
[\"0xcbca25e39f142387\",2],[\"0xf78b278be53f454c\",2],[\"0xab3c0572291feb8b\",1],\
[\"0xed99c5acb25eedf5\",3]],\"transactionVersion\":1,\"stateVersion\":1}";
Expand Down
4 changes: 2 additions & 2 deletions client/rpc/src/state/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,8 +514,8 @@ async fn should_return_runtime_version() {

// it is basically json-encoded substrate_test_runtime_client::runtime::VERSION
let result = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1,\
\"specVersion\":2,\"implVersion\":2,\"apis\":[[\"0xdf6acb689907609b\",4],\
[\"0x37e397fc7c91f5e4\",2],[\"0xd2bc9897eed08f15\",3],[\"0x40fe3ad401f8959a\",6],\
\"specVersion\":2,\"implVersion\":2,\"apis\":[[\"0xdf6acb689907609b\",5],\
[\"0x37e397fc7c91f5e4\",2],[\"0xd2bc9897eed08f15\",3],[\"0x40fe3ad401f8959a\",7],\
[\"0xbc9d89904f5b923f\",1],[\"0xc6e9a76309f39b09\",2],[\"0xdd718d5cc53262d4\",1],\
[\"0xcbca25e39f142387\",2],[\"0xf78b278be53f454c\",2],[\"0xab3c0572291feb8b\",1],\
[\"0xed99c5acb25eedf5\",3]],\"transactionVersion\":1,\"stateVersion\":1}";
Expand Down
1 change: 1 addition & 0 deletions client/transaction-pool/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ sc-client-api = { version = "4.0.0-dev", path = "../api" }
sc-transaction-pool-api = { version = "4.0.0-dev", path = "./api" }
sc-utils = { version = "4.0.0-dev", path = "../utils" }
sp-api = { version = "4.0.0-dev", path = "../../primitives/api" }
sp-block-builder = { version = "4.0.0-dev", path = "../../primitives/block-builder" }
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
sp-blockchain = { version = "4.0.0-dev", path = "../../primitives/blockchain" }
sp-core = { version = "21.0.0", path = "../../primitives/core" }
sp-runtime = { version = "24.0.0", path = "../../primitives/runtime" }
Expand Down
10 changes: 9 additions & 1 deletion frame/executive/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,12 @@ std = [
"sp-std/std",
"sp-tracing/std",
]
try-runtime = ["frame-support/try-runtime", "frame-try-runtime/try-runtime", "sp-runtime/try-runtime"]
try-runtime = [
"frame-support/try-runtime",
"frame-try-runtime/try-runtime",
"sp-runtime/try-runtime",
"frame-system/try-runtime",
"frame-try-runtime?/try-runtime",
"pallet-balances/try-runtime",
"pallet-transaction-payment/try-runtime"
]
Loading
Loading