sdk: parse money fee calldata fallibly

bin/app/src/app/schema/wallet/send.rs

@@ -104,7 +104,7 @@ pub async fn make(
             let mut fees: u64 = 0;
             for call in tx.calls.iter() {
                 if call.data.is_money_fee() {
-                    if let Ok(fee) = darkfi_serial::deserialize(&call.data.data[1..9]) {
+                    if let Ok(fee) = call.data.money_fee_value() {
                         fees = fees.saturating_add(fee);
                     }
                 }

bin/drk/src/money.rs

@@ -123,6 +123,24 @@ pub const MONEY_ALIASES_COL_TOKEN_ID: &str = "token_id";
 
 pub const BALANCE_BASE10_DECIMALS: usize = 8;
 
+const MONEY_FEE_PREFIX_LEN: usize = 9;
+
+fn parse_money_function(data: &[u8]) -> Result<MoneyFunction> {
+    let Some(func) = data.first() else {
+        return Err(Error::ParseFailed("money call data is empty"))
+    };
+
+    Ok(MoneyFunction::try_from(*func)?)
+}
+
+async fn parse_money_fee_params(data: &[u8]) -> Result<MoneyFeeParamsV1> {
+    if data.len() < MONEY_FEE_PREFIX_LEN {
+        return Err(Error::ParseFailed("money fee call data is too short"))
+    }
+
+    Ok(deserialize_async(&data[MONEY_FEE_PREFIX_LEN..]).await?)
+}
+
 impl Drk {
     /// Initialize wallet with tables for the Money contract.
     pub async fn initialize_money(&self, output: &mut Vec<String>) -> WalletDbResult<()> {
@@ -770,10 +788,10 @@ impl Drk {
 
         let call = &calls[*call_idx];
         let data = &call.data.data;
-        match MoneyFunction::try_from(data[0])? {
+        match parse_money_function(data)? {
             MoneyFunction::FeeV1 => {
                 scan_cache_log!(scan_cache, "[parse_money_call] Found Money::FeeV1 call");
-                let params: MoneyFeeParamsV1 = deserialize_async(&data[9..]).await?;
+                let params = parse_money_fee_params(data).await?;
                 nullifiers.push(params.input.nullifier);
                 if !params.output.tx_local {
                     coins.push((params.output.coin, params.output.note, false));
@@ -1080,9 +1098,9 @@ impl Drk {
         let mut nullifiers: Vec<Nullifier> = vec![];
 
         let data = &call.data.data;
-        match MoneyFunction::try_from(data[0])? {
+        match parse_money_function(data)? {
             MoneyFunction::FeeV1 => {
-                let params: MoneyFeeParamsV1 = deserialize_async(&data[9..]).await?;
+                let params = parse_money_fee_params(data).await?;
                 nullifiers.push(params.input.nullifier);
             }
             MoneyFunction::TransferV1 => {
@@ -1402,7 +1420,7 @@ impl Drk {
                 continue
             }
 
-            match MoneyFunction::try_from(call.data.data[0])? {
+            match parse_money_function(&call.data.data)? {
                 MoneyFunction::FeeV1 => {
                     return Err(Error::Custom("Fee call already exists".to_string()))
                 }

bin/explorer/src/rpc.rs

@@ -27,9 +27,7 @@ use darkfi::{
     tx::Transaction,
     util::{encoding::base64, parse::encode_base10},
 };
-use darkfi_money_contract::MoneyFunction;
-use darkfi_sdk::crypto::contract_id::MONEY_CONTRACT_ID;
-use darkfi_serial::{deserialize_async, serialize_async};
+use darkfi_serial::serialize_async;
 use monero::{consensus::encode::Encodable, VarInt};
 use tiny_keccak::{Hasher, Keccak};
 use tinyjson::JsonValue;
@@ -82,15 +80,15 @@ impl TransactionInfo {
         let mut fee = 0;
         let mut calls = Vec::with_capacity(tx.calls.len());
         for call in &tx.calls {
-            let func = call.data.data[0];
+            let func = call.data.data.first().copied();
 
-            if call.data.contract_id == *MONEY_CONTRACT_ID && func == MoneyFunction::FeeV1 as u8 {
-                fee = deserialize_async(&call.data.data[1..9]).await.unwrap();
+            if let Ok(parsed_fee) = call.data.money_fee_value() {
+                fee = parsed_fee;
             }
 
             calls.push(ContractCallInfo::new(
                 call.data.contract_id.to_string(),
-                format!("0x{:02x}", func),
+                func.map(|func| format!("0x{func:02x}")).unwrap_or_else(|| "empty".to_string()),
                 call.data.data.len() as u64,
             ));
         }
@@ -132,15 +130,15 @@ impl ExplTxInfo {
         let mut fee = 0;
         let mut calls = Vec::with_capacity(tx.calls.len());
         for call in &tx.calls {
-            let func = call.data.data[0];
+            let func = call.data.data.first().copied();
 
-            if call.data.contract_id == *MONEY_CONTRACT_ID && func == MoneyFunction::FeeV1 as u8 {
-                fee = deserialize_async(&call.data.data[1..9]).await.unwrap();
+            if let Ok(parsed_fee) = call.data.money_fee_value() {
+                fee = parsed_fee;
             }
 
             calls.push(ContractCallInfo::new(
                 call.data.contract_id.to_string(),
-                format!("0x{:02x}", func),
+                func.map(|func| format!("0x{func:02x}")).unwrap_or_else(|| "empty".to_string()),
                 call.data.data.len() as u64,
             ));
         }

script/research/tx-replayer/src/main.rs

@@ -301,8 +301,8 @@ async fn verify_transaction_wasm(
     let total_gas_used = gas_data.total_gas_used();
 
     if verify_fee {
-        // Deserialize the fee call to find the paid fee
-        let fee: u64 = match deserialize_async(&tx.calls[fee_call_idx].data.data[1..9]).await {
+        // Extract the paid fee from the fee call.
+        let fee = match tx.calls[fee_call_idx].data.money_fee_value() {
             Ok(v) => v,
             Err(_) => return Err(TxVerifyFailed::InvalidFee.into()),
         };
@@ -476,8 +476,8 @@ async fn verify_transaction_zkps(
     let total_gas_used = gas_data.total_gas_used();
 
     if verify_fee {
-        // Deserialize the fee call to find the paid fee
-        let fee: u64 = match deserialize_async(&tx.calls[fee_call_idx].data.data[1..9]).await {
+        // Extract the paid fee from the fee call.
+        let fee = match tx.calls[fee_call_idx].data.money_fee_value() {
             Ok(v) => v,
             Err(_) => return Err(TxVerifyFailed::InvalidFee.into()),
         };
@@ -630,8 +630,8 @@ async fn verify_transaction_signatures(
     let total_gas_used = gas_data.total_gas_used();
 
     if verify_fee {
-        // Deserialize the fee call to find the paid fee
-        let fee: u64 = match deserialize_async(&tx.calls[fee_call_idx].data.data[1..9]).await {
+        // Extract the paid fee from the fee call.
+        let fee = match tx.calls[fee_call_idx].data.money_fee_value() {
             Ok(v) => v,
             Err(_) => return Err(TxVerifyFailed::InvalidFee.into()),
         };

src/contract/money/src/entrypoint.rs

@@ -19,7 +19,7 @@
 use darkfi_sdk::{
     crypto::{pasta_prelude::Field, smt::EMPTY_NODES_FP, ContractId, MerkleNode, MerkleTree},
     dark_tree::DarkLeaf,
-    error::ContractResult,
+    error::{ContractError, ContractResult},
     msg,
     pasta::pallas,
     wasm, ContractCall,
@@ -40,6 +40,12 @@ use crate::{
     MONEY_CONTRACT_NULLIFIER_ROOTS_TREE, MONEY_CONTRACT_TOKEN_FREEZE_TREE,
 };
 
+fn parse_money_function(data: &[u8]) -> Result<MoneyFunction, ContractError> {
+    let Some(func) = data.first() else { return Err(ContractError::InvalidFunction) };
+
+    MoneyFunction::try_from(*func)
+}
+
 /// `Money::Fee` functions
 mod fee_v1;
 use fee_v1::{
@@ -230,7 +236,7 @@ fn get_metadata(cid: ContractId, ix: &[u8]) -> ContractResult {
     let call_idx = wasm::util::get_call_index()? as usize;
     let calls: Vec<DarkLeaf<ContractCall>> = deserialize(ix)?;
     let self_ = &calls[call_idx].data;
-    let func = MoneyFunction::try_from(self_.data[0])?;
+    let func = parse_money_function(&self_.data)?;
 
     let metadata = match func {
         MoneyFunction::FeeV1 => {
@@ -263,7 +269,7 @@ fn process_instruction(cid: ContractId, ix: &[u8]) -> ContractResult {
     let call_idx = wasm::util::get_call_index()? as usize;
     let calls: Vec<DarkLeaf<ContractCall>> = deserialize(ix)?;
     let self_ = &calls[call_idx].data;
-    let func = MoneyFunction::try_from(self_.data[0])?;
+    let func = parse_money_function(&self_.data)?;
 
     let update_data = match func {
         MoneyFunction::FeeV1 => {
@@ -302,7 +308,7 @@ fn process_instruction(cid: ContractId, ix: &[u8]) -> ContractResult {
 /// is the update data retrieved from `process_instruction()`, prefixed with the
 /// contract function.
 fn process_update(cid: ContractId, update_data: &[u8]) -> ContractResult {
-    match MoneyFunction::try_from(update_data[0])? {
+    match parse_money_function(update_data)? {
         MoneyFunction::FeeV1 => {
             let update: MoneyFeeUpdateV1 = deserialize(&update_data[1..])?;
             Ok(money_fee_process_update_v1(cid, update)?)

src/contract/money/src/entrypoint/fee_v1.rs

@@ -50,6 +50,20 @@ use crate::{
     MONEY_CONTRACT_NULLIFIER_ROOTS_TREE, MONEY_CONTRACT_ZKAS_FEE_NS_V1,
 };
 
+const MONEY_FEE_PREFIX_LEN: usize = 9;
+
+fn parse_fee_call_data(data: &[u8]) -> Result<(u64, MoneyFeeParamsV1), ContractError> {
+    if data.len() < MONEY_FEE_PREFIX_LEN {
+        msg!("[FeeV1] Error: Fee call data is too short");
+        return Err(MoneyError::InvalidFeeCall.into())
+    }
+
+    let fee = deserialize(&data[1..MONEY_FEE_PREFIX_LEN])?;
+    let params = deserialize(&data[MONEY_FEE_PREFIX_LEN..])?;
+
+    Ok((fee, params))
+}
+
 /// `get_metadata` function for `Money::FeeV1`
 pub(crate) fn money_fee_get_metadata_v1(
     _cid: ContractId,
@@ -57,9 +71,7 @@ pub(crate) fn money_fee_get_metadata_v1(
     calls: Vec<DarkLeaf<ContractCall>>,
 ) -> Result<Vec<u8>, ContractError> {
     let self_ = &calls[call_idx].data;
-    // The first 8 bytes here is the u64 fee, so we get the params from that offset.
-    // (Plus 1, which is the function identifier byte)
-    let params: MoneyFeeParamsV1 = deserialize(&self_.data[9..])?;
+    let (_, params) = parse_fee_call_data(&self_.data)?;
 
     // Public inputs for the ZK proofs we have to verify
     let mut zk_public_inputs: Vec<(String, Vec<pallas::Base>)> = vec![];
@@ -103,8 +115,7 @@ pub(crate) fn money_fee_process_instruction_v1(
     calls: Vec<DarkLeaf<ContractCall>>,
 ) -> Result<Vec<u8>, ContractError> {
     let self_ = &calls[call_idx];
-    let fee: u64 = deserialize(&self_.data.data[1..9])?;
-    let params: MoneyFeeParamsV1 = deserialize(&self_.data.data[9..])?;
+    let (fee, params) = parse_fee_call_data(&self_.data.data)?;
 
     // We should have _some_ fee paid...
     if fee == 0 {

src/contract/money/src/error.rs

@@ -114,6 +114,9 @@ pub enum MoneyError {
 
     #[error("Invalid local output")]
     InvalidLocalOutput,
+
+    #[error("Invalid fee call")]
+    InvalidFeeCall,
 }
 
 impl From<MoneyError> for ContractError {
@@ -150,6 +153,7 @@ impl From<MoneyError> for ContractError {
             MoneyError::ChildrenIndexesLengthMismatch => Self::Custom(29),
             MoneyError::BurnMissingInputs => Self::Custom(30),
             MoneyError::InvalidLocalOutput => Self::Custom(31),
+            MoneyError::InvalidFeeCall => Self::Custom(32),
         }
     }
 }

src/sdk/src/tx.rs

@@ -23,11 +23,11 @@ use std::{
 
 #[cfg(feature = "async")]
 use darkfi_serial::async_trait;
-use darkfi_serial::{SerialDecodable, SerialEncodable};
+use darkfi_serial::{deserialize, SerialDecodable, SerialEncodable};
 
 use super::{
     crypto::{ContractId, SecretKey},
-    ContractError, GenericResult,
+    ContractError, FeeError, FeeResult, GenericResult,
 };
 use crate::crypto::{DAO_CONTRACT_ID, DEPLOYOOOR_CONTRACT_ID, MONEY_CONTRACT_ID};
 
@@ -89,6 +89,15 @@ impl ContractCall {
         self.matches_contract_call_type(*MONEY_CONTRACT_ID, 0x00)
     }
 
+    /// Returns the paid native token fee encoded in a `Money::FeeV1` call.
+    pub fn money_fee_value(&self) -> FeeResult<u64> {
+        if !self.is_money_fee() || self.data.len() < 9 {
+            return Err(FeeError::InvalidFeeCall)
+        }
+
+        deserialize(&self.data[1..9]).map_err(|_| FeeError::InvalidFeeCall)
+    }
+
     /// Returns true if call is a money genesis mint.
     pub fn is_money_genesis_mint(&self) -> bool {
         self.matches_contract_call_type(*MONEY_CONTRACT_ID, 0x01)
@@ -160,6 +169,53 @@ impl ContractCall {
     }
 }
 
+#[cfg(test)]
+mod tests {
+    use darkfi_serial::serialize;
+
+    use super::*;
+    use crate::crypto::DAO_CONTRACT_ID;
+
+    #[test]
+    fn money_fee_value_extracts_paid_fee() {
+        let fee = 20_u64;
+        let mut data = vec![0x00];
+        data.extend(serialize(&fee));
+        data.extend([0xab, 0xcd]);
+
+        let call = ContractCall { contract_id: *MONEY_CONTRACT_ID, data };
+
+        assert_eq!(call.money_fee_value().unwrap(), fee);
+    }
+
+    #[test]
+    fn money_fee_value_rejects_wrong_contract() {
+        let mut data = vec![0x00];
+        data.extend(serialize(&20_u64));
+
+        let call = ContractCall { contract_id: *DAO_CONTRACT_ID, data };
+
+        assert_eq!(call.money_fee_value(), Err(FeeError::InvalidFeeCall));
+    }
+
+    #[test]
+    fn money_fee_value_rejects_wrong_function() {
+        let mut data = vec![0x01];
+        data.extend(serialize(&20_u64));
+
+        let call = ContractCall { contract_id: *MONEY_CONTRACT_ID, data };
+
+        assert_eq!(call.money_fee_value(), Err(FeeError::InvalidFeeCall));
+    }
+
+    #[test]
+    fn money_fee_value_rejects_short_data() {
+        let call = ContractCall { contract_id: *MONEY_CONTRACT_ID, data: vec![0x00, 0x01] };
+
+        assert_eq!(call.money_fee_value(), Err(FeeError::InvalidFeeCall));
+    }
+}
+
 // Avoid showing the data in the debug output since often the calldata is very long.
 impl Debug for ContractCall {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {

src/validator/verification.rs

@@ -889,13 +889,13 @@ pub async fn verify_transaction(
     }
 
     if verify_fee {
-        // Deserialize the fee call to find the paid fee
-        let fee: u64 = match deserialize_async(&tx.calls[fee_call_idx].data.data[1..9]).await {
+        // Extract the paid fee from the fee call.
+        let fee = match tx.calls[fee_call_idx].data.money_fee_value() {
             Ok(v) => v,
             Err(e) => {
                 error!(
                     target: "validator::verification::verify_transaction",
-                    "[VALIDATOR] Failed deserializing tx {tx_hash} fee call: {e}"
+                    "[VALIDATOR] Failed parsing tx {tx_hash} fee call: {e}"
                 );
                 return Err(TxVerifyFailed::InvalidFee.into())
             }