Zero malleable fields before execution

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - [#1632](https://github.com/FuelLabs/fuel-core/pull/1632): Removed `ContractsInfo` table. Contract salts and roots are no longer stored in on-chain data.
 - [#1632](https://github.com/FuelLabs/fuel-core/pull/1632): Opcode `CROO` now calculates the given contract's root on demand. `CROO` has therefore been changed to a `DependentCost` gas cost.
+- [#678](https://github.com/FuelLabs/fuel-vm/pull/678): Zero malleable fields before execution. Remove some now-obsolete GTF getters. Don't update `tx.receiptsRoot` after pushing receipts, and do it after execution instead.
 
 ## [Version 0.45.0]
 

fuel-asm/src/args.rs

@@ -167,15 +167,6 @@ crate::enum_try_from! {
         /// Set `$rA` to `tx.inputs[$rB].outputIndex`
         InputContractOutputIndex = 0x221,
 
-        /// Set `$rA` to `Memory address of tx.inputs[$rB].balanceRoot`
-        InputContractBalanceRoot = 0x222,
-
-        /// Set `$rA` to `Memory address of tx.inputs[$rB].stateRoot`
-        InputContractStateRoot = 0x223,
-
-        /// Set `$rA` to `Memory address of tx.inputs[$rB].txPointer`
-        InputContractTxPointer = 0x224,
-
         /// Set `$rA` to `Memory address of tx.inputs[$rB].contractID`
         InputContractId = 0x225,
 
@@ -230,12 +221,6 @@ crate::enum_try_from! {
         /// Set `$rA` to `tx.outputs[$rB].inputIndex`
         OutputContractInputIndex = 0x304,
 
-        /// Set `$rA` to `Memory address of tx.outputs[$rB].balanceRoot`
-        OutputContractBalanceRoot = 0x305,
-
-        /// Set `$rA` to `Memory address of tx.outputs[$rB].stateRoot`
-        OutputContractStateRoot = 0x306,
-
         /// Set `$rA` to `Memory address of tx.outputs[$rB].contractID`
         OutputContractCreatedContractId = 0x307,
 
@@ -332,11 +317,6 @@ fn encode_gtf_args() {
         GTFArgs::InputCoinPredicate,
         GTFArgs::InputCoinPredicateData,
         GTFArgs::InputCoinPredicateGasUsed,
-        GTFArgs::InputContractTxId,
-        GTFArgs::InputContractOutputIndex,
-        GTFArgs::InputContractBalanceRoot,
-        GTFArgs::InputContractStateRoot,
-        GTFArgs::InputContractTxPointer,
         GTFArgs::InputContractId,
         GTFArgs::InputMessageSender,
         GTFArgs::InputMessageRecipient,
@@ -355,8 +335,6 @@ fn encode_gtf_args() {
         GTFArgs::OutputCoinAmount,
         GTFArgs::OutputCoinAssetId,
         GTFArgs::OutputContractInputIndex,
-        GTFArgs::OutputContractBalanceRoot,
-        GTFArgs::OutputContractStateRoot,
         GTFArgs::OutputContractCreatedContractId,
         GTFArgs::OutputContractCreatedStateRoot,
         GTFArgs::WitnessDataLength,

fuel-tx/src/builder.rs

@@ -103,8 +103,6 @@ impl<T> Buildable for T where T: BuildableSet {}
 pub struct TransactionBuilder<Tx> {
     tx: Tx,
 
-    should_prepare_script: bool,
-    should_prepare_predicate: bool,
     params: ConsensusParameters,
 
     // We take the key by reference so this lib won't have the responsibility to properly
@@ -126,12 +124,7 @@ impl TransactionBuilder<Script> {
             receipts_root: Default::default(),
             metadata: None,
         };
-
-        let mut slf = Self::with_tx(tx);
-
-        slf.prepare_script(true);
-
-        slf
+        Self::with_tx(tx)
     }
 }
 
@@ -188,14 +181,10 @@ impl TransactionBuilder<Mint> {
 
 impl<Tx> TransactionBuilder<Tx> {
     fn with_tx(tx: Tx) -> Self {
-        let should_prepare_script = false;
-        let should_prepare_predicate = false;
         let sign_keys = BTreeMap::new();
 
         Self {
             tx,
-            should_prepare_script,
-            should_prepare_predicate,
             params: ConsensusParameters::standard(),
             sign_keys,
         }
@@ -277,16 +266,6 @@ impl<Tx> TransactionBuilder<Tx> {
 }
 
 impl<Tx: Buildable> TransactionBuilder<Tx> {
-    pub fn prepare_script(&mut self, should_prepare_script: bool) -> &mut Self {
-        self.should_prepare_script = should_prepare_script;
-        self
-    }
-
-    pub fn prepare_predicate(&mut self, should_prepare_predicate: bool) -> &mut Self {
-        self.should_prepare_predicate = should_prepare_predicate;
-        self
-    }
-
     pub fn sign_keys(&self) -> impl Iterator<Item = &SecretKey> {
         self.sign_keys.keys()
     }
@@ -435,19 +414,7 @@ impl<Tx: Buildable> TransactionBuilder<Tx> {
         *witness_index
     }
 
-    fn prepare_finalize(&mut self) {
-        if self.should_prepare_predicate {
-            self.tx.prepare_init_predicate();
-        }
-
-        if self.should_prepare_script {
-            self.tx.prepare_init_script();
-        }
-    }
-
     fn finalize_inner(&mut self) -> Tx {
-        self.prepare_finalize();
-
         let mut tx = core::mem::take(&mut self.tx);
 
         self.sign_keys
@@ -461,8 +428,6 @@ impl<Tx: Buildable> TransactionBuilder<Tx> {
     }
 
     pub fn finalize_without_signature_inner(&mut self) -> Tx {
-        self.prepare_finalize();
-
         let mut tx = core::mem::take(&mut self.tx);
 
         tx.precompute(&self.get_chain_id())

fuel-tx/src/tests/mod.rs

@@ -1,5 +1,4 @@
 mod offset;
-mod prepared_init;
 mod valid_cases;
 
 #[cfg(feature = "serde")]

fuel-tx/src/transaction.rs

@@ -414,30 +414,23 @@ pub trait Executable: field::Inputs + field::Outputs + field::Witnesses {
         self.inputs_mut().push(input);
     }
 
-    /// Prepare the transaction for VM initialization for script execution
-    ///
-    /// note: Fields dependent on storage/state such as balance and state roots, or tx
-    /// pointers, should already set by the client beforehand.
-    fn prepare_init_script(&mut self) -> &mut Self {
-        self.outputs_mut()
-            .iter_mut()
-            .for_each(|o| o.prepare_init_script());
-
-        self
-    }
-
-    /// Prepare the transaction for VM initialization for predicate verification
-    fn prepare_init_predicate(&mut self) -> &mut Self {
-        self.inputs_mut()
-            .iter_mut()
-            .for_each(|i| i.prepare_init_predicate());
-
-        self.outputs_mut()
-            .iter_mut()
-            .for_each(|o| o.prepare_init_predicate());
-
-        self
-    }
+    // /// Prepare the transaction for VM initialization for script or predicate execution
+    // ///
+    // /// note: Fields dependent on storage/state such as balance and state roots
+    // /// should already set by the client beforehand.
+    // fn __prepare_init_execute(&mut self) -> &mut Self {
+    //     // *self.receipts_root_mut() = Default::default();
+
+    //     self.inputs_mut()
+    //         .iter_mut()
+    //         .for_each(|o| o.prepare_init_execute());
+
+    //     self.outputs_mut()
+    //         .iter_mut()
+    //         .for_each(|o| o.prepare_init_execute());
+
+    //     self
+    // }
 }
 
 impl<T: field::Inputs + field::Outputs + field::Witnesses> Executable for T {}

fuel-tx/src/transaction/types/input.rs

@@ -737,6 +737,7 @@ impl Input {
         }
     }
 
+
     pub fn compute_message_id(
         sender: &Address,
         recipient: &Address,
@@ -770,9 +771,9 @@ impl Input {
         owner == &Self::predicate_owner(predicate)
     }
 
-    /// Prepare the output for VM predicate execution
-    pub fn prepare_init_predicate(&mut self) {
-        self.prepare_sign()
+    /// Prepare the output for script or predicate execution
+    pub fn prepare_init_execute(&mut self) {
+        self.prepare_sign(); // This currently does the same thing
     }
 }
 

fuel-tx/src/transaction/types/output.rs

@@ -217,29 +217,9 @@ impl Output {
     }
 
     /// Prepare the output for VM initialization for script execution
-    pub fn prepare_init_script(&mut self) {
-        match self {
-            Output::Change { amount, .. } => {
-                mem::take(amount);
-            }
-
-            Output::Variable {
-                to,
-                amount,
-                asset_id,
-            } => {
-                mem::take(to);
-                mem::take(amount);
-                mem::take(asset_id);
-            }
-
-            _ => (),
-        }
-    }
-
-    /// Prepare the output for VM initialization for predicate verification
-    pub fn prepare_init_predicate(&mut self) {
-        self.prepare_sign()
+    /// or predicate verification
+    pub fn prepare_init_execute(&mut self) {
+        self.prepare_sign() // Currently does the same thing
     }
 }
 

fuel-tx/src/transaction/types/script.rs

@@ -101,6 +101,19 @@ impl Default for Script {
     }
 }
 
+impl Script {
+    /// Prepare script for execution by clearing malleable fields.
+    pub fn prepare_init_execute(&mut self) {
+        *self.receipts_root_mut() = Default::default();
+        self.inputs_mut()
+            .iter_mut()
+            .for_each(Input::prepare_init_execute);
+        self.outputs_mut()
+            .iter_mut()
+            .for_each(Output::prepare_init_execute);
+    }
+}
+
 impl crate::UniqueIdentifier for Script {
     fn id(&self, chain_id: &ChainId) -> Bytes32 {
         if let Some(id) = self.cached_id() {

fuel-vm/src/checked_transaction.rs

@@ -47,11 +47,14 @@ bitflags::bitflags! {
     pub struct Checks: u32 {
         /// Basic checks defined in the specification for each transaction:
         /// https://github.com/FuelLabs/fuel-specs/blob/master/src/tx-format/transaction.md#transaction
+        /// Also ensures that malleable fields are zeroed.
         const Basic         = 0b00000001;
         /// Check that signature in the transactions are valid.
         const Signatures    = 0b00000010;
         /// Check that predicate in the transactions are valid.
         const Predicates    = 0b00000100;
+        /// Malleable fields are zeroed.
+        const ZeroMalleable = 0b00001000;
     }
 }
 

fuel-vm/src/interpreter.rs

@@ -44,6 +44,7 @@ use fuel_tx::{
 };
 use fuel_types::{
     AssetId,
+    Bytes32,
     ChainId,
     ContractId,
     Word,
@@ -300,6 +301,11 @@ impl<S, Tx, Ecal> Interpreter<S, Tx, Ecal> {
         self.receipts.as_ref().as_slice()
     }
 
+    /// Compute current receipts root
+    pub fn compute_receipts_root(&self) -> Bytes32 {
+        self.receipts.root()
+    }
+
     /// Mutable access to receipts for testing purposes.
     #[cfg(any(test, feature = "test-helpers"))]
     pub fn receipts_mut(&mut self) -> &mut ReceiptsCtx {