Merge branch 'master' into chore/add-unrecorded-blocks-trait

CHANGELOG.md

@@ -36,6 +36,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`.
 - [2485](https://github.com/FuelLabs/fuel-core/pull/2485): Hardcode the timestamp of the genesis block and version of `tai64` to avoid breaking changes for us.
 
 ### Changed

crates/fuel-core/src/query/message.rs

@@ -115,9 +115,6 @@ pub trait MessageProofData {
     /// Get the block.
     fn block(&self, id: &BlockHeight) -> StorageResult<CompressedBlock>;
 
-    /// Return all receipts in the given transaction.
-    fn receipts(&self, transaction_id: &TxId) -> StorageResult<Vec<Receipt>>;
-
     /// 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<Vec<Receipt>> {
-        self.receipts(transaction_id)
-    }
-
     fn transaction_status(
         &self,
         transaction_id: &TxId,
@@ -165,34 +158,28 @@ pub fn message_proof<T: MessageProofData + ?Sized>(
     desired_nonce: Nonce,
     commit_block_height: BlockHeight,
 ) -> StorageResult<MessageProof> {
-    // 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::<StorageError>(
-            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::<StorageError>(
+                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<T: MessageProofData + ?Sized>(
         }
     };
 
+    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) {
@@ -278,7 +269,11 @@ fn message_receipts_proof<T: MessageProofData + ?Sized>(
     // Get the message receipts from the block.
     let leaves: Vec<Vec<Receipt>> = 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 +332,173 @@ 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,
+            Nonce,
+        },
+        services::txpool::TransactionStatus,
+        tai64::Tai64,
+    };
+
+    use super::{
+        message_proof,
+        MessageProofData,
+    };
+
+    pub struct FakeDB {
+        pub blocks: HashMap<BlockHeight, CompressedBlock>,
+        pub transaction_statuses: HashMap<TxId, TransactionStatus>,
+        pub receipts: HashMap<TxId, Vec<Receipt>>,
+    }
+
+    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<Receipt>) {
+            self.receipts.insert(transaction_id, receipts);
+        }
+    }
+
+    impl MessageProofData for FakeDB {
+        fn block(&self, id: &BlockHeight) -> fuel_core_storage::Result<CompressedBlock> {
+            self.blocks.get(id).cloned().ok_or(not_found!("Block"))
+        }
+
+        fn transaction_status(
+            &self,
+            transaction_id: &TxId,
+        ) -> fuel_core_storage::Result<TransactionStatus> {
+            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<MerkleProof> {
+            // Unused in current tests
+            Ok(MerkleProof::default())
+        }
+    }
+
+    // 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_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 {
+            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);
+        database.insert_block(block_height, block.clone());
+        database.insert_transaction_status(
+            valid_tx_id,
+            TransactionStatus::Success {
+                time: Tai64::UNIX_EPOCH,
+                block_height,
+                receipts: valid_tx_receipts.clone(),
+                total_fee: 0,
+                total_gas: 0,
+                result: None,
+            },
+        );
+        database.insert_receipts(valid_tx_id, valid_tx_receipts.clone());
+
+        // Get the message proof with the valid transaction
+        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(block_height, 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,
+            TransactionStatus::Failed {
+                time: Tai64::UNIX_EPOCH,
+                block_height,
+                result: None,
+                total_fee: 0,
+                total_gas: 0,
+                receipts: invalid_tx_receipts.clone(),
+            },
+        );
+        database.insert_receipts(invalid_tx_id, invalid_tx_receipts.clone());
+
+        // When
+        // Get the message proof with the same message id
+        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
+        assert_eq!(message_proof_valid_tx, message_proof_invalid_tx);
+    }
+}

crates/fuel-core/src/query/message/test.rs

@@ -65,7 +65,6 @@ mockall::mock! {
             message_block_height: &BlockHeight,
             commit_block_height: &BlockHeight,
         ) -> StorageResult<MerkleProof>;
-        fn receipts(&self, transaction_id: &TxId) -> StorageResult<Vec<Receipt>>;
         fn transaction_status(&self, transaction_id: &TxId) -> StorageResult<TransactionStatus>;
     }
 }
@@ -97,16 +96,6 @@ 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)
-        }
-    });
-
     let commit_block_header = PartialBlockHeader {
         application: ApplicationHeader {
             da_height: 0u64.into(),
@@ -158,18 +147,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();

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.