Move tx length validation in builder (#2171)

* Move tx length validation in builder

* Move signed tx length validation in builder

* format

* Move and update consts

* Align, update comment

bindings/nodejs/lib/types/error.ts

@@ -30,8 +30,6 @@ export type ClientErrorName =
     | 'inputAddressNotFound'
     | 'invalidAmount'
     | 'invalidMnemonic'
-    | 'invalidTransactionLength'
-    | 'invalidSignedTransactionPayloadLength'
     | 'json'
     | 'missingParameter'
     | 'node'

sdk/examples/wallet/offline_signing/2_sign_transaction.rs

@@ -48,8 +48,6 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
 
     let signed_transaction = SignedTransactionPayload::new(prepared_transaction_data.transaction.clone(), unlocks)?;
 
-    signed_transaction.validate_length()?;
-
     let signed_transaction_data = SignedTransactionData {
         payload: signed_transaction,
         inputs_data: prepared_transaction_data.inputs_data,

sdk/src/client/api/block_builder/transaction.rs

@@ -3,30 +3,15 @@
 
 //! Transaction preparation and signing
 
-use packable::PackableExt;
-
 use crate::{
-    client::{
-        api::{PreparedTransactionData, SignedTransactionData},
-        ClientError,
-    },
+    client::api::{PreparedTransactionData, SignedTransactionData},
     types::block::{
         output::{Output, OutputId},
-        payload::signed_transaction::{SignedTransactionPayload, Transaction},
         protocol::ProtocolParameters,
         semantic::{SemanticValidationContext, TransactionFailureReason},
-        signature::Ed25519Signature,
-        Block, BlockId,
     },
 };
 
-const MAX_TX_LENGTH_FOR_BLOCK_WITH_8_PARENTS: usize = Block::LENGTH_MAX - Block::LENGTH_MIN - (7 * BlockId::LENGTH);
-// Length for unlocks with a single signature unlock (unlocks length + unlock type + signature type + public key +
-// signature)
-const SINGLE_UNLOCK_LENGTH: usize = 1 + 1 + Ed25519Signature::PUBLIC_KEY_LENGTH + Ed25519Signature::SIGNATURE_LENGTH;
-// Type + reference index
-const REFERENCE_ACCOUNT_NFT_UNLOCK_LENGTH: usize = 1 + 2;
-
 impl PreparedTransactionData {
     /// Verifies the semantic of a prepared transaction.
     pub fn verify_semantic(&self, protocol_parameters: &ProtocolParameters) -> Result<(), TransactionFailureReason> {
@@ -68,44 +53,3 @@ impl SignedTransactionData {
         context.validate()
     }
 }
-
-impl SignedTransactionPayload {
-    /// Verifies that the signed transaction payload doesn't exceed the block size limit with 8 parents.
-    pub fn validate_length(&self) -> Result<(), ClientError> {
-        let signed_transaction_payload_bytes = self.pack_to_vec();
-        if signed_transaction_payload_bytes.len() > MAX_TX_LENGTH_FOR_BLOCK_WITH_8_PARENTS {
-            return Err(ClientError::InvalidSignedTransactionPayloadLength {
-                length: signed_transaction_payload_bytes.len(),
-                max_length: MAX_TX_LENGTH_FOR_BLOCK_WITH_8_PARENTS,
-            });
-        }
-        Ok(())
-    }
-}
-
-impl Transaction {
-    /// Verifies that the transaction doesn't exceed the block size limit with 8 parents.
-    /// Assuming one signature unlock and otherwise reference/account/nft unlocks.
-    /// `validate_transaction_payload_length()` should later be used to check the length again with the correct
-    /// unlocks.
-    pub fn validate_length(&self) -> Result<(), ClientError> {
-        let transaction_bytes = self.pack_to_vec();
-
-        // Assuming there is only 1 signature unlock and the rest is reference/account/nft unlocks
-        let reference_account_nft_unlocks_amount = self.inputs().len() - 1;
-
-        // Max tx payload length - length for one signature unlock (there might be more unlocks, we check with them
-        // later again, when we built the transaction payload)
-        let max_length = MAX_TX_LENGTH_FOR_BLOCK_WITH_8_PARENTS
-            - SINGLE_UNLOCK_LENGTH
-            - (reference_account_nft_unlocks_amount * REFERENCE_ACCOUNT_NFT_UNLOCK_LENGTH);
-
-        if transaction_bytes.len() > max_length {
-            return Err(ClientError::InvalidTransactionLength {
-                length: transaction_bytes.len(),
-                max_length,
-            });
-        }
-        Ok(())
-    }
-}

sdk/src/client/api/block_builder/transaction_builder/mod.rs

@@ -169,11 +169,7 @@ impl Client {
             transaction_builder = transaction_builder.disable_additional_input_selection();
         }
 
-        let prepared_transaction_data = transaction_builder.finish()?;
-
-        prepared_transaction_data.transaction.validate_length()?;
-
-        Ok(prepared_transaction_data)
+        Ok(transaction_builder.finish()?)
     }
 }
 

sdk/src/client/error.rs

@@ -67,22 +67,6 @@ pub enum ClientError {
     /// Invalid mnemonic error
     #[error("invalid mnemonic {0}")]
     InvalidMnemonic(String),
-    /// The transaction is too large
-    #[error("the transaction is too large. Its length is {length}, max length is {max_length}")]
-    InvalidTransactionLength {
-        /// The found length.
-        length: usize,
-        /// The max supported length.
-        max_length: usize,
-    },
-    /// The signed transaction payload is too large
-    #[error("the signed transaction payload is too large. Its length is {length}, max length is {max_length}")]
-    InvalidSignedTransactionPayloadLength {
-        /// The found length.
-        length: usize,
-        /// The max length.
-        max_length: usize,
-    },
     /// JSON error
     #[error("{0}")]
     Json(#[from] serde_json::Error),

sdk/src/client/node_api/core/routes.rs

@@ -37,7 +37,7 @@ use crate::{
 pub(crate) static INFO_PATH: &str = "api/core/v3/info";
 
 /// Contains the info and the url from the node (useful when multiple nodes are used)
-#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
+#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct NodeInfoResponse {
     /// The returned info

sdk/src/client/secret/mod.rs

@@ -666,8 +666,6 @@ where
     } = prepared_transaction_data;
     let tx_payload = SignedTransactionPayload::new(transaction, unlocks)?;
 
-    tx_payload.validate_length()?;
-
     let data = SignedTransactionData {
         payload: tx_payload,
         inputs_data,

sdk/src/types/api/core.rs

@@ -34,7 +34,7 @@ pub struct RoutesResponse {
 
 /// Response of GET /api/core/v3/info.
 /// General information about the node.
-#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
+#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct InfoResponse {
     pub name: String,

sdk/src/types/block/payload/error.rs

@@ -42,8 +42,12 @@ pub enum PayloadError {
     InputCount(<InputCount as TryFrom<usize>>::Error),
     #[display(fmt = "invalid output count: {_0}")]
     OutputCount(<OutputCount as TryFrom<usize>>::Error),
+    #[display(fmt = "the signed transaction payload is too large. Its length is {length}, max length is {max_length}")]
+    SignedTransactionPayloadLength { length: usize, max_length: usize },
     #[display(fmt = "invalid transaction amount sum: {_0}")]
     TransactionAmountSum(u128),
+    #[display(fmt = "the transaction is too large. Its length is {length}, max length is {max_length}")]
+    TransactionLength { length: usize, max_length: usize },
     #[display(fmt = "duplicate output chain: {_0}")]
     DuplicateOutputChain(ChainId),
     #[display(fmt = "duplicate UTXO {_0} in inputs")]

sdk/src/types/block/payload/signed_transaction/mod.rs

@@ -8,6 +8,7 @@ mod transaction_id;
 
 use packable::{Packable, PackableExt};
 
+use self::transaction::MAX_TX_LENGTH_FOR_BLOCK_WITH_SINGLE_PARENT;
 pub(crate) use self::transaction::{InputCount, OutputCount};
 pub use self::{
     transaction::{Transaction, TransactionBuilder, TransactionCapabilities, TransactionCapabilityFlag},
@@ -69,9 +70,26 @@ fn verify_signed_transaction_payload(payload: &SignedTransactionPayload) -> Resu
         });
     }
 
+    payload.validate_length()?;
+
     Ok(())
 }
 
+impl SignedTransactionPayload {
+    /// Verifies that the transaction doesn't exceed the block size limit with 1 parent.
+    fn validate_length(&self) -> Result<(), PayloadError> {
+        let signed_transaction_payload_bytes = self.pack_to_vec();
+        if signed_transaction_payload_bytes.len() > MAX_TX_LENGTH_FOR_BLOCK_WITH_SINGLE_PARENT {
+            return Err(PayloadError::SignedTransactionPayloadLength {
+                length: signed_transaction_payload_bytes.len(),
+                max_length: MAX_TX_LENGTH_FOR_BLOCK_WITH_SINGLE_PARENT,
+            });
+        }
+
+        Ok(())
+    }
+}
+
 #[cfg(feature = "serde")]
 pub mod dto {
     use alloc::vec::Vec;

sdk/src/types/block/payload/signed_transaction/transaction.rs

@@ -23,11 +23,22 @@ use crate::{
             OptionalPayload, Payload, PayloadError,
         },
         protocol::{ProtocolParameters, WorkScore, WorkScoreParameters},
+        signature::Ed25519Signature,
         slot::SlotIndex,
+        Block,
     },
     utils::merkle_hasher,
 };
 
+pub(crate) const BASIC_BLOCK_LENGTH_MIN: usize = 238;
+pub(crate) const MAX_TX_LENGTH_FOR_BLOCK_WITH_SINGLE_PARENT: usize = Block::LENGTH_MAX - BASIC_BLOCK_LENGTH_MIN;
+// Length for unlocks with a single signature unlock (unlocks length + unlock type + signature type + public key +
+// signature)
+pub(crate) const SINGLE_UNLOCK_LENGTH: usize =
+    1 + 1 + Ed25519Signature::PUBLIC_KEY_LENGTH + Ed25519Signature::SIGNATURE_LENGTH;
+// Type + reference index
+pub(crate) const REFERENCE_ACCOUNT_NFT_UNLOCK_LENGTH: usize = 1 + 2;
+
 /// A builder to build a [`Transaction`].
 #[derive(Debug, Clone)]
 #[must_use]
@@ -448,7 +459,36 @@ fn verify_transaction_packable(transaction: &Transaction, _: &ProtocolParameters
     verify_transaction(transaction)
 }
 
+impl Transaction {
+    /// Verifies that the transaction doesn't exceed the block size limit with 1 parent.
+    /// Assuming one signature unlock and otherwise reference/account/nft unlocks.
+    /// `validate_transaction_payload_length()` should later be used to check the length again with the correct
+    /// unlocks.
+    fn validate_length(&self) -> Result<(), PayloadError> {
+        let transaction_bytes = self.pack_to_vec();
+
+        // Assuming there is only 1 signature unlock and the rest is reference/account/nft unlocks
+        let reference_account_nft_unlocks_amount = self.inputs().len() - 1;
+
+        // Max tx payload length - length for one signature unlock (there might be more unlocks, we check with them
+        // later again, when we built the transaction payload)
+        let max_length = MAX_TX_LENGTH_FOR_BLOCK_WITH_SINGLE_PARENT
+            - SINGLE_UNLOCK_LENGTH
+            - (reference_account_nft_unlocks_amount * REFERENCE_ACCOUNT_NFT_UNLOCK_LENGTH);
+
+        if transaction_bytes.len() > max_length {
+            return Err(PayloadError::TransactionLength {
+                length: transaction_bytes.len(),
+                max_length,
+            });
+        }
+        Ok(())
+    }
+}
+
 fn verify_transaction(transaction: &Transaction) -> Result<(), PayloadError> {
+    transaction.validate_length()?;
+
     if transaction.context_inputs().commitment().is_none() {
         for output in transaction.outputs.iter() {
             if output.features().is_some_and(|f| f.staking().is_some()) {

sdk/src/wallet/operations/transaction/sign_transaction.rs

@@ -84,8 +84,6 @@ where
 
         log::debug!("[TRANSACTION] signed transaction: {:?}", payload);
 
-        payload.validate_length()?;
-
         Ok(SignedTransactionData {
             payload,
             inputs_data: prepared_transaction_data.inputs_data.clone(),

sdk/tests/client/signing/account.rs

@@ -100,9 +100,7 @@ async fn sign_account_state_transition() -> Result<(), Box<dyn std::error::Error
     assert_eq!(unlocks.len(), 1);
     assert_eq!((*unlocks).first().unwrap().kind(), SignatureUnlock::KIND);
 
-    let tx_payload = SignedTransactionPayload::new(prepared_transaction_data.transaction.clone(), unlocks)?;
-
-    tx_payload.validate_length()?;
+    SignedTransactionPayload::new(prepared_transaction_data.transaction.clone(), unlocks)?;
 
     prepared_transaction_data.verify_semantic(protocol_parameters)?;
 
@@ -230,9 +228,7 @@ async fn account_reference_unlocks() -> Result<(), Box<dyn std::error::Error>> {
         _ => panic!("Invalid unlock"),
     }
 
-    let tx_payload = SignedTransactionPayload::new(prepared_transaction_data.transaction.clone(), unlocks)?;
-
-    tx_payload.validate_length()?;
+    SignedTransactionPayload::new(prepared_transaction_data.transaction.clone(), unlocks)?;
 
     prepared_transaction_data.verify_semantic(protocol_parameters)?;
 

sdk/tests/client/signing/basic.rs

@@ -95,9 +95,7 @@ async fn single_ed25519_unlock() -> Result<(), Box<dyn std::error::Error>> {
     assert_eq!(unlocks.len(), 1);
     assert_eq!((*unlocks).first().unwrap().kind(), SignatureUnlock::KIND);
 
-    let tx_payload = SignedTransactionPayload::new(prepared_transaction_data.transaction.clone(), unlocks)?;
-
-    tx_payload.validate_length()?;
+    SignedTransactionPayload::new(prepared_transaction_data.transaction.clone(), unlocks)?;
 
     prepared_transaction_data.verify_semantic(protocol_parameters)?;
 
@@ -215,9 +213,7 @@ async fn ed25519_reference_unlocks() -> Result<(), Box<dyn std::error::Error>> {
         _ => panic!("Invalid unlock"),
     }
 
-    let tx_payload = SignedTransactionPayload::new(prepared_transaction_data.transaction.clone(), unlocks)?;
-
-    tx_payload.validate_length()?;
+    SignedTransactionPayload::new(prepared_transaction_data.transaction.clone(), unlocks)?;
 
     prepared_transaction_data.verify_semantic(protocol_parameters)?;
 
@@ -320,9 +316,7 @@ async fn two_signature_unlocks() -> Result<(), Box<dyn std::error::Error>> {
     assert_eq!((*unlocks).first().unwrap().kind(), SignatureUnlock::KIND);
     assert_eq!((*unlocks).get(1).unwrap().kind(), SignatureUnlock::KIND);
 
-    let tx_payload = SignedTransactionPayload::new(prepared_transaction_data.transaction.clone(), unlocks)?;
-
-    tx_payload.validate_length()?;
+    SignedTransactionPayload::new(prepared_transaction_data.transaction.clone(), unlocks)?;
 
     prepared_transaction_data.verify_semantic(protocol_parameters)?;