Removed bytecode_length from the Create transaction (#709)

* Versioned `GasCosts` * Versioned `ConsensusParameters`ю Reduced default `MAX_SIZE` to be 110kb. Reduced default `MAX_CONTRACT_SIZE` to be 100kb. * Updated CHANGELOG.md * Versioned `FeeParameters` * Versioned `PredicateParameters`, `ScriptParameters`, `ContractParameters`, * Versioned `TxParameters` * Updated CHANGELOG.md * Make CI happy * Reshuffled fields `Script` and `Create` transactions to unify part used by all chargeable transactions * Updated CHANGELOG.md * Unified `Create` and `Script` logic via `ChargeableTransaction` * Updated CHANGELOG.md * Removed `bytecode_length` from the `Create` transaction * Removed not actual test * Fixed tests related to offsets
FuelLabs · Mar 26, 2024 · f4baeaf · f4baeaf
1 parent ac31acf
commit f4baeaf
Show file tree

Hide file tree

Showing 9 changed files with 12 additions and 122 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 #### Breaking
 
+- [#709](https://github.com/FuelLabs/fuel-vm/pull/709): Removed `bytecode_length` from the `Create` transaction.
 - [#706](https://github.com/FuelLabs/fuel-vm/pull/706): Unified `Create` and `Script` logic via `ChargeableTransaction`. The change is breaking because affects JSON serialization and deserialization. Now `Script` and `Create` transactions have `body` fields that include unique transactions.
 - [#703](https://github.com/FuelLabs/fuel-vm/pull/703): Reshuffled fields `Script` and `Create` transactions to unify part used by all chargeable transactions. It breaks the serialization and deserialization and requires adoption on the SDK side.
 - [#708](https://github.com/FuelLabs/fuel-vm/pull/708): Hidden `Default` params under the "test-helper" feature to avoid accidental use in production code. It is a huge breaking change for any code that has used them before in production, and instead, it should be fetched from the network. In the case of tests simply use the "test-helper" feature in your `[dev-dependencies]` section.

diff --git a/fuel-asm/src/args.rs b/fuel-asm/src/args.rs
@@ -95,9 +95,6 @@ crate::enum_try_from! {
         /// Set `$rA` to size of the transaction in memory, in bytes
         TxLength = 0x00E,
 
-        /// Set `$rA` to `tx.bytecodeLength`
-        CreateBytecodeLength = 0x100,
-
         /// Set `$rA` to `tx.bytecodeWitnessIndex`
         CreateBytecodeWitnessIndex = 0x101,
 
@@ -300,7 +297,6 @@ fn encode_gtf_args() {
         GTFArgs::ScriptInputAtIndex,
         GTFArgs::ScriptOutputAtIndex,
         GTFArgs::ScriptWitnessAtIndex,
-        GTFArgs::CreateBytecodeLength,
         GTFArgs::CreateBytecodeWitnessIndex,
         GTFArgs::CreateStorageSlotsCount,
         GTFArgs::CreateInputsCount,

diff --git a/fuel-tx/src/builder.rs b/fuel-tx/src/builder.rs
@@ -4,7 +4,6 @@ use crate::{
     transaction::{
         field::{
             self,
-            BytecodeLength,
             BytecodeWitnessIndex,
             Maturity,
             Tip,
@@ -146,7 +145,6 @@ impl TransactionBuilder<Create> {
         storage_slots.sort();
         let mut tx = Create {
             body: CreateBody {
-                bytecode_length: Default::default(),
                 bytecode_witness_index: Default::default(),
                 salt,
                 storage_slots,
@@ -158,7 +156,6 @@ impl TransactionBuilder<Create> {
             metadata: None,
         };
 
-        *tx.bytecode_length_mut() = (bytecode.as_ref().len() / 4) as Word;
         *tx.bytecode_witness_index_mut() = 0;
 
         tx.witnesses_mut().push(bytecode);

diff --git a/fuel-tx/src/tests/valid_cases/transaction.rs b/fuel-tx/src/tests/valid_cases/transaction.rs
@@ -3,7 +3,6 @@
 
 use super::{
     test_params,
-    CHAIN_ID,
     CONTRACT_PARAMS,
     SCRIPT_PARAMS,
     TX_PARAMS,
@@ -1131,65 +1130,6 @@ fn mint() {
     assert_eq!(err, ValidityError::TransactionMintIncorrectBlockHeight);
 }
 
-#[test]
-fn tx_id_bytecode_len() {
-    let rng = &mut StdRng::seed_from_u64(8586);
-
-    let maturity = 100.into();
-    let salt = rng.gen();
-
-    let w_a = vec![0xfau8; 4].into();
-    let w_b = vec![0xfau8; 8].into();
-    let w_c = vec![0xfbu8; 4].into();
-
-    let tx_a = Transaction::create(
-        0,
-        Policies::new().with_maturity(maturity),
-        salt,
-        vec![],
-        vec![],
-        vec![],
-        vec![w_a],
-    );
-
-    let tx_b = Transaction::create(
-        0,
-        Policies::new().with_maturity(maturity),
-        salt,
-        vec![],
-        vec![],
-        vec![],
-        vec![w_b],
-    );
-
-    let tx_c = Transaction::create(
-        0,
-        Policies::new().with_maturity(maturity),
-        salt,
-        vec![],
-        vec![],
-        vec![],
-        vec![w_c],
-    );
-
-    let id_a = tx_a.id(&CHAIN_ID);
-    let id_b = tx_b.id(&CHAIN_ID);
-    let id_c = tx_c.id(&CHAIN_ID);
-
-    // bytecode with different length should produce different id
-    assert_ne!(id_a, id_b);
-
-    // bytecode with same length and different content should produce same id
-    //
-    // Note that this isn't related to the checkable itself - this checks exclusively the
-    // id behavior. the witness payload for a bytecode cannot be tampered and the
-    // checkable rules should not allow this case to pass.
-    //
-    // For further reference, check
-    // https://github.com/FuelLabs/fuel-specs/blob/1856de801fabc7e52f5c010c45c3fc6d5d4e2be3/specs/protocol/tx_format.md?plain=1#L160
-    assert_eq!(id_a, id_c);
-}
-
 mod inputs {
     use super::*;
     use itertools::Itertools;

diff --git a/fuel-tx/src/transaction.rs b/fuel-tx/src/transaction.rs
@@ -154,20 +154,11 @@ impl Transaction {
         outputs: Vec<Output>,
         witnesses: Vec<Witness>,
     ) -> Create {
-        // 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);
-
         // sort incoming storage slots
         storage_slots.sort();
 
         Create {
             body: CreateBody {
-                bytecode_length,
                 bytecode_witness_index,
                 salt,
                 storage_slots,
@@ -705,16 +696,6 @@ pub mod field {
         fn policies_offset(&self) -> usize;
     }
 
-    pub trait BytecodeLength {
-        fn bytecode_length(&self) -> &Word;
-        fn bytecode_length_mut(&mut self) -> &mut Word;
-        fn bytecode_length_offset(&self) -> usize {
-            Self::bytecode_length_offset_static()
-        }
-
-        fn bytecode_length_offset_static() -> usize;
-    }
-
     pub trait BytecodeWitnessIndex {
         fn bytecode_witness_index(&self) -> &u16;
         fn bytecode_witness_index_mut(&mut self) -> &mut u16;

diff --git a/fuel-tx/src/transaction/id.rs b/fuel-tx/src/transaction/id.rs
@@ -23,7 +23,7 @@ use fuel_types::{
 
 /// Prepares transaction for signing.
 pub trait PrepareSign {
-    /// Prepares transaction for signing, zeroing required fields.
+    /// Prepares transaction for signing zeroing required fields.
     fn prepare_sign(&mut self);
 }
 

diff --git a/fuel-tx/src/transaction/types/create.rs b/fuel-tx/src/transaction/types/create.rs
@@ -1,7 +1,6 @@
 use crate::{
     transaction::{
         field::{
-            BytecodeLength,
             BytecodeWitnessIndex,
             Salt as SaltField,
             StorageSlots,
@@ -76,7 +75,6 @@ impl CreateMetadata {
 #[canonical(prefix = TransactionRepr::Create)]
 #[derivative(Eq, PartialEq, Hash)]
 pub struct CreateBody {
-    pub(crate) bytecode_length: Word,
     pub(crate) bytecode_witness_index: u16,
     pub(crate) salt: Salt,
     pub(crate) storage_slots: Vec<StorageSlot>,
@@ -144,9 +142,7 @@ impl UniqueFormatValidityChecks for Create {
             .map(|w| w.as_ref().len() as Word)
             .ok_or(ValidityError::TransactionCreateBytecodeWitnessIndex)?;
 
-        if bytecode_witness_len > contract_params.contract_max_size()
-            || bytecode_witness_len / 4 != self.body.bytecode_length
-        {
+        if bytecode_witness_len > contract_params.contract_max_size() {
             return Err(ValidityError::TransactionCreateBytecodeLen);
         }
 
@@ -266,23 +262,6 @@ mod field {
         StorageSlotRef,
     };
 
-    impl BytecodeLength for Create {
-        #[inline(always)]
-        fn bytecode_length(&self) -> &Word {
-            &self.body.bytecode_length
-        }
-
-        #[inline(always)]
-        fn bytecode_length_mut(&mut self) -> &mut Word {
-            &mut self.body.bytecode_length
-        }
-
-        #[inline(always)]
-        fn bytecode_length_offset_static() -> usize {
-            WORD_SIZE // `Transaction` enum discriminant
-        }
-    }
-
     impl BytecodeWitnessIndex for Create {
         #[inline(always)]
         fn bytecode_witness_index(&self) -> &u16 {
@@ -296,7 +275,7 @@ mod field {
 
         #[inline(always)]
         fn bytecode_witness_index_offset_static() -> usize {
-            Self::bytecode_length_offset_static() + WORD_SIZE
+            WORD_SIZE // `Transaction` enum discriminant
         }
     }
 

diff --git a/fuel-vm/src/interpreter/internal/message_tests.rs b/fuel-vm/src/interpreter/internal/message_tests.rs
@@ -17,12 +17,12 @@ use fuel_tx::{
 use test_case::test_case;
 
 #[test_case(0, 0, 0 => None)]
-#[test_case(0, 0, 1 => Some(96))]
-#[test_case(88, 0, 1 => Some(184))]
-#[test_case(0, 1, 2 => Some(168))]
-#[test_case(0, 2, 3 => Some(240))]
-#[test_case(0, 1, 3 => Some(168))]
-#[test_case(44, 2, 3 => Some(284))]
+#[test_case(0, 0, 1 => Some(88))]
+#[test_case(88, 0, 1 => Some(176))]
+#[test_case(0, 1, 2 => Some(160))]
+#[test_case(0, 2, 3 => Some(232))]
+#[test_case(0, 1, 3 => Some(160))]
+#[test_case(44, 2, 3 => Some(276))]
 #[test_case(88, 1, 1 => None)]
 // #[test_case(usize::MAX, 0, 1 => None ; "tx_offset and num_outputs should be constrained
 // but they aren't")]
@@ -39,11 +39,11 @@ fn test_absolute_output_offset(
 }
 
 #[test_case(
-    0 => with |r: Result<_, _>| check_memory(r.unwrap(), &[(96, Output::default().to_bytes())])
+    0 => with |r: Result<_, _>| check_memory(r.unwrap(), &[(88, Output::default().to_bytes())])
     ; "Output at start of memory"
 )]
 #[test_case(
-    200 => with |r: Result<_, _>| check_memory(r.unwrap(), &[(200 + 96, Output::default().to_bytes())])
+    200 => with |r: Result<_, _>| check_memory(r.unwrap(), &[(200 + 88, Output::default().to_bytes())])
     ; "Output at 200 in memory"
 )]
 #[test_case(

diff --git a/fuel-vm/src/interpreter/metadata.rs b/fuel-vm/src/interpreter/metadata.rs
@@ -21,7 +21,6 @@ use fuel_asm::{
 };
 use fuel_tx::{
     field::{
-        BytecodeLength,
         BytecodeWitnessIndex,
         Salt,
         Script as ScriptField,
@@ -533,9 +532,6 @@ impl<Tx> GTFInput<'_, Tx> {
                     }
 
                     // Create
-                    (None, Some(create), GTFArgs::CreateBytecodeLength) => {
-                        *create.bytecode_length() as Word
-                    }
                     (None, Some(create), GTFArgs::CreateBytecodeWitnessIndex) => {
                         *create.bytecode_witness_index() as Word
                     }