[eth-rpc] Fixes (#6317)

Various fixes for the release of eth-rpc & ah-westend-runtime

- Bump asset-hub westend spec version
- Fix the status of the Receipt to properly report failed transactions
- Fix value conversion between native and eth decimal representation

---------

Co-authored-by: GitHub Action <[email protected]>

cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs

@@ -124,7 +124,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: create_runtime_str!("westmint"),
 	impl_name: create_runtime_str!("westmint"),
 	authoring_version: 1,
-	spec_version: 1_016_002,
+	spec_version: 1_016_004,
 	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 16,
@@ -968,6 +968,7 @@ impl pallet_revive::Config for Runtime {
 	type Debug = ();
 	type Xcm = pallet_xcm::Pallet<Self>;
 	type ChainId = ConstU64<420_420_421>;
+	type NativeToEthRatio = ConstU32<1_000_000>; // 10^(18 - 12) Eth is 10^18, Native is 10^12.
 }
 
 impl TryFrom<RuntimeCall> for pallet_revive::Call<Runtime> {

prdoc/pr_6317.prdoc

@@ -0,0 +1,12 @@
+title: eth-rpc fixes
+doc:
+- audience: Runtime Dev
+  description: |
+    Various fixes for the release of eth-rpc & ah-westend-runtime:
+    - Bump asset-hub westend spec version
+    - Fix the status of the Receipt to properly report failed transactions
+    - Fix value conversion between native and eth decimal representation
+
+crates:
+- name: asset-hub-westend-runtime
+  bump: patch

substrate/bin/node/runtime/src/lib.rs

@@ -1427,6 +1427,7 @@ impl pallet_revive::Config for Runtime {
 	type Debug = ();
 	type Xcm = ();
 	type ChainId = ConstU64<420_420_420>;
+	type NativeToEthRatio = ConstU32<1_000_000>; // 10^(18 - 12) Eth is 10^18, Native is 10^12.
 }
 
 impl pallet_sudo::Config for Runtime {

substrate/client/service/src/lib.rs

@@ -98,7 +98,9 @@ pub use sc_transaction_pool::TransactionPoolOptions;
 pub use sc_transaction_pool_api::{error::IntoPoolError, InPoolTransaction, TransactionPool};
 #[doc(hidden)]
 pub use std::{ops::Deref, result::Result, sync::Arc};
-pub use task_manager::{SpawnTaskHandle, Task, TaskManager, TaskRegistry, DEFAULT_GROUP_NAME};
+pub use task_manager::{
+	SpawnEssentialTaskHandle, SpawnTaskHandle, Task, TaskManager, TaskRegistry, DEFAULT_GROUP_NAME,
+};
 use tokio::runtime::Handle;
 
 const DEFAULT_PROTOCOL_ID: &str = "sup";

substrate/frame/revive/rpc/Cargo.toml

@@ -77,3 +77,4 @@ example = ["hex", "hex-literal", "rlp", "secp256k1", "subxt-signer"]
 hex-literal = { workspace = true }
 pallet-revive-fixtures = { workspace = true }
 substrate-cli-test-utils = { workspace = true }
+subxt-signer = { workspace = true, features = ["unstable-eth"] }

substrate/frame/revive/rpc/examples/rust/transfer.rs

@@ -26,29 +26,30 @@ async fn main() -> anyhow::Result<()> {
 	let alith = Account::default();
 	let client = HttpClientBuilder::default().build("http://localhost:8545")?;
 
-	let baltathar = Account::from(subxt_signer::eth::dev::baltathar());
-	let value = 1_000_000_000_000_000_000u128.into(); // 1 ETH
+	let ethan = Account::from(subxt_signer::eth::dev::ethan());
+	let value = 1_000_000_000_000_000_000_000u128.into();
 
 	let print_balance = || async {
 		let balance = client.get_balance(alith.address(), BlockTag::Latest.into()).await?;
 		println!("Alith     {:?} balance: {balance:?}", alith.address());
-		let balance = client.get_balance(baltathar.address(), BlockTag::Latest.into()).await?;
-		println!("Baltathar {:?} balance: {balance:?}", baltathar.address());
+		let balance = client.get_balance(ethan.address(), BlockTag::Latest.into()).await?;
+		println!("ethan {:?} balance: {balance:?}", ethan.address());
 		anyhow::Result::<()>::Ok(())
 	};
 
 	print_balance().await?;
 	println!("\n\n=== Transferring  ===\n\n");
 
 	let hash =
-		send_transaction(&alith, &client, value, Bytes::default(), Some(baltathar.address()))
-			.await?;
+		send_transaction(&alith, &client, value, Bytes::default(), Some(ethan.address())).await?;
 	println!("Transaction hash: {hash:?}");
 
-	let ReceiptInfo { block_number, gas_used, .. } = wait_for_receipt(&client, hash).await?;
+	let ReceiptInfo { block_number, gas_used, status, .. } =
+		wait_for_receipt(&client, hash).await?;
 	println!("Receipt: ");
 	println!("- Block number: {block_number}");
 	println!("- Gas used: {gas_used}");
+	println!("- Success: {status:?}");
 
 	print_balance().await?;
 	Ok(())

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

@@ -109,11 +109,11 @@ pub fn run(cmd: CliCommand) -> anyhow::Result<()> {
 	let tokio_handle = tokio_runtime.handle();
 	let signals = tokio_runtime.block_on(async { Signals::capture() })?;
 	let mut task_manager = TaskManager::new(tokio_handle.clone(), prometheus_registry)?;
-	let spawn_handle = task_manager.spawn_handle();
+	let essential_spawn_handle = task_manager.spawn_essential_handle();
 
 	let gen_rpc_module = || {
 		let signals = tokio_runtime.block_on(async { Signals::capture() })?;
-		let fut = Client::from_url(&node_rpc_url, &spawn_handle).fuse();
+		let fut = Client::from_url(&node_rpc_url, &essential_spawn_handle).fuse();
 		pin_mut!(fut);
 
 		match tokio_handle.block_on(signals.try_until_signal(fut)) {
@@ -125,7 +125,7 @@ pub fn run(cmd: CliCommand) -> anyhow::Result<()> {
 
 	// Prometheus metrics.
 	if let Some(PrometheusConfig { port, registry }) = prometheus_config.clone() {
-		spawn_handle.spawn(
+		task_manager.spawn_handle().spawn(
 			"prometheus-endpoint",
 			None,
 			prometheus_endpoint::init_prometheus(port, registry).map(drop),

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

@@ -35,7 +35,6 @@ use pallet_revive::{
 	},
 	EthContractResult,
 };
-use sc_service::SpawnTaskHandle;
 use sp_runtime::traits::{BlakeTwo256, Hash};
 use sp_weights::Weight;
 use std::{
@@ -145,9 +144,6 @@ pub enum ClientError {
 	/// The transaction fee could not be found
 	#[error("TransactionFeePaid event not found")]
 	TxFeeNotFound,
-	/// The token decimals property was not found
-	#[error("tokenDecimals not found in properties")]
-	TokenDecimalsNotFound,
 	/// The cache is empty.
 	#[error("Cache is empty")]
 	CacheEmpty,
@@ -165,7 +161,7 @@ impl From<ClientError> for ErrorObjectOwned {
 
 /// The number of recent blocks maintained by the cache.
 /// For each block in the cache, we also store the EVM transaction receipts.
-pub const CACHE_SIZE: usize = 10;
+pub const CACHE_SIZE: usize = 256;
 
 impl<const N: usize> BlockCache<N> {
 	fn latest_block(&self) -> Option<&Arc<SubstrateBlock>> {
@@ -228,7 +224,7 @@ impl ClientInner {
 		let rpc = LegacyRpcMethods::<SrcChainConfig>::new(RpcClient::new(rpc_client.clone()));
 
 		let (native_to_evm_ratio, chain_id, max_block_weight) =
-			tokio::try_join!(native_to_evm_ratio(&rpc), chain_id(&api), max_block_weight(&api))?;
+			tokio::try_join!(native_to_evm_ratio(&api), chain_id(&api), max_block_weight(&api))?;
 
 		Ok(Self { api, rpc_client, rpc, cache, chain_id, max_block_weight, native_to_evm_ratio })
 	}
@@ -238,6 +234,11 @@ impl ClientInner {
 		value.saturating_mul(self.native_to_evm_ratio)
 	}
 
+	/// Convert an evm balance to a native balance.
+	fn evm_to_native_decimals(&self, value: U256) -> U256 {
+		value / self.native_to_evm_ratio
+	}
+
 	/// Get the receipt infos from the extrinsics in a block.
 	async fn receipt_infos(
 		&self,
@@ -274,7 +275,7 @@ impl ClientInner {
 					.checked_div(gas_price.as_u128())
 					.unwrap_or_default();
 
-				let success = events.find_first::<ExtrinsicSuccess>().is_ok();
+				let success = events.has::<ExtrinsicSuccess>()?;
 				let transaction_index = ext.index();
 				let transaction_hash = BlakeTwo256::hash(&Vec::from(ext.bytes()).encode());
 				let block_hash = block.hash();
@@ -319,16 +320,10 @@ async fn max_block_weight(api: &OnlineClient<SrcChainConfig>) -> Result<Weight,
 }
 
 /// Fetch the native to EVM ratio from the substrate chain.
-async fn native_to_evm_ratio(rpc: &LegacyRpcMethods<SrcChainConfig>) -> Result<U256, ClientError> {
-	let props = rpc.system_properties().await?;
-	let eth_decimals = U256::from(18u32);
-	let native_decimals: U256 = props
-		.get("tokenDecimals")
-		.and_then(|v| v.as_number()?.as_u64())
-		.ok_or(ClientError::TokenDecimalsNotFound)?
-		.into();
-
-	Ok(U256::from(10u32).pow(eth_decimals - native_decimals))
+async fn native_to_evm_ratio(api: &OnlineClient<SrcChainConfig>) -> Result<U256, ClientError> {
+	let query = subxt_client::constants().revive().native_to_eth_ratio();
+	let ratio = api.constants().at(&query)?;
+	Ok(U256::from(ratio))
 }
 
 /// Extract the block timestamp.
@@ -344,7 +339,10 @@ async fn extract_block_timestamp(block: &SubstrateBlock) -> Option<u64> {
 impl Client {
 	/// Create a new client instance.
 	/// The client will subscribe to new blocks and maintain a cache of [`CACHE_SIZE`] blocks.
-	pub async fn from_url(url: &str, spawn_handle: &SpawnTaskHandle) -> Result<Self, ClientError> {
+	pub async fn from_url(
+		url: &str,
+		spawn_handle: &sc_service::SpawnEssentialTaskHandle,
+	) -> Result<Self, ClientError> {
 		log::info!(target: LOG_TARGET, "Connecting to node at: {url} ...");
 		let inner: Arc<ClientInner> = Arc::new(ClientInner::from_url(url).await?);
 		log::info!(target: LOG_TARGET, "Connected to node at: {url}");
@@ -619,9 +617,10 @@ impl Client {
 		block: BlockNumberOrTagOrHash,
 	) -> Result<EthContractResult<Balance>, ClientError> {
 		let runtime_api = self.runtime_api(&block).await?;
-		let value = tx
-			.value
-			.unwrap_or_default()
+
+		let value = self
+			.inner
+			.evm_to_native_decimals(tx.value.unwrap_or_default())
 			.try_into()
 			.map_err(|_| ClientError::ConversionFailed)?;
 

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

@@ -50,16 +50,14 @@ async fn ws_client_with_retry(url: &str) -> WsClient {
 async fn test_jsonrpsee_server() -> anyhow::Result<()> {
 	// Start the node.
 	let _ = thread::spawn(move || {
-		match start_node_inline(vec![
+		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=debug",
 		]) {
-			Ok(_) => {},
-			Err(e) => {
-				panic!("Node exited with error: {}", e);
-			},
+			panic!("Node exited with error: {e:?}");
 		}
 	});
 
@@ -69,17 +67,36 @@ async fn test_jsonrpsee_server() -> anyhow::Result<()> {
 		"--rpc-port=45788",
 		"--node-rpc-url=ws://localhost:45789",
 		"--no-prometheus",
+		"-linfo,eth-rpc=debug",
 	]);
-	let _ = thread::spawn(move || match cli::run(args) {
-		Ok(_) => {},
-		Err(e) => {
-			panic!("eth-rpc exited with error: {}", e);
-		},
+	let _ = thread::spawn(move || {
+		if let Err(e) = cli::run(args) {
+			panic!("eth-rpc exited with error: {e:?}");
+		}
 	});
 
 	let client = ws_client_with_retry("ws://localhost:45788").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 value = 1_000_000_000_000_000_000_000u128.into();
+	let hash =
+		send_transaction(&account, &client, value, Bytes::default(), Some(ethan.address())).await?;
+
+	let receipt = wait_for_receipt(&client, hash).await?;
+	assert_eq!(
+		Some(ethan.address()),
+		receipt.to,
+		"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.");
+
 	// Deploy contract
 	let data = b"hello world".to_vec();
 	let value = U256::from(5_000_000_000_000u128);
@@ -96,11 +113,7 @@ async fn test_jsonrpsee_server() -> anyhow::Result<()> {
 	);
 
 	let balance = client.get_balance(contract_address, BlockTag::Latest.into()).await?;
-	assert_eq!(
-		value * 1_000_000,
-		balance,
-		"Contract balance should be the same as the value sent."
-	);
+	assert_eq!(value, balance, "Contract balance should be the same as the value sent.");
 
 	// Call contract
 	let hash =

substrate/frame/revive/src/evm/runtime.rs

@@ -313,10 +313,14 @@ pub trait EthExtra {
 			return Err(InvalidTransaction::Call);
 		}
 
+		let value = (value / U256::from(<Self::Config as crate::Config>::NativeToEthRatio::get()))
+			.try_into()
+			.map_err(|_| InvalidTransaction::Call)?;
+
 		let call = if let Some(dest) = to {
 			crate::Call::call::<Self::Config> {
 				dest,
-				value: value.try_into().map_err(|_| InvalidTransaction::Call)?,
+				value,
 				gas_limit,
 				storage_deposit_limit,
 				data: input.0,
@@ -336,7 +340,7 @@ pub trait EthExtra {
 			};
 
 			crate::Call::instantiate_with_code::<Self::Config> {
-				value: value.try_into().map_err(|_| InvalidTransaction::Call)?,
+				value,
 				gas_limit,
 				storage_deposit_limit,
 				code: code.to_vec(),
@@ -370,7 +374,7 @@ pub trait EthExtra {
 				Default::default(),
 			)
 			.into();
-		log::debug!(target: LOG_TARGET, "try_into_checked_extrinsic: encoded_len: {encoded_len:?} actual_fee: {actual_fee:?} eth_fee: {eth_fee:?}");
+		log::trace!(target: LOG_TARGET, "try_into_checked_extrinsic: encoded_len: {encoded_len:?} actual_fee: {actual_fee:?} eth_fee: {eth_fee:?}");
 
 		if eth_fee < actual_fee {
 			log::debug!(target: LOG_TARGET, "fees {eth_fee:?} too low for the extrinsic {actual_fee:?}");
@@ -381,10 +385,10 @@ pub trait EthExtra {
 		let max = actual_fee.max(eth_fee_no_tip);
 		let diff = Percent::from_rational(max - min, min);
 		if diff > Percent::from_percent(10) {
-			log::debug!(target: LOG_TARGET, "Difference between the extrinsic fees {actual_fee:?} and the Ethereum gas fees {eth_fee_no_tip:?} should be no more than 10% got {diff:?}");
+			log::trace!(target: LOG_TARGET, "Difference between the extrinsic fees {actual_fee:?} and the Ethereum gas fees {eth_fee_no_tip:?} should be no more than 10% got {diff:?}");
 			return Err(InvalidTransaction::Call.into())
 		} else {
-			log::debug!(target: LOG_TARGET, "Difference between the extrinsic fees {actual_fee:?} and the Ethereum gas fees {eth_fee_no_tip:?}:  {diff:?}");
+			log::trace!(target: LOG_TARGET, "Difference between the extrinsic fees {actual_fee:?} and the Ethereum gas fees {eth_fee_no_tip:?}:  {diff:?}");
 		}
 
 		let tip = eth_fee.saturating_sub(eth_fee_no_tip);

substrate/frame/revive/src/exec.rs

@@ -1268,8 +1268,10 @@ where
 	fn transfer(from: &T::AccountId, to: &T::AccountId, value: BalanceOf<T>) -> ExecResult {
 		// this avoids events to be emitted for zero balance transfers
 		if !value.is_zero() {
-			T::Currency::transfer(from, to, value, Preservation::Preserve)
-				.map_err(|_| Error::<T>::TransferFailed)?;
+			T::Currency::transfer(from, to, value, Preservation::Preserve).map_err(|err| {
+				log::debug!(target: LOG_TARGET, "Transfer of {value:?} from {from:?} to {to:?} failed: {err:?}");
+				Error::<T>::TransferFailed
+			})?;
 		}
 		Ok(Default::default())
 	}