fix: sorted edge case (tari-project#5590)

Description
---
fixes and edge case where aggregated bodies that are sorted by the fact
that they don't have more than 1 kernel/output/input

Motivation and Context
---
These blocks are sorted by default and we do not have to go and sort
them.

base_layer/core/src/mempool/unconfirmed_pool/unconfirmed_pool.rs

@@ -806,6 +806,7 @@ mod test {
         covenants::Covenant,
         test_helpers::{create_consensus_constants, create_consensus_rules, create_orphan_block},
         transactions::{
+            aggregated_body::AggregateBody,
             fee::Fee,
             tari_amount::MicroMinotari,
             test_helpers::{create_test_core_key_manager_with_memory_db, TestParams, UtxoTestParams},
@@ -1097,8 +1098,12 @@ mod test {
             .expect("Failed to get tx")
             .0;
         // tx1 and tx5 have a shared input. Also, tx3 and tx6 have a shared input
-        tx5.body.inputs_mut()[0] = tx1.body.inputs()[0].clone();
-        tx6.body.inputs_mut()[1] = tx3.body.inputs()[1].clone();
+        let mut inputs = tx5.body.inputs().clone();
+        inputs[0] = tx1.body.inputs()[0].clone();
+        tx5.body = AggregateBody::new(inputs, tx5.body().outputs().clone(), tx5.body().kernels().clone());
+        let mut inputs = tx6.body.inputs().clone();
+        inputs[0] = tx3.body.inputs()[1].clone();
+        tx6.body = AggregateBody::new(inputs, tx6.body().outputs().clone(), tx6.body().kernels().clone());
         let tx5 = Arc::new(tx5);
         let tx6 = Arc::new(tx6);
 

base_layer/core/src/transactions/aggregated_body.rs

@@ -104,31 +104,16 @@ impl AggregateBody {
         &self.inputs
     }
 
-    /// Should be used for tests only. Get a mutable reference to the inputs
-    pub fn inputs_mut(&mut self) -> &mut Vec<TransactionInput> {
-        &mut self.inputs
-    }
-
     /// Provide read-only access to the output list
     pub fn outputs(&self) -> &Vec<TransactionOutput> {
         &self.outputs
     }
 
-    /// Should be used for tests only. Get a mutable reference to the outputs
-    pub fn outputs_mut(&mut self) -> &mut Vec<TransactionOutput> {
-        &mut self.outputs
-    }
-
     /// Provide read-only access to the kernel list
     pub fn kernels(&self) -> &Vec<TransactionKernel> {
         &self.kernels
     }
 
-    /// Should be used for tests only. Get a mutable reference to the kernels
-    pub fn kernels_mut(&mut self) -> &mut Vec<TransactionKernel> {
-        &mut self.kernels
-    }
-
     /// Add an input to the existing aggregate body
     pub fn add_input(&mut self, input: TransactionInput) {
         self.inputs.push(input);
@@ -156,6 +141,7 @@ impl AggregateBody {
     /// Add a kernel to the existing aggregate body
     pub fn add_kernel(&mut self, kernel: TransactionKernel) {
         self.kernels.push(kernel);
+        self.sorted = false;
     }
 
     /// Add a series of kernels to the existing aggregate body
@@ -369,7 +355,8 @@ impl AggregateBody {
     }
 
     pub fn is_sorted(&self) -> bool {
-        self.sorted
+        // a block containing only a single kernel, single output and single input is sorted by default
+        self.sorted || (self.kernels.len() <= 1 && self.outputs.len() <= 1 && self.inputs.len() <= 1)
     }
 
     /// Lists the number of inputs, outputs, and kernels in the block
@@ -450,3 +437,69 @@ impl Display for AggregateBody {
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod test {
+    use tari_common_types::types::{ComAndPubSignature, Commitment, FixedHash, PublicKey, Signature};
+    use tari_script::{ExecutionStack, TariScript};
+
+    use super::*;
+    use crate::{
+        covenants::Covenant,
+        transactions::transaction_components::{EncryptedData, OutputFeatures, TransactionInputVersion},
+    };
+
+    #[test]
+    fn test_sorted() {
+        let mut body = AggregateBody::empty();
+        assert!(body.is_sorted());
+        let kernel = TransactionKernel::new_current_version(
+            KernelFeatures::default(),
+            0.into(),
+            0,
+            Commitment::default(),
+            Signature::default(),
+            None,
+        );
+        let output = TransactionOutput::default();
+        let input = TransactionInput::new_with_output_data(
+            TransactionInputVersion::get_current_version(),
+            OutputFeatures::default(),
+            Commitment::default(),
+            TariScript::default(),
+            ExecutionStack::default(),
+            ComAndPubSignature::default(),
+            PublicKey::default(),
+            Covenant::default(),
+            EncryptedData::default(),
+            ComAndPubSignature::default(),
+            FixedHash::zero(),
+            0.into(),
+        );
+
+        body.add_kernel(kernel.clone());
+        assert!(body.is_sorted());
+        assert!(!body.sorted);
+
+        body.add_input(input.clone());
+        assert!(body.is_sorted());
+        assert!(!body.sorted);
+
+        body.add_output(output.clone());
+        assert!(body.is_sorted());
+        assert!(!body.sorted);
+        body.sort();
+        assert!(body.sorted);
+
+        let mut body2 = body.clone();
+        body2.add_kernel(kernel);
+        assert!(!body2.is_sorted());
+
+        let mut body3 = body.clone();
+        body3.add_input(input);
+        assert!(!body3.is_sorted());
+
+        body.add_output(output);
+        assert!(!body.is_sorted())
+    }
+}

base_layer/core/src/transactions/coinbase_builder.rs

@@ -438,7 +438,9 @@ mod test {
             .build(rules.consensus_constants(42), rules.emission_schedule())
             .await
             .unwrap();
-        tx.body.outputs_mut()[0].features.maturity = 1;
+        let mut outputs = tx.body.outputs().clone();
+        outputs[0].features.maturity = 1;
+        tx.body = AggregateBody::new(tx.body().inputs().clone(), outputs, tx.body().kernels().clone());
         assert!(matches!(
             tx.body.check_coinbase_output(
                 block_reward,
@@ -518,6 +520,7 @@ mod test {
     use tari_key_manager::key_manager_service::KeyManagerInterface;
 
     use crate::transactions::{
+        aggregated_body::AggregateBody,
         key_manager::{TransactionKeyManagerBranch, TransactionKeyManagerInterface, TxoStage},
         test_helpers::{create_test_core_key_manager_with_memory_db, TestKeyManager},
         transaction_components::TransactionKernelVersion,

base_layer/core/src/transactions/transaction_components/test.rs

@@ -36,6 +36,7 @@ use super::*;
 use crate::{
     consensus::ConsensusManager,
     transactions::{
+        aggregated_body::AggregateBody,
         key_manager::TransactionKeyManagerInterface,
         tari_amount::{uT, MicroMinotari, T},
         test_helpers,
@@ -421,13 +422,16 @@ async fn check_cut_through() {
         .inputs()
         .clone()
         .iter()
-        .filter(|input| tx3_cut_through.body.outputs_mut().iter().any(|o| o.is_equal_to(input)))
+        .filter(|input| tx3_cut_through.body.outputs().iter().any(|o| o.is_equal_to(input)))
         .cloned()
         .collect();
+    let mut outputs = tx3_cut_through.body.outputs().clone();
+    let mut inputs = tx3_cut_through.body.inputs().clone();
     for input in double_inputs {
-        tx3_cut_through.body.outputs_mut().retain(|x| !input.is_equal_to(x));
-        tx3_cut_through.body.inputs_mut().retain(|x| *x != input);
+        outputs.retain(|x| !input.is_equal_to(x));
+        inputs.retain(|x| *x != input);
     }
+    tx3_cut_through.body = AggregateBody::new(inputs, outputs, tx3_cut_through.body.kernels().clone());
     tx3.body.sort();
     tx3_cut_through.body.sort();
 
@@ -484,8 +488,10 @@ async fn inputs_not_malleable() {
         .push(StackItem::Hash(*b"Pls put this on tha tari network"))
         .unwrap();
 
-    tx.body.inputs_mut()[0].set_script(script![Drop]).unwrap();
-    tx.body.inputs_mut()[0].input_data = stack;
+    let mut inputs = tx.body().inputs().clone();
+    inputs[0].set_script(script![Drop]).unwrap();
+    inputs[0].input_data = stack;
+    tx.body = AggregateBody::new(inputs, tx.body.outputs().clone(), tx.body().kernels().clone());
 
     let rules = ConsensusManager::builder(Network::LocalNet).build().unwrap();
     let factories = CryptoFactories::default();

base_layer/core/tests/tests/block_validation.rs

@@ -286,10 +286,12 @@ async fn inputs_are_not_malleable() {
         .await
         .unwrap();
 
-    let input_mut = block.body.inputs_mut().get_mut(0).unwrap();
+    let mut inputs = block.body.inputs().clone();
     // Put the crafted input into the block
-    input_mut.input_data = malicious_input.input_data;
-    input_mut.script_signature = malicious_input.script_signature;
+    inputs[0].input_data = malicious_input.input_data;
+    inputs[0].script_signature = malicious_input.script_signature;
+
+    block.body = AggregateBody::new(inputs, block.body.outputs().clone(), block.body.kernels().clone());
 
     let validator = BlockBodyFullValidator::new(blockchain.consensus_manager().clone(), true);
     let txn = blockchain.store().db_read_access().unwrap();