[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,9 @@ 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

@@ -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:?}");
 			}
@@ -305,8 +302,47 @@ async fn verify_transactions_in_blocks<Client: EthRpcClient + Sync>(
 }
 
 #[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<Box<dyn std::future::Future<Output = anyhow::Result<()>>>>;
+	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())
+		}),
+	];
+
+	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());
@@ -332,9 +368,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();
 
@@ -422,9 +456,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();
@@ -443,7 +475,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,9 +488,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 +523,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 +570,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 +625,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 +656,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 +671,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 +687,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 +708,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 +758,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 +833,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?;