fix(factory): validate init_params and version before deploying

contracts/factory/src/lib.rs

@@ -14,6 +14,26 @@ mod tests;
 pub mod contract_factory {
     use super::*;
 
+    /// Largest `init_params` payload accepted by `deploy_contract`, in bytes.
+    ///
+    /// `init_params` is the SCALE-encoded constructor argument list that gets
+    /// forwarded to the target contract. Nothing about it is bounded on the way
+    /// in, so an oversized payload is copied into the transaction, re-encoded
+    /// for the cross-contract call, and then written into the deployment
+    /// record's footprint — all before anything rejects it. 4 KiB is far above
+    /// any realistic ink! constructor signature (the largest templates in
+    /// `templates.rs` encode to tens of bytes) while still bounding the work a
+    /// single call can force.
+    pub const MAX_INIT_PARAMS_LEN: u32 = 4_096;
+
+    /// Largest `version` label accepted by `deploy_contract`, in bytes.
+    ///
+    /// The version is stored verbatim in `DeployedContract` and returned by
+    /// `get_deployment`, so an unbounded label is unbounded permanent storage
+    /// paid for by the factory. 64 bytes comfortably fits a semver string, a
+    /// git SHA, or a `v1.2.3-rc1` style tag.
+    pub const MAX_VERSION_LEN: u32 = 64;
+
     /// Contract types that can be deployed
     #[derive(Debug, Clone, Copy, PartialEq, Eq, scale::Encode, scale::Decode)]
     #[cfg_attr(
@@ -66,10 +86,21 @@ pub mod contract_factory {
         DeploymentFailed,
         CodeHashNotSet,
         ContractNotFound,
+        /// `init_params` was empty or longer than `MAX_INIT_PARAMS_LEN`.
+        ///
+        /// Previously never constructed: `builder::build_contract` accepted
+        /// any payload and returned `Ok`, so a caller had no way to learn that
+        /// its parameters were unusable.
         InvalidParameters,
         /// Native tokens were attached to `deploy_contract` but deployment
         /// fees are not supported. Send zero value.
         UnexpectedValue,
+        /// `version` was empty or longer than `MAX_VERSION_LEN`.
+        ///
+        /// Distinct from `InvalidParameters` so a caller can tell which half of
+        /// the request was rejected. Appended last so no existing discriminant
+        /// moves.
+        InvalidVersion,
     }
 
     /// Contract Factory storage
@@ -156,6 +187,12 @@ pub mod contract_factory {
 
         /// Deploys a new contract instance.
         ///
+        /// `config.init_params` must be the SCALE-encoded constructor argument
+        /// list for `config.contract_type`: non-empty and no longer than
+        /// `MAX_INIT_PARAMS_LEN`. `version` must be non-empty and no longer
+        /// than `MAX_VERSION_LEN`. See `validate_deployment_request` for why
+        /// the empty case is rejected.
+        ///
         /// Attaching native tokens is not supported; deployment is free.
         /// Send zero value with this call.
         #[ink(message, payable)]
@@ -167,6 +204,12 @@ pub mod contract_factory {
             if self.env().transferred_value() > 0 {
                 return Err(Error::UnexpectedValue);
             }
+
+            // Validate the request before touching storage or the builder, so
+            // a malformed call is rejected on its own terms rather than being
+            // reported as a missing code hash or a deployment failure.
+            Self::validate_deployment_request(&config, &version)?;
+
             let code_hash = self
                 .code_hashes
                 .get(config.contract_type)
@@ -255,5 +298,54 @@ pub mod contract_factory {
             }
             Ok(())
         }
+
+        /// Reject a deployment request whose `init_params` or `version` cannot
+        /// describe a real deployment.
+        ///
+        /// Empty `init_params` is rejected. `init_params` is the SCALE-encoded
+        /// constructor argument list for the target contract, and
+        /// `builder::build_contract` ignores it entirely and returns `Ok`, so
+        /// an empty payload is currently accepted and recorded as a successful
+        /// deployment. If the target's constructor expects arguments, that
+        /// deployment is un-callable; the factory is the only place that can
+        /// notice, because by the time a real instantiation runs, the params
+        /// are already baked into the record. No `DeploymentTemplate` in
+        /// `templates.rs` encodes to an empty payload, so this matches the
+        /// intended calling convention rather than inventing one.
+        ///
+        /// The factory only holds a `Hash` for each contract type, not its
+        /// constructor metadata, so it genuinely cannot type-check the payload
+        /// against the expected ABI. These are the checks that are correct
+        /// regardless of ABI, and the length bounds are what keep a single
+        /// call from forcing unbounded copying and permanent storage growth.
+        ///
+        /// The limits themselves are readable on-chain via
+        /// [`Self::deployment_limits`] so a client can check a request before
+        /// submitting it.
+        fn validate_deployment_request(
+            config: &DeploymentConfig,
+            version: &str,
+        ) -> Result<(), Error> {
+            if config.init_params.is_empty() {
+                return Err(Error::InvalidParameters);
+            }
+            if config.init_params.len() > MAX_INIT_PARAMS_LEN as usize {
+                return Err(Error::InvalidParameters);
+            }
+            if version.is_empty() {
+                return Err(Error::InvalidVersion);
+            }
+            if version.len() > MAX_VERSION_LEN as usize {
+                return Err(Error::InvalidVersion);
+            }
+            Ok(())
+        }
+
+        /// The limits `validate_deployment_request` enforces, so a
+        /// client can check a request before submitting it.
+        #[ink(message)]
+        pub fn deployment_limits(&self) -> (u32, u32) {
+            (MAX_INIT_PARAMS_LEN, MAX_VERSION_LEN)
+        }
     }
 }

contracts/factory/src/tests.rs

@@ -1,4 +1,5 @@
 use ink::env::test;
+use ink::prelude::string::String;
 use ink::prelude::vec::Vec;
 use ink::primitives::{AccountId, Hash};
 
@@ -60,14 +61,41 @@ fn test_get_deployer_contracts_empty() {
 
 // ── Deployment & admin-transfer tests (Issue #1019) ─────────────────────────
 
+/// A minimal well-formed escrow deployment request.
+///
+/// `init_params` is non-empty because `deploy_contract` now rejects an empty
+/// constructor argument list (Issue #1175). The payload is opaque here — the
+/// factory holds only a `Hash` for each type, not its constructor ABI — but it
+/// stands in for what a `DeploymentTemplate::try_encode_params` call would
+/// produce on a real deployment.
 fn escrow_config(salt_byte: u8) -> DeploymentConfig {
     DeploymentConfig {
         contract_type: ContractType::Escrow,
         salt: [salt_byte; 32],
-        init_params: Vec::new(),
+        init_params: vec![0x01, 0x02, 0x03, 0x04],
     }
 }
 
+/// A deployment request with caller-chosen `init_params` and `version`.
+fn config_with(contract_type: ContractType, init_params: Vec<u8>, version: &str) -> (DeploymentConfig, String) {
+    (
+        DeploymentConfig {
+            contract_type,
+            salt: [9u8; 32],
+            init_params,
+        },
+        version.into(),
+    )
+}
+
+/// A factory with a code hash registered for `contract_type`.
+fn factory_with_code_hash(contract_type: ContractType) -> ContractFactory {
+    let mut factory = ContractFactory::new();
+    let code_hash: Hash = [7u8; 32].into();
+    factory.set_code_hash(contract_type, code_hash).unwrap();
+    factory
+}
+
 #[ink::test]
 fn test_deploy_without_code_hash_fails() {
     let mut factory = ContractFactory::new();
@@ -173,3 +201,199 @@ fn test_deployment_getters_reflect_recorded_deployments() {
     assert!(factory.get_deployment(999).is_none());
     assert!(factory.get_deployer_contracts(accounts.charlie).is_empty());
 }
+
+// ── Deployment request validation (Issue #1175) ─────────────────────────────
+
+#[ink::test]
+fn test_deploy_rejects_empty_init_params() {
+    let mut factory = factory_with_code_hash(ContractType::Escrow);
+    let (config, version) = config_with(ContractType::Escrow, Vec::new(), "1.0.0");
+
+    assert_eq!(
+        factory.deploy_contract(config, version),
+        Err(Error::InvalidParameters)
+    );
+}
+
+#[ink::test]
+fn test_deploy_rejects_oversized_init_params() {
+    let mut factory = factory_with_code_hash(ContractType::Escrow);
+    let (config, version) = config_with(
+        ContractType::Escrow,
+        vec![0u8; MAX_INIT_PARAMS_LEN as usize + 1],
+        "1.0.0",
+    );
+
+    assert_eq!(
+        factory.deploy_contract(config, version),
+        Err(Error::InvalidParameters)
+    );
+}
+
+/// The boundary is inclusive: exactly at the limit must still be accepted.
+#[ink::test]
+fn test_deploy_accepts_init_params_at_the_limit() {
+    let mut factory = factory_with_code_hash(ContractType::Escrow);
+    let (config, version) = config_with(
+        ContractType::Escrow,
+        vec![0u8; MAX_INIT_PARAMS_LEN as usize],
+        "1.0.0",
+    );
+
+    assert!(factory.deploy_contract(config, version).is_ok());
+    assert_eq!(factory.get_deployment_count(), 1);
+}
+
+#[ink::test]
+fn test_deploy_rejects_empty_version() {
+    let mut factory = factory_with_code_hash(ContractType::Escrow);
+    let (config, version) = config_with(ContractType::Escrow, vec![1, 2, 3], "");
+
+    assert_eq!(
+        factory.deploy_contract(config, version),
+        Err(Error::InvalidVersion)
+    );
+}
+
+#[ink::test]
+fn test_deploy_rejects_oversized_version() {
+    let mut factory = factory_with_code_hash(ContractType::Escrow);
+    let long_version = "v".repeat(MAX_VERSION_LEN as usize + 1);
+    let (config, version) = config_with(ContractType::Escrow, vec![1, 2, 3], &long_version);
+
+    assert_eq!(
+        factory.deploy_contract(config, version),
+        Err(Error::InvalidVersion)
+    );
+}
+
+#[ink::test]
+fn test_deploy_accepts_version_at_the_limit() {
+    let mut factory = factory_with_code_hash(ContractType::Escrow);
+    let at_limit = "v".repeat(MAX_VERSION_LEN as usize);
+    let (config, version) = config_with(ContractType::Escrow, vec![1, 2, 3], &at_limit);
+
+    assert!(factory.deploy_contract(config, version).is_ok());
+    assert_eq!(factory.get_deployment(0).unwrap().version, at_limit);
+}
+
+/// A one-byte overage is rejected, not just a wildly oversized value.
+#[ink::test]
+fn test_validation_boundaries_are_exact() {
+    let factory = ContractFactory::new();
+
+    let (too_long_params, ok_version) = config_with(
+        ContractType::Escrow,
+        vec![0u8; MAX_INIT_PARAMS_LEN as usize + 1],
+        "1.0.0",
+    );
+    assert_eq!(
+        ContractFactory::validate_deployment_request(&too_long_params, &ok_version),
+        Err(Error::InvalidParameters)
+    );
+
+    let (at_limit_params, ok_version) = config_with(
+        ContractType::Escrow,
+        vec![0u8; MAX_INIT_PARAMS_LEN as usize],
+        "1.0.0",
+    );
+    assert!(ContractFactory::validate_deployment_request(&at_limit_params, &ok_version).is_ok());
+
+    let (params, too_long_version) = config_with(
+        ContractType::Escrow,
+        vec![1, 2, 3],
+        &"v".repeat(MAX_VERSION_LEN as usize + 1),
+    );
+    assert_eq!(
+        ContractFactory::validate_deployment_request(&params, &too_long_version),
+        Err(Error::InvalidVersion)
+    );
+
+    let (params, at_limit_version) = config_with(
+        ContractType::Escrow,
+        vec![1, 2, 3],
+        &"v".repeat(MAX_VERSION_LEN as usize),
+    );
+    assert!(ContractFactory::validate_deployment_request(&params, &at_limit_version).is_ok());
+}
+
+/// `init_params` is reported before `version`, so a caller fixing one problem
+/// at a time is told about the first one deterministically.
+#[ink::test]
+fn test_init_params_are_validated_before_version() {
+    let factory = ContractFactory::new();
+    let (config, version) = config_with(ContractType::Escrow, Vec::new(), "");
+    assert_eq!(
+        ContractFactory::validate_deployment_request(&config, &version),
+        Err(Error::InvalidParameters)
+    );
+}
+
+/// A malformed request is rejected on its own terms, not reported as a missing
+/// code hash — otherwise the caller would go looking for a code-hash problem
+/// that does not exist.
+#[ink::test]
+fn test_validation_precedes_the_code_hash_check() {
+    // No code hash registered for Escrow.
+    let mut factory = ContractFactory::new();
+    let (config, version) = config_with(ContractType::Escrow, Vec::new(), "1.0.0");
+
+    assert_eq!(
+        factory.deploy_contract(config, version),
+        Err(Error::InvalidParameters)
+    );
+}
+
+/// A well-formed request with no code hash still reports the code hash, so the
+/// new validation did not swallow the existing guard.
+#[ink::test]
+fn test_well_formed_request_still_reports_the_missing_code_hash() {
+    let mut factory = ContractFactory::new();
+    let (config, version) = config_with(ContractType::Escrow, vec![1, 2, 3], "1.0.0");
+
+    assert_eq!(
+        factory.deploy_contract(config, version),
+        Err(Error::CodeHashNotSet)
+    );
+}
+
+#[ink::test]
+fn test_failed_validation_records_nothing() {
+    let mut factory = factory_with_code_hash(ContractType::Escrow);
+    let accounts = test::default_accounts::<ink::env::DefaultEnvironment>();
+
+    for (params, version) in [
+        (Vec::new(), "1.0.0".to_string()),
+        (vec![0u8; MAX_INIT_PARAMS_LEN as usize + 1], "1.0.0".to_string()),
+        (vec![1, 2, 3], String::new()),
+        (vec![1, 2, 3], "v".repeat(MAX_VERSION_LEN as usize + 1)),
+    ] {
+        let (config, version) = config_with(ContractType::Escrow, params, &version);
+        assert!(factory.deploy_contract(config, version).is_err());
+    }
+
+    assert_eq!(factory.get_deployment_count(), 0);
+    assert!(factory.get_deployer_contracts(accounts.alice).is_empty());
+    assert!(factory.get_deployment(0).is_none());
+}
+
+#[ink::test]
+fn test_deployment_limits_are_sane() {
+    let factory = ContractFactory::new();
+    let (max_params, max_version) = factory.deployment_limits();
+
+    assert!(max_params > 0);
+    assert!(max_version > 0);
+    // A constructor argument list must fit inside the version bound's scale by
+    // a wide margin; if these ever invert, the limits are misconfigured.
+    assert!(max_params > max_version);
+}
+
+#[ink::test]
+fn test_version_is_recorded_verbatim() {
+    let mut factory = factory_with_code_hash(ContractType::Escrow);
+    let (config, version) = config_with(ContractType::Escrow, vec![1, 2, 3], "v2.0.0-rc1+build");
+    factory.deploy_contract(config, version).unwrap();
+
+    assert_eq!(factory.get_deployment(0).unwrap().version, "v2.0.0-rc1+build");
+}