Add block_transaction_size_limit to ConsensusParameters (#821)

* Add `block_transaction_size_limit` to `ConsensusParameters` * Satisfy clippy * Add support for using `ConsensusParametersV2` * Update `CHANGELOG.md` * Remove superfluous constructor `ConsensusParameters::new_v2()` * Update `CHANGELOG.md` * Remove the superfluous `ConsensusParameters::standard_v2()` constructor and use V2 by default * Update CHANGELOG.md Co-authored-by: Green Baneling <[email protected]> * Explain the changes in `ConsensusParametersV2` in a doc comment * `block_transaction_size_limit` for parametes is set to `u64::MAX` * Add test showing that it is not possible to set explicit block size limit for legacy (V1) consensus parameters --------- Co-authored-by: Green Baneling <[email protected]>
FuelLabs · Sep 16, 2024 · 2999f40 · 2999f40
1 parent 4b95333
commit 2999f40
Show file tree

Hide file tree

Showing 6 changed files with 159 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Changed
 
 #### Breaking
+- [#821](https://github.com/FuelLabs/fuel-vm/pull/821): Added `block_transaction_size_limit` to `ConsensusParameters`. It adds a new `ConensusParametersV2` as a variant of the `ConsensusParameters`.
 - [#670](https://github.com/FuelLabs/fuel-vm/pull/670): The `predicate` field of `fuel_tx::input::Coin` is now a wrapper struct `PredicateCode`.
 
 ## [Version 0.56.0]

diff --git a/fuel-tx/src/tests/valid_cases.rs b/fuel-tx/src/tests/valid_cases.rs
@@ -40,6 +40,7 @@ pub fn test_params() -> ConsensusParameters {
         Default::default(),
         Default::default(),
         Default::default(),
+        Default::default(),
     )
 }
 

diff --git a/fuel-tx/src/transaction/consensus_parameters.rs b/fuel-tx/src/transaction/consensus_parameters.rs
@@ -22,11 +22,15 @@ const MAX_GAS: u64 = 100_000_000;
 #[cfg(feature = "test-helpers")]
 const MAX_SIZE: u64 = 110 * 1024;
 
+#[derive(Debug)]
+pub struct SettingBlockTransactionSizeLimitNotSupported;
+
 /// A versioned set of consensus parameters.
 #[derive(Clone, Debug, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)]
 pub enum ConsensusParameters {
     /// Version 1 of the consensus parameters
     V1(ConsensusParametersV1),
+    V2(ConsensusParametersV2),
 }
 
 #[cfg(feature = "test-helpers")]
@@ -40,13 +44,13 @@ impl ConsensusParameters {
     #[cfg(feature = "test-helpers")]
     /// Constructor for the `ConsensusParameters` with Standard values.
     pub fn standard() -> Self {
-        ConsensusParametersV1::standard().into()
+        ConsensusParametersV2::standard().into()
     }
 
     #[cfg(feature = "test-helpers")]
     /// Constructor for the `ConsensusParameters` with Standard values around `ChainId`.
     pub fn standard_with_id(chain_id: ChainId) -> Self {
-        ConsensusParametersV1::standard_with_id(chain_id).into()
+        ConsensusParametersV2::standard_with_id(chain_id).into()
     }
 
     /// Constructor for the `ConsensusParameters`
@@ -60,9 +64,10 @@ impl ConsensusParameters {
         gas_costs: GasCosts,
         base_asset_id: AssetId,
         block_gas_limit: u64,
+        block_transaction_size_limit: u64,
         privileged_address: Address,
     ) -> Self {
-        Self::V1(ConsensusParametersV1 {
+        Self::V2(ConsensusParametersV2 {
             tx_params,
             predicate_params,
             script_params,
@@ -72,6 +77,7 @@ impl ConsensusParameters {
             gas_costs,
             base_asset_id,
             block_gas_limit,
+            block_transaction_size_limit,
             privileged_address,
         })
     }
@@ -80,69 +86,91 @@ impl ConsensusParameters {
     pub const fn tx_params(&self) -> &TxParameters {
         match self {
             Self::V1(params) => &params.tx_params,
+            Self::V2(params) => &params.tx_params,
         }
     }
 
     /// Get the predicate parameters
     pub const fn predicate_params(&self) -> &PredicateParameters {
         match self {
             Self::V1(params) => &params.predicate_params,
+            Self::V2(params) => &params.predicate_params,
         }
     }
 
     /// Get the script parameters
     pub const fn script_params(&self) -> &ScriptParameters {
         match self {
             Self::V1(params) => &params.script_params,
+            Self::V2(params) => &params.script_params,
         }
     }
 
     /// Get the contract parameters
     pub const fn contract_params(&self) -> &ContractParameters {
         match self {
             Self::V1(params) => &params.contract_params,
+            Self::V2(params) => &params.contract_params,
         }
     }
 
     /// Get the fee parameters
     pub const fn fee_params(&self) -> &FeeParameters {
         match self {
             Self::V1(params) => &params.fee_params,
+            Self::V2(params) => &params.fee_params,
         }
     }
 
     /// Get the chain ID
     pub const fn chain_id(&self) -> ChainId {
         match self {
             Self::V1(params) => params.chain_id,
+            Self::V2(params) => params.chain_id,
         }
     }
 
     /// Get the gas costs
     pub const fn gas_costs(&self) -> &GasCosts {
         match self {
             Self::V1(params) => &params.gas_costs,
+            Self::V2(params) => &params.gas_costs,
         }
     }
 
     /// Get the base asset ID
     pub const fn base_asset_id(&self) -> &AssetId {
         match self {
             Self::V1(params) => &params.base_asset_id,
+            Self::V2(params) => &params.base_asset_id,
         }
     }
 
     /// Get the block gas limit
     pub const fn block_gas_limit(&self) -> u64 {
         match self {
             Self::V1(params) => params.block_gas_limit,
+            Self::V2(params) => params.block_gas_limit,
+        }
+    }
+
+    /// Get the block transaction size limit
+    pub fn block_transaction_size_limit(&self) -> u64 {
+        match self {
+            Self::V1(_) => {
+                // In V1 there was no limit on the transaction size. For the sake of
+                // backwards compatibility we allow for a largest limit possible.
+                u64::MAX
+            }
+            Self::V2(params) => params.block_transaction_size_limit,
         }
     }
 
     /// Get the privileged address
     pub const fn privileged_address(&self) -> &Address {
         match self {
             Self::V1(params) => &params.privileged_address,
+            Self::V2(params) => &params.privileged_address,
         }
     }
 }
@@ -152,69 +180,93 @@ impl ConsensusParameters {
     pub fn set_tx_params(&mut self, tx_params: TxParameters) {
         match self {
             Self::V1(params) => params.tx_params = tx_params,
+            Self::V2(params) => params.tx_params = tx_params,
         }
     }
 
     /// Set the predicate parameters.
     pub fn set_predicate_params(&mut self, predicate_params: PredicateParameters) {
         match self {
             Self::V1(params) => params.predicate_params = predicate_params,
+            Self::V2(params) => params.predicate_params = predicate_params,
         }
     }
 
     /// Set the script parameters.
     pub fn set_script_params(&mut self, script_params: ScriptParameters) {
         match self {
             Self::V1(params) => params.script_params = script_params,
+            Self::V2(params) => params.script_params = script_params,
         }
     }
 
     /// Set the contract parameters.
     pub fn set_contract_params(&mut self, contract_params: ContractParameters) {
         match self {
             Self::V1(params) => params.contract_params = contract_params,
+            Self::V2(params) => params.contract_params = contract_params,
         }
     }
 
     /// Set the fee parameters.
     pub fn set_fee_params(&mut self, fee_params: FeeParameters) {
         match self {
             Self::V1(params) => params.fee_params = fee_params,
+            Self::V2(params) => params.fee_params = fee_params,
         }
     }
 
     /// Set the chain ID.
     pub fn set_chain_id(&mut self, chain_id: ChainId) {
         match self {
             Self::V1(params) => params.chain_id = chain_id,
+            Self::V2(params) => params.chain_id = chain_id,
         }
     }
 
     /// Set the gas costs.
     pub fn set_gas_costs(&mut self, gas_costs: GasCosts) {
         match self {
             Self::V1(params) => params.gas_costs = gas_costs,
+            Self::V2(params) => params.gas_costs = gas_costs,
         }
     }
 
     /// Set the base asset ID.
     pub fn set_base_asset_id(&mut self, base_asset_id: AssetId) {
         match self {
             Self::V1(params) => params.base_asset_id = base_asset_id,
+            Self::V2(params) => params.base_asset_id = base_asset_id,
         }
     }
 
     /// Set the block gas limit.
     pub fn set_block_gas_limit(&mut self, block_gas_limit: u64) {
         match self {
             Self::V1(params) => params.block_gas_limit = block_gas_limit,
+            Self::V2(params) => params.block_gas_limit = block_gas_limit,
+        }
+    }
+
+    /// Set the block transaction size limit.
+    pub fn set_block_transaction_size_limit(
+        &mut self,
+        block_transaction_size_limit: u64,
+    ) -> Result<(), SettingBlockTransactionSizeLimitNotSupported> {
+        match self {
+            Self::V1(_) => Err(SettingBlockTransactionSizeLimitNotSupported),
+            Self::V2(params) => {
+                params.block_transaction_size_limit = block_transaction_size_limit;
+                Ok(())
+            }
         }
     }
 
     /// Set the privileged address.
     pub fn set_privileged_address(&mut self, privileged_address: Address) {
         match self {
             Self::V1(params) => params.privileged_address = privileged_address,
+            Self::V2(params) => params.privileged_address = privileged_address,
         }
     }
 }
@@ -273,6 +325,66 @@ impl From<ConsensusParametersV1> for ConsensusParameters {
     }
 }
 
+/// A collection of parameters for convenience
+/// The difference with [`ConsensusParametersV1`]:
+/// - `block_transaction_size_limit` has been added.
+#[derive(Clone, Debug, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)]
+pub struct ConsensusParametersV2 {
+    pub tx_params: TxParameters,
+    pub predicate_params: PredicateParameters,
+    pub script_params: ScriptParameters,
+    pub contract_params: ContractParameters,
+    pub fee_params: FeeParameters,
+    pub chain_id: ChainId,
+    pub gas_costs: GasCosts,
+    pub base_asset_id: AssetId,
+    pub block_gas_limit: u64,
+    pub block_transaction_size_limit: u64,
+    /// The privileged address(user or predicate) that can perform permissioned
+    /// operations(like upgrading the network).
+    pub privileged_address: Address,
+}
+
+#[cfg(feature = "test-helpers")]
+impl ConsensusParametersV2 {
+    const DEFAULT_BLOCK_TRANSACTION_SIZE_LIMIT: u64 = 126 * 1024;
+
+    /// Constructor for the `ConsensusParameters` with Standard values.
+    pub fn standard() -> Self {
+        Self::standard_with_id(ChainId::default())
+    }
+
+    /// Constructor for the `ConsensusParameters` with Standard values around `ChainId`.
+    pub fn standard_with_id(chain_id: ChainId) -> Self {
+        Self {
+            tx_params: TxParameters::DEFAULT,
+            predicate_params: PredicateParameters::DEFAULT,
+            script_params: ScriptParameters::DEFAULT,
+            contract_params: ContractParameters::DEFAULT,
+            fee_params: FeeParameters::DEFAULT,
+            chain_id,
+            gas_costs: GasCosts::default(),
+            base_asset_id: Default::default(),
+            block_gas_limit: TxParameters::DEFAULT.max_gas_per_tx(),
+            block_transaction_size_limit: Self::DEFAULT_BLOCK_TRANSACTION_SIZE_LIMIT,
+            privileged_address: Default::default(),
+        }
+    }
+}
+
+#[cfg(feature = "test-helpers")]
+impl Default for ConsensusParametersV2 {
+    fn default() -> Self {
+        Self::standard()
+    }
+}
+
+impl From<ConsensusParametersV2> for ConsensusParameters {
+    fn from(params: ConsensusParametersV2) -> Self {
+        Self::V2(params)
+    }
+}
+
 /// The versioned fee parameters.
 #[derive(
     Copy, Clone, Debug, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize,
@@ -916,3 +1028,39 @@ pub mod typescript {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::consensus_parameters::{
+        ConsensusParametersV2,
+        SettingBlockTransactionSizeLimitNotSupported,
+    };
+
+    use super::{
+        ConsensusParameters,
+        ConsensusParametersV1,
+    };
+
+    #[test]
+    fn error_when_setting_block_size_limit_in_consensus_parameters_v1() {
+        let mut consensus_params: ConsensusParameters =
+            ConsensusParametersV1::default().into();
+
+        let result = consensus_params.set_block_transaction_size_limit(0);
+
+        assert!(matches!(
+            result,
+            Err(SettingBlockTransactionSizeLimitNotSupported)
+        ))
+    }
+
+    #[test]
+    fn ok_when_setting_block_size_limit_in_consensus_parameters_v2() {
+        let mut consensus_params: ConsensusParameters =
+            ConsensusParametersV2::default().into();
+
+        let result = consensus_params.set_block_transaction_size_limit(0);
+
+        assert!(matches!(result, Ok(())))
+    }
+}
diff --git a/fuel-vm/src/checked_transaction.rs b/fuel-vm/src/checked_transaction.rs
@@ -920,6 +920,7 @@ mod tests {
             Default::default(),
             Default::default(),
             Default::default(),
+            Default::default(),
         )
     }
 

diff --git a/fuel-vm/src/tests/outputs.rs b/fuel-vm/src/tests/outputs.rs
@@ -166,6 +166,7 @@ fn correct_change_is_provided_for_coin_outputs_create() {
         context.get_gas_costs().to_owned(),
         *context.get_base_asset_id(),
         context.get_block_gas_limit(),
+        context.get_block_transaction_size_limit(),
         *context.get_privileged_address(),
     );
     let create = create

diff --git a/fuel-vm/src/util.rs b/fuel-vm/src/util.rs
@@ -396,6 +396,10 @@ pub mod test_helpers {
             self.consensus_params.block_gas_limit()
         }
 
+        pub fn get_block_transaction_size_limit(&self) -> u64 {
+            self.consensus_params.block_transaction_size_limit()
+        }
+
         pub fn get_privileged_address(&self) -> &Address {
             self.consensus_params.privileged_address()
         }