Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct log index returns in etherium apis #2283

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Correct log index returns in etherium apis #2283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3b54398
Correct log index returns in etherium apis
maxconway Feb 3, 2025
b2fc932
Merge branch 'main' into max/bugfix/correct_eth_log_indices
maxconway Feb 5, 2025
8d6ee91
Merge branch 'main' into max/bugfix/correct_eth_log_indices
maxconway Feb 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
242 changes: 130 additions & 112 deletions zilliqa/src/api/eth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,27 +274,126 @@ fn get_balance(params: Params, node: &Arc<Mutex<Node>>) -> Result<String> {
.to_hex())
}

fn get_block_receipts(params: Params, node: &Arc<Mutex<Node>>) -> Result<Vec<TransactionReceipt>> {
let block_id: BlockId = params.one()?;
pub fn get_block_transaction_receipts_inner(
node: &MutexGuard<Node>,
block_id: impl Into<BlockId>,
) -> Result<Vec<eth::TransactionReceipt>> {
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<Node>,
block_id: impl Into<BlockId>,
txn_hash: Hash,
) -> Result<Option<eth::TransactionReceipt>> {
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<Mutex<Node>>) -> Result<Vec<TransactionReceipt>> {
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<Mutex<Node>>) -> Result<String> {
let mut params = params.sequence();
let address: Address = params.next()?;
Expand Down Expand Up @@ -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<Node>,
) -> Result<Option<eth::TransactionReceipt>> {
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<Mutex<Node>>,
Expand All @@ -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<Mutex<Node>>) -> Result<String> {
Expand Down Expand Up @@ -944,7 +950,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::<usize>() 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;
Expand Down Expand Up @@ -986,7 +1004,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;
Expand Down
28 changes: 15 additions & 13 deletions zilliqa/src/api/ots.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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::<Result<Vec<_>>>()?
.into_iter()
.flatten()
.collect::<Vec<_>>();

let block_gas_limit = node.config.consensus.eth_block_gas_limit;
let full_block = ots::BlockWithTransactions {
Expand Down Expand Up @@ -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()
Expand Down
51 changes: 51 additions & 0 deletions zilliqa/tests/it/eth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down