From d1ba55f8adffafced35a288955dca0722da01cae Mon Sep 17 00:00:00 2001 From: Parth Desai Date: Sat, 9 Dec 2023 23:09:39 +0400 Subject: [PATCH] addressing feedback - backward incompatible apis + test simplification --- crates/sp-domains-fraud-proof/src/tests.rs | 807 +++--------------- domains/client/block-preprocessor/src/lib.rs | 30 +- .../src/stateless_runtime.rs | 21 +- .../src/domain_bundle_proposer.rs | 27 +- domains/client/domain-operator/src/tests.rs | 2 +- domains/primitives/runtime/src/lib.rs | 70 +- domains/runtime/evm/src/lib.rs | 152 +--- domains/test/runtime/evm/src/lib.rs | 147 +--- domains/test/service/src/domain.rs | 17 +- domains/test/service/src/lib.rs | 4 +- 10 files changed, 175 insertions(+), 1102 deletions(-) diff --git a/crates/sp-domains-fraud-proof/src/tests.rs b/crates/sp-domains-fraud-proof/src/tests.rs index 09be993ea2d..10e810f4332 100644 --- a/crates/sp-domains-fraud-proof/src/tests.rs +++ b/crates/sp-domains-fraud-proof/src/tests.rs @@ -1,91 +1,19 @@ -use codec::{Decode, Encode}; -use domain_block_preprocessor::stateless_runtime::StatelessRuntime; -use domain_runtime_primitives::{CheckTxValidityError, DomainCoreApi}; -use domain_test_service::domain::EvmDomainClient as DomainClient; -use domain_test_service::evm_domain_test_runtime::Runtime as TestRuntime; -use domain_test_service::EcdsaKeyring::{Alice, Bob, Charlie}; +use codec::Encode; +use domain_runtime_primitives::DomainCoreApi; +use domain_test_service::EcdsaKeyring::{Alice, Charlie}; use domain_test_service::Sr25519Keyring::Ferdie; -use domain_test_service::{construct_extrinsic_generic, EvmDomainNode, GENESIS_DOMAIN_ID}; +use domain_test_service::GENESIS_DOMAIN_ID; use fp_rpc::EthereumRuntimeRPCApi; -use sc_client_api::{HeaderBackend, ProofProvider, StorageProof}; +use sc_client_api::HeaderBackend; use sc_service::{BasePath, Role}; -use sp_api::{ApiExt, BlockT, ProvideRuntimeApi, TransactionOutcome}; -use sp_domains::DomainsApi; -use sp_runtime::generic::UncheckedExtrinsic; -use sp_runtime::traits::{BlakeTwo256, Header as HeaderT}; +use sp_api::{ApiExt, ProvideRuntimeApi, TransactionOutcome}; use sp_runtime::transaction_validity::{InvalidTransaction, TransactionValidityError}; -use sp_runtime::{OpaqueExtrinsic, Storage}; -use sp_trie::{read_trie_value, LayoutV1}; -use std::collections::BTreeMap; -use std::sync::Arc; -use subspace_runtime_primitives::opaque::Block; -use subspace_test_service::{produce_block_with, produce_blocks, MockConsensusNode}; +use sp_runtime::OpaqueExtrinsic; +use subspace_test_service::MockConsensusNode; use tempfile::TempDir; -type HashFor = <::Header as HeaderT>::Hash; - -fn generate_storage_proof_for_tx_validity( - block_hash: HashFor, - client: Arc, - keys: Vec>, -) -> StorageProof { - client - .read_proof( - block_hash, - &mut keys - .iter() - .map(|k| k.as_slice()) - .collect::>() - .into_iter(), - ) - .unwrap() -} - -fn get_storage_keys_for_verifying_signed_tx_validity( - domain_node: &EvmDomainNode, - extrinsic: UncheckedExtrinsic< - domain_test_service::evm_domain_test_runtime::Address, - ::RuntimeCall, - domain_test_service::evm_domain_test_runtime::Signature, - domain_test_service::evm_domain_test_runtime::SignedExtra, - >, -) -> Vec> { - let extrinsic_signer = domain_node - .client - .runtime_api() - .extract_signer( - domain_node.client.as_ref().info().best_hash, - vec![extrinsic.clone().into()], - ) - .unwrap() - .first() - .unwrap() - .0 - .clone(); - let extrinsic_era = domain_node - .client - .runtime_api() - .extrinsic_era( - domain_node.client.as_ref().info().best_hash, - &extrinsic.clone().into(), - ) - .unwrap(); - - domain_node - .client - .runtime_api() - .storage_keys_for_verifying_transaction_validity( - domain_node.client.as_ref().info().best_hash, - extrinsic_signer, - domain_node.client.as_ref().info().best_number, - extrinsic_era, - ) - .unwrap() - .unwrap() -} - #[tokio::test(flavor = "multi_thread")] -async fn runtime_instance_assumptions_are_correct() { +async fn runtime_instance_storage_assumptions_are_correct() { let directory = TempDir::new().expect("Must be able to create temporary directory"); let mut builder = sc_cli::LoggerBuilder::new(""); @@ -102,7 +30,7 @@ async fn runtime_instance_assumptions_are_correct() { ); // Run Alice (a evm domain authority node) - let alice = domain_test_service::DomainNodeBuilder::new( + let mut alice = domain_test_service::DomainNodeBuilder::new( tokio_handle.clone(), Alice, BasePath::new(directory.path().join("alice")), @@ -110,90 +38,50 @@ async fn runtime_instance_assumptions_are_correct() { .build_evm_node(Role::Authority, GENESIS_DOMAIN_ID, &mut ferdie) .await; + let best_hash = alice.client.as_ref().info().best_hash; + let best_number = alice.client.as_ref().info().best_number; + // transfer most of the alice's balance let alice_balance = alice .client .runtime_api() - .account_basic( - alice.client.as_ref().info().best_hash, - Alice.to_account_id().into(), - ) + .account_basic(best_hash, Alice.to_account_id().into()) .unwrap() .balance .as_u128(); - let target_balance = u32::MAX - 100; - let balance_to_transfer = alice_balance - target_balance as u128; - let alice_balance_transfer_extrinsic = construct_extrinsic_generic::( - &alice.client, - pallet_balances::Call::transfer_allow_death { - dest: Charlie.to_account_id(), - value: balance_to_transfer, - }, - alice.key, - false, - 0, - 1, - ); - - alice - .send_extrinsic(alice_balance_transfer_extrinsic) - .await - .expect("Failed to send extrinsic"); + let mut alice_nonce = alice.account_nonce(); - let (slot, bundle) = ferdie.produce_slot_and_wait_for_bundle_submission().await; - assert!(bundle.is_some()); - produce_block_with!(ferdie.produce_block_with_slot(slot), alice) - .await - .unwrap(); - produce_blocks!(ferdie, alice, 10).await.unwrap(); - - let mut alice_nonce = 1; - - let transfer_to_charlie_with_big_tip_1 = construct_extrinsic_generic::( - &alice.client, + let transfer_to_charlie_with_big_tip_1 = alice.construct_extrinsic_with_tip( + alice_nonce, + (alice_balance / 3) * 2, pallet_balances::Call::transfer_allow_death { dest: Charlie.to_account_id(), value: 1, }, - alice.key, - false, - alice_nonce, - 2000000000u32, ); alice_nonce += 1; - let transfer_to_charlie_with_big_tip_2 = construct_extrinsic_generic::( - &alice.client, + let transfer_to_charlie_with_big_tip_2 = alice.construct_extrinsic_with_tip( + alice_nonce, + (alice_balance / 3) * 2, pallet_balances::Call::transfer_allow_death { dest: Charlie.to_account_id(), value: 1, }, - alice.key, - false, - alice_nonce, - 2000000000u32, ); // A runtime instance preserve changes in between let runtime_instance = alice.client.runtime_api(); - let tx_validity_error = CheckTxValidityError::InvalidTransaction { - error: TransactionValidityError::Invalid(InvalidTransaction::Payment), - storage_keys: get_storage_keys_for_verifying_signed_tx_validity( - &alice, - transfer_to_charlie_with_big_tip_2.clone(), - ), - }; - // First transaction should be okay assert_eq!( runtime_instance .check_transaction_and_do_pre_dispatch( - alice.client.as_ref().info().best_hash, + best_hash, &transfer_to_charlie_with_big_tip_1.clone().into(), - alice.client.as_ref().info().best_number, - alice.client.as_ref().info().best_hash, + best_number, + best_hash, ) .unwrap(), Ok(()) @@ -204,70 +92,82 @@ async fn runtime_instance_assumptions_are_correct() { assert_eq!( runtime_instance .check_transaction_and_do_pre_dispatch( - alice.client.as_ref().info().best_hash, + best_hash, + &transfer_to_charlie_with_big_tip_2.clone().into(), + best_number, + best_hash, + ) + .unwrap(), + Err(TransactionValidityError::Invalid( + InvalidTransaction::Payment + )) + ); + + // If second tx is executed in dedicated runtime instance it would error with `InvalidTransaction::Future` + // as the nonce at the block for this account is less than the tx's nonce. And there is no overlay + // preserved between runtime instance. + assert_eq!( + alice + .client + .runtime_api() + .check_transaction_and_do_pre_dispatch( + best_hash, &transfer_to_charlie_with_big_tip_2.clone().into(), - alice.client.as_ref().info().best_number, - alice.client.as_ref().info().best_hash, + best_number, + best_hash, ) .unwrap(), - Err(CheckTxValidityError::decode(&mut tx_validity_error.encode().as_slice()).unwrap()) + Err(TransactionValidityError::Invalid( + InvalidTransaction::Future + )) ); let test_commit_mode = vec![true, false]; for commit_mode in test_commit_mode { - // A runtime instance can rollback changes safely. - let runtime_instance = alice.client.runtime_api(); + alice_nonce = alice.account_nonce(); - alice_nonce = 1; - - let transfer_with_big_tip_1 = construct_extrinsic_generic::( - &alice.client, + let transfer_with_big_tip_1 = alice.construct_extrinsic_with_tip( + alice_nonce, + alice_balance / 3, pallet_balances::Call::transfer_allow_death { dest: Charlie.to_account_id(), value: 1, }, - alice.key, - false, - alice_nonce, - 4000000000u32 / 3, ); alice_nonce += 1; - let transfer_with_big_tip_2 = construct_extrinsic_generic::( - &alice.client, + let transfer_with_big_tip_2 = alice.construct_extrinsic_with_tip( + alice_nonce, + alice_balance / 3, pallet_balances::Call::transfer_allow_death { dest: Charlie.to_account_id(), value: 1, }, - alice.key, - false, - alice_nonce, - 4000000000u32 / 3, ); if commit_mode { alice_nonce += 1; } - let transfer_with_big_tip_3 = construct_extrinsic_generic::( - &alice.client, + let transfer_with_big_tip_3 = alice.construct_extrinsic_with_tip( + alice_nonce, + alice_balance / 3, pallet_balances::Call::transfer_allow_death { dest: Charlie.to_account_id(), value: 1, }, - alice.key, - false, - alice_nonce, - 4000000000u32 / 3, ); + // A runtime instance can rollback changes safely. + let runtime_instance = alice.client.runtime_api(); + assert!(runtime_instance .check_transaction_and_do_pre_dispatch( - alice.client.as_ref().info().best_hash, + best_hash, &transfer_with_big_tip_1.clone().into(), - alice.client.as_ref().info().best_number, - alice.client.as_ref().info().best_hash + best_number, + best_hash ) .unwrap() .is_ok()); @@ -276,17 +176,17 @@ async fn runtime_instance_assumptions_are_correct() { .execute_in_transaction(|api| { if commit_mode { TransactionOutcome::Commit(api.check_transaction_and_do_pre_dispatch( - alice.client.as_ref().info().best_hash, + best_hash, &transfer_with_big_tip_2.clone().into(), - alice.client.as_ref().info().best_number, - alice.client.as_ref().info().best_hash, + best_number, + best_hash, )) } else { TransactionOutcome::Rollback(api.check_transaction_and_do_pre_dispatch( - alice.client.as_ref().info().best_hash, + best_hash, &transfer_with_big_tip_2.clone().into(), - alice.client.as_ref().info().best_number, - alice.client.as_ref().info().best_hash, + best_number, + best_hash, )) } }) @@ -295,17 +195,16 @@ async fn runtime_instance_assumptions_are_correct() { assert_eq!( runtime_instance .check_transaction_and_do_pre_dispatch( - alice.client.as_ref().info().best_hash, + best_hash, &transfer_with_big_tip_3.clone().into(), - alice.client.as_ref().info().best_number, - alice.client.as_ref().info().best_hash + best_number, + best_hash ) .unwrap(), if commit_mode { - Err( - CheckTxValidityError::decode(&mut tx_validity_error.encode().as_slice()) - .unwrap(), - ) + Err(TransactionValidityError::Invalid( + InvalidTransaction::Payment, + )) } else { Ok(()) } @@ -314,7 +213,7 @@ async fn runtime_instance_assumptions_are_correct() { } #[tokio::test(flavor = "multi_thread")] -async fn check_bundle_validity_runtime_api_should_work() { +async fn check_tx_validity_and_pre_dispatch_runtime_api_should_work() { let directory = TempDir::new().expect("Must be able to create temporary directory"); let mut builder = sc_cli::LoggerBuilder::new(""); @@ -331,7 +230,7 @@ async fn check_bundle_validity_runtime_api_should_work() { ); // Run Alice (a evm domain authority node) - let alice = domain_test_service::DomainNodeBuilder::new( + let mut alice = domain_test_service::DomainNodeBuilder::new( tokio_handle.clone(), Alice, BasePath::new(directory.path().join("alice")), @@ -339,42 +238,20 @@ async fn check_bundle_validity_runtime_api_should_work() { .build_evm_node(Role::Authority, GENESIS_DOMAIN_ID, &mut ferdie) .await; - let mut alice_nonce = 0; - let alice_balance = alice - .client - .runtime_api() - .account_basic( - alice.client.as_ref().info().best_hash, - Alice.to_account_id().into(), - ) - .unwrap() - .balance - .as_u128(); + let mut alice_nonce = alice.account_nonce(); + let best_hash = alice.client.as_ref().info().best_hash; + let best_number = alice.client.as_ref().info().best_number; + let dummy_runtime_call = pallet_balances::Call::transfer_allow_death { + dest: Charlie.to_account_id(), + value: 1, + }; - let transfer_to_charlie = construct_extrinsic_generic::( - &alice.client, - pallet_balances::Call::transfer_allow_death { - dest: Charlie.to_account_id(), - value: alice_balance, - }, - alice.key, - false, - alice_nonce, - 0, - ); + let transfer_to_charlie = + alice.construct_extrinsic_with_tip(alice_nonce, 0, dummy_runtime_call.clone()); alice_nonce += 1; - let transfer_to_charlie_2 = construct_extrinsic_generic::( - &alice.client, - pallet_balances::Call::transfer_allow_death { - dest: Charlie.to_account_id(), - value: 8, - }, - alice.key, - false, - alice_nonce, - 1, - ); + let transfer_to_charlie_2 = + alice.construct_extrinsic_with_tip(alice_nonce, 0, dummy_runtime_call.clone()); let sample_valid_bundle_extrinsics = vec![ OpaqueExtrinsic::from_bytes(&transfer_to_charlie.encode()).unwrap(), @@ -387,10 +264,10 @@ async fn check_bundle_validity_runtime_api_should_work() { assert_eq!( runtime_instance .check_transaction_and_do_pre_dispatch( - alice.client.as_ref().info().best_hash, + best_hash, &extrinsic, - alice.client.as_ref().info().best_number, - alice.client.as_ref().info().best_hash, + best_number, + best_hash, ) .unwrap(), Ok(()) @@ -399,17 +276,7 @@ async fn check_bundle_validity_runtime_api_should_work() { } let transfer_to_charlie_3_duplicate_nonce_extrinsic = - construct_extrinsic_generic::( - &alice.client, - pallet_balances::Call::transfer_allow_death { - dest: Charlie.to_account_id(), - value: 8, - }, - alice.key, - false, - alice_nonce, - 1, - ); + alice.construct_extrinsic_with_tip(alice_nonce, 0, dummy_runtime_call.clone()); // Here tx index: 2 is invalid let sample_invalid_bundle_with_same_nonce_extrinsic = vec![ @@ -429,10 +296,10 @@ async fn check_bundle_validity_runtime_api_should_work() { assert_eq!( runtime_instance .check_transaction_and_do_pre_dispatch( - alice.client.as_ref().info().best_hash, + best_hash, extrinsic, - alice.client.as_ref().info().best_number, - alice.client.as_ref().info().best_hash, + best_number, + best_hash, ) .unwrap(), Ok(()) @@ -441,89 +308,42 @@ async fn check_bundle_validity_runtime_api_should_work() { assert_eq!( runtime_instance .check_transaction_and_do_pre_dispatch( - alice.client.as_ref().info().best_hash, + best_hash, extrinsic, - alice.client.as_ref().info().best_number, - alice.client.as_ref().info().best_hash, + best_number, + best_hash, ) .unwrap(), - Err(CheckTxValidityError::InvalidTransaction { - error: TransactionValidityError::Invalid(InvalidTransaction::Stale), - storage_keys: get_storage_keys_for_verifying_signed_tx_validity( - &alice, - transfer_to_charlie_3_duplicate_nonce_extrinsic.clone() - ), - }) + Err(TransactionValidityError::Invalid(InvalidTransaction::Stale)) ); } } } + // Resetting the nonce + alice_nonce = alice.account_nonce(); + // transfer most of the alice's balance let alice_balance = alice .client .runtime_api() - .account_basic( - alice.client.as_ref().info().best_hash, - Alice.to_account_id().into(), - ) + .account_basic(best_hash, Alice.to_account_id().into()) .unwrap() .balance .as_u128(); - let target_balance = u32::MAX - 100; - let balance_to_transfer = alice_balance - target_balance as u128; - // reset alice nonce now - alice_nonce = 0; - - let alice_balance_transfer_extrinsic = construct_extrinsic_generic::( - &alice.client, - pallet_balances::Call::transfer_allow_death { - dest: Charlie.to_account_id(), - value: balance_to_transfer, - }, - alice.key, - false, + let transfer_to_charlie_with_big_tip_1 = alice.construct_extrinsic_with_tip( alice_nonce, - 1, + (alice_balance / 3) * 2, + dummy_runtime_call.clone(), ); - alice_nonce += 1; - alice - .send_extrinsic(alice_balance_transfer_extrinsic) - .await - .expect("Failed to send extrinsic"); - - let (slot, bundle) = ferdie.produce_slot_and_wait_for_bundle_submission().await; - assert!(bundle.is_some()); - produce_block_with!(ferdie.produce_block_with_slot(slot), alice) - .await - .unwrap(); - produce_blocks!(ferdie, alice, 10).await.unwrap(); - - let transfer_to_charlie_with_big_tip_1 = construct_extrinsic_generic::( - &alice.client, - pallet_balances::Call::transfer_allow_death { - dest: Charlie.to_account_id(), - value: 1, - }, - alice.key, - false, - alice_nonce, - 2000000000u32, - ); alice_nonce += 1; - let transfer_to_charlie_with_big_tip_2 = construct_extrinsic_generic::( - &alice.client, - pallet_balances::Call::transfer_allow_death { - dest: Charlie.to_account_id(), - value: 1, - }, - alice.key, - false, + let transfer_to_charlie_with_big_tip_2 = alice.construct_extrinsic_with_tip( alice_nonce, - 2000000000u32, + (alice_balance / 3) * 2, + dummy_runtime_call.clone(), ); // In single tx context both are valid (both tx are identical except nonce), so no need to test both of them independently. @@ -533,24 +353,16 @@ async fn check_bundle_validity_runtime_api_should_work() { .client .runtime_api() .check_transaction_and_do_pre_dispatch( - alice.client.as_ref().info().best_hash, + best_hash, &OpaqueExtrinsic::from_bytes(&transfer_to_charlie_with_big_tip_1.encode()).unwrap(), - alice.client.as_ref().info().best_number, - alice.client.as_ref().info().best_hash, + best_number, + best_hash, ) .unwrap(), Ok(()) ); // Tx index: 1 is invalid since low balance to pay for the tip - let tx_validity_error = CheckTxValidityError::InvalidTransaction { - error: TransactionValidityError::Invalid(InvalidTransaction::Payment), - storage_keys: get_storage_keys_for_verifying_signed_tx_validity( - &alice, - transfer_to_charlie_with_big_tip_2.clone(), - ), - }; - let transfer_to_charlie_with_big_tip = vec![ transfer_to_charlie_with_big_tip_1.clone().into(), transfer_to_charlie_with_big_tip_2.clone().into(), @@ -563,25 +375,24 @@ async fn check_bundle_validity_runtime_api_should_work() { assert_eq!( runtime_instance .check_transaction_and_do_pre_dispatch( - alice.client.as_ref().info().best_hash, + best_hash, extrinsic, - alice.client.as_ref().info().best_number, - alice.client.as_ref().info().best_hash, + best_number, + best_hash, ) .unwrap(), - Err( - CheckTxValidityError::decode(&mut tx_validity_error.encode().as_slice()) - .unwrap() - ) + Err(TransactionValidityError::Invalid( + InvalidTransaction::Payment + )) ); } else { assert_eq!( runtime_instance .check_transaction_and_do_pre_dispatch( - alice.client.as_ref().info().best_hash, + best_hash, extrinsic, - alice.client.as_ref().info().best_number, - alice.client.as_ref().info().best_hash, + best_number, + best_hash, ) .unwrap(), Ok(()) @@ -591,378 +402,6 @@ async fn check_bundle_validity_runtime_api_should_work() { } } -#[tokio::test(flavor = "multi_thread")] -async fn check_tx_validity_runtime_api_should_work() { - let directory = TempDir::new().expect("Must be able to create temporary directory"); - - let mut builder = sc_cli::LoggerBuilder::new(""); - builder.with_colors(false); - let _ = builder.init(); - - let tokio_handle = tokio::runtime::Handle::current(); - - // Start Ferdie - let mut ferdie = MockConsensusNode::run( - tokio_handle.clone(), - Ferdie, - BasePath::new(directory.path().join("ferdie")), - ); - - // Run Alice (a evm domain authority node) - let alice = domain_test_service::DomainNodeBuilder::new( - tokio_handle.clone(), - Alice, - BasePath::new(directory.path().join("alice")), - ) - .build_evm_node(Role::Authority, GENESIS_DOMAIN_ID, &mut ferdie) - .await; - - // Run Bob (a evm domain full node) - let bob = domain_test_service::DomainNodeBuilder::new( - tokio_handle, - Bob, - BasePath::new(directory.path().join("bob")), - ) - .build_evm_node(Role::Full, GENESIS_DOMAIN_ID, &mut ferdie) - .await; - - // Bob is able to sync blocks. - produce_blocks!(ferdie, alice, 1, bob).await.unwrap(); - - let bob_nonce = bob.account_nonce(); - let transfer_to_charlie = construct_extrinsic_generic::( - &bob.client, - pallet_balances::Call::transfer_allow_death { - dest: Charlie.to_account_id(), - value: 8, - }, - bob.key, - false, - bob_nonce + 1, - 1, - ); - - produce_blocks!(ferdie, alice, 2, bob).await.unwrap(); - - let opaque_extrinsic = OpaqueExtrinsic::from_bytes(&transfer_to_charlie.encode()).unwrap(); - let extrinsic_best_hash = bob.client.as_ref().info().best_hash; - let bob_account_id = Bob.to_account_id().encode(); - let extrinsic_best_number = bob.client.as_ref().info().best_number; - - let extrinsic_era = bob - .client - .runtime_api() - .extrinsic_era(bob.client.as_ref().info().best_hash, &opaque_extrinsic) - .unwrap(); - - let test_table = vec![ - ( - Some(4), - Some(TransactionValidityError::Invalid( - InvalidTransaction::AncientBirthBlock, - )), - ), - ( - Some(3), - Some(TransactionValidityError::Invalid( - InvalidTransaction::Payment, - )), - ), - ( - Some(1), - Some(TransactionValidityError::Invalid( - InvalidTransaction::BadProof, - )), - ), - ( - Some(0), - Some(TransactionValidityError::Invalid( - InvalidTransaction::BadProof, - )), - ), - (None, None), - ]; - - for table_data in test_table { - let mut storage_keys_for_tx_validity = bob - .client - .runtime_api() - .storage_keys_for_verifying_transaction_validity( - extrinsic_best_hash, - Some(bob_account_id.clone()), - extrinsic_best_number, - extrinsic_era, - ) - .unwrap() - .unwrap(); - - let maybe_storage_key_to_remove = table_data.0; - let original_storage_keys_for_tx_validity = storage_keys_for_tx_validity.clone(); - - if let Some(storage_key_to_remove) = maybe_storage_key_to_remove { - storage_keys_for_tx_validity = storage_keys_for_tx_validity - .drain(..) - .enumerate() - .filter(|(i, _v)| *i != storage_key_to_remove) - .map(|(_i, v)| v) - .collect(); - } - - let maybe_error = table_data.1; - - let expected_out = if let Some(err) = maybe_error { - let err_with_storage_keys = CheckTxValidityError::InvalidTransaction { - error: err, - storage_keys: original_storage_keys_for_tx_validity, - }; - Err(err_with_storage_keys) - } else { - Ok(()) - }; - - let storage_proof = generate_storage_proof_for_tx_validity( - bob.client.as_ref().info().best_hash, - bob.client.clone(), - storage_keys_for_tx_validity.clone(), - ); - - let header = bob - .client - .as_ref() - .header(bob.client.as_ref().info().best_hash); - let state_root = header.unwrap().unwrap().state_root; - - let wasm_bundle = ferdie - .client - .runtime_api() - .domain_runtime_code(ferdie.client.as_ref().info().best_hash, GENESIS_DOMAIN_ID) - .unwrap() - .unwrap(); - - let mut domain_stateless_runtime = - StatelessRuntime::::new(ferdie.executor.clone().into(), wasm_bundle.into()); - - let db = storage_proof.into_memory_db::(); - let mut top_storage_map = BTreeMap::new(); - for storage_key in storage_keys_for_tx_validity.iter() { - let storage_value = read_trie_value::, _>( - &db, - &state_root, - storage_key, - None, - None, - ) - .unwrap() - .unwrap(); - top_storage_map.insert(storage_key.to_vec(), storage_value); - } - - let storage = Storage { - top: top_storage_map, - children_default: Default::default(), - }; - - domain_stateless_runtime.set_storage(storage); - - assert_eq!( - domain_stateless_runtime - .check_transaction_validity( - &opaque_extrinsic, - bob.client.as_ref().info().best_number, - bob.client.as_ref().info().best_hash - ) - .unwrap(), - expected_out - ); - } -} - -#[tokio::test(flavor = "multi_thread")] -async fn check_tx_validity_and_do_pre_dispatch_runtime_api_should_work() { - let directory = TempDir::new().expect("Must be able to create temporary directory"); - - let mut builder = sc_cli::LoggerBuilder::new(""); - builder.with_colors(false); - let _ = builder.init(); - - let tokio_handle = tokio::runtime::Handle::current(); - - // Start Ferdie - let mut ferdie = MockConsensusNode::run( - tokio_handle.clone(), - Ferdie, - BasePath::new(directory.path().join("ferdie")), - ); - - // Run Alice (a evm domain authority node) - let alice = domain_test_service::DomainNodeBuilder::new( - tokio_handle.clone(), - Alice, - BasePath::new(directory.path().join("alice")), - ) - .build_evm_node(Role::Authority, GENESIS_DOMAIN_ID, &mut ferdie) - .await; - - // Run Bob (a evm domain full node) - let bob = domain_test_service::DomainNodeBuilder::new( - tokio_handle, - Bob, - BasePath::new(directory.path().join("bob")), - ) - .build_evm_node(Role::Full, GENESIS_DOMAIN_ID, &mut ferdie) - .await; - - // Bob is able to sync blocks. - produce_blocks!(ferdie, alice, 1, bob).await.unwrap(); - - let bob_nonce = bob.account_nonce(); - let transfer_to_charlie = construct_extrinsic_generic::( - &bob.client, - pallet_balances::Call::transfer_allow_death { - dest: Charlie.to_account_id(), - value: 8, - }, - bob.key, - false, - bob_nonce, - 1, - ); - - produce_blocks!(ferdie, alice, 2, bob).await.unwrap(); - - let opaque_extrinsic = OpaqueExtrinsic::from_bytes(&transfer_to_charlie.encode()).unwrap(); - let extrinsic_best_hash = bob.client.as_ref().info().best_hash; - let bob_account_id = Bob.to_account_id().encode(); - let extrinsic_best_number = bob.client.as_ref().info().best_number; - - let extrinsic_era = bob - .client - .runtime_api() - .extrinsic_era(bob.client.as_ref().info().best_hash, &opaque_extrinsic) - .unwrap(); - - let test_table = vec![ - ( - Some(4), - Some(TransactionValidityError::Invalid( - InvalidTransaction::AncientBirthBlock, - )), - ), - ( - Some(3), - Some(TransactionValidityError::Invalid( - InvalidTransaction::Payment, - )), - ), - ( - Some(1), - Some(TransactionValidityError::Invalid( - InvalidTransaction::BadProof, - )), - ), - ( - Some(0), - Some(TransactionValidityError::Invalid( - InvalidTransaction::BadProof, - )), - ), - (None, None), - ]; - - for table_data in test_table { - let mut storage_keys_for_tx_validity = bob - .client - .runtime_api() - .storage_keys_for_verifying_transaction_validity( - extrinsic_best_hash, - Some(bob_account_id.clone()), - extrinsic_best_number, - extrinsic_era, - ) - .unwrap() - .unwrap(); - - let maybe_storage_key_to_remove = table_data.0; - let original_storage_keys_for_tx_validity = storage_keys_for_tx_validity.clone(); - - if let Some(storage_key_to_remove) = maybe_storage_key_to_remove { - storage_keys_for_tx_validity = storage_keys_for_tx_validity - .drain(..) - .enumerate() - .filter(|(i, _v)| *i != storage_key_to_remove) - .map(|(_i, v)| v) - .collect(); - } - - let maybe_error = table_data.1; - - let expected_out = if let Some(err) = maybe_error { - let err_with_storage_keys = CheckTxValidityError::InvalidTransaction { - error: err, - storage_keys: original_storage_keys_for_tx_validity, - }; - Err(err_with_storage_keys) - } else { - Ok(()) - }; - - let storage_proof = generate_storage_proof_for_tx_validity( - bob.client.as_ref().info().best_hash, - bob.client.clone(), - storage_keys_for_tx_validity.clone(), - ); - - let header = bob - .client - .as_ref() - .header(bob.client.as_ref().info().best_hash); - let state_root = header.unwrap().unwrap().state_root; - - let wasm_bundle = ferdie - .client - .runtime_api() - .domain_runtime_code(ferdie.client.as_ref().info().best_hash, GENESIS_DOMAIN_ID) - .unwrap() - .unwrap(); - - let mut domain_stateless_runtime = - StatelessRuntime::::new(ferdie.executor.clone().into(), wasm_bundle.into()); - - let db = storage_proof.into_memory_db::(); - let mut top_storage_map = BTreeMap::new(); - for storage_key in storage_keys_for_tx_validity.iter() { - let storage_value = read_trie_value::, _>( - &db, - &state_root, - storage_key, - None, - None, - ) - .unwrap() - .unwrap(); - top_storage_map.insert(storage_key.to_vec(), storage_value); - } - - let storage = Storage { - top: top_storage_map, - children_default: Default::default(), - }; - - domain_stateless_runtime.set_storage(storage); - - assert_eq!( - domain_stateless_runtime - .check_transaction_and_do_pre_dispatch( - &opaque_extrinsic, - bob.client.as_ref().info().best_number, - bob.client.as_ref().info().best_hash - ) - .unwrap(), - expected_out - ); - } -} - // #[tokio::test(flavor = "multi_thread")] // #[ignore] // async fn execution_proof_creation_and_verification_should_work() { diff --git a/domains/client/block-preprocessor/src/lib.rs b/domains/client/block-preprocessor/src/lib.rs index 302d52ef53d..43618f3394e 100644 --- a/domains/client/block-preprocessor/src/lib.rs +++ b/domains/client/block-preprocessor/src/lib.rs @@ -20,7 +20,7 @@ use codec::{Decode, Encode}; use domain_runtime_primitives::opaque::AccountId; use domain_runtime_primitives::DomainCoreApi; use sc_client_api::BlockBackend; -use sp_api::{ApiExt, HashT, ProvideRuntimeApi}; +use sp_api::{HashT, ProvideRuntimeApi}; use sp_blockchain::HeaderBackend; use sp_core::H256; use sp_domains::extrinsics::deduplicate_and_shuffle_extrinsics; @@ -268,14 +268,6 @@ where // // NOTE: for each extrinsic the checking order must follow `InvalidBundleType::checking_order` let runtime_api = self.client.runtime_api(); - let api_version = runtime_api - .api_version::>(at) - .map_err(sp_blockchain::Error::RuntimeApiError)? - .ok_or_else(|| { - sp_blockchain::Error::Application( - format!("Could not find `DomainCoreApi` api for block `{:?}`.", at).into(), - ) - })?; for (index, opaque_extrinsic) in bundle.extrinsics.iter().enumerate() { let Ok(extrinsic) = <::Extrinsic>::decode(&mut opaque_extrinsic.encode().as_slice()) @@ -313,23 +305,9 @@ where // between them. // Using `check_transaction_and_do_pre_dispatch` instead of `check_transaction_validity` // to maintain side-effect in the storage buffer. - let is_legal_tx = if api_version == 2 { - runtime_api.check_transaction_and_do_pre_dispatch( - at, - &extrinsic, - domain_block_number, - at, - )? - } else { - #[allow(deprecated)] // old check_transaction_validity - runtime_api.check_transaction_validity_before_version_2( - at, - &extrinsic, - domain_block_number, - at, - )? - } - .is_ok(); + let is_legal_tx = runtime_api + .check_transaction_and_do_pre_dispatch(at, &extrinsic, domain_block_number, at)? + .is_ok(); if !is_legal_tx { // TODO: Generate a fraud proof for this invalid bundle diff --git a/domains/client/block-preprocessor/src/stateless_runtime.rs b/domains/client/block-preprocessor/src/stateless_runtime.rs index 1e5f2eed63d..9f02bbc0b34 100644 --- a/domains/client/block-preprocessor/src/stateless_runtime.rs +++ b/domains/client/block-preprocessor/src/stateless_runtime.rs @@ -1,12 +1,13 @@ use codec::{Codec, Encode}; use domain_runtime_primitives::opaque::AccountId; -use domain_runtime_primitives::{CheckTxValidityError, DomainCoreApi}; +use domain_runtime_primitives::DomainCoreApi; use sc_executor::RuntimeVersionOf; use sp_api::{ApiError, BlockT, Core, Hasher, RuntimeVersion}; use sp_core::traits::{CallContext, CodeExecutor, FetchRuntimeCode, RuntimeCode}; use sp_messenger::messages::ExtractedStateRootsFromProof; use sp_messenger::MessengerApi; use sp_runtime::traits::NumberFor; +use sp_runtime::transaction_validity::TransactionValidityError; use sp_runtime::Storage; use sp_state_machine::BasicExternalities; use std::borrow::Cow; @@ -212,29 +213,13 @@ where ) } - /// This is stateful runtime api call and require setting of storage keys. - pub fn check_transaction_validity( - &self, - uxt: &::Extrinsic, - block_number: NumberFor, - block_hash: ::Hash, - ) -> Result, ApiError> { - >::check_transaction_validity( - self, - Default::default(), - uxt, - block_number, - block_hash, - ) - } - /// This is stateful runtime api call and require setting of storage keys. pub fn check_transaction_and_do_pre_dispatch( &self, uxt: &::Extrinsic, block_number: NumberFor, block_hash: ::Hash, - ) -> Result, ApiError> { + ) -> Result, ApiError> { >::check_transaction_and_do_pre_dispatch( self, Default::default(), diff --git a/domains/client/domain-operator/src/domain_bundle_proposer.rs b/domains/client/domain-operator/src/domain_bundle_proposer.rs index 777ad89f22d..2de5f4f0945 100644 --- a/domains/client/domain-operator/src/domain_bundle_proposer.rs +++ b/domains/client/domain-operator/src/domain_bundle_proposer.rs @@ -115,18 +115,6 @@ where // We are using one runtime api instance here to maintain storage changes in the instance's internal buffer // between runtime calls done in this loop. let runtime_api_instance = self.client.runtime_api(); - let api_version = runtime_api_instance - .api_version::>(parent_hash) - .map_err(sp_blockchain::Error::RuntimeApiError)? - .ok_or_else(|| { - sp_blockchain::Error::Application( - format!( - "Could not find `DomainCoreApi` api for block `{:?}`.", - parent_hash - ) - .into(), - ) - })?; for pending_tx in pending_iterator { let pending_tx_data = pending_tx.data(); @@ -176,22 +164,13 @@ where // some other checks, its side effect will still be part of RuntimeApiImpl's changes buffer. let transaction_validity_result = runtime_api_instance.execute_in_transaction(|api| { - let transaction_validity_result = if api_version == 2 { - api.check_transaction_and_do_pre_dispatch( + let transaction_validity_result = api + .check_transaction_and_do_pre_dispatch( parent_hash, pending_tx_data, parent_number, parent_hash, - ) - } else { - #[allow(deprecated)] // old check_transaction_validity - api.check_transaction_validity_before_version_2( - parent_hash, - pending_tx_data, - parent_number, - parent_hash, - ) - }; + ); // Only commit, if there are no errors (both ApiError and CheckTxValidityError) if let Ok(Ok(_)) = transaction_validity_result { sp_api::TransactionOutcome::Commit(transaction_validity_result) diff --git a/domains/client/domain-operator/src/tests.rs b/domains/client/domain-operator/src/tests.rs index 41d97cd0c07..dc968d41e42 100644 --- a/domains/client/domain-operator/src/tests.rs +++ b/domains/client/domain-operator/src/tests.rs @@ -2769,7 +2769,7 @@ async fn test_domain_transaction_fee_and_operator_reward() { // Transaction fee (including the tip) is deducted from alice's account let alice_free_balance_changes = pre_alice_free_balance - alice.free_balance(Alice.to_account_id()); - assert!(alice_free_balance_changes >= tip as u128); + assert!(alice_free_balance_changes >= tip); // All the transaction fee is collected as operator reward assert_eq!(alice_free_balance_changes, receipt.total_rewards); diff --git a/domains/primitives/runtime/src/lib.rs b/domains/primitives/runtime/src/lib.rs index d7c007dab50..4d76358c20d 100644 --- a/domains/primitives/runtime/src/lib.rs +++ b/domains/primitives/runtime/src/lib.rs @@ -20,9 +20,7 @@ use parity_scale_codec::{Decode, Encode}; use scale_info::TypeInfo; use sp_runtime::generic::{Era, UncheckedExtrinsic}; -use sp_runtime::traits::{ - Block as BlockT, Convert, IdentifyAccount, LookupError, NumberFor, Verify, -}; +use sp_runtime::traits::{Block as BlockT, Convert, IdentifyAccount, NumberFor, Verify}; use sp_runtime::transaction_validity::TransactionValidityError; use sp_runtime::{Digest, MultiAddress, MultiSignature}; use sp_std::vec::Vec; @@ -136,36 +134,8 @@ pub mod opaque { pub type AccountId = Vec; } -#[derive(Debug, PartialEq, Eq, Encode, Decode, TypeInfo)] -pub enum CheckTxValidityError { - /// Can not find the sender from address. - Lookup, - /// Unable to extract signer from tx - UnableToExtractSigner { error: TransactionValidityError }, - /// Transaction is invalid. - InvalidTransaction { - /// Concrete transaction validity error type. - error: TransactionValidityError, - /// Storage keys of state accessed in the validation. - storage_keys: Vec>, - }, -} - -impl From for CheckTxValidityError { - fn from(_lookup_error: LookupError) -> Self { - Self::Lookup - } -} - -#[derive(Debug, PartialEq, Eq, Encode, Decode, TypeInfo)] -pub enum VerifyTxValidityError { - /// Failed to decode the opaque account id into the runtime account type. - FailedToDecodeAccountId, -} - sp_api::decl_runtime_apis! { /// Base API that every domain runtime must implement. - #[api_version(2)] pub trait DomainCoreApi { /// Extracts the optional signer per extrinsic. fn extract_signer( @@ -196,46 +166,12 @@ sp_api::decl_runtime_apis! { /// Returns true if the extrinsic is an inherent extrinsic. fn is_inherent_extrinsic(extrinsic: &::Extrinsic) -> bool; - /// Checks the validity of extrinsic. - #[changed_in(2)] - fn check_transaction_validity( - uxt: &::Extrinsic, - block_number: NumberFor, - block_hash: ::Hash - ) -> Result<(), CheckTxValidityError>; - - /// Checks the validity of extrinsic - /// v2 can check validity of unsigned and self contained extrinsics as well. - fn check_transaction_validity( - uxt: &::Extrinsic, - block_number: NumberFor, - block_hash: ::Hash - ) -> Result<(), CheckTxValidityError>; - - /// New in v2. - /// Checks transaction and also perform pre dispatch on transaction. - /// This is useful to maintain side effect in the RuntimeApi instance's storage buffer. + /// Checks the validity of extrinsic + do pre_dispatch as well. fn check_transaction_and_do_pre_dispatch( uxt: &::Extrinsic, block_number: NumberFor, block_hash: ::Hash - ) -> Result<(), CheckTxValidityError>; - - /// Returns the storage keys of states accessed in the API `check_transaction_validity`. - #[changed_in(2)] - fn storage_keys_for_verifying_transaction_validity( - account_id: opaque::AccountId, - block_number: NumberFor, - tx_era: Option, - ) -> Result>, VerifyTxValidityError>; - - /// Returns the storage keys of states accessed in the API `check_transaction_validity` - /// In v2, signer account id is optional to support getting storage keys for unsigned tx as well. - fn storage_keys_for_verifying_transaction_validity( - maybe_account_id: Option, - block_number: NumberFor, - tx_era: Option, - ) -> Result>, VerifyTxValidityError>; + ) -> Result<(), TransactionValidityError>; /// Returns extrinsic Era if present fn extrinsic_era( diff --git a/domains/runtime/evm/src/lib.rs b/domains/runtime/evm/src/lib.rs index 3c9c904f56a..779c2384b14 100644 --- a/domains/runtime/evm/src/lib.rs +++ b/domains/runtime/evm/src/lib.rs @@ -18,7 +18,7 @@ use frame_support::dispatch::{DispatchClass, GetDispatchInfo}; use frame_support::inherent::ProvideInherent; use frame_support::traits::{ ConstU16, ConstU32, ConstU64, Currency, Everything, FindAuthor, Imbalance, OnFinalize, - OnUnbalanced, PalletInfoAccess, + OnUnbalanced, }; use frame_support::weights::constants::{ BlockExecutionWeight, ExtrinsicBaseWeight, ParityDbWeight, WEIGHT_REF_TIME_PER_MILLIS, @@ -56,7 +56,7 @@ use sp_runtime::transaction_validity::{ use sp_runtime::{ create_runtime_str, generic, impl_opaque_keys, ApplyExtrinsicResult, ConsensusEngineId, Digest, }; -pub use sp_runtime::{MultiAddress, Perbill, Permill, SaturatedConversion}; +pub use sp_runtime::{MultiAddress, Perbill, Permill}; use sp_std::marker::PhantomData; use sp_std::prelude::*; #[cfg(feature = "std")] @@ -186,7 +186,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("subspace-evm-domain"), impl_name: create_runtime_str!("subspace-evm-domain"), authoring_version: 0, - spec_version: 1, + spec_version: 0, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 0, @@ -704,35 +704,6 @@ pub fn extract_signer( .collect() } -fn storage_keys_for_verifying_tx_validity_inner( - maybe_sender: Option, - block_number: BlockNumber, - maybe_tx_era: Option, -) -> Vec> { - let mut storage_keys = sp_std::vec![ - frame_system::BlockHash::::hashed_key_for(BlockNumber::from(0u32)), - frame_support::storage::storage_prefix(System::name().as_bytes(), b"Number").to_vec(), - pallet_transaction_payment::NextFeeMultiplier::::hashed_key().to_vec() - ]; - - if let Some(sender) = maybe_sender { - storage_keys.push(frame_system::Account::::hashed_key_for(sender)); - } - - match maybe_tx_era { - None => storage_keys, - Some(era) => { - let birth_number = era - .birth(block_number.saturated_into::()) - .saturated_into::(); - storage_keys.push(frame_system::BlockHash::::hashed_key_for( - birth_number, - )); - storage_keys - } - } -} - fn extrinsic_era(extrinsic: &::Extrinsic) -> Option { extrinsic .0 @@ -741,44 +712,6 @@ fn extrinsic_era(extrinsic: &::Extrinsic) -> Option { .map(|(_, _, extra)| extra.4 .0) } -fn check_transaction_validity_inner( - lookup: &Lookup, - uxt: &::Extrinsic, - block_number: BlockNumber, - block_hash: ::Hash, -) -> Result, domain_runtime_primitives::CheckTxValidityError> -where - Lookup: sp_runtime::traits::Lookup, -{ - let maybe_signer_info = extract_signer_inner(uxt, lookup); - let maybe_signer = if let Some(signer_info) = maybe_signer_info { - let signer = signer_info.map_err(|tx_validity_error| { - domain_runtime_primitives::CheckTxValidityError::UnableToExtractSigner { - error: tx_validity_error, - } - })?; - Some(signer) - } else { - None - }; - - let tx_validity = - Executive::validate_transaction(TransactionSource::External, uxt.clone(), block_hash); - - tx_validity - .map(|_| maybe_signer) - .map_err(|tx_validity_error| { - domain_runtime_primitives::CheckTxValidityError::InvalidTransaction { - error: tx_validity_error, - storage_keys: storage_keys_for_verifying_tx_validity_inner( - maybe_signer, - block_number, - extrinsic_era(uxt), - ), - } - }) -} - #[cfg(feature = "runtime-benchmarks")] mod benches { frame_benchmarking::define_benchmarks!( @@ -958,55 +891,26 @@ impl_runtime_apis! { } } - fn check_transaction_validity(uxt: &::Extrinsic, - block_number: BlockNumber, - block_hash: ::Hash) -> Result<(), domain_runtime_primitives::CheckTxValidityError> { - let lookup = frame_system::ChainContext::::default(); - check_transaction_validity_inner(&lookup, uxt, block_number, block_hash).map(|_| ()) - } - fn check_transaction_and_do_pre_dispatch( uxt: &::Extrinsic, block_number: BlockNumber, block_hash: ::Hash - ) -> Result<(), domain_runtime_primitives::CheckTxValidityError> { - let lookup = frame_system::ChainContext::::default(); - - let maybe_signer_info = extract_signer_inner(uxt, &lookup); - let maybe_signer = if let Some(signer_info) = maybe_signer_info { - let signer = signer_info.map_err(|tx_validity_error| { - domain_runtime_primitives::CheckTxValidityError::UnableToExtractSigner { - error: tx_validity_error, - } - })?; - Some(signer) - } else { - None - }; + ) -> Result<(), TransactionValidityError> { + let lookup = frame_system::ChainContext::::default(); // Initializing block related storage required for validation System::initialize( - &(System::block_number() + BlockNumber::one()), + &(block_number + BlockNumber::one()), &block_hash, &Default::default(), ); - let storage_keys = storage_keys_for_verifying_tx_validity_inner(maybe_signer, block_number, Self::extrinsic_era(uxt)); - - let xt = uxt.clone().check(&lookup).map_err(|tx_validity_error| { - domain_runtime_primitives::CheckTxValidityError::InvalidTransaction { - error: tx_validity_error, - storage_keys: storage_keys.clone(), - } - })?; + let xt = uxt.clone().check(&lookup)?; let dispatch_info = xt.get_dispatch_info(); if dispatch_info.class == DispatchClass::Mandatory { - return Err( domain_runtime_primitives::CheckTxValidityError::InvalidTransaction { - error: InvalidTransaction::BadMandatory.into(), - storage_keys: storage_keys.clone(), - }) + return Err(InvalidTransaction::BadMandatory.into()); } let encoded = uxt.encode(); @@ -1017,50 +921,20 @@ impl_runtime_apis! { // runtime instance. match xt.signed { CheckedSignature::Signed(account_id, extra) => { - extra.pre_dispatch(&account_id, &xt.function, &dispatch_info, encoded_len).map(|_| ()).map_err(|tx_validity_error| { - domain_runtime_primitives::CheckTxValidityError::InvalidTransaction { - error: tx_validity_error, - storage_keys, - } - }) + extra.pre_dispatch(&account_id, &xt.function, &dispatch_info, encoded_len).map(|_| ()) }, CheckedSignature::Unsigned => { - Runtime::pre_dispatch(&xt.function).map(|_| ()).map_err(|tx_validity_error| { - domain_runtime_primitives::CheckTxValidityError::InvalidTransaction { - error: tx_validity_error, - storage_keys: storage_keys.clone(), - } - })?; - SignedExtra::pre_dispatch_unsigned(&xt.function, &dispatch_info, encoded_len).map(|_| ()).map_err(|tx_validity_error| { - domain_runtime_primitives::CheckTxValidityError::InvalidTransaction { - error: tx_validity_error, - storage_keys, - } - }) + Runtime::pre_dispatch(&xt.function).map(|_| ())?; + SignedExtra::pre_dispatch_unsigned(&xt.function, &dispatch_info, encoded_len).map(|_| ()) }, CheckedSignature::SelfContained(self_contained_signing_info) => { xt.function.pre_dispatch_self_contained(&self_contained_signing_info, &dispatch_info, encoded_len).ok_or(TransactionValidityError::Invalid( - InvalidTransaction::BadProof, - )).map(|_| ()).map_err(|tx_validity_error| { - domain_runtime_primitives::CheckTxValidityError::InvalidTransaction { - error: tx_validity_error, - // TODO: Verify the storage keys here - storage_keys, - } - }) + InvalidTransaction::Call, + )).map(|_| ()) } } } - fn storage_keys_for_verifying_transaction_validity( - maybe_who: Option, - block_number: BlockNumber, - maybe_tx_era: Option - ) -> Result>, domain_runtime_primitives::VerifyTxValidityError> { - let maybe_sender = maybe_who.map(|who| AccountId::decode(&mut who.as_slice()).map_err(|_| domain_runtime_primitives::VerifyTxValidityError::FailedToDecodeAccountId)).transpose()?; - Ok(storage_keys_for_verifying_tx_validity_inner(maybe_sender, block_number, maybe_tx_era)) - } - fn extrinsic_era( extrinsic: &::Extrinsic ) -> Option { diff --git a/domains/test/runtime/evm/src/lib.rs b/domains/test/runtime/evm/src/lib.rs index 26f7c59aac6..e6cab20a52e 100644 --- a/domains/test/runtime/evm/src/lib.rs +++ b/domains/test/runtime/evm/src/lib.rs @@ -18,7 +18,7 @@ use frame_support::dispatch::{DispatchClass, GetDispatchInfo}; use frame_support::inherent::ProvideInherent; use frame_support::traits::{ ConstU16, ConstU32, ConstU64, Currency, Everything, FindAuthor, Imbalance, OnFinalize, - OnUnbalanced, PalletInfoAccess, + OnUnbalanced, }; use frame_support::weights::constants::{ BlockExecutionWeight, ExtrinsicBaseWeight, ParityDbWeight, WEIGHT_REF_TIME_PER_MILLIS, @@ -55,7 +55,6 @@ use sp_runtime::transaction_validity::{ }; use sp_runtime::{ create_runtime_str, generic, impl_opaque_keys, ApplyExtrinsicResult, ConsensusEngineId, Digest, - SaturatedConversion, }; pub use sp_runtime::{MultiAddress, Perbill, Permill}; use sp_std::marker::PhantomData; @@ -703,35 +702,6 @@ pub fn extract_signer( .collect() } -fn storage_keys_for_verifying_tx_validity_inner( - maybe_sender: Option, - block_number: BlockNumber, - maybe_tx_era: Option, -) -> Vec> { - let mut storage_keys = sp_std::vec![ - frame_system::BlockHash::::hashed_key_for(BlockNumber::from(0u32)), - frame_support::storage::storage_prefix(System::name().as_bytes(), b"Number").to_vec(), - pallet_transaction_payment::NextFeeMultiplier::::hashed_key().to_vec() - ]; - - if let Some(sender) = maybe_sender { - storage_keys.push(frame_system::Account::::hashed_key_for(sender)); - } - - match maybe_tx_era { - None => storage_keys, - Some(era) => { - let birth_number = era - .birth(block_number.saturated_into::()) - .saturated_into::(); - storage_keys.push(frame_system::BlockHash::::hashed_key_for( - birth_number, - )); - storage_keys - } - } -} - fn extrinsic_era(extrinsic: &::Extrinsic) -> Option { extrinsic .0 @@ -740,44 +710,6 @@ fn extrinsic_era(extrinsic: &::Extrinsic) -> Option { .map(|(_, _, extra)| extra.4 .0) } -fn check_transaction_validity_inner( - lookup: &Lookup, - uxt: &::Extrinsic, - block_number: BlockNumber, - block_hash: ::Hash, -) -> Result, domain_runtime_primitives::CheckTxValidityError> -where - Lookup: sp_runtime::traits::Lookup, -{ - let maybe_signer_info = extract_signer_inner(uxt, lookup); - let maybe_signer = if let Some(signer_info) = maybe_signer_info { - let signer = signer_info.map_err(|tx_validity_error| { - domain_runtime_primitives::CheckTxValidityError::UnableToExtractSigner { - error: tx_validity_error, - } - })?; - Some(signer) - } else { - None - }; - - let tx_validity = - Executive::validate_transaction(TransactionSource::External, uxt.clone(), block_hash); - - tx_validity - .map(|_| maybe_signer) - .map_err(|tx_validity_error| { - domain_runtime_primitives::CheckTxValidityError::InvalidTransaction { - error: tx_validity_error, - storage_keys: storage_keys_for_verifying_tx_validity_inner( - maybe_signer, - block_number, - extrinsic_era(uxt), - ), - } - }) -} - impl_runtime_apis! { impl sp_api::Core for Runtime { fn version() -> RuntimeVersion { @@ -946,55 +878,26 @@ impl_runtime_apis! { } } - fn check_transaction_validity(uxt: &::Extrinsic, - block_number: BlockNumber, - block_hash: ::Hash) -> Result<(), domain_runtime_primitives::CheckTxValidityError> { - let lookup = frame_system::ChainContext::::default(); - check_transaction_validity_inner(&lookup, uxt, block_number, block_hash).map(|_| ()) - } - fn check_transaction_and_do_pre_dispatch( uxt: &::Extrinsic, block_number: BlockNumber, block_hash: ::Hash - ) -> Result<(), domain_runtime_primitives::CheckTxValidityError> { + ) -> Result<(), TransactionValidityError> { let lookup = frame_system::ChainContext::::default(); - let maybe_signer_info = extract_signer_inner(uxt, &lookup); - let maybe_signer = if let Some(signer_info) = maybe_signer_info { - let signer = signer_info.map_err(|tx_validity_error| { - domain_runtime_primitives::CheckTxValidityError::UnableToExtractSigner { - error: tx_validity_error, - } - })?; - Some(signer) - } else { - None - }; - // Initializing block related storage required for validation System::initialize( - &(System::block_number() + BlockNumber::one()), + &(block_number + BlockNumber::one()), &block_hash, &Default::default(), ); - let storage_keys = storage_keys_for_verifying_tx_validity_inner(maybe_signer, block_number, Self::extrinsic_era(uxt)); - - let xt = uxt.clone().check(&lookup).map_err(|tx_validity_error| { - domain_runtime_primitives::CheckTxValidityError::InvalidTransaction { - error: tx_validity_error, - storage_keys: storage_keys.clone(), - } - })?; + let xt = uxt.clone().check(&lookup)?; let dispatch_info = xt.get_dispatch_info(); if dispatch_info.class == DispatchClass::Mandatory { - return Err( domain_runtime_primitives::CheckTxValidityError::InvalidTransaction { - error: InvalidTransaction::BadMandatory.into(), - storage_keys: storage_keys.clone(), - }) + return Err(InvalidTransaction::BadMandatory.into()); } let encoded = uxt.encode(); @@ -1005,50 +908,20 @@ impl_runtime_apis! { // runtime instance. match xt.signed { CheckedSignature::Signed(account_id, extra) => { - extra.pre_dispatch(&account_id, &xt.function, &dispatch_info, encoded_len).map(|_| ()).map_err(|tx_validity_error| { - domain_runtime_primitives::CheckTxValidityError::InvalidTransaction { - error: tx_validity_error, - storage_keys, - } - }) + extra.pre_dispatch(&account_id, &xt.function, &dispatch_info, encoded_len).map(|_| ()) }, CheckedSignature::Unsigned => { - Runtime::pre_dispatch(&xt.function).map(|_| ()).map_err(|tx_validity_error| { - domain_runtime_primitives::CheckTxValidityError::InvalidTransaction { - error: tx_validity_error, - storage_keys: storage_keys.clone(), - } - })?; - SignedExtra::pre_dispatch_unsigned(&xt.function, &dispatch_info, encoded_len).map(|_| ()).map_err(|tx_validity_error| { - domain_runtime_primitives::CheckTxValidityError::InvalidTransaction { - error: tx_validity_error, - storage_keys, - } - }) + Runtime::pre_dispatch(&xt.function).map(|_| ())?; + SignedExtra::pre_dispatch_unsigned(&xt.function, &dispatch_info, encoded_len).map(|_| ()) }, CheckedSignature::SelfContained(self_contained_signing_info) => { xt.function.pre_dispatch_self_contained(&self_contained_signing_info, &dispatch_info, encoded_len).ok_or(TransactionValidityError::Invalid( - InvalidTransaction::BadProof, - )).map(|_| ()).map_err(|tx_validity_error| { - domain_runtime_primitives::CheckTxValidityError::InvalidTransaction { - error: tx_validity_error, - // TODO: Verify the storage keys here - storage_keys, - } - }) + InvalidTransaction::Call, + )).map(|_| ()) } } } - fn storage_keys_for_verifying_transaction_validity( - maybe_who: Option, - block_number: BlockNumber, - maybe_tx_era: Option - ) -> Result>, domain_runtime_primitives::VerifyTxValidityError> { - let maybe_sender = maybe_who.map(|who| AccountId::decode(&mut who.as_slice()).map_err(|_| domain_runtime_primitives::VerifyTxValidityError::FailedToDecodeAccountId)).transpose()?; - Ok(storage_keys_for_verifying_tx_validity_inner(maybe_sender, block_number, maybe_tx_era)) - } - fn extrinsic_era( extrinsic: &::Extrinsic ) -> Option { diff --git a/domains/test/service/src/domain.rs b/domains/test/service/src/domain.rs index eed107a19df..d73f4c40ab3 100644 --- a/domains/test/service/src/domain.rs +++ b/domains/test/service/src/domain.rs @@ -2,7 +2,9 @@ #![warn(missing_docs)] use crate::chain_spec::create_domain_spec; -use crate::{construct_extrinsic_generic, node_config, EcdsaKeyring, UncheckedExtrinsicFor}; +use crate::{ + construct_extrinsic_generic, node_config, BalanceOf, EcdsaKeyring, UncheckedExtrinsicFor, +}; use domain_client_operator::{BootstrapResult, Bootstrapper, OperatorStreams}; use domain_runtime_primitives::opaque::Block; use domain_runtime_primitives::{Balance, DomainCoreApi}; @@ -338,7 +340,7 @@ where self.key, false, self.account_nonce(), - 0, + 0.into(), ); self.rpc_handlers.send_transaction(extrinsic.into()).await } @@ -349,14 +351,21 @@ where nonce: u32, function: impl Into<::RuntimeCall>, ) -> UncheckedExtrinsicFor { - construct_extrinsic_generic::(&self.client, function, self.key, false, nonce, 0) + construct_extrinsic_generic::( + &self.client, + function, + self.key, + false, + nonce, + 0.into(), + ) } /// Construct an extrinsic with the given transaction tip. pub fn construct_extrinsic_with_tip( &mut self, nonce: u32, - tip: u32, + tip: BalanceOf, function: impl Into<::RuntimeCall>, ) -> UncheckedExtrinsicFor { construct_extrinsic_generic::( diff --git a/domains/test/service/src/lib.rs b/domains/test/service/src/lib.rs index 249713bfffc..72a2e4d74b1 100644 --- a/domains/test/service/src/lib.rs +++ b/domains/test/service/src/lib.rs @@ -177,7 +177,7 @@ pub fn construct_extrinsic_generic( caller: EcdsaKeyring, immortal: bool, nonce: u32, - tip: u32, + tip: BalanceOf, ) -> UncheckedExtrinsicFor where Runtime: frame_system::Config + pallet_transaction_payment::Config + Send + Sync, @@ -207,7 +207,7 @@ where }), frame_system::CheckNonce::::from(nonce.into()), frame_system::CheckWeight::::new(), - pallet_transaction_payment::ChargeTransactionPayment::::from(tip.into()), + pallet_transaction_payment::ChargeTransactionPayment::::from(tip), ); let raw_payload = generic::SignedPayload::< ::RuntimeCall,