fix(dojo-utils): return real address on already-deployed path

crates/dojo/utils/src/tx/deployer.rs

@@ -35,14 +35,19 @@ where
         Self { account, txn_config }
     }
 
-    /// Get a Call for deploying a contract via the UDC.
+    /// Get the deterministic UDC-derived contract address along with the
+    /// Call required to deploy it (or `None` for the Call if the contract
+    /// is already deployed at that address).
+    ///
+    /// The address is always returned, even on the already-deployed path,
+    /// so callers don't have to re-derive it themselves.
     pub async fn deploy_via_udc_getcall(
         &self,
         class_hash: Felt,
         salt: Felt,
         constructor_calldata: &[Felt],
         deployer_address: Felt,
-    ) -> Result<Option<(Felt, Call)>, TransactionError<A::SignError>> {
+    ) -> Result<(Felt, Option<Call>), TransactionError<A::SignError>> {
         let udc_calldata = [
             vec![class_hash, salt, deployer_address, Felt::from(constructor_calldata.len())],
             constructor_calldata.to_vec(),
@@ -53,13 +58,13 @@ where
             get_contract_address(salt, class_hash, constructor_calldata, deployer_address);
 
         if is_deployed(contract_address, &self.account.provider()).await? {
-            return Ok(None);
+            return Ok((contract_address, None));
         }
 
-        Ok(Some((
+        Ok((
             contract_address,
-            Call { calldata: udc_calldata, selector: UDC_DEPLOY_SELECTOR, to: UDC_ADDRESS },
-        )))
+            Some(Call { calldata: udc_calldata, selector: UDC_DEPLOY_SELECTOR, to: UDC_ADDRESS }),
+        ))
     }
 
     /// Deploys a contract via the UDC.
@@ -70,12 +75,11 @@ where
         constructor_calldata: &[Felt],
         deployer_address: Felt,
     ) -> Result<(Felt, TransactionResult), TransactionError<A::SignError>> {
-        let (contract_address, call) = match self
+        let (contract_address, call) = self
             .deploy_via_udc_getcall(class_hash, salt, constructor_calldata, deployer_address)
-            .await?
-        {
-            Some(res) => res,
-            None => return Ok((Felt::ZERO, TransactionResult::Noop)),
+            .await?;
+        let Some(call) = call else {
+            return Ok((contract_address, TransactionResult::Noop));
         };
 
         let InvokeTransactionResult { transaction_hash } =
@@ -121,3 +125,73 @@ where
         Err(e) => Err(e),
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use katana_runner::RunnerCtx;
+    use starknet::core::utils::get_contract_address;
+    use starknet::macros::felt;
+
+    use super::*;
+    use crate::TxnConfig;
+
+    // The default account class that katana dev predeclares on every chain.
+    // Used as the class_hash for our deploy tests so we don't need to declare
+    // a contract first.
+    const KATANA_DEV_ACCOUNT_CLASS_HASH: Felt =
+        felt!("0x07dc7899aa655b0aae51eadff6d801a58e97dd99cf4666ee59e704249e51adf2");
+
+    /// Regression: `deploy_via_udc_getcall` used to return `Option<(Felt, Call)>`
+    /// where `None` meant "already deployed" and the address was dropped on
+    /// the floor. `deploy_via_udc` then mapped that to `(Felt::ZERO, Noop)`.
+    /// After the fix both paths surface the real contract address, so
+    /// deploy is idempotent across re-runs with the same salt.
+    #[tokio::test(flavor = "multi_thread")]
+    #[katana_runner::test(accounts = 2)]
+    async fn deploy_via_udc_idempotent_returns_real_address(sequencer: &RunnerCtx) {
+        let account = sequencer.account(0);
+        let deployer = Deployer::new(account, TxnConfig { wait: true, ..Default::default() });
+
+        let class_hash = KATANA_DEV_ACCOUNT_CLASS_HASH;
+        let salt = felt!("0xabc");
+        // Account class has a single-arg constructor (public_key). Any non-zero
+        // felt works for this test; we never interact with the deployed account.
+        let calldata = vec![felt!("0xdeadbeef")];
+        let deployer_address = Felt::ZERO;
+
+        let expected_address = get_contract_address(salt, class_hash, &calldata, deployer_address);
+
+        // First call: not yet deployed. Returns (addr, Some(call)).
+        let (addr, call) = deployer
+            .deploy_via_udc_getcall(class_hash, salt, &calldata, deployer_address)
+            .await
+            .unwrap();
+        assert_eq!(addr, expected_address);
+        assert!(call.is_some(), "expected deploy Call on the not-yet-deployed path");
+
+        // Actually deploy it.
+        let (deployed_addr, _tx) =
+            deployer.deploy_via_udc(class_hash, salt, &calldata, deployer_address).await.unwrap();
+        assert_eq!(deployed_addr, expected_address);
+
+        // Second getcall with identical params: contract is already deployed
+        // at the same address. Returns (same addr, None) — this is the path
+        // that used to lose the address before the fix.
+        let (addr, call) = deployer
+            .deploy_via_udc_getcall(class_hash, salt, &calldata, deployer_address)
+            .await
+            .unwrap();
+        assert_eq!(addr, expected_address, "address must be surfaced even when already deployed");
+        assert!(call.is_none(), "no deploy Call needed on the already-deployed path");
+
+        // Second deploy_via_udc call: returns (real_address, Noop). Before
+        // the fix this returned (Felt::ZERO, Noop).
+        let (addr, tx) =
+            deployer.deploy_via_udc(class_hash, salt, &calldata, deployer_address).await.unwrap();
+        assert_eq!(addr, expected_address);
+        assert!(
+            matches!(tx, TransactionResult::Noop),
+            "already-deployed path must return Noop, got {tx:?}"
+        );
+    }
+}

crates/sozo/ops/src/migrate/mod.rs

@@ -917,23 +917,18 @@ where
 
             let deployer = Deployer::new(&self.world.account, self.txn_config);
 
-            match deployer
+            // `deploy_via_udc_getcall` returns the UDC-derived contract
+            // address plus an Option<Call> — None means the contract is
+            // already deployed at that address and no call is needed.
+            let (_contract_address, call) = deployer
                 .deploy_via_udc_getcall(
                     contract.common.class_hash,
                     contract.salt,
                     &contract.encoded_constructor_data,
                     Felt::ZERO,
                 )
-                .await?
-            {
-                Some((_, call)) => deploy_call = Some(call),
-                None => {
-                    deploy_call = {
-                        // Already deployed, no need to deploy again.
-                        None
-                    }
-                }
-            }
+                .await?;
+            deploy_call = call;
 
             is_upgradeable = contract.is_upgradeable;
         }