[pallet-revive] improve eth-rpc tests reliability

.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'

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::<Vec<_>>();
+	let args = std::env::args_os().map(|s| s.to_string_lossy().to_string()).collect::<Vec<_>>();
+	return run_with_args(args);
+}
+
+/// Parse and run command line arguments
+pub fn run_with_args(args: Vec<String>) -> sc_cli::Result<()> {
 	let mut cli = Cli::from_iter(args);
 
 	match &cli.subcommand {

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;

substrate/frame/revive/rpc/Cargo.toml

@@ -53,9 +53,8 @@ tokio = { workspace = true, features = ["full"] }
 env_logger = { workspace = true }
 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 }
-substrate-cli-test-utils = { workspace = true }
 
 [build-dependencies]
 git2 = { workspace = true }

substrate/frame/revive/rpc/src/tests.rs

@@ -37,9 +37,7 @@ use pallet_revive::{
 		HashesOrTransactionInfos, TransactionInfo, H256, U256,
 	},
 };
-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 +70,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:?}");
 			}
@@ -112,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::<jsonrpsee::core::client::Error>().unwrap() {
@@ -124,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<Client: EthRpcClient + Sync + Send>(
@@ -305,10 +313,33 @@ async fn verify_transactions_in_blocks<Client: EthRpcClient + Sync>(
 }
 
 #[tokio::test]
-async fn transfer() -> anyhow::Result<()> {
-	let _lock = SHARED_RESOURCES.write();
+async fn run_all_eth_rpc_tests() -> anyhow::Result<()> {
+	// start node and rpc server
+	let _shared = SharedResources::start();
 	let client = Arc::new(SharedResources::client().await);
 
+	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::debug!(target: LOG_TARGET, "All tests completed successfully!");
+	Ok(())
+}
+
+async fn test_transfer(client: Arc<WsClient>) -> anyhow::Result<()> {
 	let ethan = Account::from(subxt_signer::eth::dev::ethan());
 	let initial_balance = client.get_balance(ethan.address(), BlockTag::Latest.into()).await?;
 
@@ -332,10 +363,7 @@ async fn transfer() -> anyhow::Result<()> {
 	Ok(())
 }
 
-#[tokio::test]
-async fn deploy_and_call() -> anyhow::Result<()> {
-	let _lock = SHARED_RESOURCES.write();
-	let client = std::sync::Arc::new(SharedResources::client().await);
+async fn test_deploy_and_call(client: Arc<WsClient>) -> anyhow::Result<()> {
 	let account = Account::default();
 
 	// Balance transfer
@@ -422,11 +450,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();
-	let client = std::sync::Arc::new(SharedResources::client().await);
-
+async fn test_runtime_api_dry_run_addr_works(client: Arc<WsClient>) -> anyhow::Result<()> {
 	let account = Account::default();
 	let origin: [u8; 32] = account.substrate_account().into();
 	let data = b"hello world".to_vec();
@@ -443,7 +467,10 @@ async fn runtime_api_dry_run_addr_works() -> anyhow::Result<()> {
 		None,
 	);
 
-	let nonce = client.get_transaction_count(account.address(), BlockTag::Latest.into()).await?;
+	// 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::<SrcChainConfig>::from_url("ws://localhost:45789").await?;
@@ -453,10 +480,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();
-	let client = Arc::new(SharedResources::client().await);
+async fn test_invalid_transaction(client: Arc<WsClient>) -> anyhow::Result<()> {
 	let ethan = Account::from(subxt_signer::eth::dev::ethan());
 
 	let err = TransactionBuilder::new(&client)
@@ -490,11 +514,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();
-	let client = std::sync::Arc::new(SharedResources::client().await);
-
+async fn test_evm_blocks_should_match(client: Arc<WsClient>) -> anyhow::Result<()> {
 	let (node_client, node_rpc_client, _) =
 		client::connect(SharedResources::node_rpc_url()).await.unwrap();
 
@@ -539,11 +559,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();
-	let client = std::sync::Arc::new(SharedResources::client().await);
-
+async fn test_evm_blocks_hydrated_should_match(client: Arc<WsClient>) -> 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);
@@ -596,11 +612,9 @@ 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();
-	let client = Arc::new(SharedResources::client().await);
-
+async fn test_block_hash_for_tag_with_proper_ethereum_block_hash_works(
+	client: Arc<WsClient>,
+) -> 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);
@@ -629,11 +643,9 @@ 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();
-	let client = Arc::new(SharedResources::client().await);
-
+async fn test_block_hash_for_tag_with_invalid_ethereum_block_hash_fails(
+	client: Arc<WsClient>,
+) -> anyhow::Result<()> {
 	let fake_eth_hash = H256::from([0x42u8; 32]);
 
 	log::trace!(target: LOG_TARGET, "Testing with fake Ethereum hash: {fake_eth_hash:?}");
@@ -646,11 +658,9 @@ 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();
-	let client = Arc::new(SharedResources::client().await);
-
+async fn test_block_hash_for_tag_with_block_number_works(
+	client: Arc<WsClient>,
+) -> anyhow::Result<()> {
 	let block_number = client.block_number().await?;
 
 	log::trace!(target: LOG_TARGET, "Testing with block number: {block_number}");
@@ -664,10 +674,9 @@ 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();
-	let client = Arc::new(SharedResources::client().await);
+async fn test_block_hash_for_tag_with_block_tags_works(
+	client: Arc<WsClient>,
+) -> anyhow::Result<()> {
 	let account = Account::default();
 
 	let tags = vec![
@@ -687,11 +696,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);
-
+async fn test_multiple_transactions_in_block(client: Arc<WsClient>) -> anyhow::Result<()> {
 	let num_transactions = 20;
 	let alith = Account::default();
 	let ethan = Account::from(subxt_signer::eth::dev::ethan());
@@ -739,11 +744,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);
-
+async fn test_mixed_evm_substrate_transactions(client: Arc<WsClient>) -> anyhow::Result<()> {
 	let num_evm_txs = 10;
 	let num_substrate_txs = 7;
 
@@ -816,10 +817,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);
+async fn test_runtime_pallets_address_upload_code(client: Arc<WsClient>) -> anyhow::Result<()> {
 	let (node_client, node_rpc_client, _) =
 		client::connect(SharedResources::node_rpc_url()).await?;