From 3b54398606f3db803a2e5af02e7b0c3f3a0c7a89 Mon Sep 17 00:00:00 2001 From: Max Conway Date: Mon, 3 Feb 2025 11:13:37 +0000 Subject: [PATCH] Correct log index returns in etherium apis --- zilliqa/src/api/eth.rs | 242 +++++++++++++++++++++------------------- zilliqa/src/api/ots.rs | 28 ++--- zilliqa/tests/it/eth.rs | 51 +++++++++ 3 files changed, 196 insertions(+), 125 deletions(-) diff --git a/zilliqa/src/api/eth.rs b/zilliqa/src/api/eth.rs index c4f29dfdc..49ac78b14 100644 --- a/zilliqa/src/api/eth.rs +++ b/zilliqa/src/api/eth.rs @@ -274,27 +274,126 @@ fn get_balance(params: Params, node: &Arc>) -> Result { .to_hex()) } -fn get_block_receipts(params: Params, node: &Arc>) -> Result> { - let block_id: BlockId = params.one()?; +pub fn get_block_transaction_receipts_inner( + node: &MutexGuard, + block_id: impl Into, +) -> Result> { + let Some(block) = node.get_block(block_id)? else { + return Err(anyhow!("Block not found")); + }; - let node = node.lock().unwrap(); + let mut log_index = 0; + let mut receipts = Vec::new(); - // Get the block - let block = node - .get_block(block_id)? - .ok_or_else(|| anyhow!("block not found"))?; + for (transaction_index, tx_hash) in block.transactions.iter().enumerate() { + let Some(signed_transaction) = node.get_transaction_by_hash(*tx_hash)? else { + warn!( + "Failed to get TX by hash when getting TX receipt! {}", + tx_hash + ); + continue; + }; - // Get receipts for all transactions in the block - let mut receipts = Vec::new(); - for tx_hash in block.transactions { - if let Some(receipt) = get_transaction_receipt_inner(tx_hash, &node)? { - receipts.push(receipt); + let Some(receipt) = node.get_transaction_receipt(*tx_hash)? else { + debug!( + "Failed to get TX receipt when getting TX receipt! {}", + tx_hash + ); + continue; + }; + + debug!( + "get_block_transaction_receipts: hash: {:?} result: {:?}", + tx_hash, receipt + ); + + let mut logs_bloom = [0; 256]; + + let mut logs = Vec::new(); + for log in receipt.logs { + let log = match log { + Log::Evm(log) => log, + Log::Scilla(log) => log.into_evm(), + }; + let log = eth::Log::new( + log, + log_index, + transaction_index, + *tx_hash, + block.number(), + block.hash(), + ); + log_index += 1; + log.bloom(&mut logs_bloom); + logs.push(log); } + + let is_zilliqa_txn = matches!(signed_transaction.tx, SignedTransaction::Zilliqa { .. }); + + let from = signed_transaction.signer; + let v = signed_transaction.tx.sig_v(); + let r = signed_transaction.tx.sig_r(); + let s = signed_transaction.tx.sig_s(); + let transaction = signed_transaction.tx.into_transaction(); + + // Temporary on-the-fly fix for returning proper contract address for deployments from zil transactions + let contract_address = { + if is_zilliqa_txn && transaction.to_addr().is_none() { + let mut hasher = Sha256::new(); + hasher.update(signed_transaction.signer.as_slice()); + hasher.update(transaction.nonce().unwrap().to_be_bytes()); + let hashed = hasher.finalize(); + Some(Address::from_slice(&hashed[12..])) + } else { + receipt.contract_address + } + }; + + let receipt = eth::TransactionReceipt { + transaction_hash: (*tx_hash).into(), + transaction_index: transaction_index as u64, + block_hash: block.hash().into(), + block_number: block.number(), + from, + to: transaction.to_addr(), + cumulative_gas_used: receipt.cumulative_gas_used, + effective_gas_price: transaction.max_fee_per_gas(), + gas_used: receipt.gas_used, + contract_address, + logs, + logs_bloom, + ty: 0, + status: receipt.success, + v, + r, + s, + }; + + receipts.push(receipt); } Ok(receipts) } +// This has to iterate through a whole block, so get_block_transaction_receipts_inner is more efficient for multiple receipts +pub fn get_transaction_receipt_inner_slow( + node: &MutexGuard, + block_id: impl Into, + txn_hash: Hash, +) -> Result> { + let receipts = get_block_transaction_receipts_inner(node, block_id)?; + Ok(receipts + .into_iter() + .find(|r| r.transaction_hash == txn_hash.as_bytes())) +} + +fn get_block_receipts(params: Params, node: &Arc>) -> Result> { + let block_id: BlockId = params.one()?; + let node = node.lock().unwrap(); + + get_block_transaction_receipts_inner(&node, block_id) +} + fn get_code(params: Params, node: &Arc>) -> Result { let mut params = params.sequence(); let address: Address = params.next()?; @@ -703,103 +802,6 @@ pub(super) fn get_transaction_inner( Ok(Some(eth::Transaction::new(tx, block))) } -pub(super) fn get_transaction_receipt_inner( - hash: Hash, - node: &MutexGuard, -) -> Result> { - let Some(signed_transaction) = node.get_transaction_by_hash(hash)? else { - warn!("Failed to get TX by hash when getting TX receipt! {}", hash); - return Ok(None); - }; - // TODO: Return error if receipt or block does not exist. - - let Some(receipt) = node.get_transaction_receipt(hash)? else { - debug!("Failed to get TX receipt when getting TX receipt! {}", hash); - return Ok(None); - }; - - debug!( - "get_transaction_receipt_inner: hash: {:?} result: {:?}", - hash, receipt - ); - - let Some(block) = node.get_block(receipt.block_hash)? else { - warn!("Failed to get block when getting TX receipt! {}", hash); - return Ok(None); - }; - - let transaction_index = block.transactions.iter().position(|t| *t == hash).unwrap(); - - let mut logs_bloom = [0; 256]; - - let logs = receipt - .logs - .into_iter() - .map(|log| match log { - Log::Evm(log) => log, - Log::Scilla(log) => log.into_evm(), - }) - .enumerate() - .map(|(log_index, log)| { - let log = eth::Log::new( - log, - log_index, - transaction_index, - hash, - block.number(), - block.hash(), - ); - - log.bloom(&mut logs_bloom); - - log - }) - .collect(); - - let is_zilliqa_txn = matches!(signed_transaction.tx, SignedTransaction::Zilliqa { .. }); - - let from = signed_transaction.signer; - let v = signed_transaction.tx.sig_v(); - let r = signed_transaction.tx.sig_r(); - let s = signed_transaction.tx.sig_s(); - let transaction = signed_transaction.tx.into_transaction(); - - // Temporary on-the-fly fix for returning proper contract address for deployments from zil transactions - let contract_address = { - if is_zilliqa_txn && transaction.to_addr().is_none() { - let mut hasher = Sha256::new(); - hasher.update(signed_transaction.signer.as_slice()); - hasher.update(transaction.nonce().unwrap().to_be_bytes()); - let hashed = hasher.finalize(); - Some(Address::from_slice(&hashed[12..])) - } else { - receipt.contract_address - } - }; - - let receipt = eth::TransactionReceipt { - transaction_hash: hash.into(), - transaction_index: transaction_index as u64, - block_hash: block.hash().into(), - block_number: block.number(), - from, - to: transaction.to_addr(), - cumulative_gas_used: receipt.cumulative_gas_used, - effective_gas_price: transaction.max_fee_per_gas(), - gas_used: receipt.gas_used, - contract_address, - logs, - logs_bloom, - ty: 0, - status: receipt.success, - v, - r, - s, - }; - - Ok(Some(receipt)) -} - fn get_transaction_receipt( params: Params, node: &Arc>, @@ -808,7 +810,11 @@ fn get_transaction_receipt( let hash: B256 = params.one()?; let hash: Hash = hash.into(); let node = node.lock().unwrap(); - get_transaction_receipt_inner(hash, &node) + let block_hash = match node.get_transaction_receipt(hash)? { + Some(receipt) => receipt.block_hash, + None => return Ok(None), + }; + get_transaction_receipt_inner_slow(&node, block_hash, hash) } fn send_raw_transaction(params: Params, node: &Arc>) -> Result { @@ -950,7 +956,19 @@ async fn subscribe( if !filter.filter_block_hash(receipt.block_hash.into()) { continue; } - for (log_index, log) in receipt.logs.into_iter().enumerate() { + + // We track log index plus one because we have to increment before we use the log index, and log indexes are 0-based. + let mut log_index_plus_one: i64 = get_block_transaction_receipts_inner( + &node.lock().unwrap(), + receipt.block_hash, + )? + .iter() + .take_while(|x| x.transaction_index < receipt.index) + .map(|x| x.logs.len()) + .sum::() as i64; + + for log in receipt.logs.into_iter() { + log_index_plus_one += 1; // Only consider EVM logs let Log::Evm(log) = log else { continue; @@ -992,7 +1010,7 @@ async fn subscribe( ), transaction_hash: Some(receipt.tx_hash.into()), transaction_index: Some(transaction_index as u64), - log_index: Some(log_index as u64), + log_index: Some((log_index_plus_one - 1) as u64), removed: false, }; let _ = sink.send(SubscriptionMessage::from_json(&log)?).await; diff --git a/zilliqa/src/api/ots.rs b/zilliqa/src/api/ots.rs index 4e3b27b09..fa11810ea 100644 --- a/zilliqa/src/api/ots.rs +++ b/zilliqa/src/api/ots.rs @@ -13,7 +13,10 @@ use jsonrpsee::{types::Params, RpcModule}; use serde_json::{json, Value}; use super::{ - eth::{get_transaction_inner, get_transaction_receipt_inner}, + eth::{ + get_block_transaction_receipts_inner, get_transaction_inner, + get_transaction_receipt_inner_slow, + }, types::ots::{self, Operation, TraceEntry}, }; use crate::{ @@ -116,17 +119,14 @@ fn get_block_transactions( let start = usize::min(page_number * page_size, block.transactions.len()); let end = usize::min((page_number + 1) * page_size, block.transactions.len()); - let txn_results = block.transactions[start..end].iter().map(|hash| { - // There are some redundant calls between these two functions - We could optimise by combining them. - let txn = get_transaction_inner(*hash, &node)? - .ok_or_else(|| anyhow!("transaction not found: {hash}"))?; - let receipt = get_transaction_receipt_inner(*hash, &node)? - .ok_or_else(|| anyhow!("receipt not found: {hash}"))?; - - Ok::<_, anyhow::Error>((txn, receipt)) - }); - let (transactions, receipts): (Vec<_>, Vec<_>) = - itertools::process_results(txn_results, |iter| iter.unzip())?; + let receipts = get_block_transaction_receipts_inner(&node, block_number)?; + let transactions = block.transactions[start..end] + .iter() + .map(|hash| get_transaction_inner(*hash, &node)) + .collect::>>()? + .into_iter() + .flatten() + .collect::>(); let block_gas_limit = node.config.consensus.eth_block_gas_limit; let full_block = ots::BlockWithTransactions { @@ -305,7 +305,9 @@ fn search_transactions_inner( let node = node.lock().unwrap(); let receipt = ots::TransactionReceiptWithTimestamp { - receipt: get_transaction_receipt_inner(hash, &node).unwrap().unwrap(), + receipt: get_transaction_receipt_inner_slow(&node, txn_block_number, hash) + .unwrap() + .unwrap(), timestamp: timestamp .duration_since(SystemTime::UNIX_EPOCH) .unwrap() diff --git a/zilliqa/tests/it/eth.rs b/zilliqa/tests/it/eth.rs index 6155ee4e3..3101a8ab0 100644 --- a/zilliqa/tests/it/eth.rs +++ b/zilliqa/tests/it/eth.rs @@ -384,6 +384,57 @@ async fn eth_get_transaction_receipt(mut network: Network) { assert_eq!(receipt.status.unwrap_or_default(), 1.into()); } +#[zilliqa_macros::test] +async fn get_transaction_receipt_sequential_log_indexes(mut network: Network) { + let wallet = network.genesis_wallet().await; + + // Deploy a contract that can emit events + let (hash1, abi) = deploy_contract( + "tests/it/contracts/EmitEvents.sol", + "EmitEvents", + &wallet, + &mut network, + ) + .await; + + let receipt1 = network.run_until_receipt(&wallet, hash1, 50).await; + let contract_address = receipt1.contract_address.unwrap(); + + // Call emitEvents() to generate some logs in block 1 + let emit_events = abi.function("emitEvents").unwrap(); + let tx1 = TransactionRequest::new() + .to(contract_address) + .data(emit_events.encode_input(&[]).unwrap()); + + let tx1_hash = wallet.send_transaction(tx1, None).await.unwrap().tx_hash(); + + let receipt1 = network.run_until_receipt(&wallet, tx1_hash, 50).await; + + // Verify logs in first block have sequential indexes starting at 0 + assert!(receipt1.logs.len() > 1); + for (i, log) in receipt1.logs.iter().enumerate() { + assert_eq!(log.log_index.unwrap().as_u64(), i as u64); + } + + // Create another transaction in a new block + let tx2 = TransactionRequest::new() + .to(contract_address) + .data(emit_events.encode_input(&[]).unwrap()); + + let tx2_hash = wallet.send_transaction(tx2, None).await.unwrap().tx_hash(); + + let receipt2 = network.run_until_receipt(&wallet, tx2_hash, 50).await; + + // Verify logs in second block also start at index 0 + assert!(receipt2.logs.len() > 1); + for (i, log) in receipt2.logs.iter().enumerate() { + assert_eq!(log.log_index.unwrap().as_u64(), i as u64); + } + + // Verify blocks are different + assert_ne!(receipt1.block_hash, receipt2.block_hash); +} + #[zilliqa_macros::test] async fn get_logs(mut network: Network) { let wallet = network.genesis_wallet().await;