From 484f447b634d62db82d8406b4f906884f8d8c6e8 Mon Sep 17 00:00:00 2001 From: AurelienFT Date: Fri, 6 Dec 2024 15:00:27 +0100 Subject: [PATCH 1/5] Ignore failed TX receipts and add a test. --- crates/fuel-core/src/query/message.rs | 187 +++++++++++++++++++++++++- 1 file changed, 186 insertions(+), 1 deletion(-) diff --git a/crates/fuel-core/src/query/message.rs b/crates/fuel-core/src/query/message.rs index 07d2cd9ab56..fc88ff70327 100644 --- a/crates/fuel-core/src/query/message.rs +++ b/crates/fuel-core/src/query/message.rs @@ -278,7 +278,11 @@ fn message_receipts_proof( // Get the message receipts from the block. let leaves: Vec> = message_block_txs .iter() - .map(|id| database.receipts(id)) + .filter_map(|id| match database.transaction_status(id) { + Ok(TransactionStatus::Success { receipts, .. }) => Some(Ok(receipts)), + Ok(_) => None, + Err(err) => Some(Err(err)), + }) .try_collect()?; let leaves = leaves.into_iter() // Flatten the receipts after filtering on output messages @@ -337,3 +341,184 @@ pub fn message_status( Ok(MessageStatus::not_found()) } } + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use fuel_core_storage::not_found; + use fuel_core_types::{ + blockchain::block::CompressedBlock, + entities::relayer::message::MerkleProof, + fuel_tx::{ + Address, + Bytes32, + Receipt, + TxId, + }, + fuel_types::BlockHeight, + services::txpool::TransactionStatus, + tai64::Tai64, + }; + + use super::{ + message_receipts_proof, + MessageProofData, + }; + + pub struct FakeDB { + pub blocks: HashMap, + pub transaction_statuses: HashMap, + pub receipts: HashMap>, + } + + impl FakeDB { + fn new() -> Self { + Self { + blocks: HashMap::new(), + transaction_statuses: HashMap::new(), + receipts: HashMap::new(), + } + } + + fn insert_block(&mut self, block_height: BlockHeight, block: CompressedBlock) { + self.blocks.insert(block_height, block); + } + + fn insert_transaction_status( + &mut self, + transaction_id: TxId, + status: TransactionStatus, + ) { + self.transaction_statuses.insert(transaction_id, status); + } + + fn insert_receipts(&mut self, transaction_id: TxId, receipts: Vec) { + self.receipts.insert(transaction_id, receipts); + } + } + + impl MessageProofData for FakeDB { + fn block(&self, id: &BlockHeight) -> fuel_core_storage::Result { + self.blocks.get(id).cloned().ok_or(not_found!("Block")) + } + + fn transaction_status( + &self, + transaction_id: &TxId, + ) -> fuel_core_storage::Result { + self.transaction_statuses + .get(transaction_id) + .cloned() + .ok_or(not_found!("Transaction status")) + } + + fn block_history_proof( + &self, + _message_block_height: &BlockHeight, + _commit_block_height: &BlockHeight, + ) -> fuel_core_storage::Result { + // Unused in current tests + Ok(MerkleProof::default()) + } + + fn receipts( + &self, + transaction_id: &TxId, + ) -> fuel_core_storage::Result> { + self.receipts + .get(transaction_id) + .cloned() + .ok_or(not_found!("Receipts")) + } + } + + // Test will try to get the message receipt proof with a block with only valid transactions + // Then add an invalid transaction and check if the proof is still the same (meaning the invalid transaction was ignored) + #[test] + fn test_message_receipts_proof_ignore_failed() { + // Create a fake database + let mut database = FakeDB::new(); + + // Given + // Create a block with a valid transaction and receipts + let mut block = CompressedBlock::default(); + let valid_tx_id = Bytes32::new([1; 32]); + let mut valid_tx_receipts = vec![]; + for i in 0..100 { + valid_tx_receipts.push(Receipt::MessageOut { + sender: Address::default(), + recipient: Address::default(), + amount: 0, + nonce: 0.into(), + len: 32, + digest: Bytes32::default(), + data: Some(vec![i; 32]), + }); + } + block.transactions_mut().push(valid_tx_id.clone()); + database.insert_block(1u32.into(), block.clone()); + database.insert_transaction_status( + valid_tx_id.clone(), + TransactionStatus::Success { + time: Tai64::UNIX_EPOCH, + block_height: 1u32.into(), + receipts: valid_tx_receipts.clone(), + total_fee: 0, + total_gas: 0, + result: None, + }, + ); + database.insert_receipts(valid_tx_id.clone(), valid_tx_receipts.clone()); + + // Get the message proof with the valid transaction + let message_proof_valid_tx = message_receipts_proof( + &database, + valid_tx_receipts[0].message_id().unwrap(), + &[valid_tx_id], + ) + .unwrap(); + + // Add an invalid transaction with receipts to the block + let invalid_tx_id = Bytes32::new([2; 32]); + block.transactions_mut().push(invalid_tx_id.clone()); + database.insert_block(1u32.into(), block.clone()); + let mut invalid_tx_receipts = vec![]; + for i in 0..100 { + invalid_tx_receipts.push(Receipt::MessageOut { + sender: Address::default(), + recipient: Address::default(), + amount: 0, + nonce: 0.into(), + len: 33, + digest: Bytes32::default(), + data: Some(vec![i; 33]), + }); + } + database.insert_transaction_status( + invalid_tx_id.clone(), + TransactionStatus::Failed { + time: Tai64::UNIX_EPOCH, + block_height: 1u32.into(), + result: None, + total_fee: 0, + total_gas: 0, + receipts: invalid_tx_receipts.clone(), + }, + ); + database.insert_receipts(invalid_tx_id.clone(), invalid_tx_receipts.clone()); + + // When + // Get the message proof with the same message id + let message_proof_invalid_tx = message_receipts_proof( + &database, + valid_tx_receipts[0].message_id().unwrap(), + &[valid_tx_id, invalid_tx_id], + ) + .unwrap(); + + // Then + // The proof should be the same because the invalid transaction was ignored + assert_eq!(message_proof_valid_tx, message_proof_invalid_tx); + } +} From 4dcd294478b81d51cd865a08cda5d697272f1076 Mon Sep 17 00:00:00 2001 From: AurelienFT Date: Fri, 6 Dec 2024 15:02:02 +0100 Subject: [PATCH 2/5] Fix clippy. --- crates/fuel-core/src/query/message.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/fuel-core/src/query/message.rs b/crates/fuel-core/src/query/message.rs index fc88ff70327..8833f1c178d 100644 --- a/crates/fuel-core/src/query/message.rs +++ b/crates/fuel-core/src/query/message.rs @@ -456,10 +456,10 @@ mod tests { data: Some(vec![i; 32]), }); } - block.transactions_mut().push(valid_tx_id.clone()); + block.transactions_mut().push(valid_tx_id); database.insert_block(1u32.into(), block.clone()); database.insert_transaction_status( - valid_tx_id.clone(), + valid_tx_id, TransactionStatus::Success { time: Tai64::UNIX_EPOCH, block_height: 1u32.into(), @@ -469,7 +469,7 @@ mod tests { result: None, }, ); - database.insert_receipts(valid_tx_id.clone(), valid_tx_receipts.clone()); + database.insert_receipts(valid_tx_id, valid_tx_receipts.clone()); // Get the message proof with the valid transaction let message_proof_valid_tx = message_receipts_proof( @@ -481,7 +481,7 @@ mod tests { // Add an invalid transaction with receipts to the block let invalid_tx_id = Bytes32::new([2; 32]); - block.transactions_mut().push(invalid_tx_id.clone()); + block.transactions_mut().push(invalid_tx_id); database.insert_block(1u32.into(), block.clone()); let mut invalid_tx_receipts = vec![]; for i in 0..100 { @@ -496,7 +496,7 @@ mod tests { }); } database.insert_transaction_status( - invalid_tx_id.clone(), + invalid_tx_id, TransactionStatus::Failed { time: Tai64::UNIX_EPOCH, block_height: 1u32.into(), @@ -506,7 +506,7 @@ mod tests { receipts: invalid_tx_receipts.clone(), }, ); - database.insert_receipts(invalid_tx_id.clone(), invalid_tx_receipts.clone()); + database.insert_receipts(invalid_tx_id, invalid_tx_receipts.clone()); // When // Get the message proof with the same message id From ad1b926aef89c13da47eae99aa1673054b6362a2 Mon Sep 17 00:00:00 2001 From: AurelienFT Date: Fri, 6 Dec 2024 15:06:17 +0100 Subject: [PATCH 3/5] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48b6d73162d..0902187ffb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - [2369](https://github.com/FuelLabs/fuel-core/pull/2369): The `transaction_insertion_time_in_thread_pool_milliseconds` metric is properly collected. - [2413](https://github.com/FuelLabs/fuel-core/issues/2413): block production immediately errors if unable to lock the mutex. - [2389](https://github.com/FuelLabs/fuel-core/pull/2389): Fix construction of reverse iterator in RocksDB. +- [2478](https://github.com/FuelLabs/fuel-core/pull/2478): Fix proof created by `message_receipts_proof` function by ignoring the receipts from failed transactions to match `message_outbox_root`. ### Changed - [2295](https://github.com/FuelLabs/fuel-core/pull/2295): `CombinedDb::from_config` now respects `state_rewind_policy` with tmp RocksDB. From 451dfa5e4e6d667734b45cf1911ee80905c0187a Mon Sep 17 00:00:00 2001 From: AurelienFT Date: Fri, 6 Dec 2024 15:26:21 +0100 Subject: [PATCH 4/5] Update tests to match the new behavior. --- crates/fuel-core/src/query/message/test.rs | 47 +++++++++++++--------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/crates/fuel-core/src/query/message/test.rs b/crates/fuel-core/src/query/message/test.rs index 447e1c01a78..5072e865267 100644 --- a/crates/fuel-core/src/query/message/test.rs +++ b/crates/fuel-core/src/query/message/test.rs @@ -97,13 +97,17 @@ async fn can_build_message_proof() { let mut data = MockProofDataStorage::new(); let mut count = 0; - data.expect_receipts().returning(move |txn_id| { - if *txn_id == transaction_id { - Ok(receipts.to_vec()) - } else { - let r = other_receipts[count..=count].to_vec(); - count += 1; - Ok(r) + data.expect_receipts().returning({ + let receipts = receipts.to_vec(); + let other_receipts = other_receipts.to_vec(); + move |txn_id| { + if *txn_id == transaction_id { + Ok(receipts.clone()) + } else { + let r = other_receipts[count..=count].to_vec(); + count += 1; + Ok(r) + } } }); @@ -158,18 +162,23 @@ async fn can_build_message_proof() { }); let message_block_height = *message_block.header().height(); - data.expect_transaction_status() - .with(eq(transaction_id)) - .returning(move |_| { - Ok(TransactionStatus::Success { - block_height: message_block_height, - time: Tai64::UNIX_EPOCH, - result: None, - receipts: vec![], - total_gas: 0, - total_fee: 0, - }) - }); + data.expect_transaction_status().returning(move |tx_id| { + let receipts = if *tx_id == transaction_id { + receipts.to_vec() + } else { + let r = other_receipts[count..=count].to_vec(); + count += 1; + r + }; + Ok(TransactionStatus::Success { + block_height: message_block_height, + time: Tai64::UNIX_EPOCH, + result: None, + receipts, + total_gas: 0, + total_fee: 0, + }) + }); data.expect_block().times(2).returning({ let commit_block = commit_block.clone(); From 0db996728bce62aa911109746b88caa3d2da7cde Mon Sep 17 00:00:00 2001 From: AurelienFT Date: Mon, 9 Dec 2024 11:11:06 +0100 Subject: [PATCH 5/5] Remove receipts function and change test to use public function --- crates/fuel-core/src/query/message.rs | 106 ++++++++----------- crates/fuel-core/src/query/message/test.rs | 15 --- crates/types/src/entities/relayer/message.rs | 1 + 3 files changed, 44 insertions(+), 78 deletions(-) diff --git a/crates/fuel-core/src/query/message.rs b/crates/fuel-core/src/query/message.rs index 8833f1c178d..03b92d4c652 100644 --- a/crates/fuel-core/src/query/message.rs +++ b/crates/fuel-core/src/query/message.rs @@ -115,9 +115,6 @@ pub trait MessageProofData { /// Get the block. fn block(&self, id: &BlockHeight) -> StorageResult; - /// Return all receipts in the given transaction. - fn receipts(&self, transaction_id: &TxId) -> StorageResult>; - /// Get the status of a transaction. fn transaction_status( &self, @@ -138,10 +135,6 @@ impl MessageProofData for ReadView { self.block(id) } - fn receipts(&self, transaction_id: &TxId) -> StorageResult> { - self.receipts(transaction_id) - } - fn transaction_status( &self, transaction_id: &TxId, @@ -165,34 +158,28 @@ pub fn message_proof( desired_nonce: Nonce, commit_block_height: BlockHeight, ) -> StorageResult { - // Check if the receipts for this transaction actually contain this nonce or exit. - let (sender, recipient, nonce, amount, data) = database - .receipts(&transaction_id)? - .into_iter() - .find_map(|r| match r { - Receipt::MessageOut { - sender, - recipient, - nonce, - amount, - data, - .. - } if r.nonce() == Some(&desired_nonce) => { - Some((sender, recipient, nonce, amount, data)) - } - _ => None, - }) - .ok_or::( - anyhow::anyhow!("Desired `nonce` missing in transaction receipts").into(), - )?; - - let Some(data) = data else { - return Err(anyhow::anyhow!("Output message doesn't contain any `data`").into()) - }; - // Get the block id from the transaction status if it's ready. - let message_block_height = match database.transaction_status(&transaction_id) { - Ok(TransactionStatus::Success { block_height, .. }) => block_height, + let (message_block_height, (sender, recipient, nonce, amount, data)) = match database.transaction_status(&transaction_id) { + Ok(TransactionStatus::Success { block_height, receipts, .. }) => ( + block_height, + receipts.into_iter() + .find_map(|r| match r { + Receipt::MessageOut { + sender, + recipient, + nonce, + amount, + data, + .. + } if r.nonce() == Some(&desired_nonce) => { + Some((sender, recipient, nonce, amount, data)) + } + _ => None, + }) + .ok_or::( + anyhow::anyhow!("Desired `nonce` missing in transaction receipts").into(), + )? + ), Ok(TransactionStatus::Submitted { .. }) => { return Err(anyhow::anyhow!( "Unable to obtain the message block height. The transaction has not been processed yet" @@ -219,6 +206,10 @@ pub fn message_proof( } }; + let Some(data) = data else { + return Err(anyhow::anyhow!("Output message doesn't contain any `data`").into()) + }; + // Get the message fuel block header. let (message_block_header, message_block_txs) = match database.block(&message_block_height) { @@ -356,13 +347,16 @@ mod tests { Receipt, TxId, }, - fuel_types::BlockHeight, + fuel_types::{ + BlockHeight, + Nonce, + }, services::txpool::TransactionStatus, tai64::Tai64, }; use super::{ - message_receipts_proof, + message_proof, MessageProofData, }; @@ -421,28 +415,20 @@ mod tests { // Unused in current tests Ok(MerkleProof::default()) } - - fn receipts( - &self, - transaction_id: &TxId, - ) -> fuel_core_storage::Result> { - self.receipts - .get(transaction_id) - .cloned() - .ok_or(not_found!("Receipts")) - } } // Test will try to get the message receipt proof with a block with only valid transactions // Then add an invalid transaction and check if the proof is still the same (meaning the invalid transaction was ignored) #[test] - fn test_message_receipts_proof_ignore_failed() { + fn test_message_proof_ignore_failed() { // Create a fake database let mut database = FakeDB::new(); // Given // Create a block with a valid transaction and receipts let mut block = CompressedBlock::default(); + let block_height: BlockHeight = BlockHeight::new(1); + block.header_mut().set_block_height(block_height); let valid_tx_id = Bytes32::new([1; 32]); let mut valid_tx_receipts = vec![]; for i in 0..100 { @@ -457,12 +443,12 @@ mod tests { }); } block.transactions_mut().push(valid_tx_id); - database.insert_block(1u32.into(), block.clone()); + database.insert_block(block_height, block.clone()); database.insert_transaction_status( valid_tx_id, TransactionStatus::Success { time: Tai64::UNIX_EPOCH, - block_height: 1u32.into(), + block_height, receipts: valid_tx_receipts.clone(), total_fee: 0, total_gas: 0, @@ -472,17 +458,14 @@ mod tests { database.insert_receipts(valid_tx_id, valid_tx_receipts.clone()); // Get the message proof with the valid transaction - let message_proof_valid_tx = message_receipts_proof( - &database, - valid_tx_receipts[0].message_id().unwrap(), - &[valid_tx_id], - ) - .unwrap(); + let message_proof_valid_tx = + message_proof(&database, valid_tx_id, Nonce::default(), block_height) + .unwrap(); // Add an invalid transaction with receipts to the block let invalid_tx_id = Bytes32::new([2; 32]); block.transactions_mut().push(invalid_tx_id); - database.insert_block(1u32.into(), block.clone()); + database.insert_block(block_height, block.clone()); let mut invalid_tx_receipts = vec![]; for i in 0..100 { invalid_tx_receipts.push(Receipt::MessageOut { @@ -499,7 +482,7 @@ mod tests { invalid_tx_id, TransactionStatus::Failed { time: Tai64::UNIX_EPOCH, - block_height: 1u32.into(), + block_height, result: None, total_fee: 0, total_gas: 0, @@ -510,12 +493,9 @@ mod tests { // When // Get the message proof with the same message id - let message_proof_invalid_tx = message_receipts_proof( - &database, - valid_tx_receipts[0].message_id().unwrap(), - &[valid_tx_id, invalid_tx_id], - ) - .unwrap(); + let message_proof_invalid_tx = + message_proof(&database, valid_tx_id, Nonce::default(), block_height) + .unwrap(); // Then // The proof should be the same because the invalid transaction was ignored diff --git a/crates/fuel-core/src/query/message/test.rs b/crates/fuel-core/src/query/message/test.rs index 5072e865267..7c9b217aa44 100644 --- a/crates/fuel-core/src/query/message/test.rs +++ b/crates/fuel-core/src/query/message/test.rs @@ -65,7 +65,6 @@ mockall::mock! { message_block_height: &BlockHeight, commit_block_height: &BlockHeight, ) -> StorageResult; - fn receipts(&self, transaction_id: &TxId) -> StorageResult>; fn transaction_status(&self, transaction_id: &TxId) -> StorageResult; } } @@ -97,20 +96,6 @@ async fn can_build_message_proof() { let mut data = MockProofDataStorage::new(); let mut count = 0; - data.expect_receipts().returning({ - let receipts = receipts.to_vec(); - let other_receipts = other_receipts.to_vec(); - move |txn_id| { - if *txn_id == transaction_id { - Ok(receipts.clone()) - } else { - let r = other_receipts[count..=count].to_vec(); - count += 1; - Ok(r) - } - } - }); - let commit_block_header = PartialBlockHeader { application: ApplicationHeader { da_height: 0u64.into(), diff --git a/crates/types/src/entities/relayer/message.rs b/crates/types/src/entities/relayer/message.rs index 5be4cbda490..6e8e39b53c0 100644 --- a/crates/types/src/entities/relayer/message.rs +++ b/crates/types/src/entities/relayer/message.rs @@ -249,6 +249,7 @@ pub struct MerkleProof { pub proof_index: u64, } +#[derive(Debug, PartialEq, Eq)] /// Proves to da layer that this message was included in a Fuel block. pub struct MessageProof { /// Proof that message is contained within the provided block header.