diff --git a/crates/e2e-tests/src/foreign_chain_mock.rs b/crates/e2e-tests/src/foreign_chain_mock.rs index d9e1c68b6d..79d42d52f3 100644 --- a/crates/e2e-tests/src/foreign_chain_mock.rs +++ b/crates/e2e-tests/src/foreign_chain_mock.rs @@ -115,7 +115,12 @@ pub fn setup_evm_mock(server: &MockServer) -> usize { } } "eth_getTransactionReceipt" => { + // The inspector rejects receipts and logs that are not bound to the + // queried transaction, so echo the queried hash and keep the log's + // tx/block fields consistent with the receipt. + let transaction_hash = body["params"][0].as_str().expect("tx hash param"); serde_json::json!({ + "transactionHash": transaction_hash, "blockHash": format!("0x{MOCK_BLOCK_HASH}"), "blockNumber": "0xa", "status": "0x1", @@ -127,10 +132,10 @@ pub fn setup_evm_mock(server: &MockServer) -> usize { "0x0000000000000000000000000000000000000000000000000000000000008001", ], "data": "0x000000000000000000000000000000000000000000000000000006e4b5898a00", - "blockHash": "0x4c93dd4a8f347e6480b0a44f8c2b7eecdfb31d711e8d542fd60112ea5d98fb02", - "blockNumber": "0xfbf4b1", + "blockHash": format!("0x{MOCK_BLOCK_HASH}"), + "blockNumber": "0xa", "l1BatchNumber": "0x4f3c", - "transactionHash": "0x497fc5f5b5d81d6bc15cccc6d4d8be8ef6ad19376233b944a60dc435593f7234", + "transactionHash": transaction_hash, "transactionIndex": "0x0", "logIndex": "0x0", "transactionLogIndex": "0x0", diff --git a/crates/foreign-chain-inspector/src/evm/inspector.rs b/crates/foreign-chain-inspector/src/evm/inspector.rs index cf2c878810..f7f8caf219 100644 --- a/crates/foreign-chain-inspector/src/evm/inspector.rs +++ b/crates/foreign-chain-inspector/src/evm/inspector.rs @@ -64,6 +64,15 @@ where ) .await?; + // Defensive: `eth_getTransactionReceipt` looks the receipt up *by hash*, so a + // well-behaved backend always echoes back the hash we queried. + if transaction_receipt.transaction_hash != get_transaction_receipt_args.transaction_hash { + return Err(ForeignChainInspectionError::InconsistentRpcResponse { + requested_hash: get_transaction_receipt_args.transaction_hash.into(), + returned_hash: transaction_receipt.transaction_hash.into(), + }); + } + self.verify_finality_level(transaction_receipt.block_number, finality) .await?; self.verify_block_is_canonical( @@ -189,6 +198,24 @@ impl EvmExtractor { .cloned() .ok_or(ForeignChainInspectionError::LogIndexOutOfBounds)?; + // The receipt's transaction hash has already been checked against the + // requested one, so binding the log to the receipt transitively binds + // it to the requested transaction. + let log_bound_to_receipt = log.transaction_hash == rpc_response.transaction_hash + && log.block_hash == rpc_response.block_hash + && log.block_number == rpc_response.block_number; + if !log_bound_to_receipt { + return Err(ForeignChainInspectionError::LogNotBoundToReceipt { + log_index: *log_index, + log_transaction_hash: log.transaction_hash.into(), + log_block_hash: log.block_hash.into(), + log_block_number: log.block_number.as_u64(), + receipt_transaction_hash: rpc_response.transaction_hash.into(), + receipt_block_hash: rpc_response.block_hash.into(), + receipt_block_number: rpc_response.block_number.as_u64(), + }); + } + Ok(EvmExtractedValue::Log(log)) } } diff --git a/crates/foreign-chain-inspector/src/lib.rs b/crates/foreign-chain-inspector/src/lib.rs index 87b4b0ae07..4316f6ab25 100644 --- a/crates/foreign-chain-inspector/src/lib.rs +++ b/crates/foreign-chain-inspector/src/lib.rs @@ -244,12 +244,24 @@ pub enum ForeignChainInspectionError { canonical_hash: HexBytes, }, #[error( - "RPC backend returned a block that does not match the one queried by hash: requested={requested_hash}, returned={returned_hash}" + "RPC backend response does not match the hash it was queried by: requested={requested_hash}, returned={returned_hash}" )] InconsistentRpcResponse { requested_hash: HexBytes, returned_hash: HexBytes, }, + #[error( + "log at index {log_index} is not bound to its receipt: log points at tx={log_transaction_hash}, block={log_block_hash} (height {log_block_number}); receipt is tx={receipt_transaction_hash}, block={receipt_block_hash} (height {receipt_block_number})" + )] + LogNotBoundToReceipt { + log_index: u64, + log_transaction_hash: HexBytes, + log_block_hash: HexBytes, + log_block_number: u64, + receipt_transaction_hash: HexBytes, + receipt_block_hash: HexBytes, + receipt_block_number: u64, + }, #[error("The transaction's status was not success")] TransactionFailed, #[error("transaction not found")] diff --git a/crates/foreign-chain-inspector/tests/evm_inspector.rs b/crates/foreign-chain-inspector/tests/evm_inspector.rs index 6c9582da97..61db6fdb2a 100644 --- a/crates/foreign-chain-inspector/tests/evm_inspector.rs +++ b/crates/foreign-chain-inspector/tests/evm_inspector.rs @@ -1,3 +1,5 @@ +#![allow(non_snake_case)] + pub mod common; use crate::common::{FixedResponseRpcClient, SequentialResponseMockClientBuilder}; @@ -43,7 +45,7 @@ fn test_log() -> Log { transaction_index: U64([2]), transaction_hash: H256([3; 32]), block_hash: H256([4; 32]), - block_number: U64([5]), + block_number: U64::from(90), address: H160([6; 20]), data: "test_log".to_string(), topics: vec![H256([7; 32]), H256([8; 32])], @@ -77,6 +79,7 @@ macro_rules! evm_inspector_tests { hash: H256::from([0xaa; 32]), }; let tx_response = GetTransactionReceiptResponse { + transaction_hash: H256::from([3; 32]), block_hash: H256::from([4; 32]), block_number: U64::from(90), status: U64::one(), @@ -117,6 +120,7 @@ macro_rules! evm_inspector_tests { hash: H256::from([0xaa; 32]), }; let tx_response = GetTransactionReceiptResponse { + transaction_hash: H256::from([3; 32]), block_hash: H256::from([4; 32]), block_number, status: U64::one(), @@ -160,6 +164,7 @@ macro_rules! evm_inspector_tests { hash: H256::from([0xaa; 32]), }; let tx_response = GetTransactionReceiptResponse { + transaction_hash: H256::from([1; 32]), block_hash: H256::from([2; 32]), block_number: U64::from(60), status: U64::one(), @@ -197,6 +202,7 @@ macro_rules! evm_inspector_tests { hash: H256::from([0xaa; 32]), }; let tx_response = GetTransactionReceiptResponse { + transaction_hash: H256::from([1; 32]), block_hash: H256::from([2; 32]), block_number: U64::from(90), status: U64::zero(), @@ -242,6 +248,7 @@ macro_rules! evm_inspector_tests { hash: H256::from([0xaa; 32]), }; let tx_response = GetTransactionReceiptResponse { + transaction_hash: H256::from([11; 32]), block_hash: H256::from([12; 32]), block_number: U64::from(90), status: U64::one(), @@ -313,6 +320,7 @@ macro_rules! evm_inspector_tests { }; let tx_response = GetTransactionReceiptResponse { + transaction_hash: H256::from([9; 32]), block_hash: H256::from([5; 32]), block_number: U64::from(90), status: U64::one(), @@ -401,6 +409,7 @@ macro_rules! evm_inspector_tests { hash: H256::from([0xaa; 32]), }; let tx_response = GetTransactionReceiptResponse { + transaction_hash: H256::from([1; 32]), block_hash: H256::from([2; 32]), block_number: U64::from(90), status: U64::one(), @@ -445,7 +454,7 @@ macro_rules! evm_inspector_tests { transaction_index: U64([2]), transaction_hash: H256([3; 32]), block_hash: H256([4; 32]), - block_number: U64([5]), + block_number: U64::from(90), address: H160([6; 20]), data: "first_log".to_string(), topics: vec![H256([7; 32])], @@ -454,9 +463,9 @@ macro_rules! evm_inspector_tests { removed: false, log_index: U64::from(21), transaction_index: U64([20]), - transaction_hash: H256([30; 32]), + transaction_hash: H256([3; 32]), block_hash: H256([4; 32]), - block_number: U64([5]), + block_number: U64::from(90), address: H160([60; 20]), data: "second_log".to_string(), topics: vec![H256([70; 32])], @@ -468,6 +477,7 @@ macro_rules! evm_inspector_tests { hash: H256::from([0xaa; 32]), }; let tx_response = GetTransactionReceiptResponse { + transaction_hash: H256::from([3; 32]), block_hash: H256::from([4; 32]), block_number: U64::from(90), status: U64::one(), @@ -512,6 +522,7 @@ macro_rules! evm_inspector_tests { hash: H256::from([0xaa; 32]), }; let tx_response = GetTransactionReceiptResponse { + transaction_hash: H256::from([1; 32]), block_hash: H256::from([0xbb; 32]), block_number: U64::from(90), status: U64::one(), @@ -550,6 +561,139 @@ macro_rules! evm_inspector_tests { && canonical_hash == foreign_chain_inspector::HexBytes(vec![0xcc; 32]) ); } + + #[tokio::test] + async fn extract__should_reject_receipt_whose_transaction_hash_differs_from_request() + { + // Given + let requested_tx_bytes: [u8; 32] = [1; 32]; + let returned_tx_bytes: [u8; 32] = [0xdd; 32]; + let tx_id = TxHash::from(requested_tx_bytes); + + let tx_response = GetTransactionReceiptResponse { + transaction_hash: H256(returned_tx_bytes), + block_hash: H256::from([2; 32]), + block_number: U64::from(90), + status: U64::one(), + logs: vec![test_log()], + }; + + // The receipt-hash check fires before the finality lookup, so only one + // RPC call is exercised. + let mock_client = SequentialResponseMockClientBuilder::new() + .with_response(&tx_response) + .build(); + let inspector = Inspector::new(mock_client); + + // When + let response = inspector + .extract( + tx_id, + EthereumFinality::Finalized, + vec![EvmExtractor::BlockHash], + ) + .await; + + // Then + assert_matches!( + response, + Err(ForeignChainInspectionError::InconsistentRpcResponse { + requested_hash, + returned_hash, + }) if requested_hash + == foreign_chain_inspector::HexBytes(requested_tx_bytes.to_vec()) + && returned_hash + == foreign_chain_inspector::HexBytes(returned_tx_bytes.to_vec()) + ); + } + + #[rstest] + #[case::transaction_hash(Log { + transaction_hash: H256([0xdd; 32]), + ..test_log() + })] + #[case::block_hash(Log { + block_hash: H256([0xdd; 32]), + ..test_log() + })] + #[case::block_number(Log { + block_number: U64::from(91), + ..test_log() + })] + #[tokio::test] + async fn extract__should_reject_log_not_bound_to_receipt(#[case] unbound_log: Log) { + // Given + let tx_id_bytes: [u8; 32] = [3; 32]; + let tx_id = TxHash::from(tx_id_bytes); + let expected_log = unbound_log.clone(); + let bound_log = test_log(); + + // When + let response = extract_log_from_receipt_with(tx_id, unbound_log).await; + + // Then + assert_matches!( + response, + Err(ForeignChainInspectionError::LogNotBoundToReceipt { + log_index, + log_transaction_hash, + log_block_hash, + log_block_number, + receipt_transaction_hash, + receipt_block_hash, + receipt_block_number, + }) if log_index == expected_log.log_index.as_u64() + && log_transaction_hash == expected_log.transaction_hash.into() + && log_block_hash == expected_log.block_hash.into() + && log_block_number == expected_log.block_number.as_u64() + && receipt_transaction_hash + == foreign_chain_inspector::HexBytes(tx_id_bytes.to_vec()) + && receipt_block_hash == bound_log.block_hash.into() + && receipt_block_number == bound_log.block_number.as_u64() + ); + } + + /// Runs `extract` selecting `log` by its own `log_index` from a finalized, + /// canonical receipt for `tx_id` whose block fields match [`test_log`]'s. + async fn extract_log_from_receipt_with( + tx_id: TxHash, + log: Log, + ) -> Result, ForeignChainInspectionError> { + let bound_log = test_log(); + let target_log_index = log.log_index.as_u64(); + let finality_block_response = GetBlockByNumberResponse { + number: U64::from(100), + hash: H256::from([0xaa; 32]), + }; + let tx_response = GetTransactionReceiptResponse { + transaction_hash: H256(tx_id.clone().into()), + block_hash: bound_log.block_hash, + block_number: bound_log.block_number, + status: U64::one(), + logs: vec![log], + }; + let canonical_block_response = GetBlockByNumberResponse { + number: tx_response.block_number, + hash: tx_response.block_hash, + }; + + let mock_client = SequentialResponseMockClientBuilder::new() + .with_response(&tx_response) + .with_response(&finality_block_response) + .with_response(&canonical_block_response) + .build(); + let inspector = Inspector::new(mock_client); + + inspector + .extract( + tx_id, + EthereumFinality::Finalized, + vec![EvmExtractor::Log { + log_index: target_log_index, + }], + ) + .await + } } }; } diff --git a/crates/foreign-chain-rpc-interfaces/src/evm.rs b/crates/foreign-chain-rpc-interfaces/src/evm.rs index 9629baa668..d7c364d0d7 100644 --- a/crates/foreign-chain-rpc-interfaces/src/evm.rs +++ b/crates/foreign-chain-rpc-interfaces/src/evm.rs @@ -11,6 +11,7 @@ pub use ethereum_types::{H160, H256, U64}; #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct GetTransactionReceiptResponse { + pub transaction_hash: H256, pub block_hash: H256, pub block_number: U64, pub status: U64,