feat: borsh sized serialization should be fallible (#5537)

Description
---
Adds fallibility to borsh serialized size method. 

Motivation and Context
---
Audit

How Has This Been Tested?
---

What process can a PR reviewer use to test or verify this change?
---

<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->

applications/tari_base_node/src/commands/command/get_mempool_state.rs

@@ -76,7 +76,7 @@ impl CommandContext {
                     tx.body.outputs().len(),
                     tx.body.kernels().len(),
                     tx.body.inputs().len(),
-                    tx.body.sum_features_and_scripts_size(),
+                    tx.body.sum_features_and_scripts_size()?,
                 );
             }
         }

applications/tari_base_node/src/commands/command/status.rs

@@ -95,7 +95,7 @@ impl CommandContext {
                     if mempool_stats.unconfirmed_weight == 0 {
                         0
                     } else {
-                        1 + mempool_stats.unconfirmed_weight / constants.max_block_weight_excluding_coinbase()
+                        1 + mempool_stats.unconfirmed_weight / constants.max_block_weight_excluding_coinbase()?
                     },
                 ),
             );

applications/tari_console_wallet/src/ui/state/app_state.rs

@@ -775,8 +775,11 @@ impl AppStateInner {
         });
         self.data.pending_txs = pending_transactions
             .iter()
-            .map(|tx| CompletedTransactionInfo::from_completed_transaction(tx.clone(), &self.get_transaction_weight()))
-            .collect();
+            .map(|tx| {
+                CompletedTransactionInfo::from_completed_transaction(tx.clone(), &self.get_transaction_weight())
+                    .map_err(|e| UiError::TransactionError(e.to_string()))
+            })
+            .collect::<Result<Vec<_>, _>>()?;
 
         let mut completed_transactions: Vec<CompletedTransaction> = Vec::new();
         completed_transactions.extend(
@@ -807,8 +810,11 @@ impl AppStateInner {
 
         self.data.completed_txs = completed_transactions
             .iter()
-            .map(|tx| CompletedTransactionInfo::from_completed_transaction(tx.clone(), &self.get_transaction_weight()))
-            .collect();
+            .map(|tx| {
+                CompletedTransactionInfo::from_completed_transaction(tx.clone(), &self.get_transaction_weight())
+                    .map_err(|e| UiError::TransactionError(e.to_string()))
+            })
+            .collect::<Result<Vec<_>, _>>()?;
         self.updated = true;
         Ok(())
     }
@@ -851,7 +857,8 @@ impl AppStateInner {
             },
             Some(tx) => {
                 let tx =
-                    CompletedTransactionInfo::from_completed_transaction(tx.into(), &self.get_transaction_weight());
+                    CompletedTransactionInfo::from_completed_transaction(tx.into(), &self.get_transaction_weight())
+                        .map_err(|e| UiError::TransactionError(e.to_string()))?;
                 if let Some(index) = self.data.pending_txs.iter().position(|i| i.tx_id == tx_id) {
                     if tx.status == TransactionStatus::Pending && tx.cancelled.is_none() {
                         self.data.pending_txs[index] = tx;
@@ -1170,18 +1177,21 @@ pub struct CompletedTransactionInfo {
 }
 
 impl CompletedTransactionInfo {
-    pub fn from_completed_transaction(tx: CompletedTransaction, transaction_weighting: &TransactionWeight) -> Self {
+    pub fn from_completed_transaction(
+        tx: CompletedTransaction,
+        transaction_weighting: &TransactionWeight,
+    ) -> std::io::Result<Self> {
         let excess_signature = tx
             .transaction
             .first_kernel_excess_sig()
             .map(|s| s.get_signature().to_hex())
             .unwrap_or_default();
         let is_coinbase = tx.is_coinbase();
-        let weight = tx.transaction.calculate_weight(transaction_weighting);
+        let weight = tx.transaction.calculate_weight(transaction_weighting)?;
         let inputs_count = tx.transaction.body.inputs().len();
         let outputs_count = tx.transaction.body.outputs().len();
 
-        Self {
+        Ok(Self {
             tx_id: tx.tx_id,
             source_address: tx.source_address.clone(),
             destination_address: tx.destination_address.clone(),
@@ -1206,7 +1216,7 @@ impl CompletedTransactionInfo {
             weight,
             inputs_count,
             outputs_count,
-        }
+        })
     }
 }
 

applications/tari_console_wallet/src/ui/ui_error.rs

@@ -56,4 +56,6 @@ pub enum UiError {
     BurntProofFileExists,
     #[error("Channel send error: `{0}`")]
     SendError(String),
+    #[error("Transaction error: `{0}`")]
+    TransactionError(String),
 }

base_layer/core/benches/mempool.rs

@@ -53,15 +53,15 @@ mod benches {
         num_inputs: usize,
         num_outputs: usize,
         features: OutputFeatures,
-    ) -> Vec<Arc<Transaction>> {
+    ) -> std::io::Result<Vec<Arc<Transaction>>> {
         let key_manager = create_test_core_key_manager_with_memory_db();
         let mut txs = Vec::new();
         for _ in 0..num_txs {
             let (tx, _, _) =
-                tx!(T, fee: uT, inputs: num_inputs, outputs: num_outputs, features: features.clone(), &key_manager);
+                tx!(T, fee: uT, inputs: num_inputs, outputs: num_outputs, features: features.clone(), &key_manager)?;
             txs.push(Arc::new(tx));
         }
-        txs
+        Ok(txs)
     }
 
     pub fn mempool_perf_test(c: &mut Criterion) {
@@ -80,12 +80,14 @@ mod benches {
             NUM_TXNS * 1000,
             NUM_TXNS * MAX_TRANSACTION_OUTPUTS
         );
-        let transactions = runtime.block_on(generate_transactions(
-            NUM_TXNS,
-            1000,
-            MAX_TRANSACTION_OUTPUTS,
-            OutputFeatures::default(),
-        ));
+        let transactions = runtime
+            .block_on(generate_transactions(
+                NUM_TXNS,
+                1000,
+                MAX_TRANSACTION_OUTPUTS,
+                OutputFeatures::default(),
+            ))
+            .expect("Failed to get transactions");
         c.bench_function("Mempool Insert", move |b| {
             let mut idx = 0;
             b.iter(|| {

base_layer/core/src/base_node/comms_interface/inbound_handlers.rs

@@ -255,7 +255,9 @@ where B: BlockchainBackend + 'static
                 header.version = constants.blockchain_version();
                 header.pow.pow_algo = request.algo;
 
-                let constants_weight = constants.max_block_weight_excluding_coinbase();
+                let constants_weight = constants
+                    .max_block_weight_excluding_coinbase()
+                    .map_err(|e| CommsInterfaceError::InternalError(e.to_string()))?;
                 let asking_weight = if request.max_weight > constants_weight || request.max_weight == 0 {
                     constants_weight
                 } else {
@@ -298,6 +300,7 @@ where B: BlockchainBackend + 'static
                     block_template
                         .body
                         .calculate_weight(constants.transaction_weight_params())
+                        .map_err(|e| CommsInterfaceError::InternalError(e.to_string()))?
                 );
                 trace!(target: LOG_TARGET, "{}", block_template);
                 Ok(NodeCommsResponse::NewBlockTemplate(block_template))
@@ -310,7 +313,10 @@ where B: BlockchainBackend + 'static
                     target: LOG_TARGET,
                     "Prepared new block from template (hash: {}, weight: {}, {})",
                     block.hash().to_hex(),
-                    block.body.calculate_weight(constants.transaction_weight_params()),
+                    block
+                        .body
+                        .calculate_weight(constants.transaction_weight_params())
+                        .map_err(|e| CommsInterfaceError::InternalError(e.to_string()))?,
                     block.body.to_counts_string()
                 );
                 Ok(NodeCommsResponse::NewBlock {

base_layer/core/src/common/borsh.rs

@@ -37,16 +37,16 @@ impl<T: BorshDeserialize> FromBytes<T> for T {
 }
 
 pub trait SerializedSize {
-    fn get_serialized_size(&self) -> usize;
+    fn get_serialized_size(&self) -> Result<usize>;
 }
 
 impl<T: BorshSerialize> SerializedSize for T {
-    fn get_serialized_size(&self) -> usize {
+    fn get_serialized_size(&self) -> Result<usize> {
         let mut counter = ByteCounter::new();
         // The [ByteCounter] never throws an error. But be aware that we can introduce an Error into custom serialize.
         // e.g. MoneroPowData is using external functions that can throw an error. But we don't use this function for
         // it.
-        self.serialize(&mut counter).unwrap();
-        counter.get()
+        self.serialize(&mut counter)?;
+        Ok(counter.get())
     }
 }

base_layer/core/src/consensus/consensus_constants.rs

@@ -208,18 +208,18 @@ impl ConsensusConstants {
 
     /// Maximum transaction weight used for the construction of new blocks. It leaves place for 1 kernel and 1 output
     /// with default features, as well as the maximum possible value of the `coinbase_extra` field
-    pub fn max_block_weight_excluding_coinbase(&self) -> u64 {
-        self.max_block_transaction_weight - self.calculate_1_output_kernel_weight()
+    pub fn max_block_weight_excluding_coinbase(&self) -> std::io::Result<u64> {
+        Ok(self.max_block_transaction_weight - self.calculate_1_output_kernel_weight()?)
     }
 
-    fn calculate_1_output_kernel_weight(&self) -> u64 {
+    fn calculate_1_output_kernel_weight(&self) -> std::io::Result<u64> {
         let output_features = OutputFeatures { ..Default::default() };
         let max_extra_size = self.coinbase_output_features_extra_max_length() as usize;
 
         let features_and_scripts_size = self.transaction_weight.round_up_features_and_scripts_size(
-            output_features.get_serialized_size() + max_extra_size + script![Nop].get_serialized_size(),
+            output_features.get_serialized_size()? + max_extra_size + script![Nop].get_serialized_size()?,
         );
-        self.transaction_weight.calculate(1, 0, 1, features_and_scripts_size)
+        Ok(self.transaction_weight.calculate(1, 0, 1, features_and_scripts_size))
     }
 
     pub fn coinbase_output_features_extra_max_length(&self) -> u32 {

base_layer/core/src/mempool/error.rs

@@ -40,4 +40,6 @@ pub enum MempoolError {
     RwLockPoisonError,
     #[error(transparent)]
     BlockingTaskError(#[from] JoinError),
+    #[error("Internal error: {0}")]
+    InternalError(String),
 }

base_layer/core/src/mempool/mempool.rs

@@ -59,14 +59,21 @@ impl Mempool {
 
     /// Insert an unconfirmed transaction into the Mempool.
     pub async fn insert(&self, tx: Arc<Transaction>) -> Result<TxStorageResponse, MempoolError> {
-        self.with_write_access(|storage| Ok(storage.insert(tx))).await
+        self.with_write_access(|storage| {
+            storage
+                .insert(tx)
+                .map_err(|e| MempoolError::InternalError(e.to_string()))
+        })
+        .await
     }
 
     /// Inserts all transactions into the mempool.
     pub async fn insert_all(&self, transactions: Vec<Arc<Transaction>>) -> Result<(), MempoolError> {
         self.with_write_access(|storage| {
             for tx in transactions {
-                storage.insert(tx);
+                storage
+                    .insert(tx)
+                    .map_err(|e| MempoolError::InternalError(e.to_string()))?;
             }
 
             Ok(())
@@ -135,7 +142,8 @@ impl Mempool {
 
     /// Gathers and returns the stats of the Mempool.
     pub async fn stats(&self) -> Result<StatsResponse, MempoolError> {
-        self.with_read_access(|storage| Ok(storage.stats())).await
+        self.with_read_access(|storage| storage.stats().map_err(|e| MempoolError::InternalError(e.to_string())))
+            .await
     }
 
     /// Gathers and returns a breakdown of all the transaction in the Mempool.