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

Fix the runtime-version choice. #11209

Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
52 changes: 38 additions & 14 deletions client/service/src/client/call_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,26 @@ where

Ok(code)
}

fn state_with_runtime_code(
&self,
block_id: BlockId<Block>,
) -> sp_blockchain::Result<<B as backend::Backend<Block>>::State> {
Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed earlier I think here we should fetch the header first (either by block number or block hash, depending on the block id that's passed), and then we fetch the parent hash from it. The assumption we make is that the caller of this method will make the right decision on whether to use a block number or a hash.

use sp_runtime::traits::{One, Zero};

let block_num = self
.backend
.blockchain()
.block_number_from_id(&block_id)?
.ok_or_else(|| sp_blockchain::Error::UnknownBlock(block_id.to_string()))?;

let block_num_code = if block_num.is_zero() { block_num } else { block_num - One::one() };
let block_id_code = BlockId::<Block>::Number(block_num_code);

let state_code = self.backend.state_at(block_id_code)?;

Ok(state_code)
}
}

impl<Block: BlockT, B, E> Clone for LocalCallExecutor<Block, B, E>
Expand Down Expand Up @@ -151,19 +171,20 @@ where
extensions: Option<Extensions>,
) -> sp_blockchain::Result<Vec<u8>> {
let mut changes = OverlayedChanges::default();
let state = self.backend.state_at(*at)?;
let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state);
let state_data = self.backend.state_at(*at)?;
let state_code = self.state_with_runtime_code(*at)?;

let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state_code);
Comment on lines +174 to +177
Copy link
Contributor

Choose a reason for hiding this comment

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

This executor will be used in both the context of general runtime calls as well as block construction (correct me if I'm wrong here @bkchr). We will need to somehow make that distinction because in the case of block construction we don't want to use the logic of picking the code from the parent, otherwise we'd end up in a situation where: we want to build block N, therefore we use the state from N-1 and the code from N-2. In the case of block construction we basically want to keep the logic as is, i.e. to create block N we already use the code from block N-1 (since N doesn't even exist in the first place).

let runtime_code =
state_runtime_code.runtime_code().map_err(sp_blockchain::Error::RuntimeCode)?;

let runtime_code = self.check_override(runtime_code, at)?;

let at_hash = self.backend.blockchain().block_hash_from_id(at)?.ok_or_else(|| {
sp_blockchain::Error::UnknownBlock(format!("Could not find block hash for {:?}", at))
})?;

let return_data = StateMachine::new(
&state,
&state_data,
&mut changes,
&self.executor,
method,
Expand Down Expand Up @@ -205,7 +226,8 @@ where
{
let mut storage_transaction_cache = storage_transaction_cache.map(|c| c.borrow_mut());

let state = self.backend.state_at(*at)?;
let state_data = self.backend.state_at(*at)?;
let state_code = self.state_with_runtime_code(*at)?;

let changes = &mut *changes.borrow_mut();

Expand All @@ -216,15 +238,15 @@ where
// It is important to extract the runtime code here before we create the proof
// recorder to not record it. We also need to fetch the runtime code from `state` to
// make sure we use the caching layers.
let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state);
let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state_code);

let runtime_code =
state_runtime_code.runtime_code().map_err(sp_blockchain::Error::RuntimeCode)?;
let runtime_code = self.check_override(runtime_code, at)?;

match recorder {
Some(recorder) => {
let trie_state = state.as_trie_backend().ok_or_else(|| {
let trie_state = state_data.as_trie_backend().ok_or_else(|| {
Box::new(sp_state_machine::ExecutionError::UnableToGenerateProof)
as Box<dyn sp_state_machine::Error>
})?;
Expand Down Expand Up @@ -253,7 +275,7 @@ where
},
None => {
let mut state_machine = StateMachine::new(
&state,
&state_data,
changes,
&self.executor,
method,
Expand All @@ -275,10 +297,11 @@ where

fn runtime_version(&self, id: &BlockId<Block>) -> sp_blockchain::Result<RuntimeVersion> {
let mut overlay = OverlayedChanges::default();
let state = self.backend.state_at(*id)?;
let state_data = self.backend.state_at(*id)?;
let state_code = self.state_with_runtime_code(*id)?;
let mut cache = StorageTransactionCache::<Block, B::State>::default();
let mut ext = Ext::new(&mut overlay, &mut cache, &state, None);
let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state);
let mut ext = Ext::new(&mut overlay, &mut cache, &state_data, None);
let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state_code);
let runtime_code =
state_runtime_code.runtime_code().map_err(sp_blockchain::Error::RuntimeCode)?;
self.executor
Expand All @@ -292,14 +315,15 @@ where
method: &str,
call_data: &[u8],
) -> sp_blockchain::Result<(Vec<u8>, StorageProof)> {
let state = self.backend.state_at(*at)?;
let state_data = self.backend.state_at(*at)?;
let state_code = self.state_with_runtime_code(*at)?;

let trie_backend = state.as_trie_backend().ok_or_else(|| {
let trie_backend = state_data.as_trie_backend().ok_or_else(|| {
Box::new(sp_state_machine::ExecutionError::UnableToGenerateProof)
as Box<dyn sp_state_machine::Error>
})?;

let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(trie_backend);
let state_runtime_code = sp_state_machine::backend::BackendRuntimeCode::new(&state_code);
let runtime_code =
state_runtime_code.runtime_code().map_err(sp_blockchain::Error::RuntimeCode)?;
let runtime_code = self.check_override(runtime_code, at)?;
Expand Down
16 changes: 15 additions & 1 deletion client/state-db/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,21 @@ impl<BlockHash: Hash + MallocSizeOf, Key: Hash + MallocSizeOf> StateDbSync<Block
(&mut self.pruning, &self.mode)
{
loop {
if pruning.window_size() <= constraints.max_blocks.unwrap_or(0) as u64 {
// `constraitns.max_blocks` is Some(N) — only the last N states should be available,
RGafiyatullin marked this conversation as resolved.
Show resolved Hide resolved
// the rest gets pruned.
//
// `constraints.max_mem` is Some(N) — the last states using no more that N bytes are
// to be kept, the rest gets pruned.
//
// `constraints.max_blocks` is None — do not enforce max-blocks.
//
// constraints.max_mem is none — do not enforce max-mem

if constraints
.max_blocks
.map(|max_blocks| pruning.window_size() <= max_blocks as u64)
.unwrap_or(true)
{
break
}

Expand Down
24 changes: 18 additions & 6 deletions primitives/state-machine/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,14 +302,17 @@ where

/// Wrapper to create a [`RuntimeCode`] from a type that implements [`Backend`].
#[cfg(feature = "std")]
pub struct BackendRuntimeCode<'a, B, H> {
backend: &'a B,
pub struct BackendRuntimeCode<B, H> {
backend: B,
_marker: std::marker::PhantomData<H>,
}

#[cfg(feature = "std")]
impl<'a, B: Backend<H>, H: Hasher> sp_core::traits::FetchRuntimeCode
for BackendRuntimeCode<'a, B, H>
impl<B, H> sp_core::traits::FetchRuntimeCode for BackendRuntimeCode<B, H>
where
B: std::ops::Deref,
B::Target: Backend<H>,
H: Hasher,
{
fn fetch_runtime_code<'b>(&'b self) -> Option<std::borrow::Cow<'b, [u8]>> {
self.backend
Expand All @@ -321,15 +324,24 @@ impl<'a, B: Backend<H>, H: Hasher> sp_core::traits::FetchRuntimeCode
}

#[cfg(feature = "std")]
impl<'a, B: Backend<H>, H: Hasher> BackendRuntimeCode<'a, B, H>
impl<B, H: Hasher> BackendRuntimeCode<B, H>
where
H::Out: Encode,
{
/// Create a new instance.
pub fn new(backend: &'a B) -> Self {
pub fn new(backend: B) -> Self {
Self { backend, _marker: std::marker::PhantomData }
}
}

#[cfg(feature = "std")]
impl<B, H> BackendRuntimeCode<B, H>
where
B: std::ops::Deref,
B::Target: Backend<H>,
H: Hasher,
H::Out: Encode,
{
/// Return the [`RuntimeCode`] build from the wrapped `backend`.
pub fn runtime_code(&self) -> Result<RuntimeCode, &'static str> {
let hash = self
Expand Down