Rejig block selection to properly respect --at

core/src/commands/execute_block.rs

@@ -19,15 +19,14 @@ use std::{fmt::Debug, str::FromStr};
 
 use parity_scale_codec::Encode;
 use sc_executor::sp_wasm_interface::HostFunctions;
-use sp_rpc::{list::ListOrValue, number::NumberOrHex};
 use sp_runtime::{
     generic::SignedBlock,
     traits::{Block as BlockT, Header as HeaderT, NumberFor},
 };
 use substrate_rpc_client::{ws_client, ChainApi};
 
 use crate::{
-    build_executor, full_extensions, rpc_err_handler,
+    build_executor, full_extensions, hash_of, rpc_err_handler,
     state::{LiveState, SpecVersionCheck, State, TryRuntimeFeatureCheck},
     state_machine_call_with_proof, SharedParams, LOG_TARGET,
 };
@@ -95,8 +94,27 @@ where
     HostFns: HostFunctions,
 {
     let executor = build_executor::<HostFns>(&shared);
-    let ext = command
-        .state
+    let block_ws_uri = command.block_ws_uri();
+    let rpc = ws_client(&block_ws_uri).await?;
+
+    let live_state = match command.state {
+        State::Live(live_state) => live_state,
+        _ => {
+            unreachable!("execute block currently only supports Live state")
+        }
+    };
+
+    // The block we want to *execute* at is the block passed by the user
+    let execute_at = live_state
+        .at
+        .clone()
+        .map(|s| hash_of::<Block>(s.as_str()))
+        .transpose()?;
+
+    let prev_block_live_state = live_state.to_prev_block_live_state::<Block>().await?;
+
+    // Get state for the prev block
+    let ext = State::Live(prev_block_live_state)
         .to_ext::<Block, HostFns>(
             &shared,
             &executor,
@@ -106,21 +124,13 @@ where
         )
         .await?;
 
-    // get the block number associated with this block.
-    let block_ws_uri = command.block_ws_uri();
-    let rpc = ws_client(&block_ws_uri).await?;
-    let next_hash = next_hash_of::<Block>(&rpc, ext.block_hash).await?;
-
-    log::info!(target: LOG_TARGET, "fetching next block: {:?} ", next_hash);
-
-    let block = ChainApi::<(), Block::Hash, Block::Header, SignedBlock<Block>>::block(
-        &rpc,
-        Some(next_hash),
-    )
-    .await
-    .map_err(rpc_err_handler)?
-    .expect("header exists, block should also exist; qed")
-    .block;
+    // Execute the desired block on top of it
+    let block =
+        ChainApi::<(), Block::Hash, Block::Header, SignedBlock<Block>>::block(&rpc, execute_at)
+            .await
+            .map_err(rpc_err_handler)?
+            .expect("header exists, block should also exist; qed")
+            .block;
 
     // A digest item gets added when the runtime is processing the block, so we need to pop
     // the last one to be consistent with what a gossiped block would contain.
@@ -150,35 +160,3 @@ where
 
     Ok(())
 }
-
-pub(crate) async fn next_hash_of<Block: BlockT>(
-    rpc: &substrate_rpc_client::WsClient,
-    hash: Block::Hash,
-) -> sc_cli::Result<Block::Hash>
-where
-    Block: BlockT + serde::de::DeserializeOwned,
-    Block::Header: serde::de::DeserializeOwned,
-{
-    let number = ChainApi::<(), Block::Hash, Block::Header, ()>::header(rpc, Some(hash))
-        .await
-        .map_err(rpc_err_handler)
-        .and_then(|maybe_header| maybe_header.ok_or("header_not_found").map(|h| *h.number()))?;
-
-    let next = number + sp_runtime::traits::One::one();
-
-    let next_hash = match ChainApi::<(), Block::Hash, Block::Header, ()>::block_hash(
-        rpc,
-        Some(ListOrValue::Value(NumberOrHex::Number(
-            next.try_into()
-                .map_err(|_| "failed to convert number to block number")?,
-        ))),
-    )
-    .await
-    .map_err(rpc_err_handler)?
-    {
-        ListOrValue::Value(t) => t.expect("value passed in; value comes out; qed"),
-        _ => unreachable!(),
-    };
-
-    Ok(next_hash)
-}

core/src/commands/offchain_worker.rs

@@ -23,9 +23,7 @@ use sp_runtime::traits::{Block as BlockT, NumberFor};
 use substrate_rpc_client::{ws_client, ChainApi};
 
 use crate::{
-    build_executor,
-    commands::execute_block::next_hash_of,
-    full_extensions, parse, rpc_err_handler,
+    build_executor, full_extensions, hash_of, parse, rpc_err_handler,
     state::{LiveState, SpecVersionCheck, State, TryRuntimeFeatureCheck},
     state_machine_call, SharedParams, LOG_TARGET,
 };
@@ -74,10 +72,28 @@ where
     <NumberFor<Block> as FromStr>::Err: Debug,
     HostFns: HostFunctions,
 {
-    let executor = build_executor(&shared);
-    // we first build the externalities with the remote code.
-    let ext = command
-        .state
+    let executor = build_executor::<HostFns>(&shared);
+    let block_ws_uri = command.header_ws_uri();
+    let rpc = ws_client(&block_ws_uri).await?;
+
+    let live_state = match command.state {
+        State::Live(live_state) => live_state,
+        _ => {
+            unreachable!("execute block currently only supports Live state")
+        }
+    };
+
+    // The block we want to *execute* at is the block passed by the user
+    let execute_at = live_state
+        .at
+        .clone()
+        .map(|s| hash_of::<Block>(s.as_str()))
+        .transpose()?;
+
+    let prev_block_live_state = live_state.to_prev_block_live_state::<Block>().await?;
+
+    // Get state for the prev block
+    let ext = State::Live(prev_block_live_state)
         .to_ext::<Block, HostFns>(
             &shared,
             &executor,
@@ -87,13 +103,7 @@ where
         )
         .await?;
 
-    let header_ws_uri = command.header_ws_uri();
-
-    let rpc = ws_client(&header_ws_uri).await?;
-    let next_hash = next_hash_of::<Block>(&rpc, ext.block_hash).await?;
-    log::info!(target: LOG_TARGET, "fetching next header: {:?} ", next_hash);
-
-    let header = ChainApi::<(), Block::Hash, Block::Header, ()>::header(&rpc, Some(next_hash))
+    let header = ChainApi::<(), Block::Hash, Block::Header, ()>::header(&rpc, execute_at)
         .await
         .map_err(rpc_err_handler)
         .map(|maybe_header| maybe_header.ok_or("Header does not exist"))??;

core/src/state.rs

@@ -23,17 +23,19 @@ use frame_remote_externalities::{
 use parity_scale_codec::Decode;
 use sc_cli::RuntimeVersion;
 use sc_executor::{sp_wasm_interface::HostFunctions, WasmExecutor};
-use sp_api::HashT;
+use sp_api::{HashT, HeaderT};
 use sp_core::{
     hexdisplay::HexDisplay, storage::well_known_keys, traits::ReadRuntimeVersion, twox_128,
 };
+use sp_rpc::{list::ListOrValue, number::NumberOrHex};
 use sp_runtime::{
-    traits::{BlakeTwo256, Block as BlockT},
+    traits::{BlakeTwo256, Block as BlockT, CheckedSub, One},
     DeserializeOwned,
 };
+use substrate_rpc_client::{ws_client, ChainApi};
 
 use crate::{
-    ensure_try_runtime, hash_of, parse,
+    ensure_try_runtime, hash_of, parse, rpc_err_handler,
     shared_parameters::{Runtime, SharedParams},
     LOG_TARGET,
 };
@@ -80,6 +82,58 @@ pub struct LiveState {
     pub child_tree: bool,
 }
 
+impl LiveState {
+    /// Converts this `LiveState` into a `LiveState` for the previous block.
+    ///
+    /// Useful for opertations like when you want to execute a block, but also need the state of the
+    /// block *before* it.
+    pub async fn to_prev_block_live_state<Block: BlockT>(self) -> sc_cli::Result<LiveState>
+    where
+        <Block::Hash as FromStr>::Err: Debug,
+    {
+        // We want to execute the block `at`, therefore need the state of the block *before* it.
+        let at = self
+            .at
+            .clone()
+            .map(|s| hash_of::<Block>(s.as_str()))
+            .transpose()?;
+
+        // Get the block number requested by the user, or the current block number if they
+        // didn't specify one.
+        let rpc = ws_client(&self.uri).await?;
+        let number = ChainApi::<(), Block::Hash, Block::Header, ()>::header(&rpc, at)
+            .await
+            .map_err(rpc_err_handler)
+            .and_then(|maybe_header| maybe_header.ok_or("header_not_found").map(|h| *h.number()))?;
+
+        // Get the previous number.
+        let prev_number = number
+            .checked_sub(&One::one())
+            .expect("cannot get block number below 0");
+
+        // Get the previous block hash
+        let previous_hash = match ChainApi::<(), Block::Hash, Block::Header, ()>::block_hash(
+            &rpc,
+            Some(ListOrValue::Value(NumberOrHex::Number(
+                prev_number
+                    .try_into()
+                    .map_err(|_| "failed to convert number to block number")?,
+            ))),
+        )
+        .await
+        .map_err(rpc_err_handler)?
+        {
+            ListOrValue::Value(t) => t.expect("tried to fetch a block that doesn't exist"),
+            _ => unreachable!(),
+        };
+
+        Ok(LiveState {
+            at: Some(hex::encode(previous_hash)),
+            ..self
+        })
+    }
+}
+
 /// The source of runtime *state* to use.
 #[derive(Debug, Clone, clap::Subcommand)]
 pub enum State {

core/tests/execute_block.rs

@@ -60,23 +60,28 @@ async fn execute_block_works() {
                 .unwrap()
         }
 
-        let block_number = 1;
+        let block_number = 3;
         let block_hash = common::block_hash(block_number, &ws_url).await.unwrap();
 
         // Try to execute the block.
         let mut block_execution = execute_block(&ws_url, block_hash);
 
         // The execute-block command is actually executing the next block.
-        let expected_output = format!(
-            r#".*Block #{} successfully executed"#,
-            block_number.saturating_add(1)
-        );
+        let expected_output = format!(r#".*Block #{} successfully executed"#, block_number);
         let re = Regex::new(expected_output.as_str()).unwrap();
         let matched =
             common::wait_for_stream_pattern_match(block_execution.stderr.take().unwrap(), re).await;
 
-        // Assert that the block-execution process has executed a block.
+        // Assert that the block-execution process has executed the expected block.
         assert!(matched.is_ok());
+
+        // Assert that the block-execution exited succesfully
+        assert!(block_execution
+            .wait_with_output()
+            .await
+            .unwrap()
+            .status
+            .success());
     })
     .await
 }