feat(engine): programmatic UTXO freeze api - #1578
Conversation
|
Warning Rate limit exceeded@sdbondi has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 53 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📒 Files selected for processing (38)
WalkthroughMoves/introduces UTXO models ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor C as Caller
participant RM as ResourceManager
participant E as Engine
participant SS as StateStore
Note over C,RM: Freeze selected stealth UTXOs
C->>RM: freeze_utxos(utxos: Vec<UtxoId>)
RM->>E: call(ResourceAction::SetStealthUtxosFreeze,\nSetFreezeStealthUtxosArg{resource, utxos, freeze:true})
E->>SS: lock(resource_address)
alt resource is stealth and utxos non-empty
loop each utxo
E->>SS: lock(SubstateId::Utxo(UtxoAddress))
E->>SS: set utxo.is_frozen = true
SS-->>E: ok
end
E-->>RM: ok
RM-->>C: ok
else error
E-->>RM: RuntimeError (e.g., LockSubstateMismatch{id})
RM-->>C: error
end
sequenceDiagram
autonumber
actor C as Caller
participant RM as ResourceManager
participant E as Engine
Note over C,E: Vault freeze (renamed)
C->>RM: freeze_vault(vault_id)
RM->>E: call(ResourceAction::SetVaultFreeze, flags=all)
E-->>RM: ok
RM-->>C: ok
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/template_lib/src/resource/manager.rs (1)
892-906: Public API rename is breaking; add deprecated shims to honor “no breaking changes”Renaming
freeze/set_freeze/unfreezetofreeze_vault/set_freeze_vault_flags/unfreeze_vaultbreaks existing templates.Add deprecated wrappers:
/// Freezes all withdrawals, deposits and burns for the specified vault. pub fn freeze_vault(&self, vault_id: VaultId) { self.set_freeze_vault_flags(vault_id, VaultFreezeFlags::all()); } + + #[deprecated(note = "Use freeze_vault")] + pub fn freeze(&self, vault_id: VaultId) { + self.freeze_vault(vault_id) + } @@ /// Sets the freeze flags for the specified vault. pub fn set_freeze_vault_flags(&self, vault_id: VaultId, flags: VaultFreezeFlags) { @@ - resp.decode().expect("SetFreeze failed") + resp.decode().expect("SetVaultFreeze failed") } @@ - /// Unfreezes all withdrawals, deposits and burns for the specified vault. - /// Equivalent to `manager.set_freeze(FreezeFlags::empty())`. + /// Unfreezes all withdrawals, deposits and burns for the specified vault. + /// Equivalent to `set_freeze_vault_flags(VaultFreezeFlags::empty())`. pub fn unfreeze_vault(&self, vault_id: VaultId) { self.set_freeze_vault_flags(vault_id, VaultFreezeFlags::empty()); } + + #[deprecated(note = "Use unfreeze_vault")] + pub fn unfreeze(&self, vault_id: VaultId) { + self.unfreeze_vault(vault_id) + } + + #[deprecated(note = "Use set_freeze_vault_flags")] + pub fn set_freeze(&self, vault_id: VaultId, flags: VaultFreezeFlags) { + self.set_freeze_vault_flags(vault_id, flags) + }
🧹 Nitpick comments (10)
crates/template_lib/src/args/types.rs (2)
717-724: Arg naming consistency and potential TS exportThe struct name
SetFreezeStealthUtxosArgis inverted relative to the enum variantSetStealthUtxosFreeze. Consider aligning names and (optionally) exporting to TS if used over JSON-RPC.Proposed tweak:
-#[derive(Clone, Debug, Serialize, Deserialize)] -pub struct SetFreezeStealthUtxosArg { +#[derive(Clone, Debug, Serialize, Deserialize)] +#[cfg_attr(feature = "ts", derive(ts_rs::TS), ts(export))] +pub struct SetStealthUtxosFreezeArg { pub resource_address: ResourceAddress, pub utxos: Vec<UtxoId>, pub freeze: bool, }If you keep the original name, please confirm no TypeScript binding is needed.
714-715: Typo in field name: bultin → builtin (keep backward compat via alias)Minor nit:
bultinis misspelled. Suggest rename with serde alias to preserve compatibility.- GetTemplateAddress { bultin: BuiltinTemplate }, + GetTemplateAddress { + #[serde(alias = "bultin")] + builtin: BuiltinTemplate + },applications/tari_indexer/src/storage_sqlite/models/utxo.rs (1)
119-127: Fix error context strings in to_utxo_idError metadata references the wrong operation/item.
Apply:
- let commitment = - PedersenCommitmentBytes::from_hex(&self.commitment).map_err(|e| StorageError::DecodingError { - operation: "UtxoRecord::to_address", - item: "UtxoAddress", - details: format!("Failed to parse Commitment from string: {}", e), - })?; + let commitment = + PedersenCommitmentBytes::from_hex(&self.commitment).map_err(|e| StorageError::DecodingError { + operation: "UtxoRecord::to_utxo_id", + item: "UtxoId", + details: format!("Failed to parse Commitment from string: {}", e), + })?;crates/engine/tests/stealth.rs (1)
489-491: Consider extracting UTXO ID generation into a helper function.The pattern of generating UtxoIds from commitments is repeated and could be extracted for better readability and reusability.
Apply this diff to improve code organization:
+fn generate_utxo_ids<I>(resource_address: ResourceAddress, masks_and_amounts: I) -> Vec<UtxoId> +where + I: IntoIterator<Item = (&PrivateKey, u64)> +{ + masks_and_amounts + .into_iter() + .map(|(mask, amount)| { + let commitment = get_commitment_factory().commit_value(mask, amount); + UtxoId::from(commitment.to_byte_type()) + }) + .collect() +} #[test] fn freeze_then_attempt_spend() { // ... existing code ... - let utxos = mint.output_masks - .iter() - .zip(outputs) - .take(2) // Freeze the first two outputs - .map(|(mask, amount)| { - let commitment = get_commitment_factory().commit_value(mask, amount); - UtxoId::from(commitment.to_byte_type()) - }) - .collect::<Vec<_>>(); + let utxos = generate_utxo_ids( + resource_address, + mint.output_masks.iter().zip(outputs).take(2) + );crates/engine/src/runtime/working_state.rs (1)
1072-1072: Fix grammatical error in comment.Apply this diff to fix the grammar:
- .expect("FeeState guarantees that the total fee payments fit in an u64"), + .expect("FeeState guarantees that the total fee payments fit in a u64"),crates/engine/src/runtime/impl.rs (1)
1200-1204: Event label reversed for vault freeze/unfreezeWhen flags are empty we are unfreezing, not freezing.
Apply this diff:
- let action = if arg.flags.is_empty() { "freeze" } else { "unfreeze" }; + let action = if arg.flags.is_empty() { "unfreeze" } else { "freeze" };crates/template_lib/src/resource/manager.rs (2)
924-936: Tighten errors and messages; early guard on empty list
- Message still says “SetFreeze failed”. Clarify per action.
- Optional: short-circuit on empty input to avoid an engine call that will error anyway.
Apply this diff:
fn set_freeze_utxos(&self, utxos: Vec<UtxoId>, freeze: bool) { - let resp: InvokeResult = call_engine(EngineOp::ResourceInvoke, &ResourceInvokeArg { + if utxos.is_empty() { return; } + let resp: InvokeResult = call_engine(EngineOp::ResourceInvoke, &ResourceInvokeArg { resource_ref: self.resource_address.into(), action: ResourceAction::SetStealthUtxosFreeze, args: invoke_args![SetFreezeStealthUtxosArg { resource_address: self.resource_address, utxos, freeze }], }); - resp.decode().expect("SetFreeze failed") + resp.decode().expect("SetStealthUtxosFreeze failed") }
892-913: Rename docs to match new APIUpdate the reference in the comment to the new function name (
set_freeze_vault_flags).crates/template_lib/src/models/utxo.rs (2)
43-51: Be strict: require “utxo_” prefix during parsingPermissively accepting strings without the “utxo_” prefix can let malformed IDs sneak in. Enforce the prefix for round‑trip safety.
Apply this diff:
- // utxo_{resource}_{id} - let rest = s.strip_prefix("utxo_").unwrap_or(s); + // utxo_{resource}_{id} + let rest = s.strip_prefix("utxo_").ok_or(KeyParseError)?;
112-119: Fix doc commentThis is a UtxoId namespaced by a ResourceAddress, not a NonFungibleId.
Apply this diff:
-/// A NonFungibleId namespaced by a ResourceAddress. +/// A UtxoId namespaced by a ResourceAddress.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
applications/tari_indexer/src/storage_sqlite/models/utxo.rs(1 hunks)applications/tari_indexer/src/storage_sqlite/models/utxo_update.rs(1 hunks)applications/tari_indexer/src/storage_sqlite/reader.rs(1 hunks)applications/tari_indexer/src/storage_sqlite/store_factory.rs(2 hunks)applications/tari_indexer/src/substate_manager.rs(1 hunks)clients/tari_indexer_client/src/types.rs(1 hunks)clients/wallet_daemon_client/src/types.rs(1 hunks)crates/engine/src/runtime/error.rs(1 hunks)crates/engine/src/runtime/impl.rs(4 hunks)crates/engine/src/runtime/state_store.rs(1 hunks)crates/engine/src/runtime/tracker.rs(1 hunks)crates/engine/src/runtime/working_state.rs(11 hunks)crates/engine/tests/stealth.rs(2 hunks)crates/engine/tests/templates/stealth/src/lib.rs(2 hunks)crates/engine_types/src/indexed_value.rs(2 hunks)crates/engine_types/src/substate.rs(2 hunks)crates/engine_types/src/substate_serde.rs(1 hunks)crates/engine_types/src/utxo.rs(2 hunks)crates/tari_bor/src/tag.rs(0 hunks)crates/template_lib/src/args/types.rs(3 hunks)crates/template_lib/src/models/mod.rs(2 hunks)crates/template_lib/src/models/utxo.rs(1 hunks)crates/template_lib/src/prelude.rs(1 hunks)crates/template_lib/src/resource/manager.rs(4 hunks)crates/wallet/sdk/src/apis/stealth_outputs.rs(1 hunks)crates/wallet/sdk/src/apis/stealth_transfer.rs(2 hunks)crates/wallet/sdk/src/models/stealth_output.rs(1 hunks)crates/wallet/sdk/src/models/utxo_update.rs(1 hunks)crates/wallet/sdk/src/network.rs(1 hunks)crates/wallet/sdk/src/storage.rs(2 hunks)crates/wallet/sdk/tests/support/harness.rs(1 hunks)crates/wallet/sdk_services/src/indexer_jrpc_impl.rs(1 hunks)crates/wallet/sdk_services/src/utxo_scanner/utxo_recovery.rs(2 hunks)crates/wallet/storage_sqlite/src/writer.rs(2 hunks)
💤 Files with no reviewable changes (1)
- crates/tari_bor/src/tag.rs
🧰 Additional context used
🧬 Code graph analysis (29)
crates/template_lib/src/prelude.rs (2)
bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)
crates/wallet/storage_sqlite/src/writer.rs (2)
bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)
clients/tari_indexer_client/src/types.rs (3)
bindings/src/types/NonFungibleAddress.ts (1)
NonFungibleAddress(7-7)bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)
crates/wallet/sdk/tests/support/harness.rs (1)
bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)
crates/wallet/sdk/src/apis/stealth_outputs.rs (1)
bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)
clients/wallet_daemon_client/src/types.rs (1)
bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)
crates/engine/tests/templates/stealth/src/lib.rs (3)
crates/template_lib/src/resource/builder/mod.rs (1)
stealth(83-85)crates/template_lib/src/auth/access_rules.rs (1)
allow_all(137-142)crates/template_lib/src/resource/manager.rs (2)
freeze_utxos(915-917)unfreeze_utxos(920-922)
crates/wallet/sdk/src/network.rs (2)
bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)
crates/wallet/sdk/src/apis/stealth_transfer.rs (6)
bindings/src/types/SubstateId.ts (1)
SubstateId(6-6)bindings/src/types/ComponentAddress.ts (1)
ComponentAddress(6-6)bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)bindings/src/types/StealthTransferStatement.ts (1)
StealthTransferStatement(5-13)bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)bindings/src/types/VaultId.ts (1)
VaultId(6-6)
crates/wallet/sdk_services/src/utxo_scanner/utxo_recovery.rs (4)
bindings/src/types/UtxoOutput.ts (1)
UtxoOutput(6-14)bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)
crates/wallet/sdk/src/storage.rs (3)
bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)bindings/src/types/VaultId.ts (1)
VaultId(6-6)
crates/engine_types/src/indexed_value.rs (3)
bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)bindings/src/types/UtxoAddressContents.ts (1)
UtxoAddressContents(8-8)bindings/src/types/SubstateId.ts (1)
SubstateId(6-6)
crates/wallet/sdk/src/models/utxo_update.rs (1)
bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)
crates/engine/src/runtime/state_store.rs (3)
bindings/src/types/ComponentAddress.ts (1)
ComponentAddress(6-6)bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)bindings/src/types/VaultId.ts (1)
VaultId(6-6)
crates/engine/tests/stealth.rs (4)
crates/engine_types/src/crypto/helpers.rs (1)
get_commitment_factory(55-57)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)crates/template_test_tooling/src/support/stealth.rs (2)
generate_mint_statement(32-54)generate_transfer_data(103-120)crates/engine_types/src/utxo.rs (2)
new(27-32)output(34-36)
applications/tari_indexer/src/storage_sqlite/reader.rs (2)
bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)
crates/engine_types/src/substate.rs (3)
bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)bindings/src/types/Utxo.ts (1)
Utxo(4-4)bindings/src/types/SubstateValue.ts (1)
SubstateValue(12-21)
crates/wallet/sdk_services/src/indexer_jrpc_impl.rs (2)
bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)
crates/template_lib/src/resource/manager.rs (3)
bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)crates/engine/tests/templates/stealth/src/lib.rs (2)
freeze_utxos(74-76)unfreeze_utxos(78-80)crates/engine_types/src/utxo.rs (1)
freeze(46-48)
crates/engine/src/runtime/tracker.rs (1)
bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)
applications/tari_indexer/src/storage_sqlite/models/utxo_update.rs (3)
bindings/src/types/UtxoOutput.ts (1)
UtxoOutput(6-14)bindings/src/types/Shard.ts (1)
Shard(3-3)bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)
crates/engine/src/runtime/impl.rs (5)
crates/template_lib/src/models/utxo.rs (2)
resource_address(31-33)id(35-37)crates/engine_types/src/resource_container.rs (1)
resource_address(175-182)crates/engine/src/runtime/tracker.rs (1)
new(69-86)crates/engine/src/runtime/working_state.rs (1)
new(110-139)crates/engine_types/src/utxo.rs (1)
new(27-32)
applications/tari_indexer/src/substate_manager.rs (2)
bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)
applications/tari_indexer/src/storage_sqlite/models/utxo.rs (10)
bindings/src/types/Utxo.ts (1)
Utxo(4-4)bindings/src/types/UtxoOutput.ts (1)
UtxoOutput(6-14)bindings/src/types/UtxoBurnt.ts (1)
UtxoBurnt(4-4)bindings/src/types/UtxoSpent.ts (1)
UtxoSpent(4-4)bindings/src/types/UtxoUnspent.ts (1)
UtxoUnspent(5-5)bindings/src/types/WalletUtxoUpdate.ts (1)
WalletUtxoUpdate(6-6)bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)bindings/src/types/PedersenCommitmentBytes.ts (1)
PedersenCommitmentBytes(6-6)bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)
crates/engine_types/src/utxo.rs (2)
bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)
crates/template_lib/src/args/types.rs (2)
bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)
crates/wallet/sdk/src/models/stealth_output.rs (3)
bindings/src/types/ComponentAddress.ts (1)
ComponentAddress(6-6)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)
crates/template_lib/src/models/utxo.rs (6)
crates/engine_types/src/substate.rs (18)
fmt(466-478)new(69-74)new(846-852)from_str(488-533)from(302-304)from(308-310)from(314-316)from(320-322)from(326-328)from(332-334)from(338-340)from(344-346)from(350-352)from(784-786)from(790-792)from(796-798)from(802-804)from(808-810)bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)bindings/src/types/UtxoAddressContents.ts (1)
UtxoAddressContents(8-8)crates/engine_types/src/utxo.rs (1)
new(27-32)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)crates/engine_types/src/substate_serde.rs (9)
serde(176-176)serde(180-180)serde(184-184)serde(188-188)serde(192-192)serde(196-196)serde(200-200)serde(204-204)serialize(27-70)
applications/tari_indexer/src/storage_sqlite/store_factory.rs (3)
bindings/src/types/Utxo.ts (1)
Utxo(4-4)bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: check nightly
- GitHub Check: test
- GitHub Check: check stable
- GitHub Check: clippy
- GitHub Check: fmt
🔇 Additional comments (38)
clients/wallet_daemon_client/src/types.rs (1)
55-56: Import migration totari_template_lib::models::UtxoAddressverified: serde derives (Serialize,Deserialize) andts-rs::TSconfiguration match the previous definition, and no lingeringtari_engine_types::UtxoAddressimports were found.crates/wallet/sdk_services/src/utxo_scanner/utxo_recovery.rs (1)
7-7: LGTM - Import path migration aligns with the PR objectives.The imports have been correctly updated to move
UtxoAddressandUtxoIdfromtari_engine_typestotari_template_lib::models, while keepingUtxoOutputimported fromtari_engine_types. This change is consistent with the broader refactoring across the codebase where UTXO identifier/address types are being centralized in template_lib models.Also applies to: 17-17
crates/engine_types/src/indexed_value.rs (3)
18-18: LGTM - Import addition aligns with UTXO addressing enhancement.The addition of
UtxoAddressandUtxoAddressContentsto the import list is consistent with the broader PR changes that migrate UTXO types totari_template_lib::modelsand add support for UTXO substates in indexed values.Also applies to: 19-19
211-211: Parameter name standardization improves consistency.The parameter rename from
addresstoidbetter reflects that this function now works with the more generalSubstateIdtype, which can represent various substate types including UTXOs. This aligns with the addition ofSubstateId::Utxovariant mentioned in the AI summary.
218-218: LGTM - Consistent variable usage after parameter rename.All the equality checks have been correctly updated to use the renamed parameter
idinstead of the previousaddress. The logic remains functionally identical while improving consistency with the new parameter name.Also applies to: 221-221, 224-224, 227-227, 230-230, 233-233, 236-236, 239-239, 242-242
crates/wallet/sdk/tests/support/harness.rs (1)
25-25: LGTM - Import path updated consistently.The import of
UtxoIdhas been correctly moved fromtari_engine_typestotari_template_lib::models, aligning with the broader refactoring described in the PR objectives. This change maintains consistency across the codebase.crates/engine_types/src/substate_serde.rs (1)
240-240: LGTM - Test imports updated for new UTXO model location.The import update moves
UtxoAddressandUtxoIdfrom the local crate totari_template_lib::models, which is consistent with the broader PR changes. This ensures the tests continue to work with the relocated UTXO types.Also applies to: 241-241
crates/engine/src/runtime/state_store.rs (1)
15-15: LGTM - Import addition for UTXO addressing.The addition of
UtxoAddressto the import fromtari_template_lib::modelsaligns with the broader PR changes that relocate UTXO-related types to template_lib models. This import is likely needed for thedowned_utxos: IndexSet<UtxoAddress>field visible at line 36.crates/wallet/sdk/src/storage.rs (1)
9-9: LGTM - Import path consolidation for UTXO types.The imports have been correctly updated to remove
UtxoAddressandUtxoIdfromtari_engine_typesand add them to thetari_template_lib::modelsimport. The addition ofVaultIdto the models import also helps consolidate related address types in one place.Also applies to: 18-18
applications/tari_indexer/src/storage_sqlite/reader.rs (1)
30-30: LGTM - Import path migration for indexer module.The update moves
UtxoIdfromtari_engine_typestotari_template_lib::models, maintaining consistency with the broader PR refactoring. This ensures the indexer uses the centralized UTXO identifier types.crates/wallet/storage_sqlite/src/writer.rs (1)
24-24: LGTM - Import path updates for wallet storage.The imports have been correctly updated to:
- Remove
UtxoAddressandUtxoIdfromtari_engine_types- Add
UtxoAddressandUtxoIdto thetari_template_lib::modelsimportThis change aligns with the broader refactoring to centralize UTXO types in template_lib models while maintaining all functionality.
Also applies to: 45-45
crates/wallet/sdk/src/apis/stealth_outputs.rs (1)
23-23: Import path update is consistent with the broader refactoring.The change from importing UtxoAddress from tari_engine_types to tari_template_lib::models aligns with the template library's model structure for data in the Tari network. This maintains consistency across the codebase.
clients/tari_indexer_client/src/types.rs (1)
15-15: UtxoId import path update is correctly aligned.The addition of UtxoId to the template_lib_models import ensures consistent resolution with other UTXO-related types that have been moved to the models module.
applications/tari_indexer/src/substate_manager.rs (1)
43-43: Import consolidation maintains proper type resolution.Moving UtxoId from tari_engine_types to the ResourceAddress import group in tari_template_lib::models is consistent with the broader UTXO type reorganization. The code continues to function correctly with the new import path.
crates/wallet/sdk/src/network.rs (1)
16-16: Network interface correctly updated with new UtxoId path.The WalletNetworkInterface trait methods like
get_unspent_utxoscontinue to return the sameVec<(UtxoId, Utxo)>signature, but now resolve UtxoId from the models module. This maintains API compatibility while benefiting from the improved module organization.crates/wallet/sdk/src/models/stealth_output.rs (1)
5-5: UtxoAddress import migration maintains model functionality.The
StealthOutputModel::to_utxo_address()method continues to work correctly with UtxoAddress now imported from the models module. This change supports the stealth UTXO operations while maintaining the same public API.crates/template_lib/src/prelude.rs (1)
81-82: New UTXO types properly exposed in prelude.The addition of UtxoAddress and UtxoId to the public prelude enables these types to be easily accessible for template development, supporting the new programmatic UTXO freeze functionality described in the PR objectives.
crates/engine/tests/templates/stealth/src/lib.rs (3)
21-21: Signer derivation for freeze policy is well-implemented.The use of
CallerContext::transaction_signer_public_key()to create a NonFungibleAddress for the freeze rule ensures that only the transaction signer can control UTXO freezing operations.
24-24: Freeze rule correctly configured.The
freezable(rule!(non_fungible(signer)))setup ensures proper authorization for freeze operations, limiting freeze/unfreeze actions to the signer.
74-80: New freeze/unfreeze methods provide clean API.The
freeze_utxosandunfreeze_utxosmethods properly delegate to the ResourceManager, providing a clean public API for the programmatic UTXO freeze functionality as specified in the PR objectives.crates/engine/src/runtime/tracker.rs (1)
42-42: Runtime tracker correctly updated with new import path.The StateTracker's use of UtxoAddress in the
finalizemethod continues to work correctly with the new import path from tari_template_lib::models.applications/tari_indexer/src/storage_sqlite/models/utxo_update.rs (1)
5-8: Import path alignment for UtxoAddress: LGTMSwitching
UtxoAddresstotari_template_lib::modelskeeps indexer in sync with the model move. No behavioral changes here.crates/wallet/sdk/src/models/utxo_update.rs (1)
8-11: UseUtxoIdfrom template_lib models: consistent with the migrationLooks correct; downstream TS bindings already alias
UtxoIdtostring, so no serialization changes expected.applications/tari_indexer/src/storage_sqlite/models/utxo.rs (1)
10-13: Model import migration: LGTMMoving
UtxoAddress/UtxoIdto template_lib keeps storage aligned with public model surface.crates/wallet/sdk/src/apis/stealth_transfer.rs (1)
25-33: Import shift to template_lib models: good; UTXO address creation remains consistentUsing
UtxoAddress/VaultIdfromtemplate_lib::modelsmatches the type migration; helperto_utxo_addresscontinues to construct the correct address.crates/wallet/sdk_services/src/indexer_jrpc_impl.rs (1)
40-43: UnifyResourceAddress/UtxoIdorigin under template_libImport consolidation is correct. Ensure indexer client types align (no duplicate
UtxoIddefinitions leaking into JSON).crates/engine/src/runtime/error.rs (1)
89-94: Field rename toid: clearer error; verify all pattern matches updatedGood clarity improvement in
LockSubstateMismatch. Please ensure no code still destructures{ address: ... }.crates/template_lib/src/models/mod.rs (1)
42-42: Expose UTXO models via template_lib — verified: no collisions and prelude updatedUtxoAddress and UtxoId are defined in crates/template_lib/src/models/utxo.rs and are re-exported in crates/template_lib/src/prelude.rs; no duplicate UtxoAddress/UtxoId definitions or exports were found in unspent_output or elsewhere.
crates/engine/tests/stealth.rs (1)
462-533: LGTM! Well-structured test case for UTXO freeze/unfreeze operations.The test thoroughly validates the freeze functionality by:
- Freezing UTXOs and verifying that spending fails
- Unfreezing and confirming spending succeeds
- Validating the resulting UTXO output state
crates/engine_types/src/substate.rs (1)
767-772: LGTM! Essential accessor for mutable UTXO operations.The
as_utxo_mutmethod is appropriately implemented to enable in-place modification of UTXO substates, which is necessary for the freeze/unfreeze operations.applications/tari_indexer/src/storage_sqlite/store_factory.rs (1)
16-26: LGTM! Import path migration looks correct.The migration of
UtxoIdfromtari_engine_typestotari_template_lib::modelsis properly implemented.crates/engine/src/runtime/working_state.rs (2)
206-210: LGTM! Consistent error field renaming.The migration from
addresstoidfield inRuntimeError::LockSubstateMismatcherrors is consistently applied throughout the file. The renaming better reflects that the field holds aSubstateIdrather than just an address.Also applies to: 224-227, 264-268, 304-307, 315-318, 343-347, 356-361, 372-375
282-286: Good defensive programming with frozen UTXO check.The check for frozen UTXOs before spending provides a clear error message and prevents invalid operations.
crates/engine_types/src/utxo.rs (2)
50-52: LGTM! Completes the freeze/unfreeze API.The
unfreezemethod properly complements the existingfreezemethod to enable bidirectional control of UTXO freeze state.
4-5: UTXO types migration — verifiedUtxoAddress and UtxoId are defined in crates/template_lib/src/models/utxo.rs and the codebase references tari_template_lib::models for those types; no remaining imports of UtxoAddress/UtxoId from engine_types were found.
crates/engine/src/runtime/impl.rs (1)
1158-1209: Good split between vault and stealth UTXO freeze pathsMoving to SetVaultFreeze and introducing a dedicated stealth-UTXO path clarifies semantics. Once the event label above is fixed, this block looks solid.
crates/template_lib/src/models/utxo.rs (2)
65-111: Type design looks goodValue semantics (
Copy) forUtxoId, fixed‑hex serde, and conversions to/fromPedersenCommitmentBytesare solid.
53-57: Formatting uses raw hex — confirmed
ObjectKey's Display emits raw hex and ResourceAddress::as_object_key() returns &ObjectKey, so UtxoAddress::fmt prints the raw hex portion and will round‑trip with FromStr.
5fe6087 to
9969b9b
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/template_lib/src/resource/manager.rs (1)
899-906: Fix expect message and confirm action renameChange the expect() message in crates/template_lib/src/resource/manager.rs (around line 899) from "SetFreeze failed" to "SetVaultFreeze failed". ResourceAction was already renamed — matches found in manager.rs, args/types.rs, and engine/src/runtime/impl.rs; no ResourceAction::SetFreeze occurrences remain.
Apply:
- resp.decode().expect("SetFreeze failed") + resp.decode().expect("SetVaultFreeze failed")
🧹 Nitpick comments (10)
crates/engine/tests/templates/stealth/src/lib.rs (1)
74-80: Consider returning Result or asserting failures in tests.
freeze_utxos/unfreeze_utxosdrop errors (manager API returns()). If unauthorized, this will revert at runtime; consider exposing a Result here (if API allows) or add tests that assert unauthorized calls fail as expected.crates/engine_types/src/utxo.rs (1)
50-52: Add symmetric setter or docs for freeze/unfreeze.You now have
freeze()andunfreeze(). Consider a singleset_frozen(bool)to reduce branching in callers, or document semantics (no-op on burnt UTXOs).crates/template_lib/src/resource/manager.rs (3)
909-913: Docstring still references old API.Update “Equivalent to
manager.set_freeze(FreezeFlags::empty()).” to the new method name.Apply:
- /// Equivalent to `manager.set_freeze(FreezeFlags::empty())`. + /// Equivalent to `manager.set_freeze_vault_flags(vault_id, VaultFreezeFlags::empty())`.
914-922: UTXO freeze/unfreeze: dedup and accept iterable.
- Dedup UTXOs to avoid redundant engine work.
- Optional: accept IntoIterator<Item=UtxoId> for ergonomics.
Apply minimal dedup:
- pub fn freeze_utxos(&self, utxos: Vec<UtxoId>) { - self.set_freeze_utxos(utxos, true); + pub fn freeze_utxos(&self, utxos: Vec<UtxoId>) { + self.set_freeze_utxos(utxos, true); } @@ - pub fn unfreeze_utxos(&self, utxos: Vec<UtxoId>) { - self.set_freeze_utxos(utxos, false); + pub fn unfreeze_utxos(&self, utxos: Vec<UtxoId>) { + self.set_freeze_utxos(utxos, false); }And inside
set_freeze_utxos(next comment) dedup and early‑return on empty.
924-932: Hardenset_freeze_utxos: early‑return, dedup, fix expect string.
- No‑op on empty input avoids an engine call.
- Dedup with
BTreeSetprevents repeated IDs.- Update expect to reflect action name.
Apply:
- fn set_freeze_utxos(&self, utxos: Vec<UtxoId>, freeze: bool) { - let resp: InvokeResult = call_engine(EngineOp::ResourceInvoke, &ResourceInvokeArg { + fn set_freeze_utxos(&self, mut utxos: Vec<UtxoId>, freeze: bool) { + if utxos.is_empty() { + return; + } + // Dedup to avoid redundant work + let set: BTreeSet<UtxoId> = utxos.into_iter().collect(); + utxos = set.into_iter().collect(); + let resp: InvokeResult = call_engine(EngineOp::ResourceInvoke, &ResourceInvokeArg { resource_ref: self.resource_address.into(), action: ResourceAction::SetStealthUtxosFreeze, args: invoke_args![SetFreezeStealthUtxosArg { utxos, freeze }], }); - resp.decode().expect("SetFreeze failed") + resp.decode().expect("SetStealthUtxosFreeze failed") }crates/template_lib/src/models/utxo.rs (5)
40-51: Parser accepts with/withoututxo_prefix—OK; consider stricter mode.Accepting both forms is convenient; if this is ever used outside Substate parsing, consider requiring the
utxo_prefix to reduce accidental matches.
73-92: Add zero‑copy accessors for UtxoId bytes.Expose
as_bytes()andinto_array()to avoid intermediate conversions when bridging to crypto types.Apply:
impl UtxoId { @@ pub fn into_commitment_bytes(self) -> PedersenCommitmentBytes { PedersenCommitmentBytes::from_array(self.0) } pub fn to_commitment_hex_string(&self) -> String { // to_string happens to return the hex encoding of the commitment bytes. If that changes, so will this. self.to_string() } + pub fn as_bytes(&self) -> &[u8; Self::LENGTH] { + &self.0 + } + pub fn into_array(self) -> [u8; Self::LENGTH] { + self.0 + } }
88-91: Avoid coupling hex string to Display.Minor:
to_commitment_hex_stringrelying on Display works but couples behavior. Consider direct hex encoding fromself.0to future‑proof.
112-119: Fix doc string: it’s not a NonFungibleId.Replace “A NonFungibleId namespaced by a ResourceAddress.” with “A UtxoId namespaced by a ResourceAddress.”
Apply:
-/// A NonFungibleId namespaced by a ResourceAddress. +/// A UtxoId namespaced by a ResourceAddress.
156-170: Add negative tests for parser.Add cases for missing underscore, bad hex, and empty pieces.
Apply:
#[test] fn it_parses_from_strings() { @@ assert_eq!(parsed_utxo_address, utxo_address); } + + #[test] + fn it_rejects_malformed_strings() { + assert!(UtxoAddress::from_str("utxo_zzz_bad").is_err()); + assert!(UtxoAddress::from_str("utxo_").is_err()); + assert!(UtxoAddress::from_str("resource_only").is_err()); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
applications/tari_indexer/src/storage_sqlite/models/utxo.rs(1 hunks)applications/tari_indexer/src/storage_sqlite/models/utxo_update.rs(1 hunks)applications/tari_indexer/src/storage_sqlite/reader.rs(1 hunks)applications/tari_indexer/src/storage_sqlite/store_factory.rs(2 hunks)applications/tari_indexer/src/substate_manager.rs(1 hunks)clients/tari_indexer_client/src/types.rs(1 hunks)clients/wallet_daemon_client/src/types.rs(1 hunks)crates/engine/src/runtime/error.rs(1 hunks)crates/engine/src/runtime/impl.rs(4 hunks)crates/engine/src/runtime/state_store.rs(1 hunks)crates/engine/src/runtime/tracker.rs(1 hunks)crates/engine/src/runtime/working_state.rs(11 hunks)crates/engine/tests/stealth.rs(2 hunks)crates/engine/tests/templates/events/src/lib.rs(1 hunks)crates/engine/tests/templates/freeze/src/lib.rs(1 hunks)crates/engine/tests/templates/stealth/src/lib.rs(2 hunks)crates/engine_types/src/indexed_value.rs(2 hunks)crates/engine_types/src/substate.rs(2 hunks)crates/engine_types/src/substate_serde.rs(1 hunks)crates/engine_types/src/utxo.rs(2 hunks)crates/tari_bor/src/tag.rs(0 hunks)crates/template_lib/src/args/types.rs(3 hunks)crates/template_lib/src/models/metadata.rs(1 hunks)crates/template_lib/src/models/mod.rs(3 hunks)crates/template_lib/src/models/utxo.rs(1 hunks)crates/template_lib/src/prelude.rs(2 hunks)crates/template_lib/src/resource/manager.rs(4 hunks)crates/wallet/sdk/src/apis/stealth_outputs.rs(1 hunks)crates/wallet/sdk/src/apis/stealth_transfer.rs(2 hunks)crates/wallet/sdk/src/models/stealth_output.rs(1 hunks)crates/wallet/sdk/src/models/utxo_update.rs(1 hunks)crates/wallet/sdk/src/network.rs(1 hunks)crates/wallet/sdk/src/storage.rs(2 hunks)crates/wallet/sdk/tests/support/harness.rs(1 hunks)crates/wallet/sdk_services/src/indexer_jrpc_impl.rs(1 hunks)crates/wallet/sdk_services/src/utxo_scanner/utxo_recovery.rs(2 hunks)crates/wallet/storage_sqlite/src/writer.rs(2 hunks)
💤 Files with no reviewable changes (1)
- crates/tari_bor/src/tag.rs
🚧 Files skipped from review as they are similar to previous changes (26)
- crates/engine/src/runtime/state_store.rs
- crates/wallet/sdk_services/src/utxo_scanner/utxo_recovery.rs
- crates/engine/src/runtime/error.rs
- crates/wallet/sdk/src/models/utxo_update.rs
- crates/template_lib/src/prelude.rs
- applications/tari_indexer/src/substate_manager.rs
- crates/wallet/sdk/src/storage.rs
- crates/engine_types/src/substate_serde.rs
- crates/engine/src/runtime/tracker.rs
- crates/wallet/storage_sqlite/src/writer.rs
- crates/template_lib/src/args/types.rs
- crates/wallet/sdk/src/apis/stealth_outputs.rs
- applications/tari_indexer/src/storage_sqlite/store_factory.rs
- crates/engine_types/src/indexed_value.rs
- clients/wallet_daemon_client/src/types.rs
- crates/wallet/sdk/src/apis/stealth_transfer.rs
- crates/wallet/sdk/src/models/stealth_output.rs
- clients/tari_indexer_client/src/types.rs
- applications/tari_indexer/src/storage_sqlite/reader.rs
- crates/engine/src/runtime/impl.rs
- crates/engine/tests/stealth.rs
- crates/wallet/sdk_services/src/indexer_jrpc_impl.rs
- crates/template_lib/src/models/mod.rs
- crates/wallet/sdk/tests/support/harness.rs
- crates/wallet/sdk/src/network.rs
- crates/engine_types/src/substate.rs
🧰 Additional context used
🧬 Code graph analysis (9)
crates/engine/tests/templates/freeze/src/lib.rs (1)
crates/template_lib/src/resource/manager.rs (1)
get(102-104)
crates/engine/tests/templates/events/src/lib.rs (2)
crates/engine/src/runtime/impl.rs (1)
emit_event(450-474)crates/engine_types/src/indexed_value.rs (2)
metadata(105-107)metadata(314-316)
crates/template_lib/src/models/metadata.rs (2)
crates/engine_types/src/indexed_value.rs (3)
metadata(105-107)metadata(314-316)new(165-182)bindings/src/types/Metadata.ts (1)
Metadata(6-6)
applications/tari_indexer/src/storage_sqlite/models/utxo.rs (10)
bindings/src/types/Utxo.ts (1)
Utxo(4-4)bindings/src/types/UtxoOutput.ts (1)
UtxoOutput(6-14)bindings/src/types/UtxoBurnt.ts (1)
UtxoBurnt(4-4)bindings/src/types/UtxoSpent.ts (1)
UtxoSpent(4-4)bindings/src/types/UtxoUnspent.ts (1)
UtxoUnspent(5-5)bindings/src/types/WalletUtxoUpdate.ts (1)
WalletUtxoUpdate(6-6)bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)bindings/src/types/PedersenCommitmentBytes.ts (1)
PedersenCommitmentBytes(6-6)bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)
applications/tari_indexer/src/storage_sqlite/models/utxo_update.rs (3)
bindings/src/types/UtxoOutput.ts (1)
UtxoOutput(6-14)bindings/src/types/Shard.ts (1)
Shard(3-3)bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)
crates/engine/tests/templates/stealth/src/lib.rs (3)
crates/template_lib/src/resource/builder/mod.rs (2)
stealth(83-85)non_fungible(65-67)crates/template_lib/src/auth/access_rules.rs (1)
allow_all(137-142)crates/template_lib/src/resource/manager.rs (2)
freeze_utxos(915-917)unfreeze_utxos(920-922)
crates/template_lib/src/resource/manager.rs (4)
bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)crates/engine/tests/templates/stealth/src/lib.rs (2)
freeze_utxos(74-76)unfreeze_utxos(78-80)crates/engine/tests/templates/freeze/src/lib.rs (1)
freeze(38-40)crates/engine_types/src/utxo.rs (1)
freeze(46-48)
crates/template_lib/src/models/utxo.rs (6)
crates/engine_types/src/substate.rs (18)
fmt(466-478)new(69-74)new(846-852)from_str(488-533)from(302-304)from(308-310)from(314-316)from(320-322)from(326-328)from(332-334)from(338-340)from(344-346)from(350-352)from(784-786)from(790-792)from(796-798)from(802-804)from(808-810)bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)bindings/src/types/UtxoAddressContents.ts (1)
UtxoAddressContents(8-8)crates/engine_types/src/utxo.rs (1)
new(27-32)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)crates/engine_types/src/substate_serde.rs (9)
serde(176-176)serde(180-180)serde(184-184)serde(188-188)serde(192-192)serde(196-196)serde(200-200)serde(204-204)serialize(27-70)
crates/engine_types/src/utxo.rs (2)
bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check nightly
- GitHub Check: check stable
- GitHub Check: clippy
🔇 Additional comments (20)
crates/engine/tests/templates/stealth/src/lib.rs (1)
21-26: Freeze authority tied to tx signer — confirm this is intended.
NonFungibleAddress::from_public_key(CallerContext::transaction_signer_public_key())+freezable(rule!(non_fungible(signer)))gates UTXO freeze to the tx signer’s identity NFT. Verify this is the desired policy for tests and won’t make freezes depend on who submits the tx rather than a designated admin identity.crates/engine/src/runtime/working_state.rs (9)
56-58: Import path migration looks correct.
UtxoAddressmoved totari_template_lib::models.
205-209: Use of id in LockSubstateMismatch aligns with struct rename.Switched from
addresstoid.
223-227: Consistent id usage.
id: locked.substate_id().clone()is correct here.
264-268: Consistent id usage.
id: addris correct for resources.
344-347: Consistent id usage.
get_vaulterror binding looks good.
358-361: Consistent id usage.
get_vault_muterror binding looks good.
371-375: Consistent id usage.
get_resource_muterror binding looks good.
1068-1073: Grammar fix LGTM.“a u64” is correct.
1456-1460: Log message grammar fix LGTM.Updated wording reads correctly.
crates/engine_types/src/utxo.rs (1)
4-6: Type imports LGTM.Using BOR Serialize/Deserialize and moving crypto types under template_lib matches the refactor.
crates/engine/tests/templates/freeze/src/lib.rs (2)
39-40: API rename LGTM.
freeze_vaultmatches the new manager API.
43-44: API rename LGTM.
unfreeze_vaultmatches the new manager API.crates/engine/tests/templates/events/src/lib.rs (1)
36-36: Confirm macro availability without qualification.Bare
metadata!relies on it being re-exported into scope (via prelude). If any call sites omit the prelude, considertari_template_lib::metadata!or ensure prelude re-exports it.applications/tari_indexer/src/storage_sqlite/models/utxo.rs (1)
6-13: Import path updates LGTM.
UtxoAddress/UtxoIdmoved totari_template_lib::models;UtxoOutputremains intari_engine_types.applications/tari_indexer/src/storage_sqlite/models/utxo_update.rs (1)
5-8: Import path update LGTM.
UtxoAddressnow sourced fromtari_template_lib::models.crates/template_lib/src/resource/manager.rs (2)
61-62: Imports look correct for new UTXO-freeze API.
SetFreezeStealthUtxosArgandUtxoIdadditions align with the new engine actions. No issues.Also applies to: 76-78
893-896: Rename is good; API reads clearer.
freeze_vaultdelegating toset_freeze_vault_flags(..., VaultFreezeFlags::all())is sound.crates/template_lib/src/models/utxo.rs (2)
53-57: Display format aligns with SubstateId printing.Using
as_object_key()keeps it consistent with other addresses. LGTM.
121-150: borsh impls are fine; could be derived for contents only.Manual impls look correct and minimal. No change required.
9969b9b to
1866ad8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/wallet/sdk/src/models/stealth_output.rs (1)
31-33: UtxoAddress::new signature changed — update callsite to pass a UtxoIdcrates/template_lib/src/models/utxo.rs:27 defines
pub fn new(resource_address: ResourceAddress, id: UtxoId) -> Self; update the call in crates/wallet/sdk/src/models/stealth_output.rs (lines 31–33) —UtxoAddress::new(self.resource_address, self.commitment.into())— to supply aUtxoId(or an explicit conversion toUtxoId) so the types align.crates/engine/src/runtime/impl.rs (1)
1199-1205: Bug: inverted event action for vault freeze/unfreeze.When flags.is_empty() it’s an unfreeze, not freeze.
Apply:
- let action = if arg.flags.is_empty() { "freeze" } else { "unfreeze" }; + let action = if arg.flags.is_empty() { "unfreeze" } else { "freeze" };
🧹 Nitpick comments (10)
crates/engine_types/src/utxo.rs (4)
12-14: Make fields private to preserve invariants and allow evolution.Public fields let callers bypass
freeze/unfreeze/burninvariants. You already expose accessors, so keeping fields private is safer.pub struct Utxo { - pub output: Option<UtxoOutput>, - pub is_frozen: bool, + output: Option<UtxoOutput>, + is_frozen: bool, }
50-52: Avoid unfreezing burnt UTXOs (no‑op guard).Unfreezing a burnt UTXO toggles a meaningless flag and can confuse callers.
pub fn unfreeze(&mut self) { - self.is_frozen = false; + // No-op if already burnt + if self.is_burnt() { + return; + } + self.is_frozen = false; }
58-64: Provide a convenienceis_spendable()guard.This centralizes the intended predicate and reduces duplicate checks at call sites.
pub fn is_burnt(&self) -> bool { self.output.is_none() } +/// Returns true if the UTXO can be spent (i.e., exists and is not frozen). +pub fn is_spendable(&self) -> bool { + !self.is_burnt() && !self.is_frozen +}
9-11: ConfirmDebugredaction for sensitive fields.If
PrivateOutput: Debugprints raw data, logs could leak sensitive info. Ensure it is redacted or avoid derivingDebughere.If needed, drop
Debugon these structs and implement a redacted formatter:-#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Clone, Serialize, Deserialize)] pub struct Utxo { ... } -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Clone, Serialize, Deserialize)] pub struct UtxoOutput { ... }Then implement
fmt::Debugthat omits/obfuscatesoutputinternals.Also applies to: 16-18
crates/engine/tests/stealth.rs (1)
489-492: Consider extracting UTXO ID derivation logic.The logic for deriving UTXO IDs from commitments could be extracted into a helper function for reusability:
fn derive_utxo_ids(masks: &[PrivateKey], amounts: &[u64]) -> Vec<UtxoId> { masks.iter() .zip(amounts) .map(|(mask, amount)| { let commitment = get_commitment_factory().commit_value(mask, *amount); UtxoId::from(commitment.to_byte_type()) }) .collect() }crates/engine/src/runtime/impl.rs (1)
1224-1285: Optional: add events for UTXO freeze/unfreeze.Consider emitting per‑UTXO or batch events (object "utxo", actions "freeze"/"unfreeze") for auditability, mirroring vault freeze events.
crates/template_lib/src/resource/manager.rs (4)
893-895: API rename LGTM; align docs with new names.Docs still reference set_freeze(...). Update to set_freeze_vault_flags(...).
Apply:
- /// Equivalent to `manager.set_freeze(FreezeFlags::empty())`. + /// Equivalent to `manager.set_freeze_vault_flags(FreezeFlags::empty())`.Also applies to: 910-913
898-906: Clarify panic message."SetFreeze failed" is misleading after rename.
Apply:
- resp.decode().expect("SetFreeze failed") + resp.decode().expect("SetVaultFreeze failed")
914-932: Consistent error text for UTXO freeze.Align expect message with action.
Apply:
- resp.decode().expect("SetFreeze failed") + resp.decode().expect("SetStealthUtxosFreeze failed")
915-922: Optional: pre-validate non-empty UTXO list.Engine validates, but early client-side check yields clearer panic sites.
pub fn freeze_utxos(&self, utxos: Vec<UtxoId>) { - self.set_freeze_utxos(utxos, true); + assert!(!utxos.is_empty(), "freeze_utxos: UTXO list cannot be empty"); + self.set_freeze_utxos(utxos, true); } pub fn unfreeze_utxos(&self, utxos: Vec<UtxoId>) { - self.set_freeze_utxos(utxos, false); + assert!(!utxos.is_empty(), "unfreeze_utxos: UTXO list cannot be empty"); + self.set_freeze_utxos(utxos, false); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
applications/tari_indexer/src/storage_sqlite/models/utxo.rs(1 hunks)applications/tari_indexer/src/storage_sqlite/models/utxo_update.rs(1 hunks)applications/tari_indexer/src/storage_sqlite/reader.rs(1 hunks)applications/tari_indexer/src/storage_sqlite/store_factory.rs(2 hunks)applications/tari_indexer/src/substate_manager.rs(1 hunks)clients/tari_indexer_client/src/types.rs(1 hunks)clients/wallet_daemon_client/src/types.rs(1 hunks)crates/engine/src/runtime/error.rs(1 hunks)crates/engine/src/runtime/impl.rs(4 hunks)crates/engine/src/runtime/state_store.rs(1 hunks)crates/engine/src/runtime/tracker.rs(1 hunks)crates/engine/src/runtime/working_state.rs(11 hunks)crates/engine/tests/stealth.rs(2 hunks)crates/engine/tests/templates/events/src/lib.rs(1 hunks)crates/engine/tests/templates/freeze/src/lib.rs(1 hunks)crates/engine/tests/templates/stealth/src/lib.rs(2 hunks)crates/engine_types/src/indexed_value.rs(2 hunks)crates/engine_types/src/substate.rs(2 hunks)crates/engine_types/src/substate_serde.rs(1 hunks)crates/engine_types/src/utxo.rs(2 hunks)crates/tari_bor/src/tag.rs(0 hunks)crates/template_lib/src/args/types.rs(3 hunks)crates/template_lib/src/models/metadata.rs(1 hunks)crates/template_lib/src/models/mod.rs(3 hunks)crates/template_lib/src/models/utxo.rs(1 hunks)crates/template_lib/src/prelude.rs(2 hunks)crates/template_lib/src/resource/manager.rs(4 hunks)crates/wallet/sdk/src/apis/stealth_outputs.rs(1 hunks)crates/wallet/sdk/src/apis/stealth_transfer.rs(2 hunks)crates/wallet/sdk/src/models/stealth_output.rs(1 hunks)crates/wallet/sdk/src/models/utxo_update.rs(1 hunks)crates/wallet/sdk/src/network.rs(1 hunks)crates/wallet/sdk/src/storage.rs(2 hunks)crates/wallet/sdk/tests/support/harness.rs(1 hunks)crates/wallet/sdk_services/src/indexer_jrpc_impl.rs(1 hunks)crates/wallet/sdk_services/src/utxo_scanner/utxo_recovery.rs(2 hunks)crates/wallet/storage_sqlite/src/writer.rs(2 hunks)
💤 Files with no reviewable changes (1)
- crates/tari_bor/src/tag.rs
🚧 Files skipped from review as they are similar to previous changes (22)
- clients/wallet_daemon_client/src/types.rs
- crates/wallet/sdk/src/models/utxo_update.rs
- crates/engine/tests/templates/stealth/src/lib.rs
- applications/tari_indexer/src/substate_manager.rs
- crates/wallet/sdk/tests/support/harness.rs
- crates/wallet/sdk/src/storage.rs
- applications/tari_indexer/src/storage_sqlite/store_factory.rs
- applications/tari_indexer/src/storage_sqlite/models/utxo.rs
- crates/wallet/sdk_services/src/utxo_scanner/utxo_recovery.rs
- crates/template_lib/src/args/types.rs
- crates/wallet/storage_sqlite/src/writer.rs
- crates/engine_types/src/substate_serde.rs
- crates/template_lib/src/prelude.rs
- applications/tari_indexer/src/storage_sqlite/reader.rs
- crates/wallet/sdk/src/network.rs
- crates/engine/tests/templates/freeze/src/lib.rs
- crates/template_lib/src/models/utxo.rs
- crates/template_lib/src/models/mod.rs
- crates/wallet/sdk/src/apis/stealth_outputs.rs
- crates/engine/tests/templates/events/src/lib.rs
- applications/tari_indexer/src/storage_sqlite/models/utxo_update.rs
- crates/wallet/sdk_services/src/indexer_jrpc_impl.rs
🧰 Additional context used
🧬 Code graph analysis (13)
crates/wallet/sdk/src/apis/stealth_transfer.rs (1)
bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)
crates/wallet/sdk/src/models/stealth_output.rs (3)
bindings/src/types/ComponentAddress.ts (1)
ComponentAddress(6-6)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)
crates/engine/src/runtime/impl.rs (4)
bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)crates/engine_types/src/resource_container.rs (1)
resource_address(175-182)bindings/src/types/Utxo.ts (1)
Utxo(4-4)crates/engine_types/src/utxo.rs (1)
new(27-32)
crates/engine/src/runtime/tracker.rs (1)
bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)
crates/engine_types/src/utxo.rs (3)
bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)crates/engine/tests/templates/freeze/src/lib.rs (1)
unfreeze(42-44)
crates/engine/src/runtime/error.rs (1)
bindings/src/types/SubstateId.ts (1)
SubstateId(6-6)
clients/tari_indexer_client/src/types.rs (3)
bindings/src/types/NonFungibleAddress.ts (1)
NonFungibleAddress(7-7)bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)
crates/template_lib/src/models/metadata.rs (2)
crates/engine_types/src/indexed_value.rs (3)
metadata(105-107)metadata(314-316)new(165-182)bindings/src/types/Metadata.ts (1)
Metadata(6-6)
crates/template_lib/src/resource/manager.rs (7)
bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)bindings/src/types/VaultId.ts (1)
VaultId(6-6)bindings/src/types/VaultFreezeFlags.ts (1)
VaultFreezeFlags(3-3)crates/template_lib/src/args/freeze_flags.rs (2)
all(31-33)empty(15-17)crates/engine/tests/templates/stealth/src/lib.rs (2)
freeze_utxos(74-76)unfreeze_utxos(78-80)crates/engine/tests/templates/freeze/src/lib.rs (1)
freeze(38-40)crates/engine_types/src/utxo.rs (1)
freeze(46-48)
crates/engine/src/runtime/state_store.rs (1)
bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)
crates/engine_types/src/indexed_value.rs (4)
bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)bindings/src/types/UtxoAddressContents.ts (1)
UtxoAddressContents(8-8)bindings/src/types/SubstateId.ts (1)
SubstateId(6-6)bindings/src/types/Utxo.ts (1)
Utxo(4-4)
crates/engine_types/src/substate.rs (3)
bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)bindings/src/types/Utxo.ts (1)
Utxo(4-4)bindings/src/types/SubstateValue.ts (1)
SubstateValue(12-21)
crates/engine/tests/stealth.rs (3)
crates/engine_types/src/crypto/helpers.rs (1)
get_commitment_factory(55-57)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)crates/engine_types/src/utxo.rs (2)
new(27-32)output(34-36)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: check nightly
- GitHub Check: test
- GitHub Check: check stable
- GitHub Check: clippy
- GitHub Check: machete
- GitHub Check: fmt
🔇 Additional comments (22)
crates/wallet/sdk/src/models/stealth_output.rs (1)
4-8: Import relocation verified — no stale tari_engine_types::UtxoAddress imports found.Repo-wide search (including hidden/ignored files) shows UtxoAddress is imported from tari_template_lib::models; no occurrences of use tari_engine_types::UtxoAddress were found.
crates/engine_types/src/utxo.rs (2)
18-24: ts-rs: no change required — field types already implement TSRistrettoPublicKeyBytes and UtxoTag both derive ts_rs::TS in crates/template_lib_types/src/crypto/ristretto.rs and crates/template_lib_types/src/crypto/utxo_tag.rs, so deriving TS on UtxoOutput will succeed and no explicit #[cfg_attr(feature = "ts", ts(...))] mappings are required.
54-56: Resolved — burn() not setting is_frozen is acceptable; no callers rely on that coupling. Search shows no code treating "not spendable" as is_burnt() || is_frozen(); engine enforces freeze for UTXO spends (crates/engine/src/runtime/working_state.rs:279–285), uses is_burnt separately for NFTs (crates/engine/src/runtime/working_state.rs:501–504), and the wallet updates both flags independently (crates/wallet/sdk/src/apis/stealth_outputs.rs:374–395).crates/template_lib/src/models/metadata.rs (1)
148-161: Good use of $crate paths in the macro.The macro correctly uses
$crate::models::Metadatafor paths, avoiding the need for callers to importMetadata. This addresses the previous review comment about using $crate-qualified paths.crates/engine_types/src/indexed_value.rs (1)
211-211: Parameter rename improves clarity.Renaming from
addresstoidbetter reflects that this function accepts any SubstateId variant, not just addresses.crates/engine/src/runtime/state_store.rs (1)
15-15: LGTM! Import path change aligns with PR-wide refactoring.The migration of
UtxoAddressfromtari_engine_typestotari_template_lib::modelsis consistent with the broader effort to consolidate UTXO-related types in the template library.crates/engine/src/runtime/tracker.rs (1)
42-42: Import path change is consistent.The migration of
UtxoAddresstotari_template_lib::modelsaligns with the PR-wide refactoring of UTXO-related types.clients/tari_indexer_client/src/types.rs (1)
15-15: Import reorganization looks good.Consolidating
UtxoIdwith other template_lib_models imports improves code organization.crates/engine/src/runtime/error.rs (1)
89-93: Parameter rename improves error message consistency.Changing from
addresstoidin the error variant and message makes it clearer that this error applies to any SubstateId type, not just addresses.crates/engine_types/src/substate.rs (2)
349-353: LGTM! From implementation for UtxoAddress.The addition of
From<UtxoAddress>conversion enables seamless integration of UTXO addresses into the SubstateId system.
767-772: Good addition of mutable accessor for Utxo.The
as_utxo_mutmethod enables in-place modification of UTXO substates, which is essential for the freeze/unfreeze functionality.crates/engine/tests/stealth.rs (1)
463-533: Comprehensive test coverage for UTXO freeze functionality.The test thoroughly validates the freeze/unfreeze API:
- Creates UTXOs via stealth mint
- Freezes specific UTXOs by their commitment-derived IDs
- Verifies that frozen UTXOs cannot be spent
- Unfreezes the UTXOs
- Confirms successful spending after unfreezing
This provides good end-to-end coverage of the new programmatic UTXO freeze API.
crates/engine/src/runtime/working_state.rs (7)
203-211: Resolved: correct lock_id and id in LockSubstateMismatch.Now uses locked.lock_id() and includes the actual substate id. Good fix.
223-228: Accurate error context on component mutation path.Using locked.lock_id() and the precise substate id improves diagnostics.
262-269: Accurate error context on resource path.Same improvement applied consistently here.
298-307: Consistent, correct error context for NonFungible access (immut/mut).Looks good.
Also applies to: 310-319
340-347: Consistent, correct error context for Vault/Resource (immut/mut).All use locked.lock_id() and the actual id. LGTM.
Also applies to: 352-361, 366-376
273-287: Confirm lock lifecycle when downing UTXOs.You lock each UTXO (Write), then call store.down_utxo(lock_id) without an explicit unlock. If down_utxo consumes/releases the lock, all good; if not, this could leak locks.
Would you confirm that down_utxo(lock_id) consumes the lock and does not require try_unlock?
1458-1461: Grammar fix in ownership warning.“it does not own” reads correctly now.
crates/engine/src/runtime/impl.rs (1)
1224-1285: Security parity: invoke resource auth hook for UTXO freeze/unfreeze.Freeze on stealth UTXOs authorizes via access rules but skips the resource auth hook, unlike Mint/Recall/UpdateAccessRules/SetVaultFreeze. This weakens policy guarantees for resources relying on hooks.
Suggested two‑stage flow mirroring other actions (validate + hook, then mutate):
ResourceAction::SetStealthUtxosFreeze => { - let resource_address = + let resource_address = resource_ref .as_resource_address() .ok_or_else(|| RuntimeError::InvalidArgument { argument: "resource_ref", reason: "FreezeStealthUtxo resource action requires a resource address".to_string(), })?; let arg: SetFreezeStealthUtxosArg = args.assert_one_arg()?; - - self.tracker.write_with(|state_mut| { - let resource_lock = state_mut.read_lock_substate(&SubstateId::Resource(resource_address))?; - - let resource = state_mut.get_resource(&resource_lock)?; - - if !resource.resource_type().is_stealth() { - return Err(RuntimeError::InvalidArgument { - argument: "resource_ref", - reason: "FreezeStealthUtxo can only be called on stealth resources".to_string(), - }); - } - - if arg.utxos.is_empty() { - return Err(RuntimeError::InvalidArgument { - argument: "SetFreezeStealthUtxosArg", - reason: "Utxos list cannot be empty".to_string(), - }); - } - - state_mut.authorization().check_resource_access_rules( - ResourceAuthAction::Freeze, - resource.as_ownership(), - resource.access_rules(), - )?; - - for utxo in arg.utxos { - let id = SubstateId::Utxo(UtxoAddress::new(resource_address, utxo)); - let locked = state_mut.write_lock_substate(&id)?; - - let utxo = state_mut - .get_locked_substate_mut(&locked)? - .as_utxo_mut() - .ok_or_else(|| RuntimeError::LockSubstateMismatch { - lock_id: locked.lock_id(), - expected_type: "Utxo", - id, - })?; - - // Freeze is idempotent. - if arg.freeze { - utxo.freeze(); - } else { - utxo.unfreeze(); - } - state_mut.unlock_substate(locked)?; - } - - state_mut.unlock_substate(resource_lock)?; - - Ok(InvokeResult::unit()) - }) + let utxos = arg.utxos.clone(); + let freeze = arg.freeze; + + // Stage 1: validate + access rules + fetch hook + let (maybe_auth_hook, auth_caller) = self.tracker.write_with(|state_mut| { + let resource_lock = state_mut.read_lock_substate(&SubstateId::Resource(resource_address))?; + let resource = state_mut.get_resource(&resource_lock)?; + + if !resource.resource_type().is_stealth() { + return Err(RuntimeError::InvalidArgument { + argument: "resource_ref", + reason: "FreezeStealthUtxo can only be called on stealth resources".to_string(), + }); + } + if utxos.is_empty() { + return Err(RuntimeError::InvalidArgument { + argument: "SetFreezeStealthUtxosArg", + reason: "Utxos list cannot be empty".to_string(), + }); + } + + state_mut.authorization().check_resource_access_rules( + ResourceAuthAction::Freeze, + resource.as_ownership(), + resource.access_rules(), + )?; + + let auth_hook = resource.auth_hook().cloned(); + let auth_caller = state_mut.get_auth_caller()?; + state_mut.unlock_substate(resource_lock)?; + Ok::<_, RuntimeError>((auth_hook, auth_caller)) + })?; + + if let Some(auth_hook) = maybe_auth_hook { + self.invoke_resource_access_hook(auth_hook, auth_caller, ResourceAuthAction::Freeze)?; + } + + // Stage 2: mutate UTXOs + self.tracker.write_with(|state_mut| { + for utxo in utxos { + let id = SubstateId::Utxo(UtxoAddress::new(resource_address, utxo)); + let locked = state_mut.write_lock_substate(&id)?; + let utxo = state_mut + .get_locked_substate_mut(&locked)? + .as_utxo_mut() + .ok_or_else(|| RuntimeError::LockSubstateMismatch { + lock_id: locked.lock_id(), + expected_type: "Utxo", + id, + })?; + if freeze { utxo.freeze(); } else { utxo.unfreeze(); } + state_mut.unlock_substate(locked)?; + } + Ok(InvokeResult::unit()) + }) },crates/wallet/sdk/src/apis/stealth_transfer.rs (2)
25-32: UtxoAddress imports validated — no engine_types usages remainSweep (rg) shows UtxoAddress imported from tari_template_lib::models only at:
- crates/wallet/sdk_services/src/utxo_scanner/utxo_recovery.rs:17
- applications/tari_indexer/src/storage_sqlite/models/utxo_update.rs:7
- crates/engine/src/runtime/state_store.rs:15
No engine_types occurrences found.
9-9: Approve — byte-type traits originate in tari_engine_types; no re-export in template_lib.ConvertFromByteType/FromByteType/ToByteType are defined in crates/engine_types/src/byte_types.rs with multiple impls and there are no pub re-exports in crates/template_lib; importing them from tari_engine_types is correct.
1866ad8 to
aa8d3d1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/engine/src/runtime/working_state.rs (2)
203-209: LockSubstateMismatch: pass owned SubstateId (clone) instead of borrowed.Several error constructors pass
id: addr/id: addresswhereaddr/addressis likely&SubstateId. Other call sites use.clone(). Make these consistent to avoid type-mismatch or unintended moves.Apply:
@@ - let component = substate.component().ok_or_else(|| RuntimeError::LockSubstateMismatch { - lock_id: locked.lock_id(), - id: address, - expected_type: "Component", - })?; + let component = substate.component().ok_or_else(|| RuntimeError::LockSubstateMismatch { + lock_id: locked.lock_id(), + id: address.clone(), + expected_type: "Component", + })?; @@ - .ok_or_else(|| RuntimeError::LockSubstateMismatch { - lock_id: locked.lock_id(), - id: addr, - expected_type: "Resource", - })?; + .ok_or_else(|| RuntimeError::LockSubstateMismatch { + lock_id: locked.lock_id(), + id: addr.clone(), + expected_type: "Resource", + })?; @@ - let vault = substate.as_vault().ok_or_else(|| RuntimeError::LockSubstateMismatch { - lock_id: locked.lock_id(), - id: addr, - expected_type: "Vault", - })?; + let vault = substate.as_vault().ok_or_else(|| RuntimeError::LockSubstateMismatch { + lock_id: locked.lock_id(), + id: addr.clone(), + expected_type: "Vault", + })?; @@ - .ok_or_else(|| RuntimeError::LockSubstateMismatch { - lock_id: locked.lock_id(), - id: addr, - expected_type: "Vault", - })?; + .ok_or_else(|| RuntimeError::LockSubstateMismatch { + lock_id: locked.lock_id(), + id: addr.clone(), + expected_type: "Vault", + })?; @@ - .ok_or_else(|| RuntimeError::LockSubstateMismatch { - lock_id: locked.lock_id(), - id: addr, - expected_type: "Resource", - })?; + .ok_or_else(|| RuntimeError::LockSubstateMismatch { + lock_id: locked.lock_id(), + id: addr.clone(), + expected_type: "Resource", + })?;Also applies to: 260-268, 343-347, 357-361, 370-375
273-296: Fix dangling locks when downing UTXOsdown_utxo does not release the LockId it reads from LockedSubstates (it calls locked_substates.get(...) and removes the substate but never calls try_unlock). spend_stealth_utxos calls self.store.try_lock(...), then self.store.down_utxo(lock_id)? and never unlocks — this leaves locks held and will deadlock under contention.
Actionable fixes (choose one):
- Call self.store.try_unlock(lock_id)? immediately after down_utxo in crates/engine/src/runtime/working_state.rs (spend_stealth_utxos, ~lines 273–296).
- Or change crates/engine/src/runtime/state_store.rs::down_utxo (around line ~184) to consume/unlock the lock internally and document the new contract; update callers to avoid double-unlock.
🧹 Nitpick comments (4)
crates/engine/src/runtime/impl.rs (1)
1197-1207: Avoid duplicate/misleading freeze events; working_state already emits them.
state_mut.set_vault_freezeemits a vault event. The additional “resource freeze/unfreeze” here causes duplicate/conflicting events (and the action mapping is inverted).Apply:
- let payload = - Metadata::from_iter([("vault_id", arg.vault_id.to_string()), ("flags", arg.flags.to_string())]); - let action = if arg.flags.is_empty() { "freeze" } else { "unfreeze" }; - self.emit_std_event("resource", action, resource_address.into(), payload, state_mut)?; + // No extra event here; WorkingState::set_vault_freeze already emits a precise vault event.crates/template_lib/src/resource/manager.rs (2)
892-895: Vault freeze API rename reads well; minor doc polish.Docs for unfreeze still reference the old helper name. Update for accuracy.
@@ - /// Unfreezes all withdrawals, deposits and burns for the specified vault. - /// Equivalent to `manager.set_freeze(FreezeFlags::empty())`. + /// Unfreezes all withdrawals, deposits and burns for the specified vault. + /// Equivalent to `manager.set_freeze_vault_flags(vault_id, VaultFreezeFlags::empty())`.Also applies to: 911-913
274-278: Doc example nit: wrong parameter name for mint_stealth.Example uses
statementbut the API takesamount.- /// let bucket = resource_manager.mint_stealth(statement); + /// let bucket = resource_manager.mint_stealth(amount);crates/engine_types/src/utxo.rs (1)
50-53: LGTM — unfreeze added; burn no longer alters frozen flag.Verified: Utxo::unfreeze() sets is_frozen = false; burn() only clears output and leaves is_frozen unchanged; no callers assume burn implies frozen.
Add a tiny unit test asserting freeze/unfreeze idempotency and that burn does not change is_frozen.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
applications/tari_indexer/src/storage_sqlite/models/utxo.rs(1 hunks)applications/tari_indexer/src/storage_sqlite/models/utxo_update.rs(1 hunks)applications/tari_indexer/src/storage_sqlite/reader.rs(1 hunks)applications/tari_indexer/src/storage_sqlite/store_factory.rs(2 hunks)applications/tari_indexer/src/substate_manager.rs(1 hunks)clients/tari_indexer_client/src/types.rs(1 hunks)clients/wallet_daemon_client/src/types.rs(1 hunks)crates/engine/src/runtime/error.rs(1 hunks)crates/engine/src/runtime/impl.rs(4 hunks)crates/engine/src/runtime/state_store.rs(1 hunks)crates/engine/src/runtime/tracker.rs(1 hunks)crates/engine/src/runtime/working_state.rs(11 hunks)crates/engine/tests/stealth.rs(2 hunks)crates/engine/tests/templates/events/src/lib.rs(1 hunks)crates/engine/tests/templates/freeze/src/lib.rs(1 hunks)crates/engine/tests/templates/stealth/src/lib.rs(2 hunks)crates/engine_types/src/indexed_value.rs(2 hunks)crates/engine_types/src/substate.rs(2 hunks)crates/engine_types/src/substate_serde.rs(1 hunks)crates/engine_types/src/utxo.rs(2 hunks)crates/tari_bor/src/tag.rs(0 hunks)crates/template_lib/src/args/types.rs(3 hunks)crates/template_lib/src/models/metadata.rs(1 hunks)crates/template_lib/src/models/mod.rs(3 hunks)crates/template_lib/src/models/utxo.rs(1 hunks)crates/template_lib/src/prelude.rs(2 hunks)crates/template_lib/src/resource/manager.rs(4 hunks)crates/wallet/sdk/src/apis/stealth_outputs.rs(1 hunks)crates/wallet/sdk/src/apis/stealth_transfer.rs(2 hunks)crates/wallet/sdk/src/models/stealth_output.rs(1 hunks)crates/wallet/sdk/src/models/utxo_update.rs(1 hunks)crates/wallet/sdk/src/network.rs(1 hunks)crates/wallet/sdk/src/storage.rs(2 hunks)crates/wallet/sdk/tests/support/harness.rs(1 hunks)crates/wallet/sdk_services/src/indexer_jrpc_impl.rs(1 hunks)crates/wallet/sdk_services/src/utxo_scanner/utxo_recovery.rs(2 hunks)crates/wallet/storage_sqlite/src/writer.rs(2 hunks)
💤 Files with no reviewable changes (1)
- crates/tari_bor/src/tag.rs
🚧 Files skipped from review as they are similar to previous changes (27)
- crates/template_lib/src/models/metadata.rs
- crates/wallet/sdk/src/models/utxo_update.rs
- crates/wallet/sdk/src/apis/stealth_outputs.rs
- crates/wallet/sdk/tests/support/harness.rs
- crates/engine/tests/templates/events/src/lib.rs
- crates/engine/tests/templates/freeze/src/lib.rs
- crates/engine/src/runtime/tracker.rs
- applications/tari_indexer/src/storage_sqlite/models/utxo.rs
- crates/wallet/sdk_services/src/utxo_scanner/utxo_recovery.rs
- crates/engine_types/src/substate.rs
- crates/template_lib/src/models/mod.rs
- clients/wallet_daemon_client/src/types.rs
- crates/engine/src/runtime/error.rs
- crates/wallet/sdk/src/apis/stealth_transfer.rs
- crates/wallet/sdk/src/models/stealth_output.rs
- crates/template_lib/src/prelude.rs
- applications/tari_indexer/src/substate_manager.rs
- applications/tari_indexer/src/storage_sqlite/reader.rs
- applications/tari_indexer/src/storage_sqlite/store_factory.rs
- crates/wallet/storage_sqlite/src/writer.rs
- applications/tari_indexer/src/storage_sqlite/models/utxo_update.rs
- clients/tari_indexer_client/src/types.rs
- crates/engine/src/runtime/state_store.rs
- crates/wallet/sdk/src/storage.rs
- crates/wallet/sdk_services/src/indexer_jrpc_impl.rs
- crates/engine_types/src/substate_serde.rs
- crates/wallet/sdk/src/network.rs
🧰 Additional context used
🧬 Code graph analysis (9)
crates/engine/tests/templates/stealth/src/lib.rs (3)
bindings/src/types/NonFungibleAddress.ts (1)
NonFungibleAddress(7-7)crates/template_lib/src/resource/manager.rs (2)
freeze_utxos(915-917)unfreeze_utxos(920-922)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)
crates/template_lib/src/args/types.rs (1)
bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)
crates/engine/src/runtime/working_state.rs (1)
bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)
crates/engine_types/src/utxo.rs (3)
bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)crates/engine/tests/templates/freeze/src/lib.rs (1)
unfreeze(42-44)
crates/engine/tests/stealth.rs (2)
crates/engine_types/src/crypto/helpers.rs (1)
get_commitment_factory(55-57)crates/engine_types/src/utxo.rs (2)
new(27-32)output(34-36)
crates/engine/src/runtime/impl.rs (2)
crates/template_lib/src/models/utxo.rs (2)
resource_address(31-33)id(35-37)crates/engine_types/src/utxo.rs (1)
new(27-32)
crates/template_lib/src/resource/manager.rs (5)
bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)bindings/src/types/VaultId.ts (1)
VaultId(6-6)bindings/src/types/VaultFreezeFlags.ts (1)
VaultFreezeFlags(3-3)crates/engine/tests/templates/stealth/src/lib.rs (2)
freeze_utxos(74-76)unfreeze_utxos(78-80)crates/engine/tests/templates/freeze/src/lib.rs (1)
freeze(38-40)
crates/template_lib/src/models/utxo.rs (5)
crates/engine_types/src/substate.rs (18)
fmt(466-478)new(69-74)new(846-852)from_str(488-533)from(302-304)from(308-310)from(314-316)from(320-322)from(326-328)from(332-334)from(338-340)from(344-346)from(350-352)from(784-786)from(790-792)from(796-798)from(802-804)from(808-810)bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)bindings/src/types/UtxoAddressContents.ts (1)
UtxoAddressContents(8-8)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)crates/engine_types/src/substate_serde.rs (9)
serde(176-176)serde(180-180)serde(184-184)serde(188-188)serde(192-192)serde(196-196)serde(200-200)serde(204-204)serialize(27-70)
crates/engine_types/src/indexed_value.rs (3)
bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)bindings/src/types/UtxoAddressContents.ts (1)
UtxoAddressContents(8-8)bindings/src/types/SubstateId.ts (1)
SubstateId(6-6)
🔇 Additional comments (16)
crates/template_lib/src/models/utxo.rs (1)
1-171: LGTM! Well-structured UTXO types with solid parsing and serialization support.The implementation of
UtxoAddress,UtxoId, andUtxoAddressContentsis clean and follows established patterns. Good use of BorTag for type safety, comprehensive trait implementations (FromStr, Display, serialization), and proper test coverage.crates/engine_types/src/indexed_value.rs (2)
18-19: LGTM! Consistent addition of UTXO imports.The imports for
UtxoAddressandUtxoAddressContentsare properly added to support the new UTXO indexing functionality, maintaining consistency with the type relocation totari_template_lib::models.
211-260: LGTM! Clean parameter rename fromaddresstoid.The rename from
addresstoidis semantically clearer and aligns with the broader substate identification pattern. All match arms correctly use the renamed parameter.crates/engine/tests/templates/stealth/src/lib.rs (2)
21-24: LGTM! Proper initialization with transaction signer for freeze authorization.Good implementation using the transaction signer's public key as a NonFungibleAddress to gate the freeze functionality. This ensures only the transaction initiator can control UTXO freezing.
74-80: LGTM! Clean API delegation for freeze/unfreeze operations.The new public methods properly delegate to the resource manager, maintaining clean separation of concerns.
crates/engine/tests/stealth.rs (2)
8-18: LGTM! Necessary imports for commitment-based UTXO operations.Added imports support the new freeze functionality by enabling commitment generation for UTXO identification.
462-533: Comprehensive test coverage for UTXO freeze/unfreeze flow.The test thoroughly validates the freeze lifecycle: freeze UTXOs → verify spend rejection → unfreeze → verify successful spend. Good coverage of the critical path.
crates/template_lib/src/args/types.rs (3)
241-243: LGTM! Clean addition of stealth UTXO freeze variant.The new
SetStealthUtxosFreezevariant properly extends the ResourceAction enum to support UTXO-specific freezing operations.
719-723: LGTM! Well-structured freeze argument type.The
SetFreezeStealthUtxosArgstruct is appropriately designed with a list of UTXO IDs and a boolean freeze flag, following established patterns for freeze operations.
238-243: Add serde alias for backward compatibility.The rename from
SetFreezetoSetVaultFreezecould break deserialization of existing payloads. Consider adding a serde alias.Apply this diff to maintain backward compatibility:
- /// Sets the freeze flags on a vault of a resource. - SetVaultFreeze, + /// Sets the freeze flags on a vault of a resource. + #[serde(alias = "SetFreeze")] + SetVaultFreeze,crates/engine/src/runtime/working_state.rs (3)
56-57: Import path shift looks good.Using UtxoAddress from template_lib::models aligns with the PR’s relocation. No issues.
303-306: LGTM: corrected lock_id and id reporting for NF paths.Both get_non_fungible and get_non_fungible_mut now report the actual lock_id and clone the address. Good diagnostics.
Also applies to: 315-318
1458-1461: LGTM: clarified ownership warning text.The log message grammar fix improves clarity without behavior change.
crates/engine/src/runtime/impl.rs (2)
97-98: LGTM: API surface updates wired correctly.
- Imported SetFreezeStealthUtxosArg and UtxoAddress from the new locations.
- Matches the template_lib manager changes.
Also applies to: 122-123
1224-1285: Add resource auth-hook invocation to SetStealthUtxosFreeze (parity with other resource mutations).All mutating resource actions (Mint/Recall/UpdateRules/SetVaultFreeze/Withdraw/Deposit, etc.) invoke the resource’s auth hook. UTXO freeze currently skips it, weakening policy enforcement.
Apply:
- ResourceAction::SetStealthUtxosFreeze => { - let resource_address = - resource_ref - .as_resource_address() - .ok_or_else(|| RuntimeError::InvalidArgument { - argument: "resource_ref", - reason: "FreezeStealthUtxo resource action requires a resource address".to_string(), - })?; - let arg: SetFreezeStealthUtxosArg = args.assert_one_arg()?; - - self.tracker.write_with(|state_mut| { - let resource_lock = state_mut.read_lock_substate(&SubstateId::Resource(resource_address))?; - - let resource = state_mut.get_resource(&resource_lock)?; - - if !resource.resource_type().is_stealth() { - return Err(RuntimeError::InvalidArgument { - argument: "resource_ref", - reason: "FreezeStealthUtxo can only be called on stealth resources".to_string(), - }); - } - - if arg.utxos.is_empty() { - return Err(RuntimeError::InvalidArgument { - argument: "SetFreezeStealthUtxosArg", - reason: "Utxos list cannot be empty".to_string(), - }); - } - - state_mut.authorization().check_resource_access_rules( - ResourceAuthAction::Freeze, - resource.as_ownership(), - resource.access_rules(), - )?; - - for utxo in arg.utxos { - let id = SubstateId::Utxo(UtxoAddress::new(resource_address, utxo)); - let locked = state_mut.write_lock_substate(&id)?; - - let utxo = state_mut - .get_locked_substate_mut(&locked)? - .as_utxo_mut() - .ok_or_else(|| RuntimeError::LockSubstateMismatch { - lock_id: locked.lock_id(), - expected_type: "Utxo", - id, - })?; - - // Freeze is idempotent. - if arg.freeze { - utxo.freeze(); - } else { - utxo.unfreeze(); - } - state_mut.unlock_substate(locked)?; - } - - state_mut.unlock_substate(resource_lock)?; - - Ok(InvokeResult::unit()) - }) - }, + ResourceAction::SetStealthUtxosFreeze => { + let resource_address = resource_ref.as_resource_address().ok_or_else(|| RuntimeError::InvalidArgument { + argument: "resource_ref", + reason: "FreezeStealthUtxo resource action requires a resource address".to_string(), + })?; + let arg: SetFreezeStealthUtxosArg = args.assert_one_arg()?; + let utxos = arg.utxos.clone(); + let freeze = arg.freeze; + + // Stage 1: validate, authorize, fetch hook + caller, then unlock + let (maybe_auth_hook, auth_caller) = self.tracker.write_with(|state_mut| { + let resource_lock = state_mut.read_lock_substate(&SubstateId::Resource(resource_address))?; + let resource = state_mut.get_resource(&resource_lock)?; + if !resource.resource_type().is_stealth() { + return Err(RuntimeError::InvalidArgument { + argument: "resource_ref", + reason: "FreezeStealthUtxo can only be called on stealth resources".to_string(), + }); + } + if utxos.is_empty() { + return Err(RuntimeError::InvalidArgument { + argument: "SetFreezeStealthUtxosArg", + reason: "Utxos list cannot be empty".to_string(), + }); + } + state_mut.authorization().check_resource_access_rules( + ResourceAuthAction::Freeze, + resource.as_ownership(), + resource.access_rules(), + )?; + let auth_hook = resource.auth_hook().cloned(); + let auth_caller = state_mut.get_auth_caller()?; + state_mut.unlock_substate(resource_lock)?; + Ok::<_, RuntimeError>((auth_hook, auth_caller)) + })?; + + if let Some(auth_hook) = maybe_auth_hook { + self.invoke_resource_access_hook(auth_hook, auth_caller, ResourceAuthAction::Freeze)?; + } + + // Stage 2: perform freeze/unfreeze + self.tracker.write_with(|state_mut| { + for utxo_id in utxos { + let id = SubstateId::Utxo(UtxoAddress::new(resource_address, utxo_id)); + let locked = state_mut.write_lock_substate(&id)?; + let utxo = state_mut + .get_locked_substate_mut(&locked)? + .as_utxo_mut() + .ok_or_else(|| RuntimeError::LockSubstateMismatch { + lock_id: locked.lock_id(), + expected_type: "Utxo", + id, + })?; + if freeze { utxo.freeze(); } else { utxo.unfreeze(); } + state_mut.unlock_substate(locked)?; + } + Ok(InvokeResult::unit()) + }) + },crates/template_lib/src/resource/manager.rs (1)
914-932: LGTM: programmatic stealth UTXO freeze/unfreeze client API.Thin wrapper over SetStealthUtxosFreeze with clear semantics.
aa8d3d1 to
5bc5aae
Compare
* development: feat(engine): programmatic UTXO freeze api (tari-project#1578) chore: bump bindings version to 1.16.0 (tari-project#1579)
2 participants
Description
feat(engine): programmatic UTXO freeze api
Motivation and Context
Previously, there was no way to freeze UTXOs even though it is supported at the substate level.
How Has This Been Tested?
New unit tests
Breaking Changes
Summary by CodeRabbit
New Features
Breaking Changes
Improvements
Refactor