diff --git a/crates/dojo/utils/src/tx/deployer.rs b/crates/dojo/utils/src/tx/deployer.rs index 682f53bcdd..3425d9339f 100644 --- a/crates/dojo/utils/src/tx/deployer.rs +++ b/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, TransactionError> { + ) -> Result<(Felt, Option), TransactionError> { 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> { - 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:?}" + ); + } +} diff --git a/crates/sozo/ops/src/migrate/mod.rs b/crates/sozo/ops/src/migrate/mod.rs index aaf325dcec..f4c09f6178 100644 --- a/crates/sozo/ops/src/migrate/mod.rs +++ b/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 — 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; }