diff --git a/.config/nextest.toml b/.config/nextest.toml index 1e18f8b5589c..912bf2514a77 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -124,3 +124,10 @@ 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' diff --git a/Cargo.lock b/Cargo.lock index fdaaf2f3b45f..0248fe138437 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -14699,6 +14699,7 @@ dependencies = [ "sp-crypto-hashing 0.1.0", "sp-runtime 31.0.1", "sp-weights 27.0.0", + "static_init", "substrate-cli-test-utils", "substrate-prometheus-endpoint", "subxt", diff --git a/prdoc/pr_6453.prdoc b/prdoc/pr_6453.prdoc new file mode 100644 index 000000000000..5df44f11296d --- /dev/null +++ b/prdoc/pr_6453.prdoc @@ -0,0 +1,7 @@ +title: '[pallet-revive] breakdown integration tests' +doc: +- audience: Runtime Dev + description: Break down the single integration tests into multiple tests, use keccak-256 for tx.hash +crates: +- name: pallet-revive-eth-rpc + bump: minor diff --git a/substrate/frame/revive/rpc/Cargo.toml b/substrate/frame/revive/rpc/Cargo.toml index 9faf05885dfe..9f89b74c668f 100644 --- a/substrate/frame/revive/rpc/Cargo.toml +++ b/substrate/frame/revive/rpc/Cargo.toml @@ -74,6 +74,7 @@ ethabi = { version = "18.0.0" } example = ["hex-literal", "rlp", "secp256k1", "subxt-signer"] [dev-dependencies] +static_init = { workspace = true } hex-literal = { workspace = true } pallet-revive-fixtures = { workspace = true } substrate-cli-test-utils = { workspace = true } diff --git a/substrate/frame/revive/rpc/examples/js/src/lib.ts b/substrate/frame/revive/rpc/examples/js/src/lib.ts index 9db9d36cf56c..7f6fc19aea75 100644 --- a/substrate/frame/revive/rpc/examples/js/src/lib.ts +++ b/substrate/frame/revive/rpc/examples/js/src/lib.ts @@ -4,15 +4,34 @@ import { JsonRpcProvider, TransactionReceipt, TransactionResponse, + Wallet, } from 'ethers' import { readFileSync } from 'node:fs' import type { compile } from '@parity/revive' import { spawn } from 'node:child_process' +import { parseArgs } from 'node:util' type CompileOutput = Awaited> type Abi = CompileOutput['contracts'][string][string]['abi'] -const geth = process.argv.includes('--geth') +const { + values: { geth, westend, ['private-key']: privateKey }, +} = parseArgs({ + args: process.argv.slice(2), + options: { + ['private-key']: { + type: 'string', + short: 'k', + }, + geth: { + type: 'boolean', + }, + westend: { + type: 'boolean', + }, + }, +}) + if (geth) { console.log('Testing with Geth') const child = spawn( @@ -31,13 +50,15 @@ if (geth) { ) process.on('exit', () => child.kill()) - child.unref() await new Promise((resolve) => setTimeout(resolve, 500)) } -const provider = new JsonRpcProvider('http://localhost:8545') -const signer = await provider.getSigner() +const provider = new JsonRpcProvider( + westend ? 'https://westend-asset-hub-eth-rpc.polkadot.io' : 'http://localhost:8545' +) + +const signer = privateKey ? new Wallet(privateKey, provider) : await provider.getSigner() console.log(`Signer address: ${await signer.getAddress()}, Nonce: ${await signer.getNonce()}`) /** diff --git a/substrate/frame/revive/rpc/src/client.rs b/substrate/frame/revive/rpc/src/client.rs index ba2398141bec..bc4f59b5e26e 100644 --- a/substrate/frame/revive/rpc/src/client.rs +++ b/substrate/frame/revive/rpc/src/client.rs @@ -35,6 +35,7 @@ use pallet_revive::{ }, EthContractResult, }; +use sp_core::keccak_256; use sp_weights::Weight; use std::{ collections::{HashMap, VecDeque}, @@ -278,6 +279,7 @@ impl ClientInner { // Filter extrinsics from pallet_revive let extrinsics = extrinsics.iter().flat_map(|ext| { let call = ext.as_extrinsic::().ok()??; + let transaction_hash = H256(keccak_256(&call.payload)); let tx = rlp::decode::(&call.payload).ok()?; let from = tx.recover_eth_address().ok()?; let contract_address = if tx.transaction_legacy_unsigned.to.is_none() { @@ -286,12 +288,12 @@ impl ClientInner { None }; - Some((from, tx, contract_address, ext)) + Some((from, tx, transaction_hash, contract_address, ext)) }); // Map each extrinsic to a receipt stream::iter(extrinsics) - .map(|(from, tx, contract_address, ext)| async move { + .map(|(from, tx, transaction_hash, contract_address, ext)| async move { let events = ext.events().await?; let tx_fees = events.find_first::()?.ok_or(ClientError::TxFeeNotFound)?; @@ -305,7 +307,6 @@ impl ClientInner { let transaction_index = ext.index(); let block_hash = block.hash(); let block_number = block.number().into(); - let transaction_hash= ext.hash(); // get logs from ContractEmitted event let logs = events.iter() diff --git a/substrate/frame/revive/rpc/src/lib.rs b/substrate/frame/revive/rpc/src/lib.rs index f6709edc96c9..8d9d6fab829e 100644 --- a/substrate/frame/revive/rpc/src/lib.rs +++ b/substrate/frame/revive/rpc/src/lib.rs @@ -24,7 +24,7 @@ use jsonrpsee::{ types::{ErrorCode, ErrorObjectOwned}, }; use pallet_revive::{evm::*, EthContractResult}; -use sp_core::{H160, H256, U256}; +use sp_core::{keccak_256, H160, H256, U256}; use thiserror::Error; pub mod cli; @@ -135,6 +135,8 @@ impl EthRpcServer for EthRpcServerImpl { } async fn send_raw_transaction(&self, transaction: Bytes) -> RpcResult { + let hash = H256(keccak_256(&transaction.0)); + let tx = rlp::decode::(&transaction.0).map_err(|err| { log::debug!(target: LOG_TARGET, "Failed to decode transaction: {err:?}"); EthRpcError::from(err) @@ -167,7 +169,7 @@ impl EthRpcServer for EthRpcServerImpl { gas_required.into(), storage_deposit, ); - let hash = self.client.submit(call).await?; + self.client.submit(call).await?; log::debug!(target: LOG_TARGET, "send_raw_transaction hash: {hash:?}"); Ok(hash) } diff --git a/substrate/frame/revive/rpc/src/tests.rs b/substrate/frame/revive/rpc/src/tests.rs index 537cfd07964f..3d2cbe42be8e 100644 --- a/substrate/frame/revive/rpc/src/tests.rs +++ b/substrate/frame/revive/rpc/src/tests.rs @@ -27,6 +27,7 @@ use pallet_revive::{ create1, evm::{Account, BlockTag, U256}, }; +use static_init::dynamic; use std::thread; use substrate_cli_test_utils::*; @@ -60,6 +61,52 @@ fn get_contract(name: &str) -> anyhow::Result<(Vec, ethabi::Contract)> { Ok((bytecode, contract)) } +struct SharedResources { + _node_handle: std::thread::JoinHandle<()>, + _rpc_handle: std::thread::JoinHandle<()>, +} + +impl SharedResources { + fn start() -> Self { + // Start the 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", + ]) { + panic!("Node exited with error: {e:?}"); + } + }); + + // Start the rpc server. + let args = CliCommand::parse_from([ + "--dev", + "--rpc-port=45788", + "--node-rpc-url=ws://localhost:45789", + "--no-prometheus", + "-linfo,eth-rpc=debug", + ]); + + let _rpc_handle = thread::spawn(move || { + if let Err(e) = cli::run(args) { + panic!("eth-rpc exited with error: {e:?}"); + } + }); + + Self { _node_handle, _rpc_handle } + } + + async fn client() -> WsClient { + ws_client_with_retry("ws://localhost:45788").await + } +} + +#[dynamic(lazy)] +static mut SHARED_RESOURCES: SharedResources = SharedResources::start(); + macro_rules! unwrap_call_err( ($err:expr) => { match $err.downcast_ref::().unwrap() { @@ -70,41 +117,42 @@ macro_rules! unwrap_call_err( ); #[tokio::test] -async fn test_jsonrpsee_server() -> anyhow::Result<()> { - // Start the node. - let _ = 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", - ]) { - panic!("Node exited with error: {e:?}"); - } - }); - - // Start the rpc server. - let args = CliCommand::parse_from([ - "--dev", - "--rpc-port=45788", - "--node-rpc-url=ws://localhost:45789", - "--no-prometheus", - "-linfo,eth-rpc=debug", - ]); - let _ = thread::spawn(move || { - if let Err(e) = cli::run(args) { - panic!("eth-rpc exited with error: {e:?}"); - } - }); +async fn transfer() -> anyhow::Result<()> { + let _lock = SHARED_RESOURCES.write(); + let client = SharedResources::client().await; - let client = ws_client_with_retry("ws://localhost:45788").await; + let ethan = Account::from(subxt_signer::eth::dev::ethan()); + let initial_balance = client.get_balance(ethan.address(), BlockTag::Latest.into()).await?; + + let value = 1_000_000_000_000_000_000_000u128.into(); + let hash = TransactionBuilder::default() + .value(value) + .to(ethan.address()) + .send(&client) + .await?; + + let receipt = wait_for_successful_receipt(&client, hash).await?; + assert_eq!( + Some(ethan.address()), + receipt.to, + "Receipt should have the correct contract address." + ); + + let increase = + client.get_balance(ethan.address(), BlockTag::Latest.into()).await? - initial_balance; + assert_eq!(value, increase); + Ok(()) +} + +#[tokio::test] +async fn deploy_and_call() -> anyhow::Result<()> { + let _lock = SHARED_RESOURCES.write(); + let client = SharedResources::client().await; let account = Account::default(); // Balance transfer let ethan = Account::from(subxt_signer::eth::dev::ethan()); - let ethan_balance = client.get_balance(ethan.address(), BlockTag::Latest.into()).await?; - assert_eq!(U256::zero(), ethan_balance); + let initial_balance = client.get_balance(ethan.address(), BlockTag::Latest.into()).await?; let value = 1_000_000_000_000_000_000_000u128.into(); let hash = TransactionBuilder::default() @@ -120,8 +168,8 @@ async fn test_jsonrpsee_server() -> anyhow::Result<()> { "Receipt should have the correct contract address." ); - let ethan_balance = client.get_balance(ethan.address(), BlockTag::Latest.into()).await?; - assert_eq!(value, ethan_balance, "ethan's balance should be the same as the value sent."); + let updated_balance = client.get_balance(ethan.address(), BlockTag::Latest.into()).await?; + assert_eq!(value, updated_balance - initial_balance); // Deploy contract let data = b"hello world".to_vec(); @@ -169,15 +217,19 @@ async fn test_jsonrpsee_server() -> anyhow::Result<()> { wait_for_successful_receipt(&client, hash).await?; let increase = client.get_balance(contract_address, BlockTag::Latest.into()).await? - balance; assert_eq!(value, increase, "contract's balance should have increased by the value sent."); + Ok(()) +} - // Deploy revert +#[tokio::test] +async fn revert_call() -> anyhow::Result<()> { + let _lock = SHARED_RESOURCES.write(); + let client = SharedResources::client().await; let (bytecode, contract) = get_contract("revert")?; let receipt = TransactionBuilder::default() .input(contract.constructor.clone().unwrap().encode_input(bytecode, &[]).unwrap()) .send_and_wait_for_receipt(&client) .await?; - // Call doRevert let err = TransactionBuilder::default() .to(receipt.contract_address.unwrap()) .input(contract.function("doRevert")?.encode_input(&[])?.to_vec()) @@ -187,25 +239,36 @@ async fn test_jsonrpsee_server() -> anyhow::Result<()> { let call_err = unwrap_call_err!(err.source().unwrap()); assert_eq!(call_err.message(), "Execution reverted: revert message"); + Ok(()) +} - // Deploy event +#[tokio::test] +async fn event_logs() -> anyhow::Result<()> { + let _lock = SHARED_RESOURCES.write(); + let client = SharedResources::client().await; let (bytecode, contract) = get_contract("event")?; let receipt = TransactionBuilder::default() .input(bytecode) .send_and_wait_for_receipt(&client) .await?; - // Call triggerEvent let receipt = TransactionBuilder::default() .to(receipt.contract_address.unwrap()) .input(contract.function("triggerEvent")?.encode_input(&[])?.to_vec()) .send_and_wait_for_receipt(&client) .await?; assert_eq!(receipt.logs.len(), 1, "There should be one log."); + Ok(()) +} + +#[tokio::test] +async fn invalid_transaction() -> anyhow::Result<()> { + let _lock = SHARED_RESOURCES.write(); + let client = SharedResources::client().await; + let ethan = Account::from(subxt_signer::eth::dev::ethan()); - // Invalid transaction let err = TransactionBuilder::default() - .value(value) + .value(U256::from(1_000_000_000_000u128)) .to(ethan.address()) .mutate(|tx| tx.chain_id = Some(42u32.into())) .send(&client)