From 435becbe2963f68b0cb321ae436f23a34d402aa7 Mon Sep 17 00:00:00 2001 From: Marian Radu Date: Mon, 10 Nov 2025 17:13:13 +0200 Subject: [PATCH 01/12] Replace substrate_node with revive-dev-node. --- substrate/frame/revive/dev-node/node/src/command.rs | 8 ++++++-- substrate/frame/revive/dev-node/node/src/lib.rs | 1 + substrate/frame/revive/rpc/Cargo.toml | 2 +- substrate/frame/revive/rpc/src/tests.rs | 13 +++++-------- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/substrate/frame/revive/dev-node/node/src/command.rs b/substrate/frame/revive/dev-node/node/src/command.rs index 9dc20b4f85a2f..b38869a38eee8 100644 --- a/substrate/frame/revive/dev-node/node/src/command.rs +++ b/substrate/frame/revive/dev-node/node/src/command.rs @@ -56,9 +56,13 @@ impl SubstrateCli for Cli { } } -/// Parse and run command line arguments pub fn run() -> sc_cli::Result<()> { - let args = std::env::args_os().collect::>(); + let args = std::env::args_os().map(|s| s.to_string_lossy().to_string()).collect::>(); + return run_with_args(args); +} + +/// Parse and run command line arguments +pub fn run_with_args(args: Vec) -> sc_cli::Result<()> { let mut cli = Cli::from_iter(args); match &cli.subcommand { diff --git a/substrate/frame/revive/dev-node/node/src/lib.rs b/substrate/frame/revive/dev-node/node/src/lib.rs index c2065def736ae..b132f8e2fc547 100644 --- a/substrate/frame/revive/dev-node/node/src/lib.rs +++ b/substrate/frame/revive/dev-node/node/src/lib.rs @@ -17,5 +17,6 @@ pub mod chain_spec; pub(crate) mod cli; +pub mod command; pub mod rpc; pub mod service; diff --git a/substrate/frame/revive/rpc/Cargo.toml b/substrate/frame/revive/rpc/Cargo.toml index 60af7f08f8e7b..3fc16b9eca8d1 100644 --- a/substrate/frame/revive/rpc/Cargo.toml +++ b/substrate/frame/revive/rpc/Cargo.toml @@ -54,7 +54,7 @@ env_logger = { workspace = true } pallet-revive-fixtures = { workspace = true, default-features = true } pretty_assertions = { workspace = true } static_init = { workspace = true } -substrate-cli-test-utils = { workspace = true } +revive-dev-node = { workspace = true } [build-dependencies] git2 = { workspace = true } diff --git a/substrate/frame/revive/rpc/src/tests.rs b/substrate/frame/revive/rpc/src/tests.rs index e167f22f32289..9c62ebf6062af 100644 --- a/substrate/frame/revive/rpc/src/tests.rs +++ b/substrate/frame/revive/rpc/src/tests.rs @@ -39,7 +39,6 @@ use pallet_revive::{ }; use static_init::dynamic; use std::{collections::BTreeMap, sync::Arc, thread}; -use substrate_cli_test_utils::*; use subxt::{ backend::rpc::RpcClient, ext::subxt_rpcs::rpc_params, @@ -72,14 +71,12 @@ struct SharedResources { impl SharedResources { fn start() -> Self { - // Start the node. + // Start revive-dev-node let _node_handle = thread::spawn(move || { - if let Err(e) = start_node_inline(vec![ - "--dev", - "--rpc-port=45789", - "--no-telemetry", - "--no-prometheus", - "-lerror,evm=debug,sc_rpc_server=info,runtime::revive=trace", + if let Err(e) = revive_dev_node::command::run_with_args(vec![ + "--dev".to_string(), + "--rpc-port=45789".to_string(), + "-lerror,sc_rpc_server=info,runtime::revive=debug".to_string(), ]) { panic!("Node exited with error: {e:?}"); } From 530f43d7c8cc961e94a49051acb69175ea282325 Mon Sep 17 00:00:00 2001 From: Marian Radu Date: Mon, 10 Nov 2025 22:15:10 +0200 Subject: [PATCH 02/12] Fix eth-rpc runtime_api_dry_run_addr_works test. --- substrate/frame/revive/rpc/src/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/revive/rpc/src/tests.rs b/substrate/frame/revive/rpc/src/tests.rs index 9c62ebf6062af..f320dd2aa5289 100644 --- a/substrate/frame/revive/rpc/src/tests.rs +++ b/substrate/frame/revive/rpc/src/tests.rs @@ -440,8 +440,8 @@ async fn runtime_api_dry_run_addr_works() -> anyhow::Result<()> { None, ); - let nonce = client.get_transaction_count(account.address(), BlockTag::Latest.into()).await?; - let contract_address = create1(&account.address(), nonce.try_into().unwrap()); + let _nonce = client.get_transaction_count(account.address(), BlockTag::Latest.into()).await?; + let contract_address = create1(&account.address(), 0u64); let c = OnlineClient::::from_url("ws://localhost:45789").await?; let res = c.runtime_api().at_latest().await?.call(payload).await?.result.unwrap(); From ccfe32c239405005295ae68e74d195368da4f209 Mon Sep 17 00:00:00 2001 From: Marian Radu Date: Tue, 11 Nov 2025 12:25:38 +0200 Subject: [PATCH 03/12] Fix runtime_api_dry_run_addr_works test nonce query. --- Cargo.lock | 2 +- substrate/frame/revive/rpc/Cargo.toml | 2 +- substrate/frame/revive/rpc/src/tests.rs | 7 +++++-- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2414c607e7efe..41b496b98319b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13340,6 +13340,7 @@ dependencies = [ "pallet-revive-fixtures", "parity-scale-codec", "pretty_assertions", + "revive-dev-node", "revive-dev-runtime", "rlp 0.6.1", "sc-cli", @@ -13358,7 +13359,6 @@ dependencies = [ "sp-weights", "sqlx", "static_init", - "substrate-cli-test-utils", "substrate-prometheus-endpoint", "subxt 0.43.0", "subxt-signer 0.43.0", diff --git a/substrate/frame/revive/rpc/Cargo.toml b/substrate/frame/revive/rpc/Cargo.toml index 3fc16b9eca8d1..bc79ceaa764f5 100644 --- a/substrate/frame/revive/rpc/Cargo.toml +++ b/substrate/frame/revive/rpc/Cargo.toml @@ -53,8 +53,8 @@ tokio = { workspace = true, features = ["full"] } env_logger = { workspace = true } pallet-revive-fixtures = { workspace = true, default-features = true } pretty_assertions = { workspace = true } -static_init = { workspace = true } revive-dev-node = { workspace = true } +static_init = { workspace = true } [build-dependencies] git2 = { workspace = true } diff --git a/substrate/frame/revive/rpc/src/tests.rs b/substrate/frame/revive/rpc/src/tests.rs index f320dd2aa5289..5a74cd1f9383d 100644 --- a/substrate/frame/revive/rpc/src/tests.rs +++ b/substrate/frame/revive/rpc/src/tests.rs @@ -440,8 +440,11 @@ async fn runtime_api_dry_run_addr_works() -> anyhow::Result<()> { None, ); - let _nonce = client.get_transaction_count(account.address(), BlockTag::Latest.into()).await?; - let contract_address = create1(&account.address(), 0u64); + // runtime_api.at_latest() uses the latest finalized block, query nonce accordingly + let nonce = client + .get_transaction_count(account.address(), BlockTag::Finalized.into()) + .await?; + let contract_address = create1(&account.address(), nonce.try_into().unwrap()); let c = OnlineClient::::from_url("ws://localhost:45789").await?; let res = c.runtime_api().at_latest().await?.call(payload).await?.result.unwrap(); From 074fc1fe47247b02a4f30de2fea2ec085ebd7585 Mon Sep 17 00:00:00 2001 From: Marian Radu Date: Tue, 11 Nov 2025 17:50:49 +0200 Subject: [PATCH 04/12] Run all tests in a single tokio test function --- substrate/frame/revive/rpc/src/tests.rs | 85 +++++++++++++++---------- 1 file changed, 51 insertions(+), 34 deletions(-) diff --git a/substrate/frame/revive/rpc/src/tests.rs b/substrate/frame/revive/rpc/src/tests.rs index 2d3bd7e97cad4..6b0bfa58f2bbb 100644 --- a/substrate/frame/revive/rpc/src/tests.rs +++ b/substrate/frame/revive/rpc/src/tests.rs @@ -302,8 +302,49 @@ async fn verify_transactions_in_blocks( } #[tokio::test] -async fn transfer() -> anyhow::Result<()> { +async fn run_all_eth_rpc_tests() -> anyhow::Result<()> { let _lock = SHARED_RESOURCES.write(); + + type TestFn = fn() -> std::pin::Pin>>>; + // Define test functions with their names + let tests: Vec<(&str, TestFn)> = vec![ + ("transfer", || Box::pin(test_transfer())), + ("deploy_and_call", || Box::pin(test_deploy_and_call())), + ("runtime_api_dry_run_addr_works", || Box::pin(test_runtime_api_dry_run_addr_works())), + ("invalid_transaction", || Box::pin(test_invalid_transaction())), + ("evm_blocks_should_match", || Box::pin(test_evm_blocks_should_match())), + ("evm_blocks_hydrated_should_match", || Box::pin(test_evm_blocks_hydrated_should_match())), + ("block_hash_for_tag_with_proper_ethereum_block_hash_works", || { + Box::pin(test_block_hash_for_tag_with_proper_ethereum_block_hash_works()) + }), + ("block_hash_for_tag_with_invalid_ethereum_block_hash_fails", || { + Box::pin(test_block_hash_for_tag_with_invalid_ethereum_block_hash_fails()) + }), + ("block_hash_for_tag_with_block_number_works", || { + Box::pin(test_block_hash_for_tag_with_block_number_works()) + }), + ("block_hash_for_tag_with_block_tags_works", || { + Box::pin(test_block_hash_for_tag_with_block_tags_works()) + }), + ("multiple_transactions_in_block", || Box::pin(test_multiple_transactions_in_block())), + ("mixed_evm_substrate_transactions", || Box::pin(test_mixed_evm_substrate_transactions())), + ("runtime_pallets_address_upload_code", || { + Box::pin(test_runtime_pallets_address_upload_code()) + }), + ]; + + // Run each test + for (name, test_fn) in tests { + log::info!(target: LOG_TARGET, "Running test: {name}"); + test_fn().await?; + log::info!(target: LOG_TARGET, "Test passed: {name}"); + } + + log::info!(target: LOG_TARGET, "All tests completed successfully!"); + Ok(()) +} + +async fn test_transfer() -> anyhow::Result<()> { let client = Arc::new(SharedResources::client().await); let ethan = Account::from(subxt_signer::eth::dev::ethan()); @@ -329,9 +370,7 @@ async fn transfer() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] -async fn deploy_and_call() -> anyhow::Result<()> { - let _lock = SHARED_RESOURCES.write(); +async fn test_deploy_and_call() -> anyhow::Result<()> { let client = std::sync::Arc::new(SharedResources::client().await); let account = Account::default(); @@ -419,9 +458,7 @@ async fn deploy_and_call() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] -async fn runtime_api_dry_run_addr_works() -> anyhow::Result<()> { - let _lock = SHARED_RESOURCES.write(); +async fn test_runtime_api_dry_run_addr_works() -> anyhow::Result<()> { let client = std::sync::Arc::new(SharedResources::client().await); let account = Account::default(); @@ -453,9 +490,7 @@ async fn runtime_api_dry_run_addr_works() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] -async fn invalid_transaction() -> anyhow::Result<()> { - let _lock = SHARED_RESOURCES.write(); +async fn test_invalid_transaction() -> anyhow::Result<()> { let client = Arc::new(SharedResources::client().await); let ethan = Account::from(subxt_signer::eth::dev::ethan()); @@ -490,9 +525,7 @@ async fn get_evm_block_from_storage( Ok(block.0) } -#[tokio::test] -async fn evm_blocks_should_match() -> anyhow::Result<()> { - let _lock = SHARED_RESOURCES.write(); +async fn test_evm_blocks_should_match() -> anyhow::Result<()> { let client = std::sync::Arc::new(SharedResources::client().await); let (node_client, node_rpc_client, _) = @@ -539,9 +572,7 @@ async fn evm_blocks_should_match() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] -async fn evm_blocks_hydrated_should_match() -> anyhow::Result<()> { - let _lock = SHARED_RESOURCES.write(); +async fn test_evm_blocks_hydrated_should_match() -> anyhow::Result<()> { let client = std::sync::Arc::new(SharedResources::client().await); // Deploy a contract to have some transactions in the block @@ -596,9 +627,7 @@ async fn evm_blocks_hydrated_should_match() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] -async fn block_hash_for_tag_with_proper_ethereum_block_hash_works() -> anyhow::Result<()> { - let _lock = SHARED_RESOURCES.write(); +async fn test_block_hash_for_tag_with_proper_ethereum_block_hash_works() -> anyhow::Result<()> { let client = Arc::new(SharedResources::client().await); // Deploy a transaction to create a block with transactions @@ -629,9 +658,7 @@ async fn block_hash_for_tag_with_proper_ethereum_block_hash_works() -> anyhow::R Ok(()) } -#[tokio::test] -async fn block_hash_for_tag_with_invalid_ethereum_block_hash_fails() -> anyhow::Result<()> { - let _lock = SHARED_RESOURCES.write(); +async fn test_block_hash_for_tag_with_invalid_ethereum_block_hash_fails() -> anyhow::Result<()> { let client = Arc::new(SharedResources::client().await); let fake_eth_hash = H256::from([0x42u8; 32]); @@ -646,9 +673,7 @@ async fn block_hash_for_tag_with_invalid_ethereum_block_hash_fails() -> anyhow:: Ok(()) } -#[tokio::test] -async fn block_hash_for_tag_with_block_number_works() -> anyhow::Result<()> { - let _lock = SHARED_RESOURCES.write(); +async fn test_block_hash_for_tag_with_block_number_works() -> anyhow::Result<()> { let client = Arc::new(SharedResources::client().await); let block_number = client.block_number().await?; @@ -664,9 +689,7 @@ async fn block_hash_for_tag_with_block_number_works() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] -async fn block_hash_for_tag_with_block_tags_works() -> anyhow::Result<()> { - let _lock = SHARED_RESOURCES.write(); +async fn test_block_hash_for_tag_with_block_tags_works() -> anyhow::Result<()> { let client = Arc::new(SharedResources::client().await); let account = Account::default(); @@ -687,9 +710,7 @@ async fn block_hash_for_tag_with_block_tags_works() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] async fn test_multiple_transactions_in_block() -> anyhow::Result<()> { - let _lock = SHARED_RESOURCES.write(); let client = Arc::new(SharedResources::client().await); let num_transactions = 20; @@ -739,9 +760,7 @@ async fn test_multiple_transactions_in_block() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] async fn test_mixed_evm_substrate_transactions() -> anyhow::Result<()> { - let _lock = SHARED_RESOURCES.write(); let client = Arc::new(SharedResources::client().await); let num_evm_txs = 10; @@ -816,9 +835,7 @@ async fn test_mixed_evm_substrate_transactions() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] async fn test_runtime_pallets_address_upload_code() -> anyhow::Result<()> { - let _lock = SHARED_RESOURCES.write(); let client = Arc::new(SharedResources::client().await); let (node_client, node_rpc_client, _) = client::connect(SharedResources::node_rpc_url()).await?; From 227cfb2a5f474b34bec8180f50a52474db669d2a Mon Sep 17 00:00:00 2001 From: Marian Radu Date: Tue, 11 Nov 2025 18:01:13 +0200 Subject: [PATCH 05/12] Remove sequential execution configuration for eth-rpc tests. --- .config/nextest.toml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.config/nextest.toml b/.config/nextest.toml index 1bf566d789d2c..7a692be7bc7e3 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -123,10 +123,3 @@ serial-integration = { max-threads = 1 } [[profile.default.overrides]] filter = 'test(/(^ui$|_ui|ui_)/)' test-group = 'serial-integration' - -# Running eth-rpc tests sequentially -# These tests rely on a shared resource (the RPC and Node) -# and would cause race conditions due to transaction nonces if run in parallel. -[[profile.default.overrides]] -filter = 'package(pallet-revive-eth-rpc) and test(/^tests::/)' -test-group = 'serial-integration' From d4592fb8e2dd45bbb3f343fe6468e6a829688d36 Mon Sep 17 00:00:00 2001 From: Marian Radu Date: Tue, 11 Nov 2025 18:04:55 +0200 Subject: [PATCH 06/12] Remove comments. --- substrate/frame/revive/rpc/src/tests.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/substrate/frame/revive/rpc/src/tests.rs b/substrate/frame/revive/rpc/src/tests.rs index 6b0bfa58f2bbb..68797743f2b22 100644 --- a/substrate/frame/revive/rpc/src/tests.rs +++ b/substrate/frame/revive/rpc/src/tests.rs @@ -306,7 +306,6 @@ async fn run_all_eth_rpc_tests() -> anyhow::Result<()> { let _lock = SHARED_RESOURCES.write(); type TestFn = fn() -> std::pin::Pin>>>; - // Define test functions with their names let tests: Vec<(&str, TestFn)> = vec![ ("transfer", || Box::pin(test_transfer())), ("deploy_and_call", || Box::pin(test_deploy_and_call())), @@ -333,7 +332,6 @@ async fn run_all_eth_rpc_tests() -> anyhow::Result<()> { }), ]; - // Run each test for (name, test_fn) in tests { log::info!(target: LOG_TARGET, "Running test: {name}"); test_fn().await?; From 0516576b17d759048487f7ab806281f6402a257d Mon Sep 17 00:00:00 2001 From: Marian Radu Date: Tue, 11 Nov 2025 19:30:26 +0200 Subject: [PATCH 07/12] Add macro to run all tests sequentially. --- Cargo.lock | 1 - substrate/frame/revive/rpc/Cargo.toml | 1 - substrate/frame/revive/rpc/src/tests.rs | 127 ++++++++++-------------- 3 files changed, 55 insertions(+), 74 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 336e56c59a359..38b2b2e51c60d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -13358,7 +13358,6 @@ dependencies = [ "sp-timestamp", "sp-weights", "sqlx", - "static_init", "substrate-prometheus-endpoint", "subxt 0.43.0", "subxt-signer 0.43.0", diff --git a/substrate/frame/revive/rpc/Cargo.toml b/substrate/frame/revive/rpc/Cargo.toml index 08da42234c72a..cb9f124e96930 100644 --- a/substrate/frame/revive/rpc/Cargo.toml +++ b/substrate/frame/revive/rpc/Cargo.toml @@ -55,7 +55,6 @@ pallet-revive-fixtures = { workspace = true, default-features = true } pretty_assertions = { workspace = true } revive-dev-node = { workspace = true } sp-io = { workspace = true, default-features = true } -static_init = { workspace = true } [build-dependencies] git2 = { workspace = true } diff --git a/substrate/frame/revive/rpc/src/tests.rs b/substrate/frame/revive/rpc/src/tests.rs index 68797743f2b22..07b0900163c7b 100644 --- a/substrate/frame/revive/rpc/src/tests.rs +++ b/substrate/frame/revive/rpc/src/tests.rs @@ -37,7 +37,6 @@ use pallet_revive::{ HashesOrTransactionInfos, TransactionInfo, H256, U256, }, }; -use static_init::dynamic; use std::{collections::BTreeMap, sync::Arc, thread}; use subxt::{ backend::rpc::RpcClient, @@ -109,9 +108,6 @@ impl SharedResources { } } -#[dynamic(lazy)] -static mut SHARED_RESOURCES: SharedResources = SharedResources::start(); - macro_rules! unwrap_call_err( ($err:expr) => { match $err.downcast_ref::().unwrap() { @@ -121,6 +117,21 @@ macro_rules! unwrap_call_err( } ); +macro_rules! run_tests { + ($client:expr, $($test:ident),+ $(,)?) => { + $( + { + let test_name = stringify!($test); + log::debug!(target: LOG_TARGET, "Running test: {}", test_name); + match $test($client.clone()).await { + Ok(()) => log::debug!(target: LOG_TARGET, "Test passed: {}", test_name), + Err(err) => panic!("Test {} failed: {err:?}", test_name), + } + } + )+ + }; +} + // Helper functions /// Prepare multiple EVM transfer transactions with sequential nonces async fn prepare_evm_transactions( @@ -303,48 +314,32 @@ async fn verify_transactions_in_blocks( #[tokio::test] async fn run_all_eth_rpc_tests() -> anyhow::Result<()> { - let _lock = SHARED_RESOURCES.write(); - - type TestFn = fn() -> std::pin::Pin>>>; - let tests: Vec<(&str, TestFn)> = vec![ - ("transfer", || Box::pin(test_transfer())), - ("deploy_and_call", || Box::pin(test_deploy_and_call())), - ("runtime_api_dry_run_addr_works", || Box::pin(test_runtime_api_dry_run_addr_works())), - ("invalid_transaction", || Box::pin(test_invalid_transaction())), - ("evm_blocks_should_match", || Box::pin(test_evm_blocks_should_match())), - ("evm_blocks_hydrated_should_match", || Box::pin(test_evm_blocks_hydrated_should_match())), - ("block_hash_for_tag_with_proper_ethereum_block_hash_works", || { - Box::pin(test_block_hash_for_tag_with_proper_ethereum_block_hash_works()) - }), - ("block_hash_for_tag_with_invalid_ethereum_block_hash_fails", || { - Box::pin(test_block_hash_for_tag_with_invalid_ethereum_block_hash_fails()) - }), - ("block_hash_for_tag_with_block_number_works", || { - Box::pin(test_block_hash_for_tag_with_block_number_works()) - }), - ("block_hash_for_tag_with_block_tags_works", || { - Box::pin(test_block_hash_for_tag_with_block_tags_works()) - }), - ("multiple_transactions_in_block", || Box::pin(test_multiple_transactions_in_block())), - ("mixed_evm_substrate_transactions", || Box::pin(test_mixed_evm_substrate_transactions())), - ("runtime_pallets_address_upload_code", || { - Box::pin(test_runtime_pallets_address_upload_code()) - }), - ]; + // start node and rpc server + let _shared = SharedResources::start(); + let client = Arc::new(SharedResources::client().await); - for (name, test_fn) in tests { - log::info!(target: LOG_TARGET, "Running test: {name}"); - test_fn().await?; - log::info!(target: LOG_TARGET, "Test passed: {name}"); - } + run_tests!( + client, + test_transfer, + test_deploy_and_call, + test_runtime_api_dry_run_addr_works, + test_invalid_transaction, + test_evm_blocks_should_match, + test_evm_blocks_hydrated_should_match, + test_block_hash_for_tag_with_proper_ethereum_block_hash_works, + test_block_hash_for_tag_with_invalid_ethereum_block_hash_fails, + test_block_hash_for_tag_with_block_number_works, + test_block_hash_for_tag_with_block_tags_works, + test_multiple_transactions_in_block, + test_mixed_evm_substrate_transactions, + test_runtime_pallets_address_upload_code, + ); log::info!(target: LOG_TARGET, "All tests completed successfully!"); Ok(()) } -async fn test_transfer() -> anyhow::Result<()> { - let client = Arc::new(SharedResources::client().await); - +async fn test_transfer(client: Arc) -> anyhow::Result<()> { let ethan = Account::from(subxt_signer::eth::dev::ethan()); let initial_balance = client.get_balance(ethan.address(), BlockTag::Latest.into()).await?; @@ -368,8 +363,7 @@ async fn test_transfer() -> anyhow::Result<()> { Ok(()) } -async fn test_deploy_and_call() -> anyhow::Result<()> { - let client = std::sync::Arc::new(SharedResources::client().await); +async fn test_deploy_and_call(client: Arc) -> anyhow::Result<()> { let account = Account::default(); // Balance transfer @@ -456,9 +450,7 @@ async fn test_deploy_and_call() -> anyhow::Result<()> { Ok(()) } -async fn test_runtime_api_dry_run_addr_works() -> anyhow::Result<()> { - let client = std::sync::Arc::new(SharedResources::client().await); - +async fn test_runtime_api_dry_run_addr_works(client: Arc) -> anyhow::Result<()> { let account = Account::default(); let origin: [u8; 32] = account.substrate_account().into(); let data = b"hello world".to_vec(); @@ -488,8 +480,7 @@ async fn test_runtime_api_dry_run_addr_works() -> anyhow::Result<()> { Ok(()) } -async fn test_invalid_transaction() -> anyhow::Result<()> { - let client = Arc::new(SharedResources::client().await); +async fn test_invalid_transaction(client: Arc) -> anyhow::Result<()> { let ethan = Account::from(subxt_signer::eth::dev::ethan()); let err = TransactionBuilder::new(&client) @@ -523,9 +514,7 @@ async fn get_evm_block_from_storage( Ok(block.0) } -async fn test_evm_blocks_should_match() -> anyhow::Result<()> { - let client = std::sync::Arc::new(SharedResources::client().await); - +async fn test_evm_blocks_should_match(client: Arc) -> anyhow::Result<()> { let (node_client, node_rpc_client, _) = client::connect(SharedResources::node_rpc_url()).await.unwrap(); @@ -570,9 +559,7 @@ async fn test_evm_blocks_should_match() -> anyhow::Result<()> { Ok(()) } -async fn test_evm_blocks_hydrated_should_match() -> anyhow::Result<()> { - let client = std::sync::Arc::new(SharedResources::client().await); - +async fn test_evm_blocks_hydrated_should_match(client: Arc) -> anyhow::Result<()> { // Deploy a contract to have some transactions in the block let (bytes, _) = pallet_revive_fixtures::compile_module("dummy")?; let value = U256::from(5_000_000_000_000u128); @@ -625,9 +612,9 @@ async fn test_evm_blocks_hydrated_should_match() -> anyhow::Result<()> { Ok(()) } -async fn test_block_hash_for_tag_with_proper_ethereum_block_hash_works() -> anyhow::Result<()> { - let client = Arc::new(SharedResources::client().await); - +async fn test_block_hash_for_tag_with_proper_ethereum_block_hash_works( + client: Arc, +) -> anyhow::Result<()> { // Deploy a transaction to create a block with transactions let (bytes, _) = pallet_revive_fixtures::compile_module("dummy")?; let value = U256::from(5_000_000_000_000u128); @@ -656,9 +643,9 @@ async fn test_block_hash_for_tag_with_proper_ethereum_block_hash_works() -> anyh Ok(()) } -async fn test_block_hash_for_tag_with_invalid_ethereum_block_hash_fails() -> anyhow::Result<()> { - let client = Arc::new(SharedResources::client().await); - +async fn test_block_hash_for_tag_with_invalid_ethereum_block_hash_fails( + client: Arc, +) -> anyhow::Result<()> { let fake_eth_hash = H256::from([0x42u8; 32]); log::trace!(target: LOG_TARGET, "Testing with fake Ethereum hash: {fake_eth_hash:?}"); @@ -671,9 +658,9 @@ async fn test_block_hash_for_tag_with_invalid_ethereum_block_hash_fails() -> any Ok(()) } -async fn test_block_hash_for_tag_with_block_number_works() -> anyhow::Result<()> { - let client = Arc::new(SharedResources::client().await); - +async fn test_block_hash_for_tag_with_block_number_works( + client: Arc, +) -> anyhow::Result<()> { let block_number = client.block_number().await?; log::trace!(target: LOG_TARGET, "Testing with block number: {block_number}"); @@ -687,8 +674,9 @@ async fn test_block_hash_for_tag_with_block_number_works() -> anyhow::Result<()> Ok(()) } -async fn test_block_hash_for_tag_with_block_tags_works() -> anyhow::Result<()> { - let client = Arc::new(SharedResources::client().await); +async fn test_block_hash_for_tag_with_block_tags_works( + client: Arc, +) -> anyhow::Result<()> { let account = Account::default(); let tags = vec![ @@ -708,9 +696,7 @@ async fn test_block_hash_for_tag_with_block_tags_works() -> anyhow::Result<()> { Ok(()) } -async fn test_multiple_transactions_in_block() -> anyhow::Result<()> { - let client = Arc::new(SharedResources::client().await); - +async fn test_multiple_transactions_in_block(client: Arc) -> anyhow::Result<()> { let num_transactions = 20; let alith = Account::default(); let ethan = Account::from(subxt_signer::eth::dev::ethan()); @@ -758,9 +744,7 @@ async fn test_multiple_transactions_in_block() -> anyhow::Result<()> { Ok(()) } -async fn test_mixed_evm_substrate_transactions() -> anyhow::Result<()> { - let client = Arc::new(SharedResources::client().await); - +async fn test_mixed_evm_substrate_transactions(client: Arc) -> anyhow::Result<()> { let num_evm_txs = 10; let num_substrate_txs = 7; @@ -833,8 +817,7 @@ async fn test_mixed_evm_substrate_transactions() -> anyhow::Result<()> { Ok(()) } -async fn test_runtime_pallets_address_upload_code() -> anyhow::Result<()> { - let client = Arc::new(SharedResources::client().await); +async fn test_runtime_pallets_address_upload_code(client: Arc) -> anyhow::Result<()> { let (node_client, node_rpc_client, _) = client::connect(SharedResources::node_rpc_url()).await?; From cf1eb6d1b314feeac488cb181fedadc8a6e624ae Mon Sep 17 00:00:00 2001 From: Marian Radu Date: Wed, 12 Nov 2025 09:58:29 +0200 Subject: [PATCH 08/12] Update log level --- substrate/frame/revive/rpc/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/revive/rpc/src/tests.rs b/substrate/frame/revive/rpc/src/tests.rs index 07b0900163c7b..353cf90062424 100644 --- a/substrate/frame/revive/rpc/src/tests.rs +++ b/substrate/frame/revive/rpc/src/tests.rs @@ -335,7 +335,7 @@ async fn run_all_eth_rpc_tests() -> anyhow::Result<()> { test_runtime_pallets_address_upload_code, ); - log::info!(target: LOG_TARGET, "All tests completed successfully!"); + log::debug!(target: LOG_TARGET, "All tests completed successfully!"); Ok(()) } From 69e7b14024dc5b66a0987fd23fffb785cc505553 Mon Sep 17 00:00:00 2001 From: Marian Radu Date: Wed, 12 Nov 2025 13:24:02 +0200 Subject: [PATCH 09/12] Address review comments 1. Move run_tests macro to run_all_eth_rpc_tests 2. Build transactions with descending nonces in prepare_evm_transactions --- substrate/frame/revive/rpc/src/tests.rs | 218 +++++++++--------------- 1 file changed, 81 insertions(+), 137 deletions(-) diff --git a/substrate/frame/revive/rpc/src/tests.rs b/substrate/frame/revive/rpc/src/tests.rs index 353cf90062424..ad8990d12ff94 100644 --- a/substrate/frame/revive/rpc/src/tests.rs +++ b/substrate/frame/revive/rpc/src/tests.rs @@ -117,23 +117,8 @@ macro_rules! unwrap_call_err( } ); -macro_rules! run_tests { - ($client:expr, $($test:ident),+ $(,)?) => { - $( - { - let test_name = stringify!($test); - log::debug!(target: LOG_TARGET, "Running test: {}", test_name); - match $test($client.clone()).await { - Ok(()) => log::debug!(target: LOG_TARGET, "Test passed: {}", test_name), - Err(err) => panic!("Test {} failed: {err:?}", test_name), - } - } - )+ - }; -} - // Helper functions -/// Prepare multiple EVM transfer transactions with sequential nonces +/// Prepare multiple EVM transfer transactions with nonce in descending order async fn prepare_evm_transactions( client: &Arc, signer: Account, @@ -141,10 +126,12 @@ async fn prepare_evm_transactions( amount: U256, count: usize, ) -> anyhow::Result>> { - let mut nonce = client.get_transaction_count(signer.address(), BlockTag::Latest.into()).await?; + let start_nonce = + client.get_transaction_count(signer.address(), BlockTag::Latest.into()).await?; let mut transactions = Vec::new(); - for i in 0..count { + for i in (0..count).rev() { + let nonce = start_nonce.saturating_add(U256::from(i as u64)); let tx_builder = TransactionBuilder::new(client) .signer(signer.clone()) .nonce(nonce) @@ -153,7 +140,6 @@ async fn prepare_evm_transactions( transactions.push(tx_builder); log::trace!(target: LOG_TARGET, "Prepared EVM transaction {}/{count} with nonce: {nonce:?}", i + 1); - nonce = nonce.saturating_add(U256::one()); } Ok(transactions) @@ -209,28 +195,6 @@ async fn submit_evm_transactions( Ok(submitted_txs) } -/// Wait for all submitted transactions to be included in blocks -async fn wait_for_receipts( - submitted_txs: Vec<( - H256, - pallet_revive::evm::GenericTransaction, - crate::example::SubmittedTransaction, - )>, -) -> anyhow::Result< - Vec<(H256, pallet_revive::evm::GenericTransaction, pallet_revive::evm::ReceiptInfo)>, -> { - let wait_futures: Vec<_> = submitted_txs - .into_iter() - .map(|(hash, generic_tx, tx)| async move { - let receipt = tx.wait_for_receipt().await?; - Ok::<_, anyhow::Error>((hash, generic_tx, receipt)) - }) - .collect(); - - let results = futures::future::join_all(wait_futures).await; - results.into_iter().collect() -} - /// Submit substrate transactions and return futures for waiting async fn submit_substrate_transactions( substrate_txs: Vec>>, @@ -271,43 +235,62 @@ async fn submit_substrate_transactions( futures } -/// Verify that each transaction exists in its block and is accessible via RPC -/// Takes a map of block_number -> transaction_hashes for efficient verification -async fn verify_transactions_in_blocks( - client: &Arc, - blocks_map: &BTreeMap>, +/// Verify all given transaction hashes are in the specified block and accessible via RPC +async fn verify_transactions_in_single_block( + client: &Arc, + block_number: U256, + expected_tx_hashes: &[H256], ) -> anyhow::Result<()> { - for (block_number, expected_tx_hashes) in blocks_map { - // Fetch the block once for all transactions - let block = client - .get_block_by_number(BlockNumberOrTag::U256(*block_number), false) - .await? - .ok_or_else(|| anyhow!("Block {block_number} should exist"))?; - - let block_tx_hashes = match &block.transactions { - pallet_revive::evm::HashesOrTransactionInfos::Hashes(hashes) => hashes.clone(), - pallet_revive::evm::HashesOrTransactionInfos::TransactionInfos(infos) => - infos.iter().map(|info| info.hash).collect(), - }; + // Fetch the block + let block = client + .get_block_by_number(BlockNumberOrTag::U256(block_number), false) + .await? + .ok_or_else(|| anyhow!("Block {block_number} should exist"))?; + + let block_tx_hashes = match &block.transactions { + HashesOrTransactionInfos::Hashes(hashes) => hashes.clone(), + HashesOrTransactionInfos::TransactionInfos(infos) => + infos.iter().map(|info| info.hash).collect(), + }; - // Verify each expected transaction is in the block - for expected_hash in expected_tx_hashes { + // Verify all transactions in parallel + let verification_futures = expected_tx_hashes.iter().map(|tx_hash| { + let client = client.clone(); + let tx_hashes = block_tx_hashes.clone(); + async move { assert!( - block_tx_hashes.contains(expected_hash), - "Block {block_number} should contain transaction {expected_hash:?}" + tx_hashes.contains(tx_hash), + "Block {block_number} should contain transaction {tx_hash:?}" + ); + + // Fetch transaction and receipt in parallel + let (tx_result, receipt_result) = tokio::join!( + client.get_transaction_by_hash(*tx_hash), + client.get_transaction_receipt(*tx_hash) ); - // Verify we can fetch the transaction by hash - let tx = client.get_transaction_by_hash(*expected_hash).await?.ok_or_else(|| { - anyhow!("Transaction {expected_hash:?} should be retrievable by hash") - })?; + let tx = tx_result? + .ok_or_else(|| anyhow!("Transaction {tx_hash:?} should be retrievable by hash"))?; + + let receipt = receipt_result? + .ok_or_else(|| anyhow!("Receipt for {tx_hash:?} should be retrievable"))?; assert_eq!( - tx.block_number, *block_number, - "Transaction {expected_hash:?} should be in block {block_number}" + tx.block_number, block_number, + "Transaction {tx_hash:?} should be in block {block_number}" ); + + assert_eq!( + receipt.status, + Some(U256::one()), + "Transaction {tx_hash:?} should be successful" + ); + + Ok::<(), anyhow::Error>(()) } - } + }); + + futures::future::try_join_all(verification_futures).await?; Ok(()) } @@ -318,8 +301,22 @@ async fn run_all_eth_rpc_tests() -> anyhow::Result<()> { let _shared = SharedResources::start(); let client = Arc::new(SharedResources::client().await); + macro_rules! run_tests { + ($($test:ident),+ $(,)?) => { + $( + { + let test_name = stringify!($test); + log::debug!(target: LOG_TARGET, "Running test: {}", test_name); + match $test(client.clone()).await { + Ok(()) => log::debug!(target: LOG_TARGET, "Test passed: {}", test_name), + Err(err) => panic!("Test {} failed: {err:?}", test_name), + } + } + )+ + }; + } + run_tests!( - client, test_transfer, test_deploy_and_call, test_runtime_api_dry_run_addr_works, @@ -708,39 +705,14 @@ async fn test_multiple_transactions_in_block(client: Arc) -> anyhow::R // Submit all transactions let submitted_txs = submit_evm_transactions(transactions).await?; + let tx_hashes: Vec = submitted_txs.iter().map(|(hash, _, _)| *hash).collect(); log::trace!(target: LOG_TARGET, "Submitted {} transactions", submitted_txs.len()); - // Wait for all receipts in parallel - let results = wait_for_receipts(submitted_txs).await?; - - // Verify all transactions were successful and group by block - // Most of the times all transactions will be in a single block, - // but we cannot guarantee that - let mut blocks_map: BTreeMap> = BTreeMap::new(); - let mut total_txs = 0; - - for (i, (hash, _generic_tx, receipt)) in results.into_iter().enumerate() { - log::trace!(target: LOG_TARGET, "Transaction {}: hash={hash:?}, block={}", i + 1, receipt.block_number); - assert_eq!( - receipt.status.unwrap_or(U256::zero()), - U256::one(), - "Transaction should be successful" - ); - - blocks_map.entry(receipt.block_number).or_insert_with(Vec::new).push(hash); - total_txs += 1; - } - - log::trace!(target: LOG_TARGET, "All {} transactions successful, spanning {} block(s):", total_txs, blocks_map.len()); - - // Print transaction distribution across blocks (BTreeMap keeps them sorted) - for (block_num, tx_hashes) in &blocks_map { - log::trace!(target: LOG_TARGET, " Block {}: {} transaction(s) - {:?}", block_num, tx_hashes.len(), tx_hashes); - } - - // Verify each transaction exists in its respective block - verify_transactions_in_blocks(&client, &blocks_map).await?; + // All transactions should be included in the same block since nonces are in descending order + let first_receipt = submitted_txs[0].2.wait_for_receipt().await?; + // Fetch and verify block contains all transactions + verify_transactions_in_single_block(&client, first_receipt.block_number, &tx_hashes).await?; Ok(()) } @@ -769,50 +741,22 @@ async fn test_mixed_evm_substrate_transactions(client: Arc) -> anyhow: // Submit EVM transactions let evm_submitted = submit_evm_transactions(evm_transactions).await?; + let evm_tx_hashes: Vec = evm_submitted.iter().map(|(hash, _, _)| *hash).collect(); // Submit substrate transactions let substrate_futures = submit_substrate_transactions(substrate_txs).await; - // Wait for all transactions in parallel - let (evm_results, _substrate_results) = tokio::join!( - wait_for_receipts(evm_submitted), + // Wait for first EVM receipt and all substrate transactions in parallel + let (evm_first_receipt_result, _substrate_results) = tokio::join!( + async { evm_submitted[0].2.wait_for_receipt().await }, futures::future::join_all(substrate_futures) ); + // Handle the EVM receipt result + let evm_first_receipt = evm_first_receipt_result?; - // Handle results - let evm_results = evm_results?; - - // Verify all EVM transactions were successful and group by block - // Most of the times all transactions will be in a single block, - // but we cannot guarantee that - let mut blocks_map: BTreeMap> = BTreeMap::new(); - let mut total_txs = 0; - - for (i, (hash, _generic_tx, receipt)) in evm_results.into_iter().enumerate() { - log::trace!(target: LOG_TARGET, "EVM Transaction {}: hash={hash:?}, block={}", i + 1, receipt.block_number); - assert_eq!( - receipt.status.unwrap_or(U256::zero()), - U256::one(), - "EVM transaction should be successful" - ); - - blocks_map.entry(receipt.block_number).or_insert_with(Vec::new).push(hash); - total_txs += 1; - } - - log::trace!(target: LOG_TARGET, - "All {} EVM transactions successful, spanning {} block(s):", - total_txs, - blocks_map.len() - ); - - // Print transaction distribution across blocks - for (block_num, tx_hashes) in &blocks_map { - log::trace!(target: LOG_TARGET, "Block {}: {} transaction(s) - {:?}", block_num, tx_hashes.len(), tx_hashes); - } - - // Verify each EVM transaction exists in its respective block - verify_transactions_in_blocks(&client, &blocks_map).await?; + // Fetch and verify block contains all transactions + verify_transactions_in_single_block(&client, evm_first_receipt.block_number, &evm_tx_hashes) + .await?; Ok(()) } From b58ce4290ed99eb78b66719a5c178e02a758319e Mon Sep 17 00:00:00 2001 From: Marian Radu Date: Wed, 12 Nov 2025 14:06:57 +0200 Subject: [PATCH 10/12] Fix warning. --- substrate/frame/revive/rpc/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/revive/rpc/src/tests.rs b/substrate/frame/revive/rpc/src/tests.rs index ad8990d12ff94..4436c65919534 100644 --- a/substrate/frame/revive/rpc/src/tests.rs +++ b/substrate/frame/revive/rpc/src/tests.rs @@ -37,7 +37,7 @@ use pallet_revive::{ HashesOrTransactionInfos, TransactionInfo, H256, U256, }, }; -use std::{collections::BTreeMap, sync::Arc, thread}; +use std::{sync::Arc, thread}; use subxt::{ backend::rpc::RpcClient, ext::subxt_rpcs::rpc_params, From ce63c345ffd2d8c07fc6e0e7b8a4e957399fa246 Mon Sep 17 00:00:00 2001 From: Marian Radu Date: Wed, 12 Nov 2025 15:10:28 +0200 Subject: [PATCH 11/12] Address review comments. --- substrate/frame/revive/rpc/src/tests.rs | 43 +++---------------------- 1 file changed, 5 insertions(+), 38 deletions(-) diff --git a/substrate/frame/revive/rpc/src/tests.rs b/substrate/frame/revive/rpc/src/tests.rs index 4436c65919534..10fd42534e9e2 100644 --- a/substrate/frame/revive/rpc/src/tests.rs +++ b/substrate/frame/revive/rpc/src/tests.rs @@ -253,44 +253,11 @@ async fn verify_transactions_in_single_block( infos.iter().map(|info| info.hash).collect(), }; - // Verify all transactions in parallel - let verification_futures = expected_tx_hashes.iter().map(|tx_hash| { - let client = client.clone(); - let tx_hashes = block_tx_hashes.clone(); - async move { - assert!( - tx_hashes.contains(tx_hash), - "Block {block_number} should contain transaction {tx_hash:?}" - ); - - // Fetch transaction and receipt in parallel - let (tx_result, receipt_result) = tokio::join!( - client.get_transaction_by_hash(*tx_hash), - client.get_transaction_receipt(*tx_hash) - ); - - let tx = tx_result? - .ok_or_else(|| anyhow!("Transaction {tx_hash:?} should be retrievable by hash"))?; - - let receipt = receipt_result? - .ok_or_else(|| anyhow!("Receipt for {tx_hash:?} should be retrievable"))?; - - assert_eq!( - tx.block_number, block_number, - "Transaction {tx_hash:?} should be in block {block_number}" - ); - - assert_eq!( - receipt.status, - Some(U256::one()), - "Transaction {tx_hash:?} should be successful" - ); - - Ok::<(), anyhow::Error>(()) - } - }); - - futures::future::try_join_all(verification_futures).await?; + if let Some(missing_hash) = + expected_tx_hashes.iter().find(|hash| !block_tx_hashes.contains(hash)) + { + return Err(anyhow!("Transaction {missing_hash:?} not found in block {block_number}")); + } Ok(()) } From f0f51c2b33d5cab0ab0987ecb790d0e84d3ae337 Mon Sep 17 00:00:00 2001 From: "cmd[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 12 Nov 2025 13:26:02 +0000 Subject: [PATCH 12/12] Update from github-actions[bot] running command 'prdoc --audience runtime_dev --bump patch' --- prdoc/pr_10281.prdoc | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 prdoc/pr_10281.prdoc diff --git a/prdoc/pr_10281.prdoc b/prdoc/pr_10281.prdoc new file mode 100644 index 0000000000000..4ee9372a4f59f --- /dev/null +++ b/prdoc/pr_10281.prdoc @@ -0,0 +1,9 @@ +title: '[pallet-revive] improve eth-rpc tests reliability' +doc: +- audience: Runtime Dev + description: Improve eth-rpc tests reliability by replacing substrate-node with + revive-dev-node, fixing nonce query in runtime_api_dry_run_addr, and running all + tests in a single tokio test function +crates: +- name: pallet-revive-eth-rpc + bump: patch