BlockId removal: runtime-api refactor (#13255)

* BlockId removal: refactor of runtime API

It changes the arguments of:
- `ApiExt` methods:  `has_api`, `has_api_with`, `api_version`
- `CallApiAt` method: `runtime_version_at`
from: `BlockId<Block>` to: `Block::Hash`

It also changes the first argument of all generated runtime API calls from: `BlockId<Block>` to: `Block::Hash`

This PR is part of BlockId::Number refactoring analysis (paritytech/substrate#11292)

* BlockId removal: refactor of runtime API - tests

- tests adjusted to new runtime API,
- some tests migrated from block number to block hash

* benchmarking-cli: BlockId(0) migrated to info().genesis_hash

`runtime_api.call()` now requires the block hash instead of BlockId::Number.
To access the genesis hash widely used in benchmarking engine the Client
was constrained to satisfy `sp_blockchain::HeaderBackend<Block>` trait
which provides `info().genesis_hash`.

* trivial: api.call(BlockId) -> api.call(Hash)

- Migrated all `runtime_api.calls` to use Hash
- Noteworthy (?):
-- `validate_transaction_blocking` in transaction pool,

* CallApiAtParams::at changed to Block::Hash

* missed doc updated

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <git@kchr.de>

* ".git/.scripts/commands/fmt/fmt.sh"

* BlockId removal: Benchmark::consumed_weight

Little refactor around `Benchmark::consumed_weight`: `BlockId` removed.

* at_hash renamed

* wrong merge fixed

* beefy worker: merged with master

* beefy: tests: missing block problem fixed

* Apply review suggestion

* fix

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: command-bot <>

substrate/primitives/api/test/tests/decl_and_impl.rs

@@ -18,11 +18,9 @@
 use sp_api::{
 	decl_runtime_apis, impl_runtime_apis, mock_impl_runtime_apis, ApiError, ApiExt, RuntimeApiInfo,
 };
-use sp_runtime::{
-	generic::BlockId,
-	traits::{Block as BlockT, GetNodeBlockType},
-};
-use substrate_test_runtime_client::runtime::Block;
+use sp_runtime::traits::{Block as BlockT, GetNodeBlockType};
+
+use substrate_test_runtime_client::runtime::{Block, Hash};
 
 /// The declaration of the `Runtime` type and the implementation of the `GetNodeBlockType`
 /// trait are done by the `construct_runtime!` macro in a real runtime.
@@ -119,13 +117,13 @@ mock_impl_runtime_apis! {
 		}
 
 		#[advanced]
-		fn same_name(_: &BlockId<Block>) -> Result<(), ApiError> {
+		fn same_name(_: <Block as BlockT>::Hash) -> Result<(), ApiError> {
 			Ok(().into())
 		}
 
 		#[advanced]
-		fn wild_card(at: &BlockId<Block>, _: u32) -> Result<(), ApiError> {
-			if let BlockId::Number(1337) = at {
+		fn wild_card(at: <Block as BlockT>::Hash, _: u32) -> Result<(), ApiError> {
+			if Hash::repeat_byte(0x0f) == at {
 				// yeah
 				Ok(().into())
 			} else {
@@ -150,19 +148,19 @@ type TestClient = substrate_test_runtime_client::client::Client<
 fn test_client_side_function_signature() {
 	let _test: fn(
 		&RuntimeApiImpl<Block, TestClient>,
-		&BlockId<Block>,
+		<Block as BlockT>::Hash,
 		u64,
 	) -> Result<(), ApiError> = RuntimeApiImpl::<Block, TestClient>::test;
 	let _something_with_block: fn(
 		&RuntimeApiImpl<Block, TestClient>,
-		&BlockId<Block>,
+		<Block as BlockT>::Hash,
 		Block,
 	) -> Result<Block, ApiError> = RuntimeApiImpl::<Block, TestClient>::something_with_block;
 
 	#[allow(deprecated)]
 	let _same_name_before_version_2: fn(
 		&RuntimeApiImpl<Block, TestClient>,
-		&BlockId<Block>,
+		<Block as BlockT>::Hash,
 	) -> Result<String, ApiError> = RuntimeApiImpl::<Block, TestClient>::same_name_before_version_2;
 }
 
@@ -204,8 +202,8 @@ fn check_runtime_api_versions() {
 fn mock_runtime_api_has_api() {
 	let mock = MockApi { block: None };
 
-	assert!(mock.has_api::<dyn ApiWithCustomVersion<Block>>(&BlockId::Number(0)).unwrap());
-	assert!(mock.has_api::<dyn Api<Block>>(&BlockId::Number(0)).unwrap());
+	assert!(mock.has_api::<dyn ApiWithCustomVersion<Block>>(Hash::default()).unwrap());
+	assert!(mock.has_api::<dyn Api<Block>>(Hash::default()).unwrap());
 }
 
 #[test]
@@ -214,17 +212,17 @@ fn mock_runtime_api_panics_on_calling_old_version() {
 	let mock = MockApi { block: None };
 
 	#[allow(deprecated)]
-	let _ = mock.same_name_before_version_2(&BlockId::Number(0));
+	let _ = mock.same_name_before_version_2(Hash::default());
 }
 
 #[test]
 fn mock_runtime_api_works_with_advanced() {
 	let mock = MockApi { block: None };
 
-	Api::<Block>::same_name(&mock, &BlockId::Number(0)).unwrap();
-	mock.wild_card(&BlockId::Number(1337), 1).unwrap();
+	Api::<Block>::same_name(&mock, Hash::default()).unwrap();
+	mock.wild_card(Hash::repeat_byte(0x0f), 1).unwrap();
 	assert_eq!(
 		"Test error".to_string(),
-		mock.wild_card(&BlockId::Number(1336), 1).unwrap_err().to_string(),
+		mock.wild_card(Hash::repeat_byte(0x01), 1).unwrap_err().to_string(),
 	);
 }

substrate/primitives/api/test/tests/runtime_calls.rs

@@ -36,9 +36,9 @@ use sp_consensus::SelectChain;
 fn calling_function_with_strat(strat: ExecutionStrategy) {
 	let client = TestClientBuilder::new().set_execution_strategy(strat).build();
 	let runtime_api = client.runtime_api();
-	let block_id = BlockId::Number(client.chain_info().best_number);
+	let best_hash = client.chain_info().best_hash;
 
-	assert_eq!(runtime_api.benchmark_add_one(&block_id, &1).unwrap(), 2);
+	assert_eq!(runtime_api.benchmark_add_one(best_hash, &1).unwrap(), 2);
 }
 
 #[test]
@@ -57,9 +57,9 @@ fn calling_native_runtime_signature_changed_function() {
 		.set_execution_strategy(ExecutionStrategy::NativeWhenPossible)
 		.build();
 	let runtime_api = client.runtime_api();
-	let block_id = BlockId::Number(client.chain_info().best_number);
+	let best_hash = client.chain_info().best_hash;
 
-	assert_eq!(runtime_api.function_signature_changed(&block_id).unwrap(), 1);
+	assert_eq!(runtime_api.function_signature_changed(best_hash).unwrap(), 1);
 }
 
 #[test]
@@ -68,10 +68,10 @@ fn calling_wasm_runtime_signature_changed_old_function() {
 		.set_execution_strategy(ExecutionStrategy::AlwaysWasm)
 		.build();
 	let runtime_api = client.runtime_api();
-	let block_id = BlockId::Number(client.chain_info().best_number);
+	let best_hash = client.chain_info().best_hash;
 
 	#[allow(deprecated)]
-	let res = runtime_api.function_signature_changed_before_version_2(&block_id).unwrap();
+	let res = runtime_api.function_signature_changed_before_version_2(best_hash).unwrap();
 	assert_eq!(&res, &[1, 2]);
 }
 
@@ -79,16 +79,16 @@ fn calling_wasm_runtime_signature_changed_old_function() {
 fn calling_with_both_strategy_and_fail_on_wasm_should_return_error() {
 	let client = TestClientBuilder::new().set_execution_strategy(ExecutionStrategy::Both).build();
 	let runtime_api = client.runtime_api();
-	let block_id = BlockId::Number(client.chain_info().best_number);
-	assert!(runtime_api.fail_on_wasm(&block_id).is_err());
+	let best_hash = client.chain_info().best_hash;
+	assert!(runtime_api.fail_on_wasm(best_hash).is_err());
 }
 
 #[test]
 fn calling_with_both_strategy_and_fail_on_native_should_work() {
 	let client = TestClientBuilder::new().set_execution_strategy(ExecutionStrategy::Both).build();
 	let runtime_api = client.runtime_api();
-	let block_id = BlockId::Number(client.chain_info().best_number);
-	assert_eq!(runtime_api.fail_on_native(&block_id).unwrap(), 1);
+	let best_hash = client.chain_info().best_hash;
+	assert_eq!(runtime_api.fail_on_native(best_hash).unwrap(), 1);
 }
 
 #[test]
@@ -97,8 +97,8 @@ fn calling_with_native_else_wasm_and_fail_on_wasm_should_work() {
 		.set_execution_strategy(ExecutionStrategy::NativeElseWasm)
 		.build();
 	let runtime_api = client.runtime_api();
-	let block_id = BlockId::Number(client.chain_info().best_number);
-	assert_eq!(runtime_api.fail_on_wasm(&block_id).unwrap(), 1);
+	let best_hash = client.chain_info().best_hash;
+	assert_eq!(runtime_api.fail_on_wasm(best_hash).unwrap(), 1);
 }
 
 #[test]
@@ -107,8 +107,8 @@ fn calling_with_native_else_wasm_and_fail_on_native_should_work() {
 		.set_execution_strategy(ExecutionStrategy::NativeElseWasm)
 		.build();
 	let runtime_api = client.runtime_api();
-	let block_id = BlockId::Number(client.chain_info().best_number);
-	assert_eq!(runtime_api.fail_on_native(&block_id).unwrap(), 1);
+	let best_hash = client.chain_info().best_hash;
+	assert_eq!(runtime_api.fail_on_native(best_hash).unwrap(), 1);
 }
 
 #[test]
@@ -117,18 +117,18 @@ fn use_trie_function() {
 		.set_execution_strategy(ExecutionStrategy::AlwaysWasm)
 		.build();
 	let runtime_api = client.runtime_api();
-	let block_id = BlockId::Number(client.chain_info().best_number);
-	assert_eq!(runtime_api.use_trie(&block_id).unwrap(), 2);
+	let best_hash = client.chain_info().best_hash;
+	assert_eq!(runtime_api.use_trie(best_hash).unwrap(), 2);
 }
 
 #[test]
 fn initialize_block_works() {
 	let client = TestClientBuilder::new().set_execution_strategy(ExecutionStrategy::Both).build();
 	let runtime_api = client.runtime_api();
-	let block_id = BlockId::Number(client.chain_info().best_number);
+	let best_hash = client.chain_info().best_hash;
 	runtime_api
 		.initialize_block(
-			&block_id,
+			best_hash,
 			&Header::new(
 				1,
 				Default::default(),
@@ -138,7 +138,7 @@ fn initialize_block_works() {
 			),
 		)
 		.unwrap();
-	assert_eq!(runtime_api.get_block_number(&block_id).unwrap(), 1);
+	assert_eq!(runtime_api.get_block_number(best_hash).unwrap(), 1);
 }
 
 #[test]
@@ -205,10 +205,10 @@ fn call_runtime_api_with_multiple_arguments() {
 	let client = TestClientBuilder::new().set_execution_strategy(ExecutionStrategy::Both).build();
 
 	let data = vec![1, 2, 4, 5, 6, 7, 8, 8, 10, 12];
-	let block_id = BlockId::Number(client.chain_info().best_number);
+	let best_hash = client.chain_info().best_hash;
 	client
 		.runtime_api()
-		.test_multiple_arguments(&block_id, data.clone(), data.clone(), data.len() as u32)
+		.test_multiple_arguments(best_hash, data.clone(), data.clone(), data.len() as u32)
 		.unwrap();
 }
 
@@ -225,8 +225,9 @@ fn disable_logging_works() {
 
 		let client = builder.build();
 		let runtime_api = client.runtime_api();
-		let block_id = BlockId::Number(0);
-		runtime_api.do_trace_log(&block_id).expect("Logging should not fail");
+		runtime_api
+			.do_trace_log(client.chain_info().genesis_hash)
+			.expect("Logging should not fail");
 		log::error!("Logging from native works");
 	} else {
 		let executable = std::env::current_exe().unwrap();

substrate/primitives/api/test/tests/ui/mock_advanced_hash_by_reference.rs

@@ -12,7 +12,7 @@ struct MockApi;
 sp_api::mock_impl_runtime_apis! {
 	impl Api<Block> for MockApi {
 		#[advanced]
-		fn test(&self, _: BlockId<Block>) -> Result<(), ApiError> {
+		fn test(&self, _: &Hash) -> Result<(), ApiError> {
 			Ok(().into())
 		}
 	}

substrate/primitives/api/test/tests/ui/mock_advanced_hash_by_reference.stderr

@@ -1,10 +1,10 @@
-error: `BlockId` needs to be taken by reference and not by value!
-  --> tests/ui/mock_advanced_block_id_by_value.rs:12:1
+error: `Hash` needs to be taken by value and not by reference!
+  --> tests/ui/mock_advanced_hash_by_reference.rs:12:1
    |
 12 | / sp_api::mock_impl_runtime_apis! {
 13 | |     impl Api<Block> for MockApi {
 14 | |         #[advanced]
-15 | |         fn test(&self, _: BlockId<Block>) -> Result<(), ApiError> {
+15 | |         fn test(&self, _: &Hash) -> Result<(), ApiError> {
 ...  |
 18 | |     }
 19 | | }

substrate/primitives/api/test/tests/ui/mock_advanced_missing_hash.stderr

@@ -1,5 +1,5 @@
-error: If using the `advanced` attribute, it is required that the function takes at least one argument, the `BlockId`.
-  --> tests/ui/mock_advanced_missing_blockid.rs:15:3
+error: If using the `advanced` attribute, it is required that the function takes at least one argument, the `Hash`.
+  --> tests/ui/mock_advanced_missing_hash.rs:15:3
    |
 15 |         fn test(&self) -> Result<(), ApiError> {
    |         ^^