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

[kariy]

Problem

Deployer::deploy_via_udc is non-idempotent: if a contract is already deployed at the deterministic UDC-derived address, the second invocation returns Ok((Felt::ZERO, TransactionResult::Noop)) instead of Ok((real_address, Noop)).

The real address IS computed — right before we decide it's already deployed — then dropped:

// crates/dojo/utils/src/tx/deployer.rs (before)
let contract_address =
    get_contract_address(salt, class_hash, constructor_calldata, deployer_address);

if is_deployed(contract_address, &self.account.provider()).await? {
    return Ok(None);   // ← address thrown away
}

The outer deploy_via_udc then does:

None => return Ok((Felt::ZERO, TransactionResult::Noop)),   // ← zero instead of real

Impact

Any caller that relies on the return value to know what got deployed breaks on re-runs. Concrete case: we hit this in saya-ops core-contract deploy --salt X. First run: works. Second run with the same salt: returns contract_address: "0x0" in both text and --output json output. Downstream orchestration (our docker-compose bootstrap, tests/saya-tee/ when re-run) writes 0x0 into state files and the next step panics on Requested contract address 0x0 is not deployed.

Fix

Change deploy_via_udc_getcall to return (Felt, Option<Call>) instead of Option<(Felt, Call)>. The address is always surfaced; the Option<Call> encodes "does a call need to be made":

// after
pub async fn deploy_via_udc_getcall(
    ...
) -> Result<(Felt, Option<Call>), TransactionError<A::SignError>> {
    ...
    if is_deployed(contract_address, ...).await? {
        return Ok((contract_address, None));
    }

    Ok((contract_address, Some(Call { ... })))
}

deploy_via_udc unpacks that cleanly:

let (contract_address, call) = self.deploy_via_udc_getcall(...).await?;
let Some(call) = call else {
    return Ok((contract_address, TransactionResult::Noop));
};

Breaking change?

deploy_via_udc_getcall signature changed, so yes — for any direct consumer. There's one in-tree caller (sozo-ops::migrate) which was pattern-matching Some((_, call)) / None and discarding the address; it adapts to let (_, call) = ... ; deploy_call = call and actually gets shorter.

deploy_via_udc return type is unchanged ((Felt, TransactionResult)). The only behavior change is "the Felt is now correct when TransactionResult::Noop."

Test plan

• cargo check -p dojo-utils -p sozo-ops clean
• Manually reproduced: saya-ops core-contract deploy --salt 0x7ee twice against the same katana L2 dev chain. Before fix: second run returns 0x0. After fix: returns the real 0x37189b18....
• CI green (full workspace build + tests)
• sozo migrate flow retested with re-run (already-deployed path exercised)

Downstream

• Saya's saya-ops has a local workaround PR (fix(ops): recover real address on dojo-utils already-deployed path saya#64) that recomputes the address on the zero-address path. Once this merges and saya's dojo-utils git dep advances to a commit including this fix, that workaround becomes a no-op and can be removed.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Fixed contract deployment to consistently return the contract address in all deployment scenarios, including when a contract is already deployed. Previously, no address was returned for already-deployed contracts, causing inconsistent behavior in deployment workflows.

[coderabbitai]

Warning

Rate limit exceeded

@kariy has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 39 minutes and 29 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 39 minutes and 29 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f82e99b1-2bc9-4708-9c1a-beb6f32d6ccb

📥 Commits

Reviewing files that changed from the base of the PR and between 912697a and dae7ace.

📒 Files selected for processing (1)

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

Ohayo sensei! 🙏 Let me break down these changes for you.

Walkthrough

The deploy_via_udc_getcall function signature is refactored to always return the contract address alongside an optional Call. Previously returning Option<(Felt, Call)>, it now returns (Felt, Option<Call>), ensuring the contract address is available regardless of deployment status. Call sites are updated to reflect this new return shape.

Changes

Cohort / File(s)	Summary
Function Signature Update crates/dojo/utils/src/tx/deployer.rs	Modified deploy_via_udc_getcall return type from Result<Option<(Felt, Call)>, ...> to Result<(Felt, Option<Call>), ...>. When already deployed, returns (contract_address, None) instead of None. When deployment needed, returns (contract_address, Some(Call { ... })).
Call Site Update crates/sozo/ops/src/migrate/mod.rs	Updated external_contracts_calls_classes to destructure the new tuple return shape. Directly assigns deploy_call = call from the destructured Option<Call>, eliminating the previous Option pattern match and explicitly removing logic that kept deploy_call as None.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title accurately summarizes the main fix: returning the real contract address on the already-deployed path, which is the core objective of this PR.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/deploy-via-udc-return-real-address

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

crates/dojo/utils/src/tx/deployer.rs (2)

51-67: Avoid building UDC calldata before the no-op check, sensei.

Since repeated deployments are the path being fixed here, defer the calldata allocation until after is_deployed returns false.

♻️ Proposed allocation cleanup

-        let udc_calldata = [
-            vec![class_hash, salt, deployer_address, Felt::from(constructor_calldata.len())],
-            constructor_calldata.to_vec(),
-        ]
-        .concat();
-
         let contract_address =
             get_contract_address(salt, class_hash, constructor_calldata, deployer_address);
 
         if is_deployed(contract_address, &self.account.provider()).await? {
             return Ok((contract_address, None));
         }
 
+        let mut udc_calldata = Vec::with_capacity(4 + constructor_calldata.len());
+        udc_calldata.extend_from_slice(&[
+            class_hash,
+            salt,
+            deployer_address,
+            Felt::from(constructor_calldata.len()),
+        ]);
+        udc_calldata.extend_from_slice(constructor_calldata);
+
         Ok((
             contract_address,
             Some(Call { calldata: udc_calldata, selector: UDC_DEPLOY_SELECTOR, to: UDC_ADDRESS }),
         ))

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/dojo/utils/src/tx/deployer.rs` around lines 51 - 67, The code
currently builds udc_calldata before checking whether the contract is already
deployed, causing unnecessary allocation on no-op paths; change the flow in the
function that calls get_contract_address, is_deployed, and returns Option<Call>
so that you compute contract_address via get_contract_address first, call
is_deployed(contract_address, &self.account.provider()).await? and return
Ok((contract_address, None)) if deployed, and only after that allocate
udc_calldata and construct Some(Call { calldata: udc_calldata, selector:
UDC_DEPLOY_SELECTOR, to: UDC_ADDRESS }); this defers the vector allocation until
it is actually needed.

---

78-83: Ohayo sensei — this fixes the no-op address bug.

deploy_via_udc now preserves the computed contract_address when returning TransactionResult::Noop; please add/keep a regression test for this exact path so it does not regress again.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/dojo/utils/src/tx/deployer.rs` around lines 78 - 83, deploy_via_udc
currently discards the computed contract_address when it returns
TransactionResult::Noop; ensure the function (and the call site using
deploy_via_udc_getcall) always returns the computed contract_address alongside
TransactionResult::Noop (i.e., keep the contract_address value when call is
None), and add or retain a regression test that exercises the path where
deploy_via_udc/ deploy_via_udc_getcall yields None for call so that the
preserved contract_address is asserted in the result.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@crates/dojo/utils/src/tx/deployer.rs`:
- Around line 51-67: The code currently builds udc_calldata before checking
whether the contract is already deployed, causing unnecessary allocation on
no-op paths; change the flow in the function that calls get_contract_address,
is_deployed, and returns Option<Call> so that you compute contract_address via
get_contract_address first, call is_deployed(contract_address,
&self.account.provider()).await? and return Ok((contract_address, None)) if
deployed, and only after that allocate udc_calldata and construct Some(Call {
calldata: udc_calldata, selector: UDC_DEPLOY_SELECTOR, to: UDC_ADDRESS }); this
defers the vector allocation until it is actually needed.
- Around line 78-83: deploy_via_udc currently discards the computed
contract_address when it returns TransactionResult::Noop; ensure the function
(and the call site using deploy_via_udc_getcall) always returns the computed
contract_address alongside TransactionResult::Noop (i.e., keep the
contract_address value when call is None), and add or retain a regression test
that exercises the path where deploy_via_udc/ deploy_via_udc_getcall yields None
for call so that the preserved contract_address is asserted in the result.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d3b4f062-8b0d-43f7-98c8-d6536c497fb9

📥 Commits

Reviewing files that changed from the base of the PR and between a6c6047 and 912697a.

📒 Files selected for processing (2)

• crates/dojo/utils/src/tx/deployer.rs
• crates/sozo/ops/src/migrate/mod.rs

[claude]

Claude finished @kariy's task —— View job

---

Analyzing PR changes and assessing documentation impact...

Tasks

• Check if docs-repo exists
• Analyze the PR changes for user-facing impact
• Search for existing documentation about deployment utilities
• Determine if documentation updates are needed

Analysis

This PR fixes a bug in the internal dojo-utils crate where Deployer::deploy_via_udc was returning Felt::ZERO instead of the real contract address when a contract was already deployed.

Key findings:

1. Internal API: The deploy_via_udc function is part of dojo-utils, an internal Rust utility crate used by Sozo and other toolchain components. It is NOT a user-facing API documented in the book.

2. No user-facing behavior change: The public interface (deploy_via_udc) maintains the same signature (Felt, TransactionResult). The fix makes it return the correct address instead of zero - this is a bug fix that makes the API work as intended, not a new feature or breaking change.

3. No existing documentation: I searched the docs and found:

   • No documentation about deploy_via_udc or the Deployer utility
   • No documentation about the dojo-utils crate
   • The Rust SDK documentation (docs-repo/docs/pages/client/sdk/rust.md) only covers high-level client libraries like torii-client, dojo-types, and dojo-world - not internal utilities

4. Bug fix, not feature: This PR corrects incorrect behavior (returning 0x0) to return the expected value (real address). Users would have expected this to work correctly all along.

Conclusion: No documentation updates are needed because:

• This is an internal utility API not documented in the user-facing docs
• The fix corrects a bug rather than introducing new behavior
• The public API signature is unchanged
• No user-facing workflows or examples need updating

---