fix(engine)!: allow stealth revealed recall, impl burn utxo - #1584
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughReworks validator genesis bootstrap, adds stealth UTXO burn with optional value proofs, renames/elides ElGamal byte type, adds zero-nonce guards and value-proof validation, extends recall/freeze/burn APIs and templates to support stealth resources, updates TS bindings and tests, and adds test tooling for UTXO/value-proof handling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant TL as TemplateLib::ResourceManager
participant E as Engine Runtime
participant SS as State Store
participant V as Crypto/Validators
rect rgba(220,240,220,0.18)
note right of C: Client requests UTXO burn
C->>TL: burn_utxo(utxo_id, value_proof?)
TL->>E: Invoke(ResourceAction::StealthUtxoBurn{utxo_id, value_proof})
end
E->>SS: Read resource + UTXO substate
E->>E: Verify resource type == Stealth
alt proof provided AND total-supply tracking enabled
E->>V: validate_value_proof(commitment, optional_elgamal, proof)
V-->>E: Ok(value) or Err(details)
alt Ok(value)
E->>SS: Burn UTXO
E->>SS: Decrease resource.total_supply by value
else Err
E-->>TL: Err(UtxoBurnFailed{...})
end
else no proof or tracking disabled
E->>SS: Burn UTXO (no supply change)
end
E-->>TL: Ok(())
TL-->>C: Result(())
note left of E: Zero-nonce checks guard proofs/signatures early
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/engine_types/src/resource_container.rs (1)
609-625: Bug: lock_all() for Stealth returns Fungible containerReturning a Fungible container for Stealth breaks unlock() due to resource type mismatch and is inconsistent with lock_by_amount().
Apply this diff:
Self::Stealth { revealed_amount, locked_amount, .. } => { if revealed_amount.is_zero() { return Err(ResourceError::InsufficientBalance { details: "lock_all: resource container contained no funds".to_string(), }); } // Sets to zero and returns the amount let newly_locked_amount = mem::take(revealed_amount); *locked_amount += newly_locked_amount; - Ok(Self::fungible(resource_address, newly_locked_amount)) + Ok(Self::stealth(resource_address, newly_locked_amount)) },crates/template_lib/src/resource/manager.rs (1)
257-280: Fix doc example for mint_stealth (uses wrong variable)Example uses
statementbut the function takesamount: Amount.Apply:
-/// ```rust,ignore -/// let bucket = resource_manager.mint_stealth(statement); -/// ``` +/// ```rust,ignore +/// let bucket = resource_manager.mint_stealth(Amount(123)); +/// ```crates/engine/tests/templates/recall/src/lib.rs (1)
77-93: Authorization/recall mismatch — runtime authorizes by the ResourceRef but recalls from the vault without verifying they matchThe runtime checks access rules using the ResourceRef passed by ResourceManager (crates/engine/src/runtime/impl.rs — ResourceAction::Recall) but then performs the recall from the provided VaultId (crates/engine/src/runtime/working_state.rs::recall_resource_from_vault) without ensuring the ResourceRef equals the vault’s resource address. Result: calls like ResourceManager::get(self.fungible.resource_address()).recall_all(vault_id) (crates/engine/tests/templates/recall/src/lib.rs:77-93) will be authorized against the fungible resource but will actually recall whatever resource the target vault holds — a cross-resource authorization/operation mismatch.
Actionable fixes:
- Short-term (test/library): call the ResourceManager for the actual resource backing the vault (derive/use the vault’s ResourceAddress) instead of always using self.fungible.resource_address().
- Engine-level (recommended): enforce resource consistency in the recall path — verify the ResourceRef’s address matches the vault’s resource_address (or require a vault-scoped recall API) before performing the recall. Relevant spots: crates/engine/src/runtime/impl.rs (ResourceAction::Recall), crates/engine/src/runtime/working_state.rs::recall_resource_from_vault, and crates/template_lib/src/resource/manager.rs::recall_internal.
🧹 Nitpick comments (20)
crates/common_types/src/engine_signature.rs (1)
62-64: Guard should check nonce identity on the group element, not raw bytes.
is_zero()on the byte type typically means "all-zero bytes", which may not reliably detect the identity element. Move this check to after signature conversion and validate on the group element to avoid false negatives.Apply this diff to remove the byte-level check:
- if sig.public_nonce().is_zero() { - return false; - }Then add the identity check immediately after converting the signature (below the existing conversion at Line 66):
// After `let Ok(sig) = RistrettoSchnorr::convert_from_byte_type(sig) else { return false; };` if sig.get_public_nonce().is_zero() { return false; }If you prefer to retain an early check, ensure it actually decodes the nonce bytes and checks identity on the decoded point:
let nonce_bytes = sig.public_nonce(); let Ok(nonce) = nonce_bytes.try_from_byte_type() else { return false; }; if nonce.is_zero() { return false; }Please confirm whether
RistrettoPublicKeyBytes::is_zero()in this codebase denotes "identity element" or "all-zero bytes". If it’s the latter, the current guard is insufficient.applications/tari_validator_node/src/bootstrap.rs (1)
221-221: Genesis creation call is correct; consider backward‑compatible bootstrappingCalling
create_genesis_state(tx, config.network, consensus_constants.num_preshards)during startup is correct. However, nodes with pre‑existing state will skip this becausehas_bootstrappedreturns true, potentially missing new genesis items (e.g.,STEALTH_TARI_RESOURCE_ADDRESS). The PR notes a breaking change (delete data dir), but a lightweight in‑place migration would improve UX.As a follow‑up, consider extending
create_genesis_stateto opportunistically create any missing substates (e.g., check forSTEALTH_TARI_RESOURCE_ADDRESS) even if the public identity exists, or introduce a simple migration step at startup that adds missing genesis substates.applications/tari_validator_node/src/genesis_state.rs (4)
101-105: Disabling XTR total‑supply tracking: document and future‑proofComment explains the rationale well. Given this is a protocol‑level behavior, consider adding a short note about how downstream tools should infer supply (sum burns minus fee exhaust) to avoid divergence across implementations, and consider a unit/integration test asserting supply tracking is disabled for XTR.
Would you like a small test added that asserts
Resource::track_total_supply(or equivalent flag) is false forSTEALTH_TARI_RESOURCE_ADDRESSon fresh genesis?
57-69: Make genesis idempotent for partial pre‑genesis states (optional migration)Today, if
PUBLIC_IDENTITY_RESOURCE_ADDRESSexists, the function returns early, which is fine for fresh networks but blocks adding newly introduced genesis substates (e.g., XTR resource, faucets) to older dev nodes without wiping. To ease upgrades, make this function tolerant to partial state.Here’s a minimal pattern to create missing items without changing external behavior:
// Pseudocode to illustrate; can be implemented near the early-return let mut created_any = false; if !SubstateRecord::exists(tx, VersionedSubstateId::new(PUBLIC_IDENTITY_RESOURCE_ADDRESS, 0).as_versioned_ref())? { create_public_identity(...)?; created_any = true; } if !SubstateRecord::exists(tx, VersionedSubstateId::new(STEALTH_TARI_RESOURCE_ADDRESS, 0).as_versioned_ref())? { create_xtr_resource_and_faucets(...)?; created_any = true; } if !created_any { return Ok(()); }This preserves idempotency and avoids forcing data‑dir deletion on dev/local environments.
126-151: Testnet faucet initial balance: extremely large valueSeeding the faucet vault with
u64::MAXrevealed amount is fine for testnets, but consider capping to a smaller constant to reduce risk if this code is ever repurposed, and to minimize potential overflows in ancillary tooling that might cast amounts.- revealed_amount: u64::MAX.into(), + revealed_amount: (u64::MAX / 1024).into(), // or a documented, explicit large constantIf you keep
u64::MAX, add a short comment warning clients/tools to avoid lossy casts.
141-151: Faucet vault and access rules bypass at genesis (acknowledged)Directly writing the faucet vault and resource at genesis bypasses runtime access rules, which is expected. Consider a brief comment to make this explicit for future readers to avoid confusion when reading the deny‑all rules above.
crates/template_test_tooling/src/support/stealth.rs (1)
221-242: Consider adding documentation for the new public functions.Both public functions lack documentation comments explaining their purpose, parameters, return values, and when to use each variant. Since these are public APIs in a testing tooling crate, good documentation would help users understand when to choose between commitment-based and ElGamal-encrypted proofs.
Add comprehensive documentation:
+/// Generates a commitment-based value proof with mask knowledge. +/// +/// This creates a Schnorr signature proving knowledge of the commitment opening +/// (mask) and that the commitment equals `mask * G + value * H`. +/// +/// # Parameters +/// - `value`: The positive amount to prove knowledge of +/// - `mask`: The secret key used as the commitment mask +/// +/// # Returns +/// A `StealthValueProof` with a `Commitment` knowledge proof variant +/// +/// # Panics +/// Panics if `value` is not positive pub fn generate_value_proof_mask_knowledge(value: Amount, mask: &RistrettoSecretKey) -> StealthValueProof { +/// Generates an ElGamal-encrypted value proof using a reveal key. +/// +/// This creates a knowledge proof that can be verified using the viewable +/// balance via the provided reveal key, assuming the original proof was +/// validated correctly. +/// +/// # Parameters +/// - `value`: The positive amount to prove knowledge of +/// - `reveal_key`: The public key used for ElGamal encryption verification +/// +/// # Returns +/// A `StealthValueProof` with an `ElgamalEncrypted` knowledge proof variant +/// +/// # Panics +/// Panics if `value` is not positive pub fn generate_value_proof_elgamal(value: Amount, reveal_key: RistrettoPublicKeyBytes) -> StealthValueProof {crates/template_lib/src/resource/builder/stealth.rs (3)
15-16: Fix doc: “Confidential” → “Stealth”.The builder is for stealth resources; the doc header is misleading.
-/// Implements the builder pattern for Confidential resources. +/// Implements the builder pattern for Stealth resources.
186-201: Doc examples reference ResourceBuilder::confidential() but should use ::stealth().Prevents confusion in user guides and IDE hovers.
-/// ResourceBuilder::confidential() +/// ResourceBuilder::stealth() -/// .with_authorization_hook(CallerContext::current_component_address(), "my_hook") +/// .with_authorization_hook(CallerContext::current_component_address(), "my_hook") -/// ResourceBuilder::confidential() +/// ResourceBuilder::stealth() -/// .with_authorization_hook(*alloc.address(), "my_hook") +/// .with_authorization_hook(*alloc.address(), "my_hook")
162-165: API polish: accept Into for with_image_url for consistency.Other string-taking builders accept Into; do the same here.
- pub fn with_image_url(self, url: String) -> Self { + pub fn with_image_url<S: Into<String>>(self, url: S) -> Self { - self.add_metadata(IMAGE_URL, url) + self.add_metadata(IMAGE_URL, url.into()) }bindings/src/types/ViewableBalanceProof.ts (1)
30-32: Doc wording/notation: clarify decryption phrasing and notation
- Suggest replacing “brute forcing” with “bounded search over valid amount domain” to avoid implying full discrete‑log brute force.
- Prefer p·R (or pR) over R.p for consistency with earlier notation (P = p·G).
Note: Update the Rust doc comments that generate this file via ts-rs; do not edit this TS file directly.
crates/template_test_tooling/src/read_only_state_store.rs (1)
59-62: Avoid unwrap to prevent panics in test toolingEven in test tooling, prefer an error over unwrap. Consider mapping the type error.
Apply this diff:
- Ok(substate.into_substate_value().into_utxo().unwrap()) + Ok(substate + .into_substate_value() + .into_utxo() + .ok_or_else(|| StateStoreError::CustomStr("Expected UTXO substate".to_string()))?)crates/engine/tests/stealth.rs (1)
543-601: Burn-then-spend negative test is comprehensive; fix minor comment.Test correctly proves value knowledge per UTXO, burns, then rejects spend and verifies burnt flags. Update the stale inline comment.
Apply:
- .take(2) // Freeze the first two outputs + .take(2) // Burn the first two outputscrates/template_lib/src/args/types.rs (1)
321-331: Documentation improvements needed for resource discriminator variants.While the code is functionally correct, the inline comments could be more detailed to explain the purpose of each variant:
- The
Everythingvariant lacks documentation about when/how it should be used- The comments for other variants are minimal
Consider adding more comprehensive documentation:
pub enum ResourceDiscriminator { - /// Select all tokens + /// Select all tokens regardless of type. This is typically used for operations that need to + /// operate on all resources in a vault or bucket without discrimination. Everything, - /// Select a specific amount of fungible (public or stealth) tokens + /// Select a specific amount of fungible tokens (applies to both public and stealth tokens). + /// The amount must not exceed the available balance. Fungible { amount: Amount }, - /// Select specific non-fungible tokens + /// Select specific non-fungible tokens by their IDs. All specified tokens must exist + /// and be available in the source container. NonFungible { tokens: BTreeSet<NonFungibleId> }, - /// Select specific confidential commitments and a revealed amount + /// Select specific confidential commitments by their commitment bytes, along with a + /// revealed amount that will be made public. Used for partial reveals of confidential funds. Confidential {crates/engine/tests/templates/burn/src/lib.rs (2)
19-44: Consider adding error handling for resource creation.While the constructor is functional, it would benefit from explicit error handling or validation to ensure all resources are created successfully. Currently, if any resource creation fails, it will panic.
Consider wrapping the resource creation in proper error handling:
pub fn new(confidential_supply: ConfidentialOutputStatement) -> Component<Self> { - let fungible = ResourceBuilder::fungible() - .burnable(rule!(allow_all)) - .initial_supply(1_000_000); + let fungible = ResourceBuilder::fungible() + .burnable(rule!(allow_all)) + .initial_supply(1_000_000) + // Consider adding validation or error handling here
46-55: Consider batching withdrawals before burning.The current implementation burns each resource type sequentially. For better efficiency and atomicity, consider collecting all buckets first, then burning them:
pub fn burn_all(&mut self) { - let bucket = self.fungible.withdraw_all(); - bucket.burn(); - let bucket = self.stealth.withdraw_all(); - bucket.burn(); - let bucket = self.non_fungible.withdraw_all(); - bucket.burn(); - let bucket = self.confidential.withdraw_all(); - bucket.burn(); + // Collect all buckets first + let buckets = vec![ + self.fungible.withdraw_all(), + self.stealth.withdraw_all(), + self.non_fungible.withdraw_all(), + self.confidential.withdraw_all(), + ]; + + // Then burn them all + for bucket in buckets { + bucket.burn(); + } }This approach ensures all withdrawals succeed before any burns occur, providing better transactional semantics.
crates/engine/src/runtime/impl.rs (1)
1339-1339: Add explicit zero-nonce guard in validate_value_proof (ElGamal branch).
validate_elgamal_verifiable_balance_proof already rejects identity R (crates/engine_types/src/crypto/elgamal.rs), but crates/engine_types/src/stealth/value_proof.rs does not check the provided ElgamalVerifiableBalanceBytes.public_nonce — add an elgamal.public_nonce.is_zero() check and return UtxoBurnFailed on zero before using encrypted - reveal_key.crates/template_lib/src/resource/manager.rs (2)
508-517: Helper is good; align error context (optional)Consider a more specific expect message to aid debugging (matches existing style).
Apply:
- let bucket_id: BucketId = resp.decode().expect("Failed to decode Bucket"); + let bucket_id: BucketId = resp.decode().expect("[mint_internal] Failed to decode BucketId");
958-971: Polish burn_utxo error message; confirm return typeMinor nit: message says “BurnStealthUtxos” (plural) but the action/arg is singular. Also, if the engine returns unit, the decode will infer
(). If it returns something else, bind it to force the type.Apply:
- resp.decode().expect("BurnStealthUtxos failed") + resp.decode().expect("[burn_utxo] BurnStealthUtxo failed")If the engine returns a non-unit, please bind it explicitly:
let _: () = resp.decode().expect("[burn_utxo] BurnStealthUtxo failed");crates/engine_types/src/crypto/elgamal.rs (1)
235-246: Deduplicate conversions using existing traitReuse ConvertFromByteType to implement TryFrom.
Apply:
-impl TryFrom<&ElgamalVerifiableBalanceBytes> for ElgamalVerifiableBalance { - type Error = tari_utilities::ByteArrayError; - - fn try_from(value: &ElgamalVerifiableBalanceBytes) -> Result<Self, Self::Error> { - let encrypted = RistrettoPublicKey::convert_from_byte_type(&value.encrypted)?; - let public_nonce = RistrettoPublicKey::convert_from_byte_type(&value.public_nonce)?; - Ok(ElgamalVerifiableBalance { - encrypted, - public_nonce, - }) - } -} +impl TryFrom<&ElgamalVerifiableBalanceBytes> for ElgamalVerifiableBalance { + type Error = tari_utilities::ByteArrayError; + fn try_from(value: &ElgamalVerifiableBalanceBytes) -> Result<Self, Self::Error> { + ElgamalVerifiableBalance::convert_from_byte_type(value) + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (41)
applications/tari_validator_node/src/bootstrap.rs(2 hunks)applications/tari_validator_node/src/genesis_state.rs(2 hunks)applications/tari_validator_node/src/lib.rs(1 hunks)bindings/src/index.ts(3 hunks)bindings/src/types/ElgamalVerifiableBalanceBytes.ts(1 hunks)bindings/src/types/PrivateOutput.ts(1 hunks)bindings/src/types/StealthValueProof.ts(1 hunks)bindings/src/types/ValueKnowledgeProof.ts(1 hunks)bindings/src/types/ViewableBalanceProof.ts(1 hunks)crates/common_types/src/engine_signature.rs(1 hunks)crates/engine/src/runtime/impl.rs(5 hunks)crates/engine/src/runtime/working_state.rs(1 hunks)crates/engine/tests/burn.rs(1 hunks)crates/engine/tests/confidential.rs(1 hunks)crates/engine/tests/recall.rs(4 hunks)crates/engine/tests/stealth.rs(4 hunks)crates/engine/tests/templates/burn/Cargo.toml(1 hunks)crates/engine/tests/templates/burn/src/lib.rs(1 hunks)crates/engine/tests/templates/nft/basic_nft/src/lib.rs(1 hunks)crates/engine/tests/templates/recall/src/lib.rs(5 hunks)crates/engine/tests/templates/stealth/src/lib.rs(2 hunks)crates/engine_types/src/crypto/elgamal.rs(5 hunks)crates/engine_types/src/crypto/helpers.rs(4 hunks)crates/engine_types/src/crypto/messages.rs(1 hunks)crates/engine_types/src/crypto/output.rs(2 hunks)crates/engine_types/src/crypto/utxo_spend.rs(1 hunks)crates/engine_types/src/hashing.rs(2 hunks)crates/engine_types/src/resource_container.rs(2 hunks)crates/engine_types/src/stealth/mod.rs(1 hunks)crates/engine_types/src/stealth/transfer.rs(1 hunks)crates/engine_types/src/stealth/value_proof.rs(1 hunks)crates/template_lib/src/args/types.rs(4 hunks)crates/template_lib/src/models/viewable_balance.rs(1 hunks)crates/template_lib/src/resource/builder/confidential.rs(4 hunks)crates/template_lib/src/resource/builder/stealth.rs(1 hunks)crates/template_lib/src/resource/manager.rs(6 hunks)crates/template_lib_types/src/crypto/mod.rs(2 hunks)crates/template_lib_types/src/crypto/ristretto.rs(1 hunks)crates/template_lib_types/src/crypto/value_proof.rs(1 hunks)crates/template_test_tooling/src/read_only_state_store.rs(2 hunks)crates/template_test_tooling/src/support/stealth.rs(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (27)
bindings/src/types/ValueKnowledgeProof.ts (2)
bindings/src/types/SchnorrSignatureBytes.ts (1)
SchnorrSignatureBytes(5-5)bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)
crates/engine/tests/templates/burn/src/lib.rs (3)
crates/template_lib/src/resource/builder/confidential.rs (1)
new(29-41)crates/template_lib/src/resource/builder/stealth.rs (1)
new(30-42)crates/template_lib/src/models/vault.rs (1)
from_bucket(199-204)
crates/engine_types/src/stealth/value_proof.rs (2)
crates/engine_types/src/crypto/helpers.rs (2)
commit_amount(59-61)convert_amount_to_secret(78-89)crates/engine_types/src/crypto/messages.rs (1)
value_proof_message(67-72)
crates/template_lib_types/src/crypto/value_proof.rs (5)
bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/SchnorrSignatureBytes.ts (1)
SchnorrSignatureBytes(5-5)bindings/src/types/Amount.ts (1)
Amount(12-12)bindings/src/types/StealthValueProof.ts (1)
StealthValueProof(9-15)bindings/src/types/ValueKnowledgeProof.ts (1)
ValueKnowledgeProof(5-23)
crates/engine_types/src/resource_container.rs (1)
bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)
crates/template_test_tooling/src/support/stealth.rs (6)
crates/engine_types/src/crypto/helpers.rs (1)
commit_amount(59-61)bindings/src/types/ValueKnowledgeProof.ts (1)
ValueKnowledgeProof(5-23)bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/StealthValueProof.ts (1)
StealthValueProof(9-15)crates/template_lib_types/src/amount/amount.rs (1)
is_positive(66-68)crates/engine_types/src/crypto/messages.rs (1)
value_proof_message(67-72)
crates/engine/tests/recall.rs (3)
bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)crates/engine_types/src/resource_container.rs (1)
withdraw(289-355)crates/engine_types/src/substate.rs (1)
vault(618-623)
bindings/src/types/StealthValueProof.ts (2)
bindings/src/types/Amount.ts (1)
Amount(12-12)bindings/src/types/ValueKnowledgeProof.ts (1)
ValueKnowledgeProof(5-23)
crates/template_lib/src/resource/builder/confidential.rs (3)
crates/template_lib/src/resource/builder/stealth.rs (1)
disable_total_supply_tracking(211-214)crates/template_lib/src/resource/builder/fungible.rs (1)
disable_total_supply_tracking(390-393)crates/template_lib/src/resource/builder/non_fungible.rs (1)
disable_total_supply_tracking(187-190)
crates/engine_types/src/crypto/messages.rs (3)
bindings/src/types/PedersenCommitmentBytes.ts (1)
PedersenCommitmentBytes(6-6)bindings/src/types/Amount.ts (1)
Amount(12-12)crates/engine_types/src/hashing.rs (1)
engine_hasher64(35-37)
crates/template_lib/src/resource/builder/stealth.rs (2)
crates/template_lib/src/resource/builder/fungible.rs (1)
initial_supply(414-421)bindings/src/types/Amount.ts (1)
Amount(12-12)
crates/engine/tests/confidential.rs (3)
crates/engine/tests/stealth.rs (2)
test(83-83)setup(39-65)crates/engine_types/src/resource.rs (1)
total_supply(199-201)crates/engine/tests/templates/confidential/faucet/src/lib.rs (1)
total_supply(89-91)
crates/engine/tests/stealth.rs (4)
bindings/src/types/SchnorrSignatureBytes.ts (1)
SchnorrSignatureBytes(5-5)crates/engine_types/src/utxo.rs (2)
new(27-32)is_burnt(62-64)crates/template_test_tooling/src/support/stealth.rs (3)
generate_mint_statement(40-62)generate_transfer_data(111-128)generate_value_proof_mask_knowledge(221-234)crates/engine_types/src/crypto/helpers.rs (1)
get_commitment_factory(50-52)
crates/engine/tests/templates/stealth/src/lib.rs (2)
bindings/src/types/StealthValueProof.ts (1)
StealthValueProof(9-15)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)
crates/engine_types/src/crypto/output.rs (1)
bindings/src/types/ElgamalVerifiableBalanceBytes.ts (1)
ElgamalVerifiableBalanceBytes(4-7)
bindings/src/types/PrivateOutput.ts (3)
bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)bindings/src/types/ElgamalVerifiableBalanceBytes.ts (1)
ElgamalVerifiableBalanceBytes(4-7)
crates/engine_types/src/stealth/transfer.rs (1)
crates/wallet/sdk/src/apis/stealth_transfer.rs (1)
transfer(330-625)
applications/tari_validator_node/src/bootstrap.rs (1)
applications/tari_validator_node/src/genesis_state.rs (1)
create_genesis_state(57-118)
crates/engine/tests/burn.rs (6)
crates/engine_types/src/resource_container.rs (1)
confidential(74-90)crates/template_test_tooling/src/support/confidential.rs (1)
generate_confidential_output_statement(17-22)crates/engine/tests/templates/burn/src/lib.rs (1)
new(19-44)crates/template_lib/src/resource/builder/confidential.rs (2)
new(29-41)initial_supply(231-238)crates/template_test_tooling/src/read_only_state_store.rs (1)
new(21-23)crates/template_lib/src/resource/manager.rs (2)
from(975-977)total_supply(760-763)
crates/engine_types/src/crypto/elgamal.rs (3)
crates/engine_types/src/resource_container.rs (1)
proof(419-438)bindings/src/types/ElgamalVerifiableBalanceBytes.ts (1)
ElgamalVerifiableBalanceBytes(4-7)crates/engine_types/src/byte_types.rs (11)
convert_from_byte_type(30-31)convert_from_byte_type(62-64)convert_from_byte_type(79-81)convert_from_byte_type(101-105)convert_from_byte_type(124-129)to_byte_type(24-24)to_byte_type(51-56)to_byte_type(70-73)to_byte_type(87-93)to_byte_type(111-118)to_byte_type(135-137)
crates/template_lib/src/args/types.rs (4)
bindings/src/types/StealthValueProof.ts (1)
StealthValueProof(9-15)bindings/src/types/Amount.ts (1)
Amount(12-12)bindings/src/types/NonFungibleId.ts (1)
NonFungibleId(6-6)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)
crates/template_test_tooling/src/read_only_state_store.rs (2)
bindings/src/types/Utxo.ts (1)
Utxo(4-4)bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)
crates/engine_types/src/crypto/helpers.rs (1)
bindings/src/types/SchnorrSignatureBytes.ts (1)
SchnorrSignatureBytes(5-5)
crates/template_lib/src/resource/manager.rs (4)
bindings/src/types/StealthValueProof.ts (1)
StealthValueProof(9-15)crates/engine/tests/templates/recall/src/lib.rs (1)
recall_all(77-93)bindings/src/types/VaultId.ts (1)
VaultId(6-6)bindings/src/types/UtxoId.ts (1)
UtxoId(3-3)
crates/engine/tests/templates/recall/src/lib.rs (5)
crates/engine/tests/templates/burn/src/lib.rs (1)
new(19-44)crates/template_lib/src/resource/builder/stealth.rs (1)
new(30-42)crates/engine_types/src/resource_container.rs (4)
stealth(92-102)fungible(56-64)non_fungible(66-72)confidential(74-90)crates/template_lib/src/models/vault.rs (1)
from_bucket(199-204)crates/template_lib/src/resource/manager.rs (2)
recall_all(582-587)get(104-106)
crates/engine/src/runtime/impl.rs (3)
crates/engine_types/src/resource_container.rs (2)
stealth(92-102)resource_address(176-183)crates/template_lib/src/args/types.rs (2)
resource_address(449-454)resource_address(645-650)crates/engine_types/src/stealth/value_proof.rs (1)
validate_value_proof(23-103)
applications/tari_validator_node/src/genesis_state.rs (2)
bindings/src/types/Network.ts (1)
Network(6-6)bindings/src/types/NumPreshards.ts (1)
NumPreshards(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). (6)
- GitHub Check: check nightly
- GitHub Check: check stable
- GitHub Check: file licenses
- GitHub Check: test
- GitHub Check: machete
- GitHub Check: clippy
🔇 Additional comments (58)
crates/engine/tests/templates/nft/basic_nft/src/lib.rs (1)
47-49: LGTM: formatting-only changePurely a readability improvement to the builder chain; no behavior change.
crates/engine_types/src/crypto/utxo_spend.rs (1)
15-20: Good hardening — add a negative test for the zero‑nonce ownership proofNo tests reference the "Ownership proof public nonce cannot be zero" guard; it lives at crates/engine_types/src/crypto/utxo_spend.rs (owner_proof.public_nonce().is_zero()) and is exercised via crates/engine/src/runtime/working_state.rs:290 — add a unit/integration test that supplies a zero public nonce and asserts ResourceError::InvalidSpend.
applications/tari_validator_node/src/lib.rs (1)
28-28: Module rename wired correctlyAdding
mod genesis_state;aligns with the new bootstrap flow. No other references to the old module remain in this file.Please confirm there are no lingering
state_bootstrapimports/usages elsewhere in the crate after this rename.applications/tari_validator_node/src/bootstrap.rs (1)
84-85: Import update matches new APISwitch to
genesis_state::create_genesis_stateis correct and consistent with the module rename.applications/tari_validator_node/src/genesis_state.rs (1)
57-61: Renamed entrypoint is clear and keeps the same contract
create_genesis_state<TTx>(...)reads well, retains the original generics/constraints, and is an appropriate public entrypoint for genesis initialization.crates/template_test_tooling/src/support/stealth.rs (3)
1-27: LGTM! Clean import organization and appropriate type selection.The imports are well-organized and all appear to be used appropriately within the module. The use of specific cryptographic types and test tooling types is correct for value proof generation functionality.
236-242: LGTM! Straightforward ElGamal proof generation.The function correctly creates an ElGamal-encrypted knowledge proof variant. The implementation is clean and the assertion for positive value is consistent with the other function.
221-224: Keep assertion in test helper — no change required.This function is in crates/template_test_tooling and is only used from tests (crates/engine/tests/stealth.rs), so panicking on invalid input is acceptable for a test-only helper; production code performs validation via Results (e.g., ResourceContainer::withdraw).
crates/template_lib/src/resource/builder/stealth.rs (1)
226-233: Incorrect — do not change Stealth::initial_supply to return ResourceAddressRuntime returns a Bucket for resource creation with any mint_arg (resource_invoke Create sets output_bucket = Some(Bucket::from_id(...)) — crates/engine/src/runtime/impl.rs) and MintArg::Stealth yields ResourceContainer::stealth (crates/engine/src/runtime/working_state.rs); many templates call ResourceBuilder::stealth().initial_supply(...) and expect a Bucket (e.g. crates/engine/tests/templates/stealth/src/lib.rs, crates/engine/tests/templates/burn/src/lib.rs), so the proposed API change would break callers.
Likely an incorrect or invalid review comment.
crates/template_test_tooling/src/read_only_state_store.rs (1)
59-62: Add: Read-only UTXO lookup — LGTMMethod aligns with existing getters and SubstateId usage.
crates/engine_types/src/stealth/transfer.rs (1)
34-38: Zero-nonce guard — good hardeningEarly rejection of zero public nonce strengthens proof validation and avoids degenerate signatures.
Please confirm this is intended to also apply to the “no inputs and no outputs” edge case. With this guard, a zero nonce will be rejected even there (previous code allowed a bespoke check in that branch).
crates/template_lib/src/resource/builder/confidential.rs (3)
24-24: Builder flag added with sane default — LGTMNew is_total_supply_tracking_enabled field defaults to true. Good default.
Also applies to: 39-39
84-100: API ergonomics: disable_total_supply_tracking() — LGTMClear docs and consistent with other builders (fungible/non‑fungible/stealth).
254-255: Plumbing the flag into ResourceManager::create — LGTMAssuming ResourceManager::create signature matches, this will correctly propagate the builder choice.
Please confirm ResourceManager::create’s parameter order/signature matches this boolean at the end to avoid accidental misplacement.
crates/template_lib_types/src/crypto/mod.rs (1)
17-17: Expose value_proof module — LGTMModule wired and re-exported as expected.
Also applies to: 28-29
crates/engine_types/src/stealth/mod.rs (1)
6-6: Re-export stealth value_proof — LGTMKeeps the stealth namespace coherent.
Also applies to: 10-10
bindings/src/types/ElgamalVerifiableBalanceBytes.ts (1)
4-7: Rename OK — no lingering CompressedElgamalVerifiableBalance referencesSearch output shows only ElgamalVerifiableBalanceBytes usages in engine_types and bindings (e.g., crates/engine_types/src/crypto/elgamal.rs, value_proof.rs, crypto/output.rs and bindings/src/types/ElgamalVerifiableBalanceBytes.ts); no matches for CompressedElgamalVerifiableBalance.
crates/engine_types/src/resource_container.rs (1)
899-901: UtxoBurnFailed added — mark ResourceError #[non_exhaustive] or update downstream
Adding a variant to a public, non-#[non_exhaustive] enum can break exhaustive matches in downstream crates; either add #[non_exhaustive] to ResourceError or coordinate a semver‑major change and update dependents. I scanned this repo and found no local match that enumerates ResourceError exhaustively without a fallback; the only match on ResourceError in tests that inspects the variant is at crates/engine_types/src/stealth/value_proof.rs:159-164 and already has a fallback. Cannot verify external downstream crates — audit dependents.crates/engine/tests/templates/burn/Cargo.toml (1)
1-14: LGTM! Standard test template Cargo.toml setup.The configuration correctly sets up the burn test template crate with required dependencies and library targets. The
cdylibcrate type is appropriate for dynamic library generation and the relative path totari_template_libaligns with workspace conventions.crates/engine_types/src/crypto/output.rs (2)
10-10: Type rename looks correct.The import change from
CompressedElgamalVerifiableBalancetoElgamalVerifiableBalanceBytesaligns with the broader PR refactoring to use byte-based type representations.
21-21: Type field update consistent with rename.The
viewable_balancefield type change fromOption<CompressedElgamalVerifiableBalance>toOption<ElgamalVerifiableBalanceBytes>is consistent with the import change and the broader type renaming across the codebase.crates/template_lib_types/src/crypto/ristretto.rs (1)
53-55: Zero-check helper method is well-implemented.The
is_zeromethod correctly checks if all bytes are zero using the idiomaticalliterator method. This is a useful utility for validating zero nonces in cryptographic contexts, particularly for the stealth value proof functionality being added.crates/template_lib/src/models/viewable_balance.rs (1)
31-32: Documentation improvement adds valuable context.The added documentation properly explains the decryption mechanism ("The value is decrypted by brute forcing E - R.p = v.G"), providing important clarity for developers working with viewable balance proofs. The period addition fixes punctuation consistency.
crates/engine_types/src/crypto/messages.rs (1)
67-72: Value proof message function follows established patterns.The new
value_proof_messagefunction correctly implements the domain-separated hash pattern consistent with other message hashing functions in this file. It properly uses theValueProofdomain label for cryptographic domain separation.crates/engine_types/src/hashing.rs (1)
131-131: New ValueProof domain label properly integrated.The
ValueProofvariant addition toEngineHashDomainLabelenum and correspondingas_label()implementation follows the established pattern. This provides proper domain separation for value proof hashing operations.Also applies to: 158-158
crates/engine/tests/burn.rs (2)
8-86: Comprehensive multi-resource burn test is well-designed.The test thoroughly validates burn functionality across all four resource types (fungible, non-fungible, confidential, stealth) by:
- Creating initial resources with proper supplies
- Verifying initial total_supply values
- Executing burn_all operation
- Confirming all vault balances are zero
- Verifying all total_supply values are zero
The test structure is clear and follows good testing practices with proper assertions.
13-14: Verified — post-generation mutation of output_revealed_amount is acceptable.generate_confidential_output_statement returns a ConfidentialOutputStatement with revealed amounts = 0; tests (e.g. crates/engine/tests/recall.rs and burn.rs) explicitly set output_revealed_amount afterwards to establish the revealed initial supply. Validation (engine_types::confidential::validate_confidential_statement) only requires revealed amounts be non-negative and allows proofs to contain both confidential outputs and revealed amounts, so this mutation aligns with initialization expectations.
crates/engine/tests/templates/stealth/src/lib.rs (2)
8-8: Import addition supports new functionality.The new import of
StealthValueProofenables the batch burn functionality implementation.
84-88: Batch burn method is well-implemented.The
burn_utxosmethod provides a clean API for burning multiple UTXOs with associated proofs. The implementation correctly:
- Accepts a vector of
(UtxoId, StealthValueProof)pairs- Iterates through each pair
- Calls
burn_utxowith the proof for each UTXOThe method signature and implementation align with the broader stealth UTXO burn support being added.
bindings/src/types/StealthValueProof.ts (1)
9-15: LGTM: new StealthValueProof type is clear and consistent.Matches Amount and ValueKnowledgeProof definitions; naming and docs look good.
crates/engine/tests/recall.rs (3)
50-50: Deposit of buckets.3 adds stealth vault coverage.Good addition to bring the stealth resource under account management for subsequent recall.
66-78: Stealth recall path exercised end-to-end.The additional recall_stealth invocation and assertions align with engine changes to accept stealth in fungible recalls.
82-97: Balance checks via read-only store are robust.Switching to direct state reads improves test reliability over parsing execution results.
bindings/src/types/ValueKnowledgeProof.ts (1)
5-23: LGTM: discriminated union mirrors Rust variants.Docs and field names match the underlying cryptographic intent; integrates cleanly with StealthValueProof.
crates/engine/tests/stealth.rs (1)
297-302: Invalid ownership proof construction is solid.Replacing the public nonce while keeping the signature ensures verification fails as intended.
crates/engine/src/runtime/working_state.rs (1)
711-727: Accept stealth in fungible recall — update messages & verify Vault::withdrawChange is correct; apply the message/log tweaks below and verify Vault::withdraw correctly handles stealth (decrements revealed amount, respects freeze/lock flags). I found ResourceType::is_fungible/is_stealth at crates/template_lib/src/resource/mod.rs; a Vault::withdraw implementation was not located by the quick search — please confirm its location and semantics.
- if !vault_mut.resource_type().is_fungible() && !vault_mut.resource_type().is_stealth() { + if !vault_mut.resource_type().is_fungible() && !vault_mut.resource_type().is_stealth() { return Err(RuntimeError::InvalidArgument { argument: "resource", reason: format!( - "Vault {} contains a {} resource but a fungible was requested", + "Vault {} contains a {} resource but a fungible or stealth was requested", vault_id, vault_mut.resource_type() ), }); } @@ - debug!( + debug!( target: LOG_TARGET, - "Recalling {} fungible tokens on resource: {}", amount, resource_address + "Recalling {} tokens on resource: {}", amount, resource_address );bindings/src/types/PrivateOutput.ts (1)
2-2: LGTM — type rename wired through; old type removed.Repository search found no references to CompressedElgamalVerifiableBalance; PrivateOutput imports ElgamalVerifiableBalanceBytes and bindings export the new type.
bindings/src/index.ts (1)
35-35: Exports updated; no stale consumers found. ripgrep shows no occurrences of CompressedElgamalVerifiableBalance in the repo; bindings now export ElgamalVerifiableBalanceBytes, StealthValueProof, and ValueKnowledgeProof.crates/engine/tests/confidential.rs (1)
72-79: Consistent variable binding update.The test correctly reads the resource from the new read-only state store, and the assertion now checks for the expected total supply of zero. This aligns with the broader architectural shift to use a read-only state store for resource queries.
crates/template_lib_types/src/crypto/value_proof.rs (1)
1-35: LGTM! Well-structured value proof types.The new
StealthValueProofandValueKnowledgeProoftypes are well-designed with appropriate documentation. The two variants (CommitmentandElgamalEncrypted) provide flexible options for proving knowledge of UTXOs, aligning with the stealth burn functionality.crates/template_lib/src/args/types.rs (1)
721-725: LGTM! Well-defined burn argument structure.The
BurnStealthUtxoArgstruct is properly designed with an optional value proof, allowing for flexible burning scenarios where the proof is only required when total supply tracking is enabled.crates/engine_types/src/crypto/helpers.rs (1)
91-93: Clean trait-based conversion implementation.The change to use
try_from_byte_type()aligns with the broader refactoring towards trait-based conversions. This approach is cleaner and more consistent with the rest of the codebase.crates/engine_types/src/stealth/value_proof.rs (2)
23-33: Add explicit validation for negative values.Good defensive programming with the negative value check. This prevents potential underflow issues in total supply tracking.
91-91: Use ofexpectis justified here.The
expectcall on line 91 is safe because the negative check on line 28 guarantees thatproof.valueis non-negative. The error message accurately describes this invariant.crates/engine/src/runtime/impl.rs (3)
1219-1225: Good validation for empty UTXO list.The check for an empty UTXO list prevents unnecessary processing and provides a clear error message.
1248-1255: Correct mutable reference handling.The rename from
utxotoutxo_mutproperly reflects that this is a mutable reference, improving code clarity.
1271-1353: Well-structured stealth UTXO burn implementation.The new
StealthUtxoBurnaction is properly implemented with:
- Resource type validation (must be stealth)
- Optional value proof validation when total supply tracking is enabled
- Proper authorization checks
- Correct total supply adjustment
The flow correctly handles the lock/unlock pattern and error cases.
crates/template_lib/src/resource/manager.rs (4)
46-46: Type import looks correctStealthValueProof path aligns with the new bindings/types.
50-66: Args surface extension acknowledgedNew BurnStealthUtxoArg import and related args look consistent with the new action.
732-742: Recall helper LGTMDecoding to BucketId and constructing Bucket is consistent with mint_internal.
558-583: Breaking rename: recall_fungible_all -> recall_all — verifiedNo occurrences of
recall_fungible_allremain;recall_allis defined at crates/template_lib/src/resource/manager.rs and is used in tests/runtime (crates/engine/tests/templates/recall/src/lib.rs, crates/engine/src/runtime/working_state.rs, crates/engine_types/src/vault.rs).crates/engine/tests/templates/recall/src/lib.rs (4)
15-16: Stealth resource integration looks solidAdding a stealth vault, address, and wiring into constructor/returns is consistent.
Also applies to: 44-48, 53-54, 63-64
67-75: withdraw_some: new stealth path LGTMFour-bucket return and stealth withdraw of 10 is OK for tests.
121-125: Confirm recall_fungible_amount works for StealthStealth recall via
recall_fungible_amountassumes the engine treats stealth revealed amounts like fungible recalls. Verify engine support; if not, a dedicated stealth recall discriminator may be needed.Run:
21-27: Constructor signature changed — extra return value (5-tuple); update callers/tests
crates/engine/tests/templates/recall/src/lib.rs now returns (Component, ResourceAddress, ResourceAddress, ResourceAddress, ResourceAddress). Repo search found no in-repo Recall::new callers; verify and update any external templates/tests/consumers to destructure the added value.crates/engine_types/src/crypto/elgamal.rs (3)
51-55: Zero-nonce guard is correctRejecting identity public nonce strengthens proof validation.
248-256: ToByteType impl LGTMUses the new bytes type correctly.
138-169: Approve — ElgamalVerifiableBalanceBytes conversions and TS bindings are consistentRust conversions exist in crates/engine_types/src/crypto/elgamal.rs; TS type is present and exported (bindings/src/types/ElgamalVerifiableBalanceBytes.ts, bindings/src/index.ts); no CompressedElgamalVerifiableBalance references found.
Test Results (CI)441 tests +27 428 ✅ +14 1h 23m 1s ⏱️ + 36m 1s For more details on these failures, see this check. Results for commit 8558141. ± Comparison against base commit 411a5b8. |
2 participants
Description
fix(engine)!: allow stealth revealed recall, impl burn utxo
Motivation and Context
Allow recall of reveald steal funds
Allow burning of UTXOs
How Has This Been Tested?
New unit tests
What process can a PR reviewer use to test or verify this change?
Breaking Changes
Summary by CodeRabbit
New Features
Breaking Changes
Bug Fixes
Documentation
Tests