Fixed no zeroing malleable fields for Create transaction

CHANGELOG.md

@@ -10,7 +10,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Changed
 
 #### Breaking
-- [#765](https://github.com/FuelLabs/fuel-vm/pull/765): corrected the gas units for WDOP and WQOP
+
+- [#767](https://github.com/FuelLabs/fuel-vm/pull/767): Fixed no zeroing malleable fields for `Create` transaction.
+- [#765](https://github.com/FuelLabs/fuel-vm/pull/765): Corrected the gas units for WDOP and WQOP.
 
 ## [Version 0.53.0]
 

fuel-vm/src/interpreter.rs

@@ -528,11 +528,6 @@ pub trait ExecutableTransaction:
             }) if *input_index as usize == input)
         })
     }
-
-    /// Prepares the transaction for execution.
-    fn prepare_init_execute(&mut self) {
-        self.prepare_sign()
-    }
 }
 
 impl ExecutableTransaction for Create {
@@ -547,8 +542,6 @@ impl ExecutableTransaction for Create {
     fn transaction_type() -> Word {
         TransactionRepr::Create as Word
     }
-
-    fn prepare_init_execute(&mut self) {}
 }
 
 impl ExecutableTransaction for Script {

fuel-vm/src/interpreter/initialization.rs

@@ -39,7 +39,7 @@ where
         runtime_balances: RuntimeBalances,
         gas_limit: Word,
     ) -> Result<(), RuntimeError<S::DataError>> {
-        tx.prepare_init_execute();
+        tx.prepare_sign();
         self.tx = tx;
 
         self.initial_balances = initial_balances.clone();

fuel-vm/src/tests/validation.rs

@@ -1,3 +1,13 @@
+use crate::{
+    checked_transaction::{
+        CheckPredicateParams,
+        EstimatePredicates,
+    },
+    prelude::{
+        MemoryInstance,
+        *,
+    },
+};
 use core::str::FromStr;
 use fuel_asm::{
     op,
@@ -6,14 +16,17 @@ use fuel_asm::{
 use fuel_tx::{
     field::{
         Inputs,
+        Outputs,
         ReceiptsRoot,
+        StorageSlots,
     },
-    input::coin::CoinSigned,
+    input::coin::CoinPredicate,
     ConsensusParameters,
+    Input,
     TransactionBuilder,
+    UtxoId,
 };
 use fuel_types::BlockHeight;
-use fuel_vm::prelude::*;
 use rand::{
     rngs::StdRng,
     Rng,
@@ -53,115 +66,237 @@ fn transaction_can_be_executed_after_maturity() {
     assert!(result.is_ok());
 }
 
-/// Malleable fields should not affect validity of the block
+/// Malleable fields should not affect validity of the create transaction
 #[test]
-fn malleable_fields_do_not_affect_validity() {
-    let rng = &mut StdRng::seed_from_u64(8586);
-
+fn malleable_fields_do_not_affect_validity_of_create() {
     let params = ConsensusParameters::default();
 
     let tx_start_ptr = params.tx_params().tx_offset();
     let tx_size_ptr = tx_start_ptr - 8;
+    let witness_count_offset =
+        <Create as StorageSlots>::storage_slots_offset_static() - 8;
 
-    let tx = TransactionBuilder::script(
-        vec![
-            // Log tx id (hash)
-            op::movi(0x21, 32),
-            op::logd(0x00, 0x00, 0x00, 0x21),
-            // Load tx size
-            op::movi(0x21, tx_size_ptr as u32),
-            op::lw(0x21, 0x21, 0),
-            // Make heap space for tx bytes, chain_id and computed tx hash
-            op::addi(0x22, 0x21, 8 + 32),
-            op::aloc(0x22),
-            // Construct the chain_id and tx bytes for hashing
-            op::addi(0x22, RegId::HP, 32), // Chain id position
-            op::gtf_args(0x20, 0x00, GTFArgs::ScriptData),
-            op::mcpi(0x22, 0x20, 8), // Copy chain id
-            op::movi(0x20, tx_start_ptr as u32),
-            op::addi(0x23, 0x22, 8),   // Tx bytes position
-            op::mcp(0x23, 0x20, 0x21), // Copy tx bytes
-            // Assert that there is exactly one witness
-            op::gtf_args(0x26, 0x00, GTFArgs::ScriptWitnessesCount),
-            op::eq(0x26, 0x26, RegId::ONE),
-            op::jnzf(0x26, 0x00, 1),
-            op::ret(0),
-            // Zero out the only witness (in the heap)
-            op::gtf_args(0x26, 0x00, GTFArgs::WitnessData),
-            op::gtf_args(0x27, 0x00, GTFArgs::WitnessDataLength),
-            op::sub(0x27, 0x26, 0x20), // Offset in relative to the tx bytes
-            op::add(0x27, 0x27, RegId::HP), // Redirect the pointer to heap
-            op::addi(0x26, 0x27, 32 + 8), // Offset of tx bytes in heap
-            op::gtf_args(0x27, 0x00, GTFArgs::WitnessDataLength),
-            op::mcl(0x26, 0x27),
-            // Zero out witness count
-            op::gtf_args(0x26, 0x00, GTFArgs::Script),
-            op::subi(0x26, 0x26, 8), // Offset to get the witness count address
-            op::sub(0x26, 0x26, 0x20), // Offset in relative to the tx bytes
-            op::add(0x26, 0x26, RegId::HP), // Redirect the pointer to heap
-            op::addi(0x26, 0x26, 32 + 8), // Offset of tx bytes in heap
-            op::sw(0x26, RegId::ZERO, 0), // Zero out the witness count
-            // Actually hash
-            op::subi(0x24, 0x21, 64 + 8 - 8), // Offset ptr
-            op::s256(RegId::HP, 0x22, 0x24),  // Compute tx id hash
-            op::movi(0x25, 32),               // Hash size
-            op::logd(0x00, 0x00, RegId::HP, 0x25), // Log computed txid
-            op::logd(0, 0, 0x22, 0x24),
-            // Done
-            op::ret(0x00),
-        ]
-        .into_iter()
-        .collect(),
-        params.chain_id().to_be_bytes().to_vec(),
-    )
-    .add_unsigned_coin_input(
-        SecretKey::random(rng),
-        UtxoId::new([3; 32].into(), 0),
-        123456789,
-        AssetId::default(),
-        Default::default(),
-    )
-    .script_gas_limit(1_000_000)
-    .finalize();
-
-    let run_tx = |tx: Script| {
-        let original_id = tx.id(&params.chain_id());
-
-        let vm = Interpreter::<_, _, Script>::with_memory_storage();
-        let mut client = MemoryClient::from_txtor(vm.into());
-        let receipts =
-            client.transact(tx.into_checked(0u32.into(), &params).expect("valid tx"));
-
-        let start_id = receipts[0].data().unwrap();
-        let computed_id = receipts[1].data().unwrap();
+    let predicate_bytecode = [
+        // Load tx size
+        op::movi(0x21, tx_size_ptr as u32),
+        op::lw(0x21, 0x21, 0),
+        // Make heap space for tx bytes, chain_id and computed tx hash
+        op::addi(0x22, 0x21, 8 + 32),
+        op::aloc(0x22),
+        // Construct the chain_id and tx bytes for hashing
+        op::addi(0x22, RegId::HP, 32), // Chain id position
+        op::gtf_args(0x20, 0x00, GTFArgs::InputCoinPredicateData),
+        op::mcpi(0x22, 0x20, 8), // Copy chain id
+        op::movi(0x20, tx_start_ptr as u32),
+        op::addi(0x23, 0x22, 8),   // Tx bytes position
+        op::mcp(0x23, 0x20, 0x21), // Copy tx bytes
+        // Assert that there is exactly one witness
+        op::gtf_args(0x26, 0x00, GTFArgs::ScriptWitnessesCount),
+        op::eq(0x26, 0x26, RegId::ONE),
+        op::jnzf(0x26, 0x00, 1),
+        op::ret(0),
+        // Zero out witness count
+        op::addi(0x26, 0x23, witness_count_offset as u16), // Offset to witness count
+        op::sw(0x26, RegId::ZERO, 0),                      // Zero out the witness count
+        // Actually hash. We use property that contract bytecode has zero size.
+        op::s256(RegId::HP, 0x22, 0x21), // Compute tx id hash
+        op::movi(0x25, 32),              // Hash size
+        // Compare two hashes
+        op::meq(0x26, RegId::HP, 0x0, 0x25),
+        // Done
+        op::ret(0x26),
+    ]
+    .into_iter()
+    .collect();
+    let predicate_data = params.chain_id().to_be_bytes().to_vec();
+    let predicate_owner = Input::predicate_owner(&predicate_bytecode);
 
-        assert_eq!(*original_id, start_id);
-        assert_eq!(*original_id, computed_id);
+    let mut tx = TransactionBuilder::create(vec![].into(), Salt::zeroed(), vec![])
+        .add_input(Input::coin_predicate(
+            UtxoId::new([4; 32].into(), 0),
+            predicate_owner,
+            123456789,
+            AssetId::default(),
+            Default::default(),
+            0,
+            predicate_bytecode,
+            predicate_data,
+        ))
+        .add_output(Output::contract_created(
+            Contract::EMPTY_CONTRACT_ID,
+            Default::default(),
+        ))
+        .add_output(Output::change(
+            Default::default(),
+            Default::default(),
+            Default::default(),
+        ))
+        .finalize();
+    tx.estimate_predicates(&CheckPredicateParams::from(&params), MemoryInstance::new())
+        .expect("Should estimate predicate");
 
-        original_id
-    };
-
-    let original = run_tx(tx.clone());
+    let run_tx = |tx: Create| tx.into_checked(0u32.into(), &params).map(|_| ());
+    let result = run_tx(tx.clone());
+    assert_eq!(result, Ok(()));
 
     // Check that modifying the input coin doesn't affect validity
     {
         let mut tx = tx.clone();
 
         match tx.inputs_mut()[0] {
-            Input::CoinSigned(CoinSigned {
+            Input::CoinPredicate(CoinPredicate {
                 ref mut tx_pointer, ..
             }) => *tx_pointer = TxPointer::from_str("123456780001").unwrap(),
             _ => unreachable!(),
         };
+
+        match tx.outputs_mut()[1] {
+            Output::Change { ref mut amount, .. } => {
+                *amount = 123456789;
+            }
+            _ => unreachable!(),
+        };
         let result = run_tx(tx);
-        assert_eq!(result, original);
+        assert_eq!(result, Ok(()));
     }
+}
+
+/// Malleable fields should not affect validity of the script
+#[test]
+fn malleable_fields_do_not_affect_validity_of_script() {
+    let params = ConsensusParameters::default();
+
+    let tx_start_ptr = params.tx_params().tx_offset();
+    let tx_size_ptr = tx_start_ptr - 8;
+
+    let predicate_bytecode = [
+        // Load tx size
+        op::movi(0x21, tx_size_ptr as u32),
+        op::lw(0x21, 0x21, 0),
+        // Make heap space for tx bytes, chain_id and computed tx hash
+        op::addi(0x22, 0x21, 8 + 32),
+        op::aloc(0x22),
+        // Construct the chain_id and tx bytes for hashing
+        op::addi(0x22, RegId::HP, 32), // Chain id position
+        op::gtf_args(0x20, 0x00, GTFArgs::InputCoinPredicateData),
+        op::mcpi(0x22, 0x20, 8), // Copy chain id
+        op::movi(0x20, tx_start_ptr as u32),
+        op::addi(0x23, 0x22, 8),   // Tx bytes position
+        op::mcp(0x23, 0x20, 0x21), // Copy tx bytes
+        // Actually hash
+        op::addi(0x24, 0x21, 8),         // Offset ptr
+        op::s256(RegId::HP, 0x22, 0x24), // Compute tx id hash
+        op::movi(0x25, 32),              // Hash size
+        // Compare two hashes
+        op::meq(0x26, RegId::HP, 0x0, 0x25),
+        // Done
+        op::ret(0x26),
+    ]
+    .into_iter()
+    .collect();
+    let predicate_data = params.chain_id().to_be_bytes().to_vec();
+    let predicate_owner = Input::predicate_owner(&predicate_bytecode);
 
-    // Check that modifying the receipts root doesn't affect validity
+    let mut tx = TransactionBuilder::script(vec![], vec![])
+        .add_input(Input::coin_predicate(
+            UtxoId::new([4; 32].into(), 0),
+            predicate_owner,
+            123456789,
+            AssetId::default(),
+            Default::default(),
+            0,
+            predicate_bytecode,
+            predicate_data,
+        ))
+        .add_input(Input::contract(
+            Default::default(),
+            Default::default(),
+            Default::default(),
+            Default::default(),
+            Contract::EMPTY_CONTRACT_ID,
+        ))
+        .add_output(Output::variable(
+            Default::default(),
+            Default::default(),
+            Default::default(),
+        ))
+        .add_output(Output::change(
+            Default::default(),
+            Default::default(),
+            Default::default(),
+        ))
+        .add_output(Output::contract(1, Default::default(), Default::default()))
+        .script_gas_limit(1_000_000)
+        .finalize();
+    tx.estimate_predicates(&CheckPredicateParams::from(&params), MemoryInstance::new())
+        .expect("Should estimate predicate");
+
+    let run_tx = |tx: Script| tx.into_checked(0u32.into(), &params).map(|_| ());
+    let result = run_tx(tx.clone());
+    assert_eq!(result, Ok(()));
+
+    // Check that modifying the input coin doesn't affect validity
     {
-        let mut tx = tx;
+        let mut tx = tx.clone();
+
         *tx.receipts_root_mut() = [1u8; 32].into();
+
+        match tx.inputs_mut()[0] {
+            Input::CoinPredicate(CoinPredicate {
+                ref mut tx_pointer, ..
+            }) => *tx_pointer = TxPointer::from_str("123456780001").unwrap(),
+            _ => unreachable!(),
+        };
+
+        match tx.inputs_mut()[1] {
+            Input::Contract(input::contract::Contract {
+                ref mut utxo_id,
+                ref mut balance_root,
+                ref mut state_root,
+                ref mut tx_pointer,
+                ..
+            }) => {
+                *utxo_id = UtxoId::new([1; 32].into(), 0);
+                *balance_root = [2; 32].into();
+                *state_root = [3; 32].into();
+                *tx_pointer = TxPointer::from_str("123456780001").unwrap();
+            }
+            _ => unreachable!(),
+        };
+
+        match tx.outputs_mut()[0] {
+            Output::Variable {
+                ref mut to,
+                ref mut amount,
+                ref mut asset_id,
+            } => {
+                *to = [123; 32].into();
+                *amount = 123456789;
+                *asset_id = [213; 32].into();
+            }
+            _ => unreachable!(),
+        };
+
+        match tx.outputs_mut()[1] {
+            Output::Change { ref mut amount, .. } => {
+                *amount = 123456789;
+            }
+            _ => unreachable!(),
+        };
+
+        match tx.outputs_mut()[2] {
+            Output::Contract(output::contract::Contract {
+                ref mut balance_root,
+                ref mut state_root,
+                ..
+            }) => {
+                *balance_root = [7; 32].into();
+                *state_root = [8; 32].into();
+            }
+            _ => unreachable!(),
+        };
+
         let result = run_tx(tx);
-        assert_eq!(result, original);
+        assert_eq!(result, Ok(()));
     }
 }