From 912697a848bf166e9ab35f51a03c0487ad8e6a0e Mon Sep 17 00:00:00 2001 From: Ammar Arif Date: Wed, 22 Apr 2026 13:00:58 -0500 Subject: [PATCH 1/3] fix(dojo-utils): return real contract_address on already-deployed path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Deployer::deploy_via_udc` computes the deterministic UDC-derived contract address, checks `is_deployed` at that address, and when the contract is already there returns `Ok((Felt::ZERO, Noop))`. The real address is computed then dropped. Callers relying on the return value (including everything that expects idempotent deploys) see zero. The `deploy_via_udc_getcall` signature reinforces this — it returns `Option<(Felt, Call)>` where `None` means "already deployed", and the address computed along the way is lost. Change: - `deploy_via_udc_getcall` now returns `(Felt, Option)`. The address is always surfaced; the Option encodes whether a deploy call is needed. - `deploy_via_udc` unwraps that to `(address, TransactionResult::Noop)` on the already-deployed path, with the correct address. The one external caller in `sozo-ops::migrate` was pattern-matching `Some((_, call))` / `None` and discarding the address — it adapts to the new shape trivially (`let (_, call) = ...await?; deploy_call = call`), in fact becoming shorter and clearer. Breaking change for any direct consumer of `deploy_via_udc_getcall`, but the return type change is mechanical to fix at each site. Reproduction before this change: run `saya-ops core-contract deploy --salt X` twice against the same L2. First run succeeds, second run returns contract_address="0x0" in its JSON output, which propagates into any downstream orchestration and causes subsequent txns targeting the deployed contract to fail with "Requested contract address 0x0 is not deployed." Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/dojo/utils/src/tx/deployer.rs | 26 +++++++++++++++----------- crates/sozo/ops/src/migrate/mod.rs | 17 ++++++----------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/crates/dojo/utils/src/tx/deployer.rs b/crates/dojo/utils/src/tx/deployer.rs index 682f53bcdd..2741c5ebcc 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 } = 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; } From 1bee8ed20dbbc662e3abad980668d488878ac856 Mon Sep 17 00:00:00 2001 From: Ammar Arif Date: Wed, 22 Apr 2026 13:08:36 -0500 Subject: [PATCH 2/3] test(dojo-utils): regression test for deploy_via_udc idempotency Spins up a katana_runner, deploys the predeclared account class at a fixed salt, then verifies both `deploy_via_udc_getcall` and `deploy_via_udc` return the real contract address on the already-deployed path. Before this change the tests would fail with address=Felt::ZERO on the second call; after, both paths return the deterministic UDC-derived address and deploy_via_udc returns TransactionResult::Noop. Uses the same harness pattern as the existing waiter tests (\`#[katana_runner::test]\` + \`RunnerCtx\`) so no new dev-deps needed. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/dojo/utils/src/tx/deployer.rs | 77 ++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/crates/dojo/utils/src/tx/deployer.rs b/crates/dojo/utils/src/tx/deployer.rs index 2741c5ebcc..5f7e1c9731 100644 --- a/crates/dojo/utils/src/tx/deployer.rs +++ b/crates/dojo/utils/src/tx/deployer.rs @@ -125,3 +125,80 @@ 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:?}" + ); + } +} From dae7acec63c2489347246bd964db4319ebac4ea7 Mon Sep 17 00:00:00 2001 From: Ammar Arif Date: Wed, 22 Apr 2026 13:21:52 -0500 Subject: [PATCH 3/3] style(dojo-utils): rustfmt per nightly-2024-08-28 CI's fmt check uses the pinned nightly-2024-08-28 rustfmt which collapses the chained-await blocks and the multi-line assert_eq! in the new test onto single lines. Purely cosmetic. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/dojo/utils/src/tx/deployer.rs | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/crates/dojo/utils/src/tx/deployer.rs b/crates/dojo/utils/src/tx/deployer.rs index 5f7e1c9731..3425d9339f 100644 --- a/crates/dojo/utils/src/tx/deployer.rs +++ b/crates/dojo/utils/src/tx/deployer.rs @@ -170,10 +170,8 @@ mod tests { 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(); + 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 @@ -183,18 +181,13 @@ mod tests { .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_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(); + 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),