Add bytecode length to tx (#123)

A signed transaction must contain a bytecode length so it won't be
malleable.

Prior to this commit, it checked dynamically; but then the witnesses are
cleared before the tx id computation, so the length is lost.

This commit introduces the attribute that will concretely store the
bytecode len so it will be properly signed and checked.

fuel-tx/src/builder.rs

@@ -17,24 +17,27 @@ pub struct TransactionBuilder<'a> {
 
 impl<'a> TransactionBuilder<'a> {
     pub fn create(
-        bytecode_witness_index: u8,
+        bytecode: Witness,
         salt: Salt,
         static_contracts: Vec<ContractId>,
         storage_slots: Vec<StorageSlot>,
     ) -> Self {
-        let tx = Transaction::create(
+        let mut tx = Transaction::create(
+            Default::default(),
             Default::default(),
             Default::default(),
             Default::default(),
             Default::default(),
-            bytecode_witness_index,
             salt,
             static_contracts,
             storage_slots,
             Default::default(),
             Default::default(),
             Default::default(),
         );
+
+        tx._set_bytecode(bytecode);
+
         let sign_keys = Vec::new();
 
         Self { tx, sign_keys }

fuel-tx/src/transaction.rs

@@ -55,6 +55,7 @@ pub enum Transaction {
         gas_limit: Word,
         byte_price: Word,
         maturity: Word,
+        bytecode_length: Word,
         bytecode_witness_index: u8,
         salt: Salt,
         static_contracts: Vec<ContractId>,
@@ -118,7 +119,7 @@ impl Transaction {
         }
     }
 
-    pub const fn create(
+    pub fn create(
         gas_price: Word,
         gas_limit: Word,
         byte_price: Word,
@@ -131,11 +132,19 @@ impl Transaction {
         outputs: Vec<Output>,
         witnesses: Vec<Witness>,
     ) -> Self {
+        // TODO consider split this function in two; one that will trust a provided bytecod len,
+        // and other that will return a resulting, failing if the witness index isn't present
+        let bytecode_length = witnesses
+            .get(bytecode_witness_index as usize)
+            .map(|witness| witness.as_ref().len() as Word / 4)
+            .unwrap_or(0);
+
         Self::Create {
             gas_price,
             gas_limit,
             byte_price,
             maturity,
+            bytecode_length,
             bytecode_witness_index,
             salt,
             static_contracts,
@@ -430,6 +439,7 @@ mod tests {
             gas_limit: 0,
             byte_price: 0,
             maturity: 0,
+            bytecode_length: 0,
             bytecode_witness_index: 0,
             salt: Default::default(),
             static_contracts: vec![],
@@ -444,6 +454,7 @@ mod tests {
             gas_limit: 0,
             byte_price: 0,
             maturity: 0,
+            bytecode_length: 0,
             bytecode_witness_index: 0,
             salt: Default::default(),
             static_contracts: vec![],

fuel-tx/src/transaction/internals.rs

@@ -1,6 +1,7 @@
 use crate::{Input, Output, Transaction, Witness};
 
 use alloc::vec::Vec;
+use fuel_asm::Word;
 
 #[cfg(feature = "internals")]
 impl Transaction {
@@ -23,6 +24,11 @@ impl Transaction {
     pub fn set_script(&mut self, script: Vec<u8>) -> Option<()> {
         self._set_script(script)
     }
+
+    /// Set the transaction bytecode, if create variant. Return none otherwise.
+    pub fn set_bytecode(&mut self, bytecode: Witness) -> Option<()> {
+        self._set_bytecode(bytecode)
+    }
 }
 
 impl Transaction {
@@ -56,4 +62,23 @@ impl Transaction {
             Self::Create { .. } => None,
         }
     }
+
+    pub(crate) fn _set_bytecode(&mut self, bytecode: Witness) -> Option<()> {
+        match self {
+            Self::Script { .. } => None,
+            Self::Create {
+                bytecode_length,
+                bytecode_witness_index,
+                witnesses,
+                ..
+            } => {
+                *bytecode_length = (bytecode.as_ref().len() / 4) as Word;
+                *bytecode_witness_index = witnesses.len() as u8;
+
+                witnesses.push(bytecode);
+
+                Some(())
+            }
+        }
+    }
 }

fuel-tx/src/transaction/txio.rs

@@ -105,6 +105,7 @@ impl io::Read for Transaction {
                 gas_limit,
                 byte_price,
                 maturity,
+                bytecode_length,
                 bytecode_witness_index,
                 salt,
                 static_contracts,
@@ -114,17 +115,12 @@ impl io::Read for Transaction {
                 witnesses,
                 ..
             } => {
-                let bytecode_length = witnesses
-                    .get(*bytecode_witness_index as usize)
-                    .map(|witness| witness.as_ref().len() as Word / 4)
-                    .unwrap_or(0);
-
                 let buf = bytes::store_number_unchecked(buf, TransactionRepr::Create as Word);
                 let buf = bytes::store_number_unchecked(buf, *gas_price);
                 let buf = bytes::store_number_unchecked(buf, *gas_limit);
                 let buf = bytes::store_number_unchecked(buf, *byte_price);
                 let buf = bytes::store_number_unchecked(buf, *maturity);
-                let buf = bytes::store_number_unchecked(buf, bytecode_length);
+                let buf = bytes::store_number_unchecked(buf, *bytecode_length);
                 let buf = bytes::store_number_unchecked(buf, *bytecode_witness_index);
                 let buf = bytes::store_number_unchecked(buf, static_contracts.len() as Word);
                 let buf = bytes::store_number_unchecked(buf, storage_slots.len() as Word);
@@ -251,7 +247,7 @@ impl io::Write for Transaction {
                 let (gas_limit, buf) = unsafe { bytes::restore_number_unchecked(buf) };
                 let (byte_price, buf) = unsafe { bytes::restore_number_unchecked(buf) };
                 let (maturity, buf) = unsafe { bytes::restore_number_unchecked(buf) };
-                let (_bytecode_length, buf) = unsafe { bytes::restore_u16_unchecked(buf) };
+                let (bytecode_length, buf) = unsafe { bytes::restore_number_unchecked(buf) };
                 let (bytecode_witness_index, buf) = unsafe { bytes::restore_u8_unchecked(buf) };
                 let (static_contracts_len, buf) = unsafe { bytes::restore_usize_unchecked(buf) };
                 let (storage_slots_len, buf) = unsafe { bytes::restore_u16_unchecked(buf) };
@@ -306,6 +302,7 @@ impl io::Write for Transaction {
                     gas_limit,
                     byte_price,
                     maturity,
+                    bytecode_length,
                     bytecode_witness_index,
                     salt,
                     static_contracts,

fuel-tx/src/transaction/validation.rs

@@ -306,17 +306,21 @@ impl Transaction {
                 inputs,
                 outputs,
                 witnesses,
+                bytecode_length,
                 bytecode_witness_index,
                 static_contracts,
                 storage_slots,
                 ..
             } => {
-                match witnesses.get(*bytecode_witness_index as usize) {
-                    Some(witness) if witness.as_ref().len() as u64 > CONTRACT_MAX_SIZE => {
-                        Err(ValidationError::TransactionCreateBytecodeLen)?
-                    }
-                    None => Err(ValidationError::TransactionCreateBytecodeWitnessIndex)?,
-                    _ => (),
+                let bytecode_witness_len = witnesses
+                    .get(*bytecode_witness_index as usize)
+                    .map(|w| w.as_ref().len() as Word)
+                    .ok_or(ValidationError::TransactionCreateBytecodeWitnessIndex)?;
+
+                if bytecode_witness_len > CONTRACT_MAX_SIZE
+                    || bytecode_witness_len / 4 != *bytecode_length
+                {
+                    return Err(ValidationError::TransactionCreateBytecodeLen);
                 }
 
                 if static_contracts.len() > MAX_STATIC_CONTRACTS as usize {