feat!: implement optional access-rule based UTXO spending - #1653
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis PR introduces a comprehensive refactoring of stealth UTXO handling, authorization, and signing flows. Key changes include replacing ownership proofs with SpendCondition-based access rules, wrapping collection types in Arc and IndexSet, refactoring signer API to use contextual patterns, renaming transaction builder finalization methods, and migrating TypeScript export targets. Changes
Sequence Diagram(s)sequenceDiagram
participant Signer as Signer API
participant Builder as Transaction Builder
participant Crypto as Crypto Layer
Note over Signer,Builder: Old Pattern
Signer->>Builder: sign_with_context(key_id, pubkey, builder)
Builder->>Builder: build()
Signer->>Crypto: sign(message)
Note over Signer,Builder: New Pattern
Signer->>Signer: with_context(pubkey)
Signer->>Builder: sign(key_id, builder)
Builder->>Builder: finish()
Builder->>Crypto: sign(message)
sequenceDiagram
participant Handler as Wallet Handler
participant Output as Output Creation
participant Condition as Spend Condition
Note over Handler,Condition: UTXO Spending Validation
Handler->>Output: resolve stealth outputs
Output->>Condition: validate_spend_condition(output, input)
alt Signed variant
Condition->>Condition: check badge in auth scope
else AccessRule variant
Condition->>Condition: enforce access rule via authorization
end
Condition-->>Output: success or error
Output-->>Handler: validated inputs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (107)
⛔ Files not processed due to max files limit (14)
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 |
f73a1e7 to
956ee01
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
crates/template_builtin/templates/faucet/src/lib.rs (1)
28-56: Update TypeScript bindings to removerequired_signerfield.The Rust implementation correctly removes
required_signerfromStealthInputsStatement, but the TypeScript bindings atbindings/src/types/StealthInputsStatement.ts:21still declare this field. This creates a breaking change for TypeScript clients. The bindings must be updated to remove therequired_signerfield and update the documentation to reflect that signer authorization now occurs at the transaction level.crates/template_lib/src/auth/access_rules.rs (2)
111-120: ClarifyMOfNdocumentation and semanticsThe new
RequireRule::MOfN(u16, Box<[RuleRequirement]>)variant is a useful addition, but the doc string says “Requires N of the specified conditions to be met”, which is ambiguous given the nameMOfNand the singleu16parameter. It would be clearer to phrase it as “Requires M of the provided conditions to be met” and (if applicable) document expectations likeM > 0andM <= requirements.len()and where those are enforced.
325-364: Macro docs should match the actualm_of_nsyntaxThe
rule!macro docs refer ton_ofbut the actual macro and example usem_of_n(...). To avoid user confusion, consider aligning the wording, e.g. “usingany_of,all_ofandm_of_nconstructs”, and updating the comment aboven_of_ruleaccordingly.crates/engine/src/runtime/impl.rs (1)
2491-2501: Respect the caller-provided spend condition when minting claimed UTXOsThe new access-rule plumbing lets wallets choose a
SpendCondition, but we ignore it here and always forceSigned(self.seal_signer_public_key). As a result, any burn claim UTXO is locked to the transaction’s seal signer, so the optional M-of-N flow never kicks in for claimed outputs. Please plumboutput_data.spend_condition(added alongside the other spend-condition changes in this PR) into the mintedUtxoOutput; otherwise multi-sig claim spends remain impossible.
🧹 Nitpick comments (27)
crates/template_lib/src/models/component.rs (1)
75-79: Consider implementingTryFrom<&[u8]>trait for consistency.While
from_bytesis functional, implementing theTryFrom<&[u8]>trait would be more idiomatic and consistent with similar types in the codebase. TheVaultIdtype invault.rs(lines 120-123) follows this pattern for identical functionality.Replace the custom method with a trait implementation:
- pub fn from_bytes(bytes: &[u8]) -> Result<Self, KeyParseError> { - let key = ObjectKey::try_from(bytes)?; - Ok(Self::new(key)) - } +} + +impl TryFrom<&[u8]> for ComponentAddress { + type Error = KeyParseError; + + fn try_from(value: &[u8]) -> Result<Self, Self::Error> { + let key = ObjectKey::try_from(value)?; + Ok(Self::new(key)) + } }This allows users to use the standard
ComponentAddress::try_from(bytes)orbytes.try_into()patterns.applications/tari_validator_node_cli/src/command/transaction.rs (1)
182-218: Typedfunction_name/method_nameconversion viatry_into()looks correct; consider adding clearer CLI error contextThe switch to
function: function_name.try_into()?andmethod: method_name.try_into()?correctly enforces the new typed function/method identifiers and will fail fast on invalid CLI input, which aligns with the broader refactor described in the PR. Error propagation via?intoanyhow::Erroris also consistent with the rest of this module.Two minor, optional points to consider:
- If the
TryFromerror type is fairly low-level, you may want to wrap it with additional context so CLI users see which name was invalid (e.g., “invalid function name 'foo'”) rather than a bare parse error.- It’s worth running a quick manual check with an intentionally invalid function/method name to ensure the surfaced error message is understandable from a UX perspective.
crates/p2p/src/conversions/transaction.rs (1)
56-72: Avoid panicking inFrom<&Transaction>; prefer a fallible conversion
From<&Transaction> for proto::transaction::Transactioncurrently does:bor_encoded: encode_to_vec(transaction).expect("Failed to encode transaction"),This will panic if encoding ever fails, which can take the node down in the middle of normal network operation. Since encoding is fallible, it would be cleaner to:
- Expose
TryFrom<&Transaction>(or a dedicatedto_proto(&Transaction) -> anyhow::Result<_>) and plumb the error up, or- At least wrap the panic in a more contextual error path where it’s caught/logged.
Given the existing
// TODO: no paniccomment, consider addressing this as part of this refactor so serialization errors can be surfaced instead of aborting the process.crates/p2p/proto/common.proto (1)
42-50: NewSubstateRequirement/OptionalVersiondefinitions line up with existing version typesThe new
SubstateRequirement(substate_id +OptionalVersion) andOptionalVersionmessages look appropriate and match the rest of the proto surface (e.g., otherversionfields are alsouint32). Using a nested message forOptionalVersionis a good fit for representingOption<u32>via presence/absence in generated code.It may be worth adding a brief comment in this file (or higher-level docs) clarifying that
versionis optional and that absence, not0, represents “any version”, to avoid ambiguity for non-Rust client implementers.crates/p2p/src/conversions/common.rs (2)
211-220:TryFrom<proto::common::SubstateRequirement>correctly reconstructs domain type; consider richer error contextThe conversion from
proto::common::SubstateRequirementto the domainSubstateRequirementis straightforward and looks correct:
SubstateId::from_bytes(&val.substate_id)?decodes the ID from the rawbytesfield.val.version.map(|v| v.version)correctly maps presence ofOptionalVersion→Option<u32>.SubstateRequirement::new(substate_id, version)cleanly encapsulates construction.If you expect malformed
substate_idbytes from the network, you might consider wrapping thefrom_bytesfailure with additional context (similar to thecontext("...")calls used earlier in this file) to make debugging easier, but the current behavior is functionally sound.
222-243: From‑impls cover owned and borrowed SubstateRequirement variants and preserve round‑trip semanticsThe trio of
Fromimplementations:
From<SubstateRequirement>delegating toFrom<&SubstateRequirement>,From<&SubstateRequirement>building the proto withsubstate_id.to_bytes()andversion().map(|v| OptionalVersion { version: v }), andFrom<SubstateRequirementRef<'_>>doing the same for the reference wrapper,provide a nice, ergonomic surface for both owned and borrowed
SubstateRequirementvalues. Together with theTryFromabove, this should give clean round-tripping between Rust domain types and proto.Given that there are external bindings (e.g.,
bindings/src/types/SubstateRequirement.ts) mirroring{ substate_id: string; version: number | null }, it would be valuable to add a small unit test that:
- Constructs a
SubstateRequirementwith and without a version,- Converts to
proto::common::SubstateRequirementand back,- Asserts equality of both
substate_idandversion.This will help catch any future drift between the Rust domain types and the proto/TS representations.
crates/template_lib_types/src/max_vec.rs (1)
99-106: Deserialize impl enforces bound correctly; consider a JSON round-trip testThe custom
Deserializeimpl correctly rejects sequences withlen > Nand mirrors the pattern used in other bounded types. You might optionally add a positiveserde_jsonround‑trip test (like the BOR one) to guard against future changes in human‑readable formats, but the current behavior is functionally sound.crates/engine/src/runtime/tracker_auth.rs (1)
175-213: Clarify semantics forRequireRule::MOfNedge casesThe M‑of‑N implementation is straightforward and short‑circuits once
satisfied == *n. However, semantics for corner cases are implicit:
n > requirements.len()will always returnfalse(rule is unsatisfiable).n == 0will also always returnfalserather than being trivially satisfied.If
0-of-Nor “over‑subscribed” rules should be rejected at construction time or treated asAllowAll, consider either:
- Validating
nwhen building the rule and failing early, or- Explicitly handling
n == 0(and optionallyn > len) here with a clear branch, plus docs/tests to lock in behavior.crates/transaction/src/v1/unsealed.rs (1)
11-15: Explicit Schnorr signing support onUnsealedTransactionV1looks consistent with the new signing modelThe split between
add_signer(seal_signer, secret)andadd_signature(public_key, signature)plus theSignable<&RistrettoPublicKeyBytes>/IntoSigned<&RistrettoPublicKeyBytes>impls matches the patterns used onUnsignedTransactionand the builder. Message construction viaTransactionSignature::create_message_v1(1, context, &self.transaction)is consistent with the v1 schema, and the newadd_signaturesimply wrapsTransactionSignature::new, so there’s no hidden behavior change.If you expect the schema version to evolve, consider plumbing
self.schema_version()intocreate_message_v1in all call sites to avoid future drift, but as-is this is fine for v1-only code.Also applies to: 54-58, 60-63, 139-145, 158-164
applications/tari_walletd/src/handlers/accounts.rs (1)
1009-1033: Stealth transfer signing sequence looks correct; consider tightening the type flowThe multi‑step signing in
handle_stealth_transferappears logically sound:
authorized_sealed_signer()creates a context‑aware signable transaction.main_signer_api = sdk.signer_api().with_context(&main_pk)is then used to:
- Attach an additional signer’s signature when
transfer.additional_signeris present, and- Apply all required UTXO spend key signatures via
sign_with_stealth_key(...)on the same transaction value.- Finally
sdk.signer_api().sign(transfer.main_signer.key_id, transaction)?adds the seal signature with the main signer key.This matches the model where access‑rule/UTXO signatures are added first, then the final transaction seal is applied.
Because the
Nonebranch of thematchcallstransaction.finish()early while theSomebranch leaves the type as returned bymain_signer_api.sign, you’re relying onsign_with_stealth_key(and the finalsign) being generic over both shapes. That’s fine if the traits are wired as intended, but it’s subtle; having tests that cover both “with additional_signer” and “without additional_signer but with utxo_spend_keys” would help guard against regressions. If you find the type flow confusing, an optional clean‑up would be to always sign on the same intermediate type and call.finish()in one place after all context‑based signatures have been applied.crates/template_lib/src/models/stealth.rs (1)
6-8: Stealth input/statement simplification matches the new spend model; make sure bindings are updatedThe Rust-side model changes look good:
StealthInputnow only carries the commitment, with ownership proofs and signer requirements pushed into SpendCondition + transaction‑level signatures instead of per‑UTXO Schnorr proofs.StealthInputsStatementdroppingrequired_signerand usingnew(inputs, revealed_amount)with:
assert!(!revealed_amount.is_negative()), andassert!(!inputs.is_empty() || !revealed_amount.is_zero())
correctly enforces that the statement is non‑negative and not completely empty.StealthInputsStatement::new_revealed_onlyandStealthTransferStatement::revealed_onlynow compose cleanly without any signer parameter, which aligns with the PR’s goal of removingrequired_signerfrom input statements.Given these struct shape changes, please ensure the TS bindings in
bindings/src/types/StealthInput.tsandStealthInputsStatement.tsare updated to drop theowner_proofandrequired_signerfields and their associated docs so that client code doesn’t rely on fields that no longer exist in the Rust models.Also applies to: 38-46, 51-56, 59-74, 80-86, 89-95
crates/engine/src/runtime/auth.rs (1)
6-71: AuthorizationScope refactor toIndexSetandArclooks soundThe move to
Arc<IndexSet<NonFungibleAddress>>forvirtual_proofsandIndexSet<ProofId>forproofs, plus the new helpers (empty,contains_badge[_of_resource],contains_proof,add_proof,remove_proof), is internally consistent and preserves determinism while deduplicating entries. Theempty()constructor also aligns withCallScope::newusage.If you find yourself needing a default
AuthorizationScopemore broadly, consider addingimpl Defaultdelegating toempty()to make that intent explicit.crates/wallet/crypto/src/stealth.rs (2)
28-98: Avoid cloninginputs_to_spendwhen buildinginputs_statementYou construct
inputs_to_spend, then clone it just to build an intermediateStealthInputsStatementbefore recreating the statement again in the return value. You can avoid the clone by buildinginputs_statementonce and reusing it both for the balance proof and the finalStealthTransferStatement.For example:
let inputs_statement = StealthInputsStatement { inputs: inputs_to_spend, revealed_amount: revealed_input_amount, }; let balance_proof = if num_outputs == 0 && num_inputs == 0 { None } else { Some(generate_stealth_balance_proof_signature( &agg_input_mask, &agg_output_mask, &inputs_statement, &outputs_statement, )) }; Ok(StealthTransferStatement { inputs_statement, outputs_statement, balance_proof, })This keeps semantics the same and avoids an extra allocation/copy of the inputs vector.
156-189: Tests cover basic range-proof validity; consider multi-output coverage (optional)The tests validate that:
- a correctly constructed statement passes
validate_stealth_outputs_statement, and- tampering with
minimum_value_promiseis detected.You might optionally add a multi-output case (and/or a case with a non-
Noneresource_view_key) to exercise aggregated range proofs and viewable-balance proofs, but the current coverage is already solid.crates/wallet/sdk/src/apis/signer.rs (1)
18-37:with_contextallows non‑Copycontexts but sign helpers requireCtx: Copy
SignerApi<'a, TSpec, Ctx>is generic over anyCtx, andwith_contextaccepts an unconstrainedCtx. However, all the signing helpers live underimpl<Ctx: Copy>, so callingwith_contextwith a non‑Copytype yields aSignerApithat no longer has any of thesign*/generate_*methods.If this isn’t intentional, consider constraining
with_context:pub fn with_context<Ctx: Copy>(self, context: Ctx) -> SignerApi<'a, TSpec, Ctx> { ... }or adding a second constructor for non‑
Copycontexts with a different usage pattern, to make the API behavior less surprising.crates/wallet/sdk/src/apis/stealth_outputs.rs (1)
639-686:create_output_witnesscorrectly wires Secret statements but hard‑codes a simpleSignedspend condition*The function now returns a
SecretStealthOutputStatementwith:
- a
SecretOutputStatementbuilt from a fresh mask, random sender nonce, and optionalresource_view_key, andspend_condition: SpendCondition::Signed(output_owner_public_key.to_byte_type()).This is correct for “single‑owner” outputs and matches the new engine model. However, it also means this API cannot create outputs with more complex access rules (M‑of‑N, etc.); those must be constructed elsewhere.
If the intent is for wallet code to be able to author M‑of‑N outputs directly, consider either:
- adding a variant of this helper that accepts a
SpendCondition, or- changing the signature to take a
SpendConditionparameter with a default ofSigned(owner_pk)at higher layers.Otherwise, the current behavior is fine for simple wallets.
crates/template_test_tooling/src/support/stealth.rs (2)
116-121:StealthSecretTransferDataandNO_INPUTSare reasonable, but rely onMaskAndValue: Into<InputSpec>
StealthSecretTransferData(masks + statement) aligns with how the rest of the test tooling consumes transfer data.
NO_INPUTSis typed asiter::Empty<MaskAndValue>and is intended to be passed intogenerate_transfer_data, which expectsII: IntoIterator<Item = IS>withIS: Into<InputSpec>. This relies on there being anInto<InputSpec>implementation forMaskAndValue(likely viaimpl From<MaskAndValue> for InputSpec>).If that impl exists,
NO_INPUTSis fine. If not, you’ll get type errors when using it and might want to retype it asiter::Empty<InputSpec>instead.
180-248:generate_transfer_data_internalmatches the new Secret / SpendCondition model*This helper:
- builds
SecretStealthOutputStatements fromOutputSpec(dropping zero‑value outputs viafilter(|os| os.value() > 0)),- derives a default
SpendCondition::Signedby treating the random output mask as the owner key forSignedBytests, or uses a specified condition verbatim,- converts
inputsintoStealthInputWitnessusingInputSpec::mask_and_value(), and- calls
stealth::create_transfer_statementwith those witnesses and output statements.This is exactly what you want for flexible test construction of both simple and complex spend conditions. The only behavioral quirk to be aware of is that zero‑value outputs are silently dropped; if you ever need to test zero‑value UTXOs, you’ll want to adjust that filter.
Otherwise this looks good.
crates/engine/tests/stealth.rs (3)
261-292: Balance‑proof failure assertion tracks the new error shapeThe test now asserts on
ResourceError::InvalidBalanceProof { details: "Balance proof signature verification failed".to_string() }. This is precise but also a bit brittle to error‑message wording. If this string changes in the engine, the test will fail even though semantics are correct.You could consider relaxing the detail comparison (e.g., assert it contains
"Balance proof signature"), similar toassert_reject_reasonusage elsewhere, to make the test less fragile to minor message tweaks.
361-411: High‑output count test remains meaningful; consider time‑sensitivity
many_outputs_in_one_transferstill asserts the configured max outputs behaviour and uses.add_signerwith the matching mask. Comments note relatively high verification times; the current limit of 8 avoids the worst‑case. No functional issues here.If these limits change frequently, you may want to centralize the test amount and expectations into a helper so they track
limits::STEALTH_LIMITS.max_outputsmore flexibly.
663-734: New 3‑of‑4 access‑rule test exercises the intended M‑of‑N semantics
transfer_restricted_by_access_rules_n_of_m:
- Creates 4 keys and a 3‑of‑4 rule via
rule!(m_of_n(...)).- Mints a single UTXO with
SpendCondition::AccessRule(rule).- First executes a transfer with only 2 of the 3 required signatures and expects
AccessDeniedforNativeAction::StealthUtxoSpend.- Then retries with 3 signers, expecting success and exactly 2 resulting UTXOs.
This is a clean, focused test of access‑rule‑based spending and looks correct.
You might also want a dedicated 1‑of‑N or N‑of‑N test in a follow‑up to cover trivial and boundary cases for the same rule machinery, but not blocking for this PR.
crates/wallet/sdk/src/apis/key_manager.rs (2)
198-212:generate_stealth_owner_keydoes appropriate validation and reuseThis helper:
- Converts
RistrettoPublicKeyBytestoRistrettoPublicKeyviatry_from_byte_type, mapping failures toInvalidKeyId.- Fetches the account key via
get_key(key_id)(reusing existing error paths).- Delegates to
crypto_api.derive_stealth_owner_secret.The layering is clean. The only minor downside is that
InvalidKeyIdis also used for malformed nonces, not just logical key id errors, but the error message makes that clear.If you later distinguish between malformed public data and internal key id issues, consider a more specific variant (e.g.
InvalidPublicNonce) instead of overloadingInvalidKeyId.
410-428: Error enum extensions are appropriate and backward‑compatible
InvalidKeyId { details }is a reasonable way to surface malformed public nonces or other id‑related issues at the API boundary.- Keeping
CipherErrorasFrom<CipherError>preserves previous behaviour for password‑related encryption/decryption.No conflicts with
IsNotFoundError; existing matches remain exhaustive.If
InvalidKeyIdis only ever used for malformed public nonces, you might rename it later to be more specific, but that’s not required for this PR.crates/transaction/src/builder/mod.rs (4)
88-92: Guardingwith_authorized_seal_signerafter signatures is sensibleCalling
panic_if_signed()after switching to an authorized seal signer prevents users from accidentally mutating already‑signed builders, which would silently invalidate signatures. The panic is preferable to producing subtly invalid transactions.Longer‑term, a typed builder state (e.g.,
UnsignedBuildervsSignedBuilder) would avoid runtime panics, but this is acceptable for now.
172-185:call_functiongeneric overTryInto<FunctionName>improves type‑safetyAllowing any
T: TryInto<FunctionName>means callers can pass&str,String, or pre‑validatedFunctionName. The explicit.expectwith a friendly message for overly long names is appropriate for this builder API.If you later want to surface this as a recoverable error, you could introduce a fallible variant (e.g.,
try_call_function) that returns aResult<Self, BuilderError>.
321-372: Usingpanic_if_signedon all mutating paths prevents stale signaturesAll methods that mutate the underlying
UnsignedTransaction(fee instructions, main instructions, inputs, epochs) now callpanic_if_signed()instead of silently leaving signatures in place. This is a clear correctness improvement: users must either:
- Build/finish after signing, or
- Avoid mutations after adding signatures.
No functional regressions seen.
If you’d like, I can sketch a small doc snippet for the builder API explaining the “no mutation after
add_signer” rule to help downstream users avoid surprises.
445-457:finishcentralizes default signer behaviour and signature attachment
finish:
- If no signatures exist, calls
with_authorized_seal_signer()by default.- Otherwise leaves the builder unchanged.
- Then attaches collected signatures via
with_signatures.This keeps the “auto‑authorized signer” behaviour in one place and ensures
build_and_sealand any future call sites share consistent semantics.If some flows must not auto‑add an authorized seal signer, you might consider a separate
finish_without_default_signerin a later PR to avoid implicit assumptions in advanced scenarios.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (86)
applications/tari_app_utilities/src/transaction_executor.rs(1 hunks)applications/tari_validator_node/src/transaction_validators/network.rs(1 hunks)applications/tari_validator_node_cli/src/command/transaction.rs(2 hunks)applications/tari_wallet_cli/src/command/transaction.rs(2 hunks)applications/tari_walletd/src/handlers/accounts.rs(13 hunks)applications/tari_walletd/src/handlers/confidential.rs(4 hunks)applications/tari_walletd/src/handlers/nfts.rs(2 hunks)applications/tari_walletd/src/handlers/transaction.rs(4 hunks)applications/tari_walletd/src/handlers/validator.rs(1 hunks)clients/wallet_daemon_client/src/component_address.rs(1 hunks)clients/wallet_daemon_client/src/types.rs(46 hunks)crates/engine/src/runtime/actions.rs(1 hunks)crates/engine/src/runtime/auth.rs(1 hunks)crates/engine/src/runtime/error.rs(0 hunks)crates/engine/src/runtime/impl.rs(4 hunks)crates/engine/src/runtime/scope.rs(3 hunks)crates/engine/src/runtime/tracker_auth.rs(7 hunks)crates/engine/src/runtime/working_state.rs(5 hunks)crates/engine/tests/account.rs(1 hunks)crates/engine/tests/asserts.rs(1 hunks)crates/engine/tests/composability.rs(3 hunks)crates/engine/tests/events.rs(2 hunks)crates/engine/tests/signature.rs(4 hunks)crates/engine/tests/stealth.rs(22 hunks)crates/engine/tests/tariswap.rs(2 hunks)crates/engine/tests/test.rs(3 hunks)crates/engine_types/src/crypto/messages.rs(1 hunks)crates/engine_types/src/hashing.rs(0 hunks)crates/engine_types/src/limits.rs(1 hunks)crates/engine_types/src/resource_container.rs(1 hunks)crates/engine_types/src/stealth/outputs.rs(2 hunks)crates/engine_types/src/stealth/transfer.rs(1 hunks)crates/engine_types/src/utxo.rs(3 hunks)crates/p2p/proto/common.proto(1 hunks)crates/p2p/proto/rpc.proto(1 hunks)crates/p2p/proto/transaction.proto(0 hunks)crates/p2p/src/conversions/common.rs(3 hunks)crates/p2p/src/conversions/transaction.rs(1 hunks)crates/p2p/src/lib.rs(1 hunks)crates/template_builtin/templates/faucet/src/lib.rs(2 hunks)crates/template_lib/src/auth/access_rules.rs(8 hunks)crates/template_lib/src/models/component.rs(2 hunks)crates/template_lib/src/models/stealth.rs(5 hunks)crates/template_lib/src/models/unspent_output.rs(2 hunks)crates/template_lib_types/src/bytes.rs(2 hunks)crates/template_lib_types/src/lib.rs(2 hunks)crates/template_lib_types/src/max_bytes.rs(1 hunks)crates/template_lib_types/src/max_string.rs(4 hunks)crates/template_lib_types/src/max_vec.rs(1 hunks)crates/template_lib_types/src/misc.rs(1 hunks)crates/template_test_tooling/src/support/confidential.rs(4 hunks)crates/template_test_tooling/src/support/mod.rs(1 hunks)crates/template_test_tooling/src/support/spec.rs(1 hunks)crates/template_test_tooling/src/support/stealth.rs(7 hunks)crates/template_test_tooling/src/template_test.rs(3 hunks)crates/transaction/Cargo.toml(1 hunks)crates/transaction/src/builder/mod.rs(10 hunks)crates/transaction/src/builder/tests.rs(2 hunks)crates/transaction/src/unsigned_transaction.rs(3 hunks)crates/transaction/src/v1/instruction.rs(3 hunks)crates/transaction/src/v1/signature.rs(3 hunks)crates/transaction/src/v1/unsealed.rs(4 hunks)crates/transaction/src/v1/unsigned.rs(1 hunks)crates/transaction_manifest/src/generator.rs(2 hunks)crates/transaction_manifest/tests/parser.rs(2 hunks)crates/wallet/crypto/src/balance_proof.rs(1 hunks)crates/wallet/crypto/src/bullet_proof.rs(1 hunks)crates/wallet/crypto/src/confidential.rs(3 hunks)crates/wallet/crypto/src/stealth.rs(7 hunks)crates/wallet/crypto/src/unblinded_statement.rs(3 hunks)crates/wallet/crypto/tests/stealth_transfer_statement.rs(6 hunks)crates/wallet/crypto/tests/viewable_balance_proof.rs(1 hunks)crates/wallet/sdk/src/apis/confidential_crypto.rs(3 hunks)crates/wallet/sdk/src/apis/confidential_transfer.rs(3 hunks)crates/wallet/sdk/src/apis/key_manager.rs(10 hunks)crates/wallet/sdk/src/apis/signer.rs(1 hunks)crates/wallet/sdk/src/apis/stealth_crypto.rs(4 hunks)crates/wallet/sdk/src/apis/stealth_outputs.rs(9 hunks)crates/wallet/sdk/src/apis/stealth_transfer/api.rs(7 hunks)crates/wallet/sdk/src/apis/stealth_transfer/params.rs(2 hunks)crates/wallet/sdk/src/apis/stealth_transfer/types.rs(1 hunks)crates/wallet/sdk/src/models/account.rs(1 hunks)crates/wallet/sdk/src/models/key.rs(3 hunks)integration_tests/src/wallet_daemon_client.rs(1 hunks)utilities/tariswap_test_bench/src/accounts.rs(2 hunks)utilities/tariswap_test_bench/src/tariswap.rs(3 hunks)
💤 Files with no reviewable changes (3)
- crates/engine_types/src/hashing.rs
- crates/engine/src/runtime/error.rs
- crates/p2p/proto/transaction.proto
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-04T10:10:24.258Z
Learnt from: sdbondi
Repo: tari-project/tari-ootle PR: 1629
File: applications/tari_walletd/src/handlers/accounts.rs:1001-1002
Timestamp: 2025-11-04T10:10:24.258Z
Learning: In applications/tari_walletd/src/handlers/accounts.rs, the expect() on Memo::new_pay_ref_and_bytes_truncate at line 1002 is safe and intentional. PayRef is validated to be at most 64 bytes during address decoding (PayRef::MAX_LEN = 64), and the function only returns None if payref exceeds 252 bytes (Memo::MAX_BYTES_LENGTH - 1). Since 64 < 252, None is impossible with a valid PayRef.
Applied to files:
crates/template_lib_types/src/max_bytes.rsutilities/tariswap_test_bench/src/accounts.rscrates/engine/tests/stealth.rs
🧬 Code graph analysis (48)
crates/engine_types/src/resource_container.rs (1)
bindings/src/types/PedersenCommitmentBytes.ts (1)
PedersenCommitmentBytes(6-6)
crates/template_builtin/templates/faucet/src/lib.rs (3)
crates/engine/src/runtime/impl.rs (1)
emit_event(437-460)bindings/src/types/StealthInputsStatement.ts (1)
StealthInputsStatement(9-22)crates/template_lib/src/models/stealth.rs (2)
new_revealed_only(29-35)new_revealed_only(72-74)
crates/template_lib/src/models/component.rs (2)
crates/template_lib/src/models/resource.rs (1)
new(43-45)crates/template_lib/src/models/vault.rs (2)
new(73-75)try_from(121-124)
applications/tari_walletd/src/handlers/validator.rs (2)
crates/wallet/sdk/src/models/key.rs (2)
derived(360-362)index(313-315)crates/transaction/src/v1/unsigned.rs (1)
builder(37-39)
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)
applications/tari_app_utilities/src/transaction_executor.rs (2)
crates/engine/src/runtime/auth.rs (1)
new(25-30)crates/engine/src/transaction/processor.rs (1)
new(92-108)
crates/transaction/src/v1/unsigned.rs (1)
crates/transaction/src/v1/signature.rs (1)
create_message_v1(121-132)
crates/engine_types/src/crypto/messages.rs (1)
bindings/src/types/PedersenCommitmentBytes.ts (1)
PedersenCommitmentBytes(6-6)
applications/tari_walletd/src/handlers/nfts.rs (2)
crates/transaction/src/v1/unsigned.rs (1)
builder(37-39)crates/transaction/src/transaction.rs (1)
builder(47-49)
crates/engine_types/src/stealth/outputs.rs (2)
bindings/src/types/StealthOutputsStatement.ts (1)
StealthOutputsStatement(9-24)bindings/src/types/UtxoOutput.ts (1)
UtxoOutput(6-14)
crates/p2p/src/conversions/common.rs (3)
bindings/src/types/SubstateId.ts (1)
SubstateId(6-6)bindings/src/types/SubstateAddress.ts (1)
SubstateAddress(3-3)bindings/src/types/SubstateRequirement.ts (1)
SubstateRequirement(3-3)
crates/transaction/src/unsigned_transaction.rs (4)
crates/transaction/src/builder/mod.rs (6)
add_signer(420-425)new(54-60)add_signature(427-430)signatures(432-434)finish(445-457)into_signed(527-532)crates/transaction/src/v1/unsealed.rs (6)
add_signer(54-58)new(34-39)add_signature(60-63)signatures(81-83)into_signed(150-155)into_signed(161-163)crates/transaction/src/v1/unsigned.rs (1)
new(41-60)crates/common_types/src/signable.rs (1)
into_signed(15-15)
crates/engine/src/runtime/impl.rs (2)
crates/engine/src/wasm/environment.rs (1)
state_mut(171-173)bindings/src/helpers/consts.ts (1)
XTR(10-10)
crates/engine/tests/tariswap.rs (1)
crates/template_lib_types/src/max_string.rs (1)
new_checked(24-31)
crates/wallet/crypto/src/balance_proof.rs (3)
bindings/src/types/StealthInputsStatement.ts (1)
StealthInputsStatement(9-22)bindings/src/types/StealthOutputsStatement.ts (1)
StealthOutputsStatement(9-24)bindings/src/types/Amount.ts (1)
Amount(12-12)
crates/template_test_tooling/src/template_test.rs (2)
crates/engine/src/transaction/processor.rs (1)
new(92-108)crates/engine/src/runtime/auth.rs (1)
proofs(53-55)
crates/transaction_manifest/tests/parser.rs (2)
bindings/src/types/Instruction.ts (1)
Instruction(18-45)bindings/src/helpers/consts.ts (1)
XTR(10-10)
crates/template_lib_types/src/max_vec.rs (3)
crates/template_lib_types/src/bytes.rs (6)
deref(39-41)into_vec(23-25)as_slice(31-33)as_ref(63-65)deref_mut(45-47)deserialize(73-80)crates/template_lib_types/src/max_bytes.rs (12)
deref(20-22)new_checked(26-33)new_unchecked(41-43)into_vec(49-51)empty(53-55)as_slice(57-59)as_ref(63-65)deref_mut(69-72)default(76-78)try_from(84-86)try_from(92-94)deserialize(116-139)crates/template_lib_types/src/max_string.rs (9)
deref(18-20)new_checked(24-31)as_ref(39-41)deref_mut(45-48)try_from(71-73)try_from(79-81)try_from(87-89)try_from(95-97)deserialize(59-65)
crates/engine_types/src/stealth/transfer.rs (2)
bindings/src/types/StealthTransferStatement.ts (1)
StealthTransferStatement(6-14)bindings/src/types/Amount.ts (1)
Amount(12-12)
crates/wallet/sdk/src/apis/stealth_transfer/api.rs (3)
bindings/src/types/SubstateId.ts (1)
SubstateId(6-6)bindings/src/types/AccountWithAddress.ts (1)
AccountWithAddress(5-5)bindings/src/types/wallet-daemon-client/KeyBranch.ts (1)
KeyBranch(3-10)
crates/wallet/crypto/tests/viewable_balance_proof.rs (2)
crates/wallet/crypto/src/confidential.rs (1)
create_output_statement(70-127)crates/wallet/crypto/src/unblinded_statement.rs (2)
value(63-65)mask(67-69)
applications/tari_walletd/src/handlers/transaction.rs (3)
crates/transaction/src/v1/unsigned.rs (1)
builder(37-39)crates/transaction/src/transaction.rs (2)
builder(47-49)signatures(99-103)crates/transaction/src/builder/mod.rs (1)
signatures(432-434)
crates/wallet/sdk/src/apis/confidential_transfer.rs (1)
crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(71-73)
crates/template_test_tooling/src/support/spec.rs (3)
crates/template_lib/src/models/unspent_output.rs (1)
signed_by(56-61)crates/wallet/crypto/src/unblinded_statement.rs (1)
value(63-65)crates/wallet/sdk/src/apis/stealth_transfer/types.rs (1)
value(29-31)
applications/tari_walletd/src/handlers/accounts.rs (4)
bindings/src/types/wallet-daemon-client/KeyId.ts (1)
KeyId(4-4)crates/wallet/sdk/src/models/key.rs (2)
derived(360-362)new(309-311)crates/wallet/sdk/src/apis/stealth_transfer/api.rs (3)
transfer(292-610)new(69-86)inputs(210-210)crates/transaction/src/unsigned_transaction.rs (1)
inputs(79-83)
crates/transaction/src/v1/unsealed.rs (8)
bindings/src/types/SchnorrSignatureBytes.ts (1)
SchnorrSignatureBytes(5-5)bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)crates/transaction/src/builder/mod.rs (5)
add_signer(420-425)add_signature(427-430)new(54-60)as_signing_message(519-521)into_signed(527-532)crates/transaction/src/unsigned_transaction.rs (4)
add_signer(133-141)add_signature(143-147)as_signing_message(182-186)into_signed(192-197)bindings/src/types/TransactionSignature.ts (1)
TransactionSignature(5-5)crates/transaction/src/v1/signature.rs (8)
sign_v1(86-100)public_key(58-60)public_key(117-119)signature(54-56)signature(113-115)new(27-29)new(82-84)create_message_v1(121-132)bindings/src/types/UnsealedTransactionV1.ts (1)
UnsealedTransactionV1(5-5)crates/common_types/src/signable.rs (2)
as_signing_message(9-9)into_signed(15-15)
crates/template_lib/src/models/unspent_output.rs (4)
bindings/src/types/AccessRule.ts (1)
AccessRule(7-7)bindings/src/types/ViewableBalanceProof.ts (1)
ViewableBalanceProof(27-62)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)
crates/wallet/crypto/src/unblinded_statement.rs (4)
bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)bindings/src/types/Memo.ts (1)
Memo(3-3)crates/engine_types/src/crypto/helpers.rs (1)
commit_u64_amount(74-76)
crates/template_lib_types/src/bytes.rs (2)
crates/template_lib_types/src/max_bytes.rs (2)
into_vec(49-51)as_slice(57-59)crates/template_lib_types/src/max_vec.rs (2)
into_vec(45-47)as_slice(53-55)
crates/engine/src/runtime/auth.rs (5)
bindings/src/types/NonFungibleAddress.ts (1)
NonFungibleAddress(7-7)bindings/src/types/ProofId.ts (1)
ProofId(6-6)bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)crates/engine/src/runtime/scope.rs (1)
new(33-44)crates/engine_types/src/resource_container.rs (1)
resource_address(175-182)
crates/template_lib/src/auth/access_rules.rs (4)
bindings/src/types/RestrictedAccessRule.ts (1)
RestrictedAccessRule(7-10)bindings/src/types/RuleRequirement.ts (1)
RuleRequirement(10-14)bindings/src/types/RequireRule.ts (1)
RequireRule(7-10)bindings/src/types/AccessRule.ts (1)
AccessRule(7-7)
crates/template_lib_types/src/max_string.rs (2)
crates/template_lib_types/src/max_bytes.rs (3)
new_checked(26-33)try_from(84-86)try_from(92-94)crates/template_lib_types/src/max_vec.rs (3)
new_checked(22-29)try_from(80-82)try_from(88-90)
crates/engine/tests/signature.rs (1)
crates/template_test_tooling/src/support/stealth.rs (1)
generate_transfer_data(122-143)
crates/wallet/sdk/src/apis/stealth_crypto.rs (3)
crates/wallet/sdk/src/apis/key_manager.rs (1)
new(66-81)crates/wallet/sdk/src/apis/stealth_outputs.rs (1)
new(82-94)crates/wallet/sdk/src/apis/confidential_outputs.rs (1)
new(40-50)
crates/template_lib/src/models/stealth.rs (2)
bindings/src/types/StealthInput.ts (1)
StealthInput(8-18)bindings/src/types/StealthInputsStatement.ts (1)
StealthInputsStatement(9-22)
crates/template_test_tooling/src/support/confidential.rs (2)
crates/template_lib_types/src/encrypted_data.rs (1)
min_size(31-33)crates/engine_types/src/resource.rs (1)
view_key(128-130)
crates/wallet/crypto/src/stealth.rs (3)
crates/wallet/crypto/src/balance_proof.rs (1)
generate_stealth_balance_proof_signature(38-55)crates/wallet/crypto/src/bullet_proof.rs (1)
generate_extended_bullet_proof(20-70)crates/engine_types/src/stealth/outputs.rs (2)
stmt(45-80)validate_stealth_outputs_statement(34-83)
crates/template_test_tooling/src/support/stealth.rs (2)
crates/template_test_tooling/src/support/spec.rs (1)
mask_and_value(58-60)crates/wallet/crypto/src/stealth.rs (1)
create_transfer_statement(28-98)
crates/wallet/sdk/src/apis/signer.rs (2)
crates/wallet/sdk/src/apis/key_manager.rs (2)
sign_with_stealth_key(352-360)sign_with_explicit_key(384-394)crates/wallet/sdk/src/key_managers/backend.rs (1)
sign(25-39)
crates/wallet/crypto/tests/stealth_transfer_statement.rs (2)
crates/wallet/crypto/src/stealth.rs (1)
create_transfer_statement(28-98)crates/template_lib_types/src/encrypted_data.rs (1)
empty(27-29)
crates/engine/tests/stealth.rs (2)
crates/template_test_tooling/src/support/stealth.rs (5)
iter(58-58)iter(66-66)generate_mint_statement(45-72)generate_transfer_data(122-143)generate_transfer_data_with_view_key(145-167)crates/template_test_tooling/src/support/assert_error.rs (2)
assert_access_denied_for_action(20-24)assert_reject_reason(10-17)
crates/engine/src/runtime/scope.rs (3)
crates/engine/src/runtime/auth.rs (2)
new(25-30)empty(32-37)crates/engine/src/runtime/working_state.rs (1)
new(110-138)crates/template_lib/src/args/types.rs (1)
proof_id(661-666)
crates/wallet/sdk/src/apis/stealth_outputs.rs (2)
crates/template_lib/src/models/unspent_output.rs (1)
signed_by(56-61)crates/wallet/sdk/src/models/stealth_output.rs (1)
from(72-80)
clients/wallet_daemon_client/src/types.rs (4)
bindings/src/types/wallet-daemon-client/KeyId.ts (1)
KeyId(4-4)bindings/src/types/wallet-daemon-client/AccountsCreateStealthTransferStatementRequest.ts (1)
AccountsCreateStealthTransferStatementRequest(4-4)bindings/src/types/wallet-daemon-client/TransferStatementRequest.ts (1)
TransferStatementRequest(7-12)bindings/src/types/wallet-daemon-client/AccountsCreateStealthTransferStatementResponse.ts (1)
AccountsCreateStealthTransferStatementResponse(5-9)
crates/wallet/sdk/src/apis/key_manager.rs (5)
bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)crates/wallet/sdk/src/apis/stealth_crypto.rs (1)
new(36-38)crates/wallet/sdk/src/models/key.rs (20)
new(309-311)secret(166-168)branch(317-319)index(313-315)key_id(74-76)key_id(145-147)key_id(170-172)secret_key(243-245)sign(178-182)public_key(82-84)public_key(141-143)public_key(239-241)from(151-156)from(186-191)from(195-200)from(204-209)from(213-218)from(290-292)from(296-298)from(391-396)crates/wallet/sdk/src/apis/signer.rs (3)
sign_with_stealth_key(65-76)sign_with_explicit_key(78-91)sign(58-63)crates/wallet/sdk/src/key_managers/backend.rs (1)
sign(25-39)
crates/transaction/src/builder/mod.rs (3)
crates/template_test_tooling/src/template_test.rs (2)
call_function(341-368)call_method(371-399)crates/engine/src/transaction/processor.rs (2)
call_function(535-578)call_method(580-646)crates/transaction/src/unsigned_transaction.rs (1)
finish(156-158)
crates/wallet/sdk/src/models/key.rs (2)
bindings/src/types/wallet-daemon-client/KeyId.ts (1)
KeyId(4-4)bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)
crates/engine/src/runtime/working_state.rs (3)
crates/engine_types/src/stealth/outputs.rs (1)
stmt(45-80)bindings/src/types/NonFungibleAddress.ts (1)
NonFungibleAddress(7-7)crates/template_lib/src/models/non_fungible.rs (1)
from_public_key(233-238)
⏰ 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). (4)
- GitHub Check: check nightly
- GitHub Check: test
- GitHub Check: check stable
- GitHub Check: clippy
Test Results (CI)495 tests - 23 495 ✅ - 5 53m 26s ⏱️ - 33m 57s Results for commit 956ee01. ± Comparison against base commit a1e36bb. This pull request removes 29 and adds 6 tests. Note that renamed tests count towards both. |
956ee01 to
233765e
Compare
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/p2p/src/conversions/transaction.rs (1)
65-72: ReplaceFromwithTryFromto handle encoding failures.The
From<&Transaction>implementation usesexpect()(line 69), which violates the trait's infallibility contract. TheFromtrait should never panic, but encoding can fail.Replace the
Fromimplementation withTryFrom:-impl From<&Transaction> for proto::transaction::Transaction { - fn from(transaction: &Transaction) -> Self { +impl TryFrom<&Transaction> for proto::transaction::Transaction { + type Error = anyhow::Error; + + fn try_from(transaction: &Transaction) -> Result<Self, Self::Error> { - proto::transaction::Transaction { - // TODO: no panic - bor_encoded: encode_to_vec(transaction).expect("Failed to encode transaction"), - } + Ok(proto::transaction::Transaction { + bor_encoded: encode_to_vec(transaction)?, + }) } }You'll also need to update callers of this conversion (e.g., line 38 in the
NewTransactionMessageconversion above) to handle theResult:impl From<NewTransactionMessage> for proto::transaction::NewTransactionMessage { fn from(msg: NewTransactionMessage) -> Self { Self { - transaction: Some((&msg.transaction).into()), + transaction: Some((&msg.transaction).try_into().expect("Failed to encode transaction")), } } }Or make that conversion fallible as well by changing it to
TryFrom.
♻️ Duplicate comments (5)
crates/template_lib_types/src/max_vec.rs (1)
31-39: The past review comment regarding the safety documentation is still relevant.The safety documentation on line 36 incorrectly refers to "
bytes" instead of "elems". This should be corrected as suggested in the previous review.applications/tari_walletd/src/handlers/nfts.rs (1)
296-309: Final NFT transfer seal signer should usefee_payer_key_id, notaccount_owner_key_idIn
handle_transfer, the transaction is effectively signed twice withaccount_owner_key_id:
- Inside the
with_authorized_seal_signer().map(...)closure (Line 305).- Again on the finished transaction (Line 309).
Yet the comment at Line 301 states “Seal signer is the fee payer account”, and
fee_payer_key_idis computed but never used for the final seal. This contradicts the intended multi-signer model and can lead to incorrect fee-payer semantics.You likely want the final seal to use
fee_payer_key_id:- let transaction = sdk.signer_api().sign(account_owner_key_id, transaction)?; + let transaction = sdk.signer_api().sign(fee_payer_key_id, transaction)?;This preserves the existing owner signing inside the
mapclosure while ensuring the fee payer actually seals the transaction.applications/tari_walletd/src/handlers/transaction.rs (1)
151-163: Fix per-signer signing context inhandle_submit.
req.other_signersare still signed using the main signer's context (main_signer_pk), so each additional signature is tagged with the wrong public key and will fail verification whenever an additional signer differs from the seal signer. The finalseal_signersignature also omits an explicit context.Apply a per-signer context when signing
other_signersand use the main signer's context for the final seal signature, for example:- let main_signer = sdk.key_manager_api().get_public_key(req.seal_signer)?; - let main_signer_pk = main_signer.public_key.to_byte_type(); - let local_signer = sdk.signer_api().with_context(&main_signer_pk); - for key in req.other_signers { - builder = local_signer.sign(key, builder)?; - } - - let transaction = sdk.signer_api().sign(req.seal_signer, builder.finish())?; + let main_signer = sdk.key_manager_api().get_public_key(req.seal_signer)?; + let main_signer_pk = main_signer.public_key.to_byte_type(); + for key in req.other_signers { + let signer_key = sdk.key_manager_api().get_public_key(key)?; + let signer_pk = signer_key.public_key.to_byte_type(); + builder = sdk + .signer_api() + .with_context(&signer_pk) + .sign(signer_key.key_id, builder)?; + } + + let transaction = sdk + .signer_api() + .with_context(&main_signer_pk) + .sign(req.seal_signer, builder.finish())?;clients/wallet_daemon_client/src/types.rs (1)
1108-1115: Clarify invariants forutxo_signersin stealth transfer statement response.Adding
utxo_signers: Vec<StealthUtxoSpendKeyId>provides clients the extra context needed for UTXO-level signing. Consider documenting:
- Whether
utxo_signers.len()relates tostatements.len()or input count- How positions in
utxo_signerscorrespond to specific inputsThis will help prevent client-side bugs when wiring up signing flows.
crates/wallet/sdk/src/apis/stealth_outputs.rs (1)
567-582: Ownership check for non-Signedspend conditions may misclassify outputs as spendable.The logic at lines 568-572 returns
truewhensigned_by()isNone(forSpendCondition::AccessRule), marking outputs with complex M-of-N rules asUnspentregardless of whether this wallet can satisfy them.This could:
- Inflate displayed balances with unspendable outputs
- Lead to failed transactions when users attempt to spend M-of-N outputs they don't fully control
Consider either:
- Marking
signed_by() == Noneoutputs with a distinct status (e.g.,OutputStatus::ComplexCondition)- Adding minimal access rule evaluation to determine spendability
🧹 Nitpick comments (14)
crates/p2p/src/conversions/common.rs (1)
211-220: Rename variable to match its type.The variable is named
substate_specificationbut its type isSubstateRequirement, which is misleading.Apply this diff:
fn try_from(val: proto::common::SubstateRequirement) -> Result<Self, Self::Error> { let substate_id = SubstateId::from_bytes(&val.substate_id)?; let version = val.version.map(|v| v.version); - let substate_specification = SubstateRequirement::new(substate_id, version); - Ok(substate_specification) + let substate_requirement = SubstateRequirement::new(substate_id, version); + Ok(substate_requirement) }crates/transaction/src/v1/signature.rs (1)
92-92: Consider using a constant or the transaction's schema version.The schema version is hardcoded to
1in bothsign_v1andverify_v1. While this matches the v1 naming, it could become a maintenance issue if multiple schema versions need to coexist or if the version needs to change.Consider one of these approaches:
Option 1: Extract to a constant:
+const SCHEMA_VERSION_V1: u16 = 1; + pub fn sign_v1( secret_key: &RistrettoSecretKey, seal_signer: &RistrettoPublicKeyBytes, transaction: &UnsignedTransactionV1, ) -> Self { let public_key = RistrettoPublicKey::from_secret_key(secret_key); - let message = Self::create_message_v1(1, seal_signer, transaction); + let message = Self::create_message_v1(SCHEMA_VERSION_V1, seal_signer, transaction);Option 2: Use the transaction's schema version if available (requires passing it through
UnsignedTransactionV1).Also applies to: 103-103
crates/wallet/sdk/src/apis/signer.rs (2)
18-21: Consider making thecontextfield private.The
contextfield is currently public, which exposes internal state. If direct access is not required, consider making it private and adding a public accessor method (e.g.,pub fn context(&self) -> &Ctx) to improve encapsulation.Apply this diff if you want to encapsulate the field:
pub struct SignerApi<'a, TSpec: WalletSdkSpec, Ctx = ()> { key_manager: KeyManagerApi<'a, TSpec>, - context: Ctx, + pub(crate) context: Ctx, }
39-92: Let me search the codebase directly to verify the key_manager implementation:Consider adding documentation to clarify the context parameter and its intended use.
The
Ctx: Copybound is working as designed—all actual usages in the codebase pass reference types (which are Copy, meaning they can be copied bitwise without transferring ownership). This design choice successfully keeps the API simple. However, adding doc comments would improve clarity by explaining:
- What the context parameter represents
- Examples of appropriate context types (public keys, addresses, etc.)
- How the
with_contextmethod is intended to be usedThis is an optional enhancement for better API documentation.
crates/transaction/src/unsigned_transaction.rs (2)
133-141: Avoid cloningUnsignedTransactionV1inadd_signer
add_signercurrently matches on&mut selfand clones the innerUnsignedTransactionV1, even thoughselfis being consumed. This is both unnecessary and inconsistent withwith_signatures, which matches onselfand moves the inner value.You can simplify and avoid the clone:
- pub fn add_signer( - mut self, - seal_signer: &RistrettoPublicKeyBytes, - key: &RistrettoSecretKey, - ) -> UnsealedTransactionV1 { - match &mut self { - Self::V1(tx) => UnsealedTransactionV1::new(tx.clone(), vec![]).add_signer(seal_signer, key), - } - } + pub fn add_signer( + self, + seal_signer: &RistrettoPublicKeyBytes, + key: &RistrettoSecretKey, + ) -> UnsealedTransactionV1 { + match self { + Self::V1(tx) => UnsealedTransactionV1::new(tx, vec![]).add_signer(seal_signer, key), + } + }This keeps the behavior the same while avoiding an extra allocation/copy and matches the pattern used in
with_signatures.
143-147: Similarly avoid clone inadd_signature
add_signaturehas the same pattern asadd_signer: it consumesselfbut still matches on&mut selfand clones the inner transaction.You can mirror the move-based approach:
- pub fn add_signature(mut self, signature: TransactionSignature) -> UnsealedTransactionV1 { - match &mut self { - Self::V1(tx) => UnsealedTransactionV1::new(tx.clone(), vec![signature]), - } - } + pub fn add_signature(self, signature: TransactionSignature) -> UnsealedTransactionV1 { + match self { + Self::V1(tx) => UnsealedTransactionV1::new(tx, vec![signature]), + } + }This removes the clone and aligns the implementation with
with_signatures.crates/template_lib_types/src/max_vec.rs (1)
77-91: Consider a more descriptive error type for TryFrom.Both
TryFromimplementations usetype Error = (), which doesn't convey why the conversion failed. While this is idiomatic for simple cases, a custom error type (e.g.,MaxVecError::LengthExceeded { max: usize, actual: usize }) would improve debuggability.Example:
pub enum MaxVecError { LengthExceeded { max: usize, actual: usize }, } impl<const N: usize, T> TryFrom<Vec<T>> for MaxVec<N, T> { type Error = MaxVecError; fn try_from(value: Vec<T>) -> Result<Self, Self::Error> { let len = value.len(); Self::new_checked(value).ok_or(MaxVecError::LengthExceeded { max: N, actual: len }) } }bindings/src/types/Bytes.ts (1)
1-9: Bytes alias is fine; consider documenting encoding in higher-level docsThe
Bytes = stringalias plus the doc comment is consistent with the CBORBytesrepresentation. Any clarification about whether this string is hex/base64 is better handled in the generator or external docs rather than editing this generated file.crates/transaction_manifest/src/generator.rs (1)
91-99: Identifier validation via TryInto is correct; consider generalizing error textUsing
to_string().try_into()?forfunctionandmethodproperly enforces name constraints during manifest generation and surfaces them asManifestError::InvalidInstruction, which is the right place to fail.If you want more accurate diagnostics, you could avoid assuming the only failure mode is length and delegate to the underlying error:
- function: function_name - .to_string() - .try_into() - .map_err(|e| ManifestError::InvalidInstruction { - reason: format!("Function name is too long: {}", e), - })?, + function: function_name + .to_string() + .try_into() + .map_err(|e| ManifestError::InvalidInstruction { + reason: format!("Failed to convert function name: {}", e), + })?, @@ - method: function_name - .to_string() - .try_into() - .map_err(|e| ManifestError::InvalidInstruction { - reason: format!("Method name is too long: {}", e), - })?, + method: function_name + .to_string() + .try_into() + .map_err(|e| ManifestError::InvalidInstruction { + reason: format!("Failed to convert method name: {}", e), + })?,Also applies to: 128-136
crates/engine/tests/templates/stealth/src/lib.rs (1)
71-74: Verify behavior when revealed outputs are present in static_programmatic_transfer
static_programmatic_transfercallsResourceManager::get(resource).stealth_transfer(transfer)and discards the returned bucket. In contrast,programmatic_transferalways deposits the resulting bucket intosupply_vault.If
transfer.outputs_statementcan contain a non-zero revealed output amount here, those funds will effectively be dropped. If this helper is intended only for zero-revealed-output statements, consider:
- Documenting that assumption and/or
- Adding a debug assertion on the revealed output amount, or
- Depositing the bucket into an explicit vault like in
programmatic_transfer.crates/template_lib/src/auth/access_rules.rs (1)
115-120: Add validation for MOfN threshold at construction time.Verification confirms the concern is valid. The
MOfN(u16, Box<[RuleRequirement]>)variant can be constructed without validating thatm <= n. Examples:
- The macro at line 400 constructs
MOfNdirectly without validation- No
implor validation function exists forRequireRule- At runtime (tracker_auth.rs:200-210), if
m > n, the loop silently exits and returnsOk(false)instead of catching the invalid constraintAdd validation either in the macro expansion or via a builder method to ensure
m <= requirements.len()before construction succeeds.applications/tari_validator_node/src/transaction_validators/network.rs (1)
34-40: Swappedactual/expectedvalues now match log semanticsUsing
actual: tx_networkandexpected: self.networkmatches the log message (“TX != Current”) and is the intuitive mapping for a mismatch error. Behavior remains the same aside from payload semantics.As a small improvement, you could tighten the
network_mismatchtest to assert the concreteactualandexpectedvalues instead of wildcards, to lock this mapping in and avoid future regressions.Also applies to: 91-100
crates/template_test_tooling/src/support/spec.rs (1)
7-47: Consider derivingDebug/Cloneon test-spec types.
OutputSpec,SpendConditionSpec, andInputSpecare likely only used in tests; adding#[derive(Debug, Clone)]would make them easier to inspect and reuse in assertions without changing semantics.Also applies to: 49-67
crates/wallet/sdk/src/apis/stealth_transfer/api.rs (1)
301-309: Stealth transfer construction and signer selection mostly look good; double-check pure-UTXO cases.
- Enforcing
owner_account.owner_key_id()in Lines 301–309 matches the new requirement that spending needs secrets, not just view-only state.- Fee vs. main intent signer selection in Lines 391–398 and 439–453 is sensible: you use the account key when revealed funds or badge usage demand it, otherwise derive a nonce key, and you avoid duplicating a signature when both roles share the same key.
- Output creation via
TryInto<StealthOutputToCreate>and passingoutput_revealed_amountthroughTransferStatementParamsin Lines 505–523 fits the newPayTo/StealthOutputToCreatemodel.- Building
utxo_spend_keysfrom(account_key_id, public_nonce)in Lines 586–593 and threading them throughStealthTransferOutputin Lines 605–612 is a clear way to carry per-input spend info to later layers.One subtle edge case to re-verify: when both fee and transfer inputs are fully confidential (no revealed amounts) and no badge is used, both
fee_signerandmain_intent_signerend up being nonce-based keys, whileutxo_spend_keysstill embedaccount_key_id. That’s fine if UTXO spend authorization is driven solely byutxo_spend_keys(deriving the correct ephemeral owners) and does not require a directaccount_key_idsignature, but it would fail if the engine expects an explicit signature fromaccount_key_iditself. Please confirm the intended behavior with your authorization logic/tests.Also applies to: 391-398, 439-453, 505-523, 586-593, 605-612
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (107)
applications/tari_app_utilities/src/transaction_executor.rs(1 hunks)applications/tari_validator_node/src/transaction_validators/network.rs(1 hunks)applications/tari_validator_node_cli/src/command/transaction.rs(2 hunks)applications/tari_wallet_cli/src/command/transaction.rs(2 hunks)applications/tari_walletd/src/handlers/accounts.rs(15 hunks)applications/tari_walletd/src/handlers/confidential.rs(4 hunks)applications/tari_walletd/src/handlers/nfts.rs(2 hunks)applications/tari_walletd/src/handlers/transaction.rs(4 hunks)applications/tari_walletd/src/handlers/validator.rs(1 hunks)applications/tari_walletd/web_ui/src/services/api/hooks/useAccounts.ts(2 hunks)bindings/src/index.ts(3 hunks)bindings/src/tari-indexer-client.ts(0 hunks)bindings/src/types/Account.ts(1 hunks)bindings/src/types/Bytes.ts(1 hunks)bindings/src/types/RequireRule.ts(1 hunks)bindings/src/types/SpendCondition.ts(1 hunks)bindings/src/types/StealthInput.ts(1 hunks)bindings/src/types/StealthInputsStatement.ts(0 hunks)bindings/src/types/StealthUnspentOutput.ts(1 hunks)bindings/src/types/UtxoOutput.ts(1 hunks)bindings/src/types/WalletTransaction.ts(1 hunks)bindings/src/types/tari-indexer-client/GetTemplateDefinitionRequest.ts(0 hunks)bindings/src/types/wallet-types/AccountsCreateStealthTransferStatementResponse.ts(1 hunks)bindings/src/types/wallet-types/PayTo.ts(1 hunks)bindings/src/types/wallet-types/StealthTransfer.ts(1 hunks)bindings/src/types/wallet-types/StealthUtxoSpendKeyId.ts(1 hunks)bindings/src/types/wallet-types/TransferOutput.ts(2 hunks)bindings/src/wallet-daemon-client.ts(0 hunks)bindings/src/wallet-types.ts(1 hunks)clients/wallet_daemon_client/src/component_address.rs(1 hunks)clients/wallet_daemon_client/src/types.rs(46 hunks)crates/consensus_types/src/certificates/quorum_certificate.rs(0 hunks)crates/engine/src/runtime/actions.rs(1 hunks)crates/engine/src/runtime/auth.rs(1 hunks)crates/engine/src/runtime/error.rs(0 hunks)crates/engine/src/runtime/impl.rs(4 hunks)crates/engine/src/runtime/scope.rs(3 hunks)crates/engine/src/runtime/tracker_auth.rs(7 hunks)crates/engine/src/runtime/working_state.rs(5 hunks)crates/engine/tests/account.rs(1 hunks)crates/engine/tests/asserts.rs(1 hunks)crates/engine/tests/composability.rs(3 hunks)crates/engine/tests/events.rs(2 hunks)crates/engine/tests/signature.rs(4 hunks)crates/engine/tests/stealth.rs(22 hunks)crates/engine/tests/tariswap.rs(2 hunks)crates/engine/tests/templates/stealth/src/lib.rs(1 hunks)crates/engine/tests/test.rs(3 hunks)crates/engine_types/src/crypto/messages.rs(1 hunks)crates/engine_types/src/hashing.rs(0 hunks)crates/engine_types/src/limits.rs(1 hunks)crates/engine_types/src/resource_container.rs(1 hunks)crates/engine_types/src/stealth/outputs.rs(2 hunks)crates/engine_types/src/stealth/transfer.rs(1 hunks)crates/engine_types/src/utxo.rs(3 hunks)crates/p2p/proto/common.proto(1 hunks)crates/p2p/proto/rpc.proto(1 hunks)crates/p2p/proto/transaction.proto(0 hunks)crates/p2p/src/conversions/common.rs(3 hunks)crates/p2p/src/conversions/transaction.rs(1 hunks)crates/p2p/src/lib.rs(1 hunks)crates/template_builtin/templates/faucet/src/lib.rs(2 hunks)crates/template_lib/src/auth/access_rules.rs(8 hunks)crates/template_lib/src/models/component.rs(2 hunks)crates/template_lib/src/models/stealth.rs(5 hunks)crates/template_lib/src/models/unspent_output.rs(2 hunks)crates/template_lib_types/src/bytes.rs(2 hunks)crates/template_lib_types/src/lib.rs(2 hunks)crates/template_lib_types/src/max_bytes.rs(1 hunks)crates/template_lib_types/src/max_string.rs(4 hunks)crates/template_lib_types/src/max_vec.rs(1 hunks)crates/template_lib_types/src/misc.rs(1 hunks)crates/template_test_tooling/src/read_only_state_store.rs(1 hunks)crates/template_test_tooling/src/support/confidential.rs(4 hunks)crates/template_test_tooling/src/support/mod.rs(1 hunks)crates/template_test_tooling/src/support/spec.rs(1 hunks)crates/template_test_tooling/src/support/stealth.rs(7 hunks)crates/template_test_tooling/src/template_test.rs(3 hunks)crates/transaction/Cargo.toml(1 hunks)crates/transaction/src/builder/mod.rs(10 hunks)crates/transaction/src/builder/tests.rs(2 hunks)crates/transaction/src/unsigned_transaction.rs(3 hunks)crates/transaction/src/v1/instruction.rs(3 hunks)crates/transaction/src/v1/signature.rs(3 hunks)crates/transaction/src/v1/unsealed.rs(4 hunks)crates/transaction/src/v1/unsigned.rs(1 hunks)crates/transaction_manifest/src/generator.rs(2 hunks)crates/transaction_manifest/tests/parser.rs(2 hunks)crates/wallet/crypto/src/balance_proof.rs(1 hunks)crates/wallet/crypto/src/bullet_proof.rs(1 hunks)crates/wallet/crypto/src/confidential.rs(3 hunks)crates/wallet/crypto/src/stealth.rs(7 hunks)crates/wallet/crypto/src/unblinded_statement.rs(3 hunks)crates/wallet/crypto/tests/stealth_transfer_statement.rs(6 hunks)crates/wallet/crypto/tests/viewable_balance_proof.rs(1 hunks)crates/wallet/sdk/src/apis/confidential_crypto.rs(3 hunks)crates/wallet/sdk/src/apis/confidential_transfer.rs(3 hunks)crates/wallet/sdk/src/apis/key_manager.rs(10 hunks)crates/wallet/sdk/src/apis/signer.rs(1 hunks)crates/wallet/sdk/src/apis/stealth_crypto.rs(4 hunks)crates/wallet/sdk/src/apis/stealth_outputs.rs(9 hunks)crates/wallet/sdk/src/apis/stealth_transfer/api.rs(10 hunks)crates/wallet/sdk/src/apis/stealth_transfer/params.rs(4 hunks)crates/wallet/sdk/src/apis/stealth_transfer/types.rs(2 hunks)crates/wallet/sdk/src/models/account.rs(1 hunks)crates/wallet/sdk/src/models/key.rs(3 hunks)integration_tests/src/wallet_daemon_client.rs(3 hunks)
⛔ Files not processed due to max files limit (3)
- utilities/tariswap_test_bench/src/accounts.rs
- utilities/tariswap_test_bench/src/tariswap.rs
- utilities/traffic-sim/src/sim.rs
💤 Files with no reviewable changes (8)
- bindings/src/tari-indexer-client.ts
- bindings/src/types/StealthInputsStatement.ts
- crates/engine/src/runtime/error.rs
- bindings/src/types/tari-indexer-client/GetTemplateDefinitionRequest.ts
- crates/engine_types/src/hashing.rs
- crates/consensus_types/src/certificates/quorum_certificate.rs
- crates/p2p/proto/transaction.proto
- bindings/src/wallet-daemon-client.ts
✅ Files skipped from review due to trivial changes (1)
- crates/p2p/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (31)
- crates/template_test_tooling/src/support/mod.rs
- crates/transaction/src/v1/unsigned.rs
- applications/tari_walletd/src/handlers/validator.rs
- crates/engine_types/src/resource_container.rs
- crates/template_test_tooling/src/support/confidential.rs
- crates/engine/tests/asserts.rs
- crates/engine/tests/account.rs
- crates/template_lib_types/src/misc.rs
- crates/engine/tests/composability.rs
- crates/wallet/sdk/src/models/account.rs
- crates/p2p/proto/common.proto
- crates/template_lib_types/src/max_bytes.rs
- crates/p2p/proto/rpc.proto
- crates/engine/tests/tariswap.rs
- crates/transaction/src/builder/tests.rs
- crates/engine/tests/signature.rs
- crates/template_lib_types/src/max_string.rs
- crates/template_lib_types/src/bytes.rs
- crates/wallet/crypto/src/unblinded_statement.rs
- crates/template_lib_types/src/lib.rs
- applications/tari_wallet_cli/src/command/transaction.rs
- crates/wallet/crypto/src/balance_proof.rs
- crates/template_test_tooling/src/template_test.rs
- crates/engine/src/runtime/impl.rs
- crates/template_builtin/templates/faucet/src/lib.rs
- crates/engine_types/src/limits.rs
- crates/engine/src/runtime/actions.rs
- crates/engine/tests/events.rs
- crates/transaction/Cargo.toml
- crates/wallet/sdk/src/apis/confidential_transfer.rs
- crates/engine/tests/test.rs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-04T10:10:24.258Z
Learnt from: sdbondi
Repo: tari-project/tari-ootle PR: 1629
File: applications/tari_walletd/src/handlers/accounts.rs:1001-1002
Timestamp: 2025-11-04T10:10:24.258Z
Learning: In applications/tari_walletd/src/handlers/accounts.rs, the expect() on Memo::new_pay_ref_and_bytes_truncate at line 1002 is safe and intentional. PayRef is validated to be at most 64 bytes during address decoding (PayRef::MAX_LEN = 64), and the function only returns None if payref exceeds 252 bytes (Memo::MAX_BYTES_LENGTH - 1). Since 64 < 252, None is impossible with a valid PayRef.
Applied to files:
applications/tari_walletd/src/handlers/nfts.rscrates/engine/tests/stealth.rscrates/wallet/sdk/src/apis/stealth_outputs.rs
🧬 Code graph analysis (27)
crates/engine/tests/templates/stealth/src/lib.rs (3)
bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)bindings/src/types/StealthTransferStatement.ts (1)
StealthTransferStatement(6-14)crates/template_lib/src/resource/manager.rs (1)
get(104-106)
crates/template_test_tooling/src/read_only_state_store.rs (3)
bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)bindings/src/types/Utxo.ts (1)
Utxo(4-4)bindings/src/types/SubstateId.ts (1)
SubstateId(6-6)
bindings/src/types/RequireRule.ts (1)
bindings/src/types/RuleRequirement.ts (1)
RuleRequirement(10-14)
bindings/src/types/UtxoOutput.ts (2)
bindings/src/types/PrivateOutput.ts (1)
PrivateOutput(6-11)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)
crates/wallet/sdk/src/apis/stealth_transfer/params.rs (3)
bindings/src/types/AccessRule.ts (1)
AccessRule(7-7)bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)bindings/src/types/Amount.ts (1)
Amount(12-12)
applications/tari_walletd/src/handlers/transaction.rs (2)
crates/transaction/src/transaction.rs (2)
builder(47-49)signatures(99-103)crates/transaction/src/builder/mod.rs (1)
signatures(432-434)
crates/engine_types/src/stealth/outputs.rs (3)
bindings/src/types/StealthOutputsStatement.ts (1)
StealthOutputsStatement(9-24)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)bindings/src/types/UtxoOutput.ts (1)
UtxoOutput(6-6)
bindings/src/types/StealthUnspentOutput.ts (2)
bindings/src/types/UnspentOutput.ts (1)
UnspentOutput(18-33)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)
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/engine_types/src/crypto/messages.rs (1)
bindings/src/types/PedersenCommitmentBytes.ts (1)
PedersenCommitmentBytes(6-6)
crates/engine/src/runtime/tracker_auth.rs (2)
bindings/src/types/AccessRule.ts (1)
AccessRule(7-7)bindings/src/types/RequireRule.ts (1)
RequireRule(7-11)
crates/template_lib/src/models/component.rs (2)
crates/template_lib/src/models/resource.rs (1)
new(43-45)crates/template_lib/src/models/vault.rs (2)
new(73-75)try_from(121-124)
integration_tests/src/wallet_daemon_client.rs (5)
bindings/src/types/UtxoInputSelection.ts (1)
UtxoInputSelection(3-3)crates/engine/src/runtime/impl.rs (1)
stealth_transfer(2809-2825)crates/engine/src/runtime/mod.rs (1)
stealth_transfer(204-209)crates/engine/src/transaction/processor.rs (1)
stealth_transfer(385-408)crates/transaction/src/builder/mod.rs (1)
stealth_transfer(204-206)
crates/template_lib/src/models/unspent_output.rs (4)
bindings/src/types/AccessRule.ts (1)
AccessRule(7-7)bindings/src/types/ViewableBalanceProof.ts (1)
ViewableBalanceProof(27-62)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)
crates/wallet/crypto/tests/stealth_transfer_statement.rs (3)
crates/wallet/crypto/src/stealth.rs (1)
create_transfer_statement(28-98)crates/template_lib_types/src/encrypted_data.rs (1)
empty(27-29)crates/template_lib/src/models/stealth.rs (1)
new(59-69)
applications/tari_walletd/src/handlers/accounts.rs (4)
crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(71-73)bindings/src/types/Memo.ts (1)
Memo(3-3)crates/template_lib/src/models/bucket.rs (1)
stealth_transfer(118-133)crates/wallet/sdk/src/models/key.rs (2)
derived(360-362)new(309-311)
crates/wallet/sdk/src/apis/stealth_transfer/api.rs (4)
bindings/src/types/SubstateId.ts (1)
SubstateId(6-6)bindings/src/types/AccountWithAddress.ts (1)
AccountWithAddress(5-5)bindings/src/types/OutputStatus.ts (1)
OutputStatus(3-3)crates/wallet/sdk/src/apis/stealth_outputs.rs (1)
params(704-717)
crates/wallet/sdk/src/apis/key_manager.rs (2)
crates/wallet/crypto/src/encryption.rs (2)
decrypt_with_password(30-98)encrypt_with_password(100-134)crates/wallet/sdk/src/key_managers/backend.rs (1)
sign(25-39)
crates/template_lib/src/models/stealth.rs (4)
bindings/src/types/PedersenCommitmentBytes.ts (1)
PedersenCommitmentBytes(6-6)bindings/src/types/RangeProofBytes.ts (1)
RangeProofBytes(9-9)bindings/src/types/StealthInput.ts (1)
StealthInput(7-12)bindings/src/types/StealthInputsStatement.ts (1)
StealthInputsStatement(8-17)
crates/engine/tests/stealth.rs (2)
crates/template_test_tooling/src/support/stealth.rs (4)
generate_mint_statement(45-72)outputs(189-220)generate_transfer_data(122-137)generate_transfer_data_with_view_key(139-161)crates/template_lib/src/models/stealth.rs (1)
new(59-69)
crates/wallet/crypto/src/stealth.rs (5)
crates/wallet/crypto/src/balance_proof.rs (1)
generate_stealth_balance_proof_signature(38-55)crates/wallet/crypto/src/viewable_balance_proof.rs (1)
create_viewable_balance_proof(22-91)crates/template_lib/src/models/stealth.rs (1)
revealed_output_amount(101-103)bindings/src/types/StealthInput.ts (1)
StealthInput(7-12)crates/engine_types/src/stealth/outputs.rs (1)
validate_stealth_outputs_statement(34-83)
crates/wallet/sdk/src/apis/signer.rs (1)
crates/wallet/sdk/src/apis/key_manager.rs (2)
sign_with_stealth_key(352-360)sign_with_explicit_key(384-394)
crates/transaction/src/unsigned_transaction.rs (3)
crates/transaction/src/v1/unsealed.rs (5)
add_signer(54-58)new(34-39)signatures(81-83)into_signed(150-155)into_signed(161-163)crates/transaction/src/v1/signature.rs (6)
new(27-29)new(82-84)signature(54-56)signature(113-115)public_key(58-60)public_key(117-119)crates/common_types/src/signable.rs (1)
into_signed(15-15)
crates/template_test_tooling/src/support/stealth.rs (3)
bindings/src/types/StealthOutputsStatement.ts (1)
StealthOutputsStatement(9-24)bindings/src/types/StealthTransferStatement.ts (1)
StealthTransferStatement(6-14)crates/template_lib/src/models/stealth.rs (3)
revealed_output_amount(101-103)revealed_input_amount(97-99)new(59-69)
crates/wallet/sdk/src/apis/stealth_outputs.rs (8)
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(6-14)bindings/src/types/UtxoAddress.ts (1)
UtxoAddress(4-4)bindings/src/types/PedersenCommitmentBytes.ts (1)
PedersenCommitmentBytes(6-6)crates/template_lib/src/models/bucket.rs (1)
stealth_transfer(118-133)crates/template_lib/src/models/unspent_output.rs (1)
signed_by(56-61)crates/wallet/sdk/src/apis/stealth_crypto.rs (1)
generate_transfer_statement(48-75)
clients/wallet_daemon_client/src/types.rs (1)
bindings/src/types/StealthTransferStatement.ts (1)
StealthTransferStatement(6-14)
crates/wallet/sdk/src/apis/stealth_transfer/types.rs (5)
crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(71-73)crates/engine/src/runtime/impl.rs (1)
stealth_transfer(2809-2825)crates/engine/src/runtime/mod.rs (1)
stealth_transfer(204-209)crates/engine/src/transaction/processor.rs (1)
stealth_transfer(385-408)crates/transaction/src/builder/mod.rs (1)
stealth_transfer(204-206)
⏰ 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 stable
- GitHub Check: check nightly
- GitHub Check: clippy
- GitHub Check: test
- GitHub Check: machete
233765e to
902ff3c
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/p2p/src/conversions/transaction.rs (1)
65-72: Critical: Encoding failure causes panic in infallible conversion.The
Fromtrait implementation at line 69 usesexpect(), which will panic ifencode_to_vecfails. This violates the trait's infallible contract and can crash the node in production when converting transactions for P2P transmission.Apply one of these fixes:
Option 1: Change to TryFrom (recommended)
-impl From<&Transaction> for proto::transaction::Transaction { +impl TryFrom<&Transaction> for proto::transaction::Transaction { + type Error = anyhow::Error; + - fn from(transaction: &Transaction) -> Self { + fn try_from(transaction: &Transaction) -> Result<Self, Self::Error> { - proto::transaction::Transaction { - // TODO: no panic - bor_encoded: encode_to_vec(transaction).expect("Failed to encode transaction"), - } + Ok(proto::transaction::Transaction { + bor_encoded: encode_to_vec(transaction)?, + }) } }Option 2: Guarantee encoding success
If Transaction encoding is guaranteed to succeed by design, document this invariant and replace
expect()withunwrap_or_elsethat logs the error before panicking to aid debugging.
♻️ Duplicate comments (5)
crates/engine_types/src/limits.rs (1)
55-58: Reconcilemax_m_of_n_signaturesvalue with the "32 KiB" commentEchoing the earlier review:
max_m_of_n_signatures: 1024labelled as “32 KiB” is ambiguous/misleading unless each signature is assumed to be 32 bytes. For common 64‑byte signatures, 1024 entries would be ~64 KiB. Please either:
- Adjust the comment to state the assumed bytes‑per‑signature and resulting total, or
- Change the numeric limit to align with the intended 32 KiB budget.
This will make the limit’s intent clear to future readers.
crates/template_lib_types/src/max_vec.rs (2)
6-7: Align Borsh deserialization with the serde length checkRight now the type derives
borsh::BorshSerializebut only has a customserde::Deserializeenforcinglen <= N. That means any futureBorshDeserialize(e.g. a simple derive added later) could accidentally bypass the length check and allow constructing invalidMaxVecinstances.To keep invariants consistent across formats, consider adding a custom
BorshDeserializeimpl forMaxVec<N, T>(gated on theborshfeature) that mirrors the serde implementation and the existingMaxStringpattern: deserialize intoVec<T>, checklenagainstN, and return a descriptive error if it exceeds the maximum.Also applies to: 99-107
31-39: Fixnew_uncheckedsafety docs and consider narrowing visibilityThe safety comment still refers to
bytesinstead ofelems, which is confusing for an unsafe API. Also, if this constructor is only intended for tests, its visibility could be reduced.- /// # Safety - /// The caller must ensure that the length of `bytes` is less than or equal to `N`. + /// # Safety + /// The caller must ensure that the length of `elems` is less than or equal to `N`.If
new_uncheckedis not required outside this crate, consider making itpub(crate)or gating it undercfg(test)to reduce the chance of breaking the invariant in production code.clients/wallet_daemon_client/src/types.rs (1)
1111-1116: Document howutxo_signersaligns withstatements/signing_keys(length, order, M-of-N)
utxo_signers: Vec<StealthUtxoSpendKeyId>is a useful addition for UTXO-level signing, but from the type alone it’s unclear:
- whether
utxo_signers.len()is expected to matchstatements.len()(or some other collection), and- how positions map in M-of-N scenarios and multi-input/multi-output statements.
Suggest adding Rust doc comments (and mirroring this in API docs/TS bindings) spelling out the invariants and mapping rules to avoid client-side wiring mistakes.
crates/wallet/sdk/src/apis/stealth_outputs.rs (1)
258-273: Spend condition classification is reasonable; ensure downstream “spendable” views useis_condition_spendableThe new
is_spendable_condition/is_spendable_access_rulehelpers and theis_condition_spendableflag onStealthOutputModelare a good way to distinguish simpleSigned/AllowAllconditions from complexRestrictedrules. Invalidate_utxo, you still treat anySpendConditionwheresigned_by()returnsNoneas matching the derived stealth address:if output .spend_condition .signed_by() .is_none_or(|pk| *pk == stealth_address) { // Unspent } else { // Invalid }With the new flag, this means complex access rules will be stored as
OutputStatus::Unspentbut withis_condition_spendable == false, which is a sensible split as long as:
- input selection and “spendable balance” calculations only consider outputs where
is_condition_spendableis true, and- the UI clearly distinguishes “owned but not directly spendable by this wallet alone” from normal unspent funds.
Can you confirm that all places which compute selectable inputs or “available balance” use
is_condition_spendable(and not justOutputStatus::Unspent) when deciding what is actually spendable? Otherwise, complex access‑rule outputs could still make the wallet look more spendable than it really is.Also applies to: 577-601, 632-652
🧹 Nitpick comments (14)
applications/tari_walletd/web_ui/src/routes/StealthUtxoList/components/StatusChip.tsx (1)
15-19: Apply tooltip consistently in both Chip and Avatar branchesThe new
tooltipprop is wired into theChipviatitle={tooltip}, but whenshowTitleisfalsethe component returns only anAvatarand ignorestooltip. Callers passing a spend-condition tooltip will see it only whenshowTitleis true, which is surprising and inconsistent.Consider also applying
title={tooltip}to theAvatarin the no-title branch:let bgColor = colorList[status]; let background = null; if (!showTitle) { - return <Avatar sx={{ bgcolor: bgColor, height: 22, width: 22 }}>{iconList[status]}</Avatar>; + return ( + <Avatar + title={tooltip} + sx={{ bgcolor: bgColor, height: 22, width: 22 }} + > + {iconList[status]} + </Avatar> + ); } else { return ( <Chip - title={tooltip} + title={tooltip} avatar={<Avatar sx={{ bgcolor: bgColor, background: background }}>{iconList[status]}</Avatar>} label={status} style={{ color: colorList[status], borderColor: colorList[status] }} variant="outlined" /> ); }Also applies to: 29-29, 43-49
crates/engine_types/src/limits.rs (1)
48-52: Newpubfield changesStealthLimitsAPI surfaceAdding
pub max_m_of_n_signatures: usizemakes the struct layout change source‑breaking for any external crates constructingStealthLimitswith struct literals. Given this is a breaking PR, that may be acceptable, but if consumers aren’t meant to tweak this limit directly, consider either narrowing the visibility (e.g.,pub(crate)) or introducing a constructor/Defaultto decouple future limit changes from the public struct shape.crates/engine_types/src/crypto/messages.rs (1)
59-65: Clarify commitment representation invalue_proof_message(optional)The use of
PedersenCommitmentByteshere is consistent with the current API, but the coexistence of bothPedersenCommitmentandPedersenCommitmentBytesin the same module can be easy to mix up. Consider adding a brief doc comment onvalue_proof_messageclarifying that the commitment must be the canonical serialized bytes (as opposed to a group element), to prevent accidental misuse in future refactors.clients/wallet_daemon_client/src/types.rs (2)
1134-1142: Clarify/defaultpay_tousage for common stealth transfer flowsMaking
pay_to: PayTomandatory onStealthTransfermakes the access-rule target explicit, which is good for flexibility, but every client now has to choose a variant correctly. If there’s a canonical “simple transfer” pattern (e.g., pay to the recipient with the default spend condition), consider:
- documenting that pattern prominently, and/or
- exposing a helper constructor or builder on the client side that fills in the typical
PayTovalue.That would reduce friction and misconfiguration risk when upgrading straightforward stealth flows to this new API.
1176-1183: Confirm how “no custom access rule” is represented inUtxoInfo.spend_condition
UtxoInfonow exposes a non-optionalspend_condition: SpendCondition, yet the feature is described as “optional access-rule based UTXO spending”. For older/simple UTXOs with no extra rule, this likely relies on a sentinelSpendConditionvariant (e.g., “default/none/owner-only”).Please confirm and, if so, document that convention so clients know how to interpret it. If there truly are UTXOs with no meaningful condition at this layer, consider
Option<SpendCondition>instead to make that absence explicit.applications/tari_walletd/web_ui/src/routes/StealthUtxoList/StealthUtxoList.tsx (1)
110-110: Consider formatting the spend condition tooltip for better readability.The spend condition is currently displayed using
JSON.stringify, which may be difficult for users to read. Consider formatting it with proper indentation or creating a custom formatter forSpendConditiontypes.For better readability, you could use:
- <StatusChip status={utxo.status} tooltip={JSON.stringify(utxo.spend_condition)} /> + <StatusChip status={utxo.status} tooltip={JSON.stringify(utxo.spend_condition, null, 2)} />Or create a dedicated formatter function for
SpendConditionobjects.crates/engine/src/runtime/tracker_auth.rs (1)
175-213: Clarify intended semantics forRequireRule::MOfNwhenn == 0The
MOfNbranch correctly enforces “at least n of these requirements are satisfied” forn > 0. Forn == 0, this implementation always returnsfalse(even for an empty requirement list), which may or may not match the intended semantics. If0-of-Nshould be vacuously true, consider special-casingn == 0to returnOk(true)early.crates/transaction/src/v1/instruction.rs (1)
15-20: Strengthening function/method names withFunctionNamelooks goodUsing
FunctionNameforCallFunction.functionandCallMethod.methodtightens validation while keeping the external JSON/TS surface as plain strings via the TS annotation. The encode/decode test for aCallFunctioninstruction confirms serde compatibility; you may optionally add a similar test for aCallMethodinstruction for symmetry.Also applies to: 31-56, 126-166, 260-277
crates/transaction/src/unsigned_transaction.rs (1)
133-147: Unnecessarymutin pattern matching.Both
add_signerandadd_signaturetakemut selfbut only use&mut selfin the match, then clone the inner value. Since you're consumingselfand cloning anyway, you can simplify:- pub fn add_signer( - mut self, + pub fn add_signer( + self, seal_signer: &RistrettoPublicKeyBytes, key: &RistrettoSecretKey, ) -> UnsealedTransactionV1 { - match &mut self { - Self::V1(tx) => UnsealedTransactionV1::new(tx.clone(), vec![]).add_signer(seal_signer, key), + match self { + Self::V1(tx) => UnsealedTransactionV1::new(tx, vec![]).add_signer(seal_signer, key), } } - pub fn add_signature(mut self, signature: TransactionSignature) -> UnsealedTransactionV1 { - match &mut self { - Self::V1(tx) => UnsealedTransactionV1::new(tx.clone(), vec![signature]), + pub fn add_signature(self, signature: TransactionSignature) -> UnsealedTransactionV1 { + match self { + Self::V1(tx) => UnsealedTransactionV1::new(tx, vec![signature]), } }crates/template_lib/src/models/stealth.rs (1)
38-75: Stealth input invariants look sound; consider tightening docs around zero-amount cases
StealthInputsStatement::newcorrectly enforces a non‑negative revealed amount and that you don’t create a completely empty statement (inputs.is_empty() && revealed_amount.is_zero()).new_revealed_onlyinheriting that check is fine, but it does mean zero‑amount revealed‑only input statements will panic — if that’s intentional, it would help to spell it out in the doc comment.Also, the comment on
StealthInput(“statement for stealth outputs to spend as inputs”) is a bit confusing; consider rewording to “stealth inputs” for clarity.Also applies to: 88-95
crates/wallet/crypto/src/stealth.rs (1)
28-98: Stealth transfer construction is consistent; consider reusinginputs_statementto avoid duplicationThe new
create_transfer_statement/create_outputs_statementflow correctly:
- Converts
StealthInputWitnessintoStealthInputusingto_commitment().to_byte_type().- Aggregates input/output masks over ExactSizeIterator-backed iterators.
- Skips the balance proof when both
num_inputsandnum_outputsare zero, while still passing full statements intogenerate_stealth_balance_proof_signature.One small clean‑up: you construct
StealthInputsStatementonce forbalance_proofand then re‑create it inline in the finalStealthTransferStatement. Reusing the sameinputs_statementvalue would reduce duplication and keep invariants in one place:- let inputs_statement = StealthInputsStatement { - inputs: inputs_to_spend.clone(), - revealed_amount: revealed_input_amount, - }; + let inputs_statement = StealthInputsStatement { + inputs: inputs_to_spend.clone(), + revealed_amount: revealed_input_amount, + }; … - Ok(StealthTransferStatement { - inputs_statement: StealthInputsStatement { - inputs: inputs_to_spend, - revealed_amount: revealed_input_amount, - }, - outputs_statement, - balance_proof, - }) + Ok(StealthTransferStatement { + inputs_statement, + outputs_statement, + balance_proof, + })Functionally it’s identical, just a bit clearer.
Also applies to: 100-140, 156-174
crates/wallet/sdk/src/apis/key_manager.rs (1)
198-213: Stealth owner key derivation and signing helpers look good; confirmKeyIdcopy semantics insign_with_contextThe new pieces here line up well:
generate_stealth_owner_keycleanly converts the public nonce fromRistrettoPublicKeyBytes, derives the account key, and delegates toStealthCryptoApi::derive_stealth_owner_secret, surfacing parse errors viaInvalidKeyId.sign_with_stealth_key,sign_with_context, andsign_with_explicit_keygive a coherent API: derived keys go through the keystore’ssign, while imported/explicit keys reuse the same Schnorr signing logic and return a uniformSignatureOutput.KeyManagerApiError::key_store_errorcentralizes keystore error wrapping, which simplifies the variousmap_errsites.One thing to double‑check: in
sign_with_contextyoumatch &key_idand then callself.get_key(key_id)?in the non‑derived branch. This pattern depends onKeyIdbeingCopy; if it ever stops beingCopy, moving it while it’s borrowed by&key_idwill fail to compile. If you want this to be robust regardless ofKeyId’s traits, you could instead match onkey_idby value and clone in the non‑derived branch, or take&KeyIdas a parameter and avoid moving it at all.Also applies to: 337-350, 352-395, 410-436
crates/wallet/sdk/src/apis/signer.rs (1)
18-37: Context‑awareSignerApiis well‑structured; note theCtx: CopyconstraintThe refactor to
SignerApi<'a, TSpec, Ctx>with:
with_contextto bind a signing context,generate_signature/generate_stealth_key_signaturereturningSignatureOutput, andsign*helpers applying signatures viaIntoSigned<Ctx>is clean and lines up with the new key manager capabilities.
Requiring
Ctx: Copyon the main impl keeps usage simple for small contexts (e.g.&RistrettoPublicKeyBytes), but it does mean you can’t use heavier, non‑Copy context types. If you foresee needing owned contexts (e.g. structs or vectors), you might consider loosening this toCtx: Cloneand cloning into the key manager calls instead.Also applies to: 39-92, 94-106
crates/transaction/src/builder/mod.rs (1)
88-92:panic_if_signedis a good safety net; consider covering network/dry‑run as wellAdding
panic_if_signedand invoking it from the various mutating builders (with_fee_instructions*,add_fee_instruction,add_instruction,with_instructions,add_input,with_inputs,with_min_epoch,with_max_epoch, andwith_authorized_seal_signer) is a solid way to avoid silently producing transactions whose signatures no longer match the payload.If
networkanddry_runparticipate in the signing message (which they do via the unsigned transaction), you might also want to callpanic_if_signedfromfor_networkandwith_dry_runto enforce the same “no mutation after signing” rule there.Also applies to: 321-372, 378-390, 436-443
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (107)
applications/tari_app_utilities/src/transaction_executor.rs(1 hunks)applications/tari_validator_node/src/transaction_validators/network.rs(1 hunks)applications/tari_validator_node_cli/src/command/transaction.rs(2 hunks)applications/tari_wallet_cli/src/command/transaction.rs(2 hunks)applications/tari_walletd/src/handlers/accounts.rs(15 hunks)applications/tari_walletd/src/handlers/confidential.rs(4 hunks)applications/tari_walletd/src/handlers/nfts.rs(2 hunks)applications/tari_walletd/src/handlers/stealth_utxos.rs(1 hunks)applications/tari_walletd/src/handlers/transaction.rs(4 hunks)applications/tari_walletd/src/handlers/validator.rs(1 hunks)applications/tari_walletd/web_ui/src/routes/StealthUtxoList/StealthUtxoList.tsx(1 hunks)applications/tari_walletd/web_ui/src/routes/StealthUtxoList/components/StatusChip.tsx(3 hunks)applications/tari_walletd/web_ui/src/services/api/hooks/useAccounts.ts(2 hunks)bindings/src/index.ts(3 hunks)bindings/src/tari-indexer-client.ts(0 hunks)bindings/src/types/Account.ts(1 hunks)bindings/src/types/Bytes.ts(1 hunks)bindings/src/types/RequireRule.ts(1 hunks)bindings/src/types/SpendCondition.ts(1 hunks)bindings/src/types/StealthInput.ts(1 hunks)bindings/src/types/StealthInputsStatement.ts(0 hunks)bindings/src/types/StealthUnspentOutput.ts(1 hunks)bindings/src/types/UtxoOutput.ts(1 hunks)bindings/src/types/WalletTransaction.ts(1 hunks)bindings/src/types/tari-indexer-client/GetTemplateDefinitionRequest.ts(0 hunks)bindings/src/types/wallet-types/AccountsCreateStealthTransferStatementResponse.ts(1 hunks)bindings/src/types/wallet-types/PayTo.ts(1 hunks)bindings/src/types/wallet-types/StealthTransfer.ts(1 hunks)bindings/src/types/wallet-types/StealthUtxoSpendKeyId.ts(1 hunks)bindings/src/types/wallet-types/TransferOutput.ts(2 hunks)bindings/src/types/wallet-types/UtxoInfo.ts(1 hunks)bindings/src/wallet-daemon-client.ts(0 hunks)bindings/src/wallet-types.ts(1 hunks)clients/wallet_daemon_client/src/component_address.rs(1 hunks)clients/wallet_daemon_client/src/types.rs(46 hunks)crates/common_types/src/signable.rs(1 hunks)crates/consensus_types/src/certificates/quorum_certificate.rs(0 hunks)crates/engine/src/runtime/actions.rs(1 hunks)crates/engine/src/runtime/auth.rs(1 hunks)crates/engine/src/runtime/error.rs(0 hunks)crates/engine/src/runtime/impl.rs(4 hunks)crates/engine/src/runtime/scope.rs(3 hunks)crates/engine/src/runtime/tracker_auth.rs(7 hunks)crates/engine/src/runtime/working_state.rs(5 hunks)crates/engine/tests/account.rs(1 hunks)crates/engine/tests/asserts.rs(1 hunks)crates/engine/tests/composability.rs(3 hunks)crates/engine/tests/events.rs(2 hunks)crates/engine/tests/signature.rs(4 hunks)crates/engine/tests/stealth.rs(22 hunks)crates/engine/tests/tariswap.rs(2 hunks)crates/engine/tests/templates/stealth/src/lib.rs(1 hunks)crates/engine/tests/test.rs(3 hunks)crates/engine_types/src/crypto/messages.rs(1 hunks)crates/engine_types/src/hashing.rs(0 hunks)crates/engine_types/src/limits.rs(1 hunks)crates/engine_types/src/resource_container.rs(2 hunks)crates/engine_types/src/stealth/outputs.rs(2 hunks)crates/engine_types/src/stealth/transfer.rs(1 hunks)crates/engine_types/src/utxo.rs(3 hunks)crates/p2p/proto/common.proto(1 hunks)crates/p2p/proto/rpc.proto(1 hunks)crates/p2p/proto/transaction.proto(0 hunks)crates/p2p/src/conversions/common.rs(3 hunks)crates/p2p/src/conversions/transaction.rs(1 hunks)crates/p2p/src/lib.rs(1 hunks)crates/template_builtin/templates/faucet/src/lib.rs(2 hunks)crates/template_lib/src/auth/access_rules.rs(8 hunks)crates/template_lib/src/models/component.rs(2 hunks)crates/template_lib/src/models/stealth.rs(5 hunks)crates/template_lib/src/models/unspent_output.rs(2 hunks)crates/template_lib_types/src/bytes.rs(2 hunks)crates/template_lib_types/src/lib.rs(2 hunks)crates/template_lib_types/src/max_bytes.rs(1 hunks)crates/template_lib_types/src/max_string.rs(4 hunks)crates/template_lib_types/src/max_vec.rs(1 hunks)crates/template_lib_types/src/misc.rs(1 hunks)crates/template_test_tooling/src/read_only_state_store.rs(1 hunks)crates/template_test_tooling/src/support/confidential.rs(4 hunks)crates/template_test_tooling/src/support/mod.rs(1 hunks)crates/template_test_tooling/src/support/spec.rs(1 hunks)crates/template_test_tooling/src/support/stealth.rs(7 hunks)crates/template_test_tooling/src/template_test.rs(3 hunks)crates/transaction/Cargo.toml(1 hunks)crates/transaction/src/builder/mod.rs(11 hunks)crates/transaction/src/builder/tests.rs(2 hunks)crates/transaction/src/unsigned_transaction.rs(3 hunks)crates/transaction/src/v1/instruction.rs(3 hunks)crates/transaction/src/v1/signature.rs(3 hunks)crates/transaction/src/v1/unsealed.rs(4 hunks)crates/transaction/src/v1/unsigned.rs(1 hunks)crates/transaction_manifest/src/generator.rs(2 hunks)crates/transaction_manifest/tests/parser.rs(2 hunks)crates/wallet/crypto/src/balance_proof.rs(1 hunks)crates/wallet/crypto/src/bullet_proof.rs(1 hunks)crates/wallet/crypto/src/confidential.rs(3 hunks)crates/wallet/crypto/src/stealth.rs(7 hunks)crates/wallet/crypto/src/unblinded_statement.rs(3 hunks)crates/wallet/crypto/tests/stealth_transfer_statement.rs(6 hunks)crates/wallet/crypto/tests/viewable_balance_proof.rs(1 hunks)crates/wallet/sdk/src/apis/confidential_crypto.rs(3 hunks)crates/wallet/sdk/src/apis/confidential_transfer.rs(3 hunks)crates/wallet/sdk/src/apis/key_manager.rs(10 hunks)crates/wallet/sdk/src/apis/signer.rs(1 hunks)crates/wallet/sdk/src/apis/stealth_crypto.rs(4 hunks)crates/wallet/sdk/src/apis/stealth_outputs.rs(12 hunks)crates/wallet/sdk/src/apis/stealth_transfer/api.rs(11 hunks)
⛔ Files not processed due to max files limit (15)
- crates/wallet/sdk/src/apis/stealth_transfer/params.rs
- crates/wallet/sdk/src/apis/stealth_transfer/types.rs
- crates/wallet/sdk/src/key_managers/backend.rs
- crates/wallet/sdk/src/models/account.rs
- crates/wallet/sdk/src/models/key.rs
- crates/wallet/sdk/src/models/stealth_output.rs
- crates/wallet/storage_sqlite/migrations/2023-02-08-122514_initial/up.sql
- crates/wallet/storage_sqlite/src/models/stealth_output.rs
- crates/wallet/storage_sqlite/src/reader.rs
- crates/wallet/storage_sqlite/src/schema.rs
- crates/wallet/storage_sqlite/src/writer.rs
- integration_tests/src/wallet_daemon_client.rs
- utilities/tariswap_test_bench/src/accounts.rs
- utilities/tariswap_test_bench/src/tariswap.rs
- utilities/traffic-sim/src/sim.rs
💤 Files with no reviewable changes (8)
- bindings/src/tari-indexer-client.ts
- crates/engine_types/src/hashing.rs
- crates/consensus_types/src/certificates/quorum_certificate.rs
- bindings/src/types/StealthInputsStatement.ts
- crates/engine/src/runtime/error.rs
- bindings/src/types/tari-indexer-client/GetTemplateDefinitionRequest.ts
- crates/p2p/proto/transaction.proto
- bindings/src/wallet-daemon-client.ts
🚧 Files skipped from review as they are similar to previous changes (44)
- crates/transaction/Cargo.toml
- bindings/src/types/SpendCondition.ts
- crates/engine/src/runtime/actions.rs
- applications/tari_walletd/src/handlers/nfts.rs
- crates/template_test_tooling/src/support/confidential.rs
- bindings/src/types/StealthInput.ts
- bindings/src/types/RequireRule.ts
- crates/template_lib_types/src/bytes.rs
- bindings/src/types/wallet-types/TransferOutput.ts
- applications/tari_wallet_cli/src/command/transaction.rs
- crates/wallet/crypto/tests/viewable_balance_proof.rs
- bindings/src/types/wallet-types/StealthUtxoSpendKeyId.ts
- crates/engine/tests/templates/stealth/src/lib.rs
- bindings/src/types/wallet-types/AccountsCreateStealthTransferStatementResponse.ts
- applications/tari_app_utilities/src/transaction_executor.rs
- bindings/src/types/Bytes.ts
- crates/wallet/crypto/src/bullet_proof.rs
- applications/tari_walletd/src/handlers/validator.rs
- crates/engine_types/src/stealth/transfer.rs
- crates/p2p/proto/rpc.proto
- crates/template_lib/src/models/component.rs
- bindings/src/index.ts
- crates/transaction/src/v1/unsigned.rs
- crates/engine_types/src/resource_container.rs
- bindings/src/types/wallet-types/PayTo.ts
- crates/engine_types/src/utxo.rs
- applications/tari_validator_node/src/transaction_validators/network.rs
- crates/transaction/src/builder/tests.rs
- bindings/src/types/UtxoOutput.ts
- crates/wallet/sdk/src/apis/confidential_crypto.rs
- crates/template_lib_types/src/max_bytes.rs
- crates/template_lib_types/src/lib.rs
- crates/engine/tests/tariswap.rs
- crates/engine/tests/events.rs
- crates/p2p/proto/common.proto
- crates/transaction_manifest/tests/parser.rs
- applications/tari_walletd/src/handlers/confidential.rs
- bindings/src/types/wallet-types/StealthTransfer.ts
- crates/p2p/src/lib.rs
- applications/tari_validator_node_cli/src/command/transaction.rs
- crates/template_test_tooling/src/support/spec.rs
- applications/tari_walletd/web_ui/src/services/api/hooks/useAccounts.ts
- crates/engine/tests/asserts.rs
- bindings/src/wallet-types.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-04T10:10:24.258Z
Learnt from: sdbondi
Repo: tari-project/tari-ootle PR: 1629
File: applications/tari_walletd/src/handlers/accounts.rs:1001-1002
Timestamp: 2025-11-04T10:10:24.258Z
Learning: In applications/tari_walletd/src/handlers/accounts.rs, the expect() on Memo::new_pay_ref_and_bytes_truncate at line 1002 is safe and intentional. PayRef is validated to be at most 64 bytes during address decoding (PayRef::MAX_LEN = 64), and the function only returns None if payref exceeds 252 bytes (Memo::MAX_BYTES_LENGTH - 1). Since 64 < 252, None is impossible with a valid PayRef.
Applied to files:
crates/engine_types/src/limits.rscrates/engine/tests/stealth.rscrates/wallet/sdk/src/apis/stealth_outputs.rs
🧬 Code graph analysis (22)
crates/common_types/src/signable.rs (4)
crates/transaction/src/v1/unsealed.rs (2)
to_signing_message(134-136)to_signing_message(142-144)crates/transaction/src/builder/mod.rs (1)
to_signing_message(519-521)crates/transaction/src/v1/unsigned.rs (1)
to_signing_message(181-183)crates/transaction/src/unsigned_transaction.rs (1)
to_signing_message(182-186)
applications/tari_walletd/web_ui/src/routes/StealthUtxoList/StealthUtxoList.tsx (1)
applications/tari_walletd/web_ui/src/routes/StealthUtxoList/components/StatusChip.tsx (1)
StatusChip(29-56)
crates/engine/tests/stealth.rs (3)
crates/template_test_tooling/src/support/assert_error.rs (2)
assert_access_denied_for_action(20-24)assert_reject_reason(10-17)crates/template_test_tooling/src/support/stealth.rs (4)
generate_mint_statement(45-72)outputs(189-220)generate_transfer_data(122-137)generate_transfer_data_with_view_key(139-161)crates/template_lib/src/models/stealth.rs (1)
new(59-69)
crates/template_lib/src/models/unspent_output.rs (4)
bindings/src/types/AccessRule.ts (1)
AccessRule(7-7)bindings/src/types/ViewableBalanceProof.ts (1)
ViewableBalanceProof(27-62)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)
crates/wallet/crypto/src/unblinded_statement.rs (4)
bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)bindings/src/types/Memo.ts (1)
Memo(3-3)crates/engine_types/src/crypto/helpers.rs (1)
commit_u64_amount(74-76)
crates/engine_types/src/stealth/outputs.rs (3)
bindings/src/types/StealthOutputsStatement.ts (1)
StealthOutputsStatement(9-24)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)bindings/src/types/UtxoOutput.ts (1)
UtxoOutput(6-6)
crates/template_lib_types/src/max_string.rs (1)
crates/template_lib_types/src/max_bytes.rs (3)
new_checked(26-33)try_from(84-86)try_from(92-94)
crates/engine_types/src/crypto/messages.rs (1)
bindings/src/types/PedersenCommitmentBytes.ts (1)
PedersenCommitmentBytes(6-6)
crates/wallet/sdk/src/apis/stealth_crypto.rs (2)
crates/wallet/sdk/src/apis/key_manager.rs (1)
new(66-81)crates/template_lib/src/models/stealth.rs (1)
new(59-69)
crates/engine/tests/account.rs (1)
crates/storage_sqlite/src/global/models/template.rs (1)
try_into(63-77)
crates/engine/src/runtime/working_state.rs (4)
bindings/src/types/UtxoOutput.ts (1)
UtxoOutput(6-6)bindings/src/types/StealthInput.ts (1)
StealthInput(7-12)crates/engine_types/src/stealth/transfer.rs (1)
validate_transfer_balance(27-137)bindings/src/types/NonFungibleAddress.ts (1)
NonFungibleAddress(7-7)
crates/engine/tests/signature.rs (2)
crates/template_test_tooling/src/support/assert_error.rs (1)
assert_reject_reason(10-17)crates/template_test_tooling/src/support/stealth.rs (1)
generate_transfer_data(122-137)
crates/wallet/sdk/src/apis/stealth_transfer/api.rs (2)
bindings/src/types/SubstateId.ts (1)
SubstateId(6-6)bindings/src/types/OutputStatus.ts (1)
OutputStatus(3-3)
crates/template_lib/src/models/stealth.rs (4)
bindings/src/types/PedersenCommitmentBytes.ts (1)
PedersenCommitmentBytes(6-6)bindings/src/types/RangeProofBytes.ts (1)
RangeProofBytes(9-9)bindings/src/types/StealthInput.ts (1)
StealthInput(7-12)bindings/src/types/StealthInputsStatement.ts (1)
StealthInputsStatement(8-17)
crates/wallet/crypto/tests/stealth_transfer_statement.rs (2)
crates/wallet/crypto/src/stealth.rs (1)
create_transfer_statement(28-98)crates/template_lib/src/models/stealth.rs (1)
new(59-69)
crates/transaction/src/unsigned_transaction.rs (2)
crates/transaction/src/v1/unsealed.rs (8)
add_signer(54-58)new(34-39)add_signature(60-63)signatures(81-83)to_signing_message(134-136)to_signing_message(142-144)into_signed(150-155)into_signed(161-163)crates/transaction/src/builder/mod.rs (7)
add_signer(420-425)new(54-60)add_signature(427-430)signatures(432-434)finish(445-457)to_signing_message(519-521)into_signed(527-532)
applications/tari_walletd/src/handlers/transaction.rs (3)
crates/transaction/src/v1/unsealed.rs (1)
signatures(81-83)crates/transaction/src/v1/transaction.rs (1)
signatures(69-71)crates/transaction/src/builder/mod.rs (1)
signatures(432-434)
crates/wallet/sdk/src/apis/key_manager.rs (4)
crates/wallet/sdk/src/apis/stealth_crypto.rs (1)
new(36-38)crates/wallet/sdk/src/apis/signer.rs (3)
sign_with_stealth_key(65-76)sign_with_explicit_key(78-91)sign(58-63)crates/transaction/src/v1/signature.rs (6)
signature(54-56)signature(113-115)sign(31-41)public_key(58-60)public_key(117-119)from(148-159)crates/wallet/sdk/src/key_managers/backend.rs (1)
sign(25-39)
crates/wallet/sdk/src/apis/signer.rs (1)
crates/wallet/sdk/src/key_managers/backend.rs (1)
sign(25-39)
crates/template_test_tooling/src/support/stealth.rs (6)
bindings/src/types/StealthOutputsStatement.ts (1)
StealthOutputsStatement(9-24)bindings/src/types/StealthTransferStatement.ts (1)
StealthTransferStatement(6-14)crates/template_lib/src/models/stealth.rs (3)
revealed_output_amount(101-103)revealed_input_amount(97-99)new(59-69)crates/engine_types/src/crypto/elgamal.rs (3)
from(160-162)from(166-171)try_from(241-248)crates/template_lib_types/src/crypto/ristretto.rs (1)
try_from(69-71)crates/wallet/crypto/src/stealth.rs (1)
create_transfer_statement(28-98)
crates/transaction/src/builder/mod.rs (1)
crates/transaction/src/v1/unsigned.rs (1)
to_signing_message(181-183)
crates/wallet/sdk/src/apis/stealth_outputs.rs (5)
crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(71-73)bindings/src/types/AccessRule.ts (1)
AccessRule(7-7)bindings/src/types/StealthTransferStatement.ts (1)
StealthTransferStatement(6-14)bindings/src/types/OutputStatus.ts (1)
OutputStatus(3-3)crates/template_lib/src/models/unspent_output.rs (1)
signed_by(56-61)
⏰ 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). (4)
- GitHub Check: test
- GitHub Check: check stable
- GitHub Check: check nightly
- GitHub Check: clippy
| // Assert that it encodes to a BOR bytes value using the Bytes variant | ||
| let val: Value = tari_bor::decode(&serialized).unwrap(); | ||
| assert_eq!( | ||
| val, | ||
| Value::Array(vec![ | ||
| Value::Integer(1.into()), | ||
| Value::Integer(2.into()), | ||
| Value::Integer(3.into()), | ||
| Value::Integer(4.into()), | ||
| Value::Integer(5.into()), | ||
| ]) | ||
| ); |
There was a problem hiding this comment.
Update serde test comment to match the asserted BOR representation
The test asserts that the encoded value is a Value::Array([...]), but the comment still mentions encoding to a "BOR bytes value using the Bytes variant", which looks like a copy‑paste from a bytes-specific test.
You could make the comment match the behavior:
- // Assert that it encodes to a BOR bytes value using the Bytes variant
+ // Assert that it encodes to a BOR array value🤖 Prompt for AI Agents
In crates/template_lib_types/src/max_vec.rs around lines 141 to 152, the inline
test comment incorrectly states the encoding is a "BOR bytes value using the
Bytes variant" while the assertion checks for a Value::Array of integers; update
the comment to accurately describe that the encoded BOR value is an Array of
Integer values (e.g., "Assert that it encodes to a BOR Value::Array of
integers") so the comment matches the test assertion.
| pub fn get_all_utxos(&self) -> Result<Vec<(UtxoAddress, Utxo)>, StateStoreError> { | ||
| let mut utxos = Vec::new(); | ||
| self.with_substates(|id, substate| { | ||
| if let SubstateId::Utxo(utxo_addr) = id { | ||
| let utxo = substate.substate_value().as_utxo().unwrap(); | ||
| utxos.push((utxo_addr.clone(), utxo.clone())); | ||
| } | ||
| })?; | ||
| Ok(utxos) | ||
| } |
There was a problem hiding this comment.
Fix type of utxo_addr when pushing into utxos
Inside the with_substates closure, id is &SubstateId, so utxo_addr is a reference (likely &UtxoAddress). Calling .clone() on that reference clones the reference, not the underlying address, so
utxos.push((utxo_addr.clone(), utxo.clone()));attempts to push (&UtxoAddress, Utxo) into Vec<(UtxoAddress, Utxo)>, which will not compile.
Mirror the existing get_all_resources pattern and take an owned UtxoAddress from the reference:
- self.with_substates(|id, substate| {
- if let SubstateId::Utxo(utxo_addr) = id {
- let utxo = substate.substate_value().as_utxo().unwrap();
- utxos.push((utxo_addr.clone(), utxo.clone()));
- }
- })?;
+ self.with_substates(|id, substate| {
+ if let SubstateId::Utxo(utxo_addr) = id {
+ let utxo = substate.substate_value().as_utxo().unwrap();
+ // If `UtxoAddress` is `Copy`, this mirrors `get_all_resources`; if not, switch to
+ // `(*utxo_addr).clone()` instead.
+ utxos.push((*utxo_addr, utxo.clone()));
+ }
+ })?;If Utxo cloning becomes expensive with many entries, you may also want a streaming iterator-style API in future to avoid collecting/cloning all UTXOs at once, but that’s optional for this test tooling.
🤖 Prompt for AI Agents
In crates/template_test_tooling/src/read_only_state_store.rs around lines
95-104, the closure matches SubstateId by reference so utxo_addr is
&UtxoAddress; fix by taking an owned UtxoAddress before pushing (mirror
get_all_resources): extract an owned address (e.g. let addr =
(*utxo_addr).clone(); or let addr = utxo_addr.to_owned();) and then push (addr,
utxo.clone()) into utxos so the Vec has type Vec<(UtxoAddress, Utxo)>.
| // We assume that all inputs being spent require a signature. This is fine because we currently filter out | ||
| // inputs that have complex access rules from input selection. | ||
| let utxo_spend_keys = inputs_to_spend | ||
| .inputs | ||
| .iter() | ||
| .chain(&fee_inputs_to_spend.inputs) | ||
| .map(|i| StealthUtxoSpendKeyId { | ||
| account_key_id, | ||
| public_nonce: i.public_nonce, | ||
| }) | ||
| .collect(); |
There was a problem hiding this comment.
🧩 Analysis chain
Potential duplicate spend keys when fee and transfer inputs overlap.
The chain of inputs_to_spend.inputs.iter().chain(&fee_inputs_to_spend.inputs) may produce duplicate StealthUtxoSpendKeyId entries if the same input is used for both fee and transfer (e.g., when spending XTR that was locked as fee change). Consider whether duplicates are acceptable or if deduplication is needed.
🏁 Script executed:
#!/bin/bash
# Check if StealthUtxoSpendKeyId implements Hash/Eq for potential deduplication
ast-grep --pattern 'struct StealthUtxoSpendKeyId {
$$$
}'
# Check how utxo_spend_keys is consumed downstream
rg -n "utxo_spend_keys" --type rust -C3Length of output: 2669
I need to search the codebase for the actual implementation to verify the concern. Let me run shell scripts to investigate further.
🏁 Script executed:
#!/bin/bash
# Find StealthUtxoSpendKeyId struct definition
rg -n "struct StealthUtxoSpendKeyId" --type rust -B2 -A5
# Also look for pub struct variant
rg -n "pub struct StealthUtxoSpendKeyId" --type rust -B2 -A5Length of output: 1403
🏁 Script executed:
#!/bin/bash
# Find where inputs_to_spend and fee_inputs_to_spend are selected/populated
rg -n "inputs_to_spend\s*=" --type rust -C2 | head -80
# Check the struct InputsToSpend
rg -n "struct InputsToSpend" --type rust -B2 -A6Length of output: 2956
🏁 Script executed:
#!/bin/bash
# Check if inputs are deduplicated or if overlap is possible
rg -n "fee_inputs_to_spend" --type rust -C4 | head -100Length of output: 7729
Let me investigate the input selection logic more closely to understand if overlap actually occurs and what the implications are.
🏁 Script executed:
#!/bin/bash
# Find lock_inputs_for_transfer implementation to see if it can select fee change
rg -n "fn lock_inputs_for_transfer" --type rust -A20
# Find lock_fee_inputs implementation
rg -n "fn lock_fee_inputs" --type rust -A20Length of output: 3714
🏁 Script executed:
#!/bin/bash
# Check if there's any logic that prevents the same input from being in both collections
rg -n "is_on_chain" --type rust -C3 | head -60
# Look for InputSpendData structure to understand public_nonce
rg -n "struct InputSpendData" --type rust -B2 -A10Length of output: 5458
🏁 Script executed:
#!/bin/bash
# Check what sign_with_stealth_key does - would duplicate signing be a problem?
rg -n "fn sign_with_stealth_key" --type rust -A15
# Search for any existing deduplication logic
rg -n "\.collect::<HashSet" --type rust -C2Length of output: 13066
Let me check if there are tests or explicit comments about this behavior:
🏁 Script executed:
#!/bin/bash
# Search for tests related to fee change or duplicate input selection
rg -n "fee.*change.*spend" --type rust -i -C3
# Check if there's any explicit comment about duplicates or deduplication being intentional
rg -n "duplicate.*spend\|spend.*duplicate" --type rust -i -C3Length of output: 2116
🏁 Script executed:
#!/bin/bash
# Look for transaction signing tests to understand if duplicate signing is handled
rg -n "sign_with_stealth_key" --type rust -C5 | head -100Length of output: 3408
Based on my investigation, the original review comment's concern is valid and substantiated. Here are the findings:
Confirmed Issues:
-
Overlap is intentional and possible: Code comment at lines 429-430 explicitly states "important to add this after we add the fee change, because this allows us to spend the fee change UTXO (XTR case)" — confirming fee change UTXOs selected in
fee_inputs_to_spendcan also be selected ininputs_to_spend. -
No deduplication exists: The code chains iterators without filtering:
inputs_to_spend.inputs.iter().chain(&fee_inputs_to_spend.inputs), allowing duplicateStealthUtxoSpendKeyIdentries if same input (samepublic_nonce) appears in both collections. -
Deduplication is technically possible:
StealthUtxoSpendKeyIdderivesHashandEq, making it safe forHashSet-based deduplication. -
Duplicate signing occurs: The signing loop (accounts.rs:1031) iterates each key and calls
sign_with_stealth_key(), so duplicates would cause the same key to sign the transaction twice.
Recommendation:
Convert utxo_spend_keys from Vec to HashSet (or deduplicate before collection) to prevent redundant signing:
let utxo_spend_keys = inputs_to_spend
.inputs
.iter()
.chain(&fee_inputs_to_spend.inputs)
.map(|i| StealthUtxoSpendKeyId {
account_key_id,
public_nonce: i.public_nonce,
})
.collect::<HashSet<_>>() // Deduplicate here🤖 Prompt for AI Agents
In crates/wallet/sdk/src/apis/stealth_transfer/api.rs around lines 586 to 596,
the code chains inputs_to_spend and fee_inputs_to_spend into a Vec of
StealthUtxoSpendKeyId which can contain duplicates (same public_nonce) and
causes duplicate signing; change the collection to deduplicate (e.g. collect
into a HashSet of StealthUtxoSpendKeyId) or otherwise filter duplicates before
collecting so each spend key appears only once (if the surrounding API needs a
Vec, convert the HashSet back to Vec after deduplication).
2 participants
Description
feat!: implement optional access-rule based UTXO spending
feat: support for M of N access rules
test: m-of-n test
fix: remove redundant signature from output, use transaction signature instead
fix: remove required_signer from input statement (since the transfer statement no longer has the required proof to spend)
Motivation and Context
Mainly to allow m of n spending of utxos.
UTXO owner signature authorised spending of the UTXO, however transaction signatures are used for authorisation in all other non-stealth cases. This PR removes the need to the signature, and instead requires that each spend is signed for on the transaction level. This also ensures that the final transaction where the transfer statement is placed, must have been seen by the spender.
The required signer was to bind the transfer to some (semi-trusted) final tx signer. This has been removed since the statement no longer contains all the required proof data to spend, and so is useless if placed into an unauthorised transaction.
How Has This Been Tested?
New unit test
What process can a PR reviewer use to test or verify this change?
Breaking Changes
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.