feat!: adds UTXO memos to encrypted data and wallet - #1595
Conversation
WalkthroughAdds optional memos to confidential and stealth outputs across crypto, SDK, daemon, client bindings, storage, CLI, and web UI; refactors EncryptedData into template_lib_types backed by MaxBytes/MaxString; updates encryption/decryption APIs to carry memos and DecryptedData; persists memos to SQLite and updates tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as CLI/Web UI
participant Daemon
participant SDK
participant Crypto as Wallet Crypto
participant DB
User->>UI: Initiate transfer (optional memo)
UI->>Daemon: TransferRequest { ..., memo/output_memo }
Daemon->>SDK: Build params { ..., memo }
SDK->>Crypto: encrypt_data(amount, mask, pk, sk, memo)
Crypto-->>SDK: EncryptedData
SDK->>Daemon: Output statements { memo }
Daemon->>DB: Persist outputs (memo_json)
DB-->>Daemon: OK
Daemon-->>UI: Response (includes memo)
sequenceDiagram
autonumber
participant Owner
participant Daemon
participant SDK
participant Crypto
participant DB
Owner->>Daemon: List outputs
Daemon->>DB: Load outputs (memo_json)
DB-->>Daemon: Outputs + memo_json
Daemon->>SDK: decrypt_unblind(..., skip_memo=false)
SDK->>Crypto: decrypt_data(enc_key, commitment, encrypted_data, false)
Crypto-->>SDK: DecryptedData { value, mask, memo? }
SDK-->>Daemon: Models { memo propagated }
Daemon-->>Owner: UtxoInfo/OutputModel { memo }
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/template_lib_types/src/encrypted_data.rs (1)
20-24: Missing import for size_ofsize_of::() requires an import. The diff above adds use tari_template_abi::rust::mem::size_of, which is consistent with no_std usage in this crate. Without it, this will not compile.
🧹 Nitpick comments (9)
crates/template_lib_types/src/serde_helpers.rs (1)
18-29: LGTM! Consider adding#[must_use]attributes.The implementation is correct and follows the expected pattern for query methods. Both methods are properly marked as
const fn, enabling compile-time evaluation.Consider adding
#[must_use]attributes to both methods to align with Rust standard library conventions (e.g.,Vec::len(),Vec::is_empty()). This prevents accidental misuse and satisfies clippy'smust_use_candidatelint:impl<'a> BytesCow<'a> { + #[must_use] pub const fn len(&self) -> usize { match self { BytesCow::Borrowed(v) => v.len(), BytesCow::Owned(v) => v.len(), } } + #[must_use] pub const fn is_empty(&self) -> bool { self.len() == 0 } }crates/wallet/storage_sqlite/migrations/2023-02-08-122514_initial/up.sql (1)
155-155: Consider adding a CHECK constraint for memo size validation.The PR description mentions a 253-byte limit for memos, but the
memo_jsoncolumns are defined asTEXT NULLwithout size constraints. While SQLite TEXT can store very large values and the limit is likely enforced at the application layer, adding a database-level CHECK constraint would provide defense-in-depth validation.For example:
memo_json TEXT NULL CHECK(length(memo_json) IS NULL OR length(memo_json) <= 512),Note: The constraint should account for JSON serialization overhead, so 512 bytes provides headroom for a 253-byte memo plus JSON structure.
Also applies to: 249-249
bindings/src/types/wallet-daemon-client/UtxoInfo.ts (1)
3-3: LGTM: Memo support added to UtxoInfo.The import and optional field addition correctly integrate memo support.
Note: The TypeScript pattern
memo?: Memo | nullis redundant since?:already makes the field optional. However, this is consistent with other types in the codebase and does not cause issues.Also applies to: 11-11
bindings/src/types/wallet-daemon-client/ProofsGenerateRequest.ts (1)
3-3: LGTM: Memo support added to ProofsGenerateRequest.The import and optional field addition correctly enable memo data in proof generation requests.
Note: The pattern
memo?: Memo | nullis redundant (see comment on UtxoInfo.ts) but consistent with the codebase style.Also applies to: 14-14
crates/wallet/crypto/tests/output_statement.rs (1)
124-130: Consider using a named helper for sender_public_nonce.The inline key pair generation works correctly, but replacing
test_sender_public_nonce()withcreate_key_pair_from_seed(0)makes the intent slightly less clear. Iftest_sender_public_nonce()is no longer available, consider extracting this to a local helper function in the test module for better readability.Apply this diff to improve clarity:
+fn test_sender_public_nonce() -> RistrettoPublicKey { + let (_sk, pk) = create_key_pair_from_seed(0); + pk +} + fn make_output_statements<A: Into<Amount> + Copy>(amounts: &[A]) -> Vec<UnblindedStealthOutputStatement> { amounts .iter() .map(|&amount| { // ... existing code ... let statement = UnblindedOutputStatement { amount, mask: output_mask, resource_view_key: None, - sender_public_nonce: { - let (_sk, pk) = create_key_pair_from_seed(0); - pk - }, + sender_public_nonce: test_sender_public_nonce(), minimum_value_promise: 0, encrypted_data: EncryptedData::try_from(vec![0; EncryptedData::min_size()]).unwrap(), memo: None, };applications/tari_walletd/src/handlers/confidential.rs (1)
254-267: TODO: Memo support not yet implemented for output proofs.The TODO comment indicates that memo support is not yet available in the output proof path. This is explicitly acknowledged by the developer.
Would you like me to open a tracking issue for implementing memo support in
handle_create_output_proof?crates/template_lib_types/src/memo.rs (3)
37-45: Small ergonomics: provide fallible TryFrom constructorsnew_message/new_bytes return Option. Consider implementing TryFrom<&str>/TryFrom<&[u8]> (or dedicated try_new_*) returning a Result with an error describing overflow. Aids caller diagnostics.
Also applies to: 47-51
78-85: Encoding/decoding choices are efficient; minor doc fixCompact 1-byte length is good. Unknown-tag fallback preserves payload—nice forward-compat choice.
Nit: comment says “255) - 1 (enum tag + length (u8))” but you subtract 2 bytes (tag+len) to get 253. Fix comment for clarity.
- /// EncryptedData memo size (255) - 1 (enum tag + length (u8)) + /// EncryptedData memo size (255) - 2 (enum tag + length (u8)) = 253 bytes usableAlso applies to: 100-132
229-236: Test name nitit_borsh_encodes_to_max_bytes uses custom encode_into (not borsh). Rename to avoid confusion.
- fn it_borsh_encodes_to_max_bytes() { + fn it_encodes_within_encrypted_data_payload_limit() {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (59)
Cargo.toml(1 hunks)applications/tari_swarm_daemon/src/process_manager/processes/minotari_wallet.rs(1 hunks)applications/tari_wallet_cli/src/command/transaction.rs(4 hunks)applications/tari_walletd/src/handlers/accounts.rs(7 hunks)applications/tari_walletd/src/handlers/confidential.rs(7 hunks)applications/tari_walletd/src/handlers/stealth_utxos.rs(1 hunks)applications/tari_walletd/web_ui/src/components/Memo.tsx(1 hunks)applications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/components/SendMoney.tsx(2 hunks)applications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/steps/FormStep.tsx(2 hunks)applications/tari_walletd/web_ui/src/routes/StealthUtxoList/StealthUtxoList.tsx(4 hunks)applications/tari_walletd/web_ui/src/services/api/hooks/useAccounts.ts(4 hunks)bindings/src/index.ts(1 hunks)bindings/src/types/Memo.ts(1 hunks)bindings/src/types/wallet-daemon-client/ConfidentialTransferRequest.ts(2 hunks)bindings/src/types/wallet-daemon-client/ProofsGenerateRequest.ts(2 hunks)bindings/src/types/wallet-daemon-client/StealthTransferRequest.ts(2 hunks)bindings/src/types/wallet-daemon-client/UtxoInfo.ts(1 hunks)clients/wallet_daemon_client/src/types.rs(5 hunks)crates/engine_types/src/confidential/claim.rs(1 hunks)crates/engine_types/src/crypto/output.rs(1 hunks)crates/p2p/src/conversions/transaction.rs(1 hunks)crates/template_lib/src/models/mod.rs(0 hunks)crates/template_lib/src/models/unspent_output.rs(1 hunks)crates/template_lib/src/prelude.rs(1 hunks)crates/template_lib_types/src/encrypted_data.rs(3 hunks)crates/template_lib_types/src/lib.rs(1 hunks)crates/template_lib_types/src/max_bytes.rs(1 hunks)crates/template_lib_types/src/max_string.rs(1 hunks)crates/template_lib_types/src/memo.rs(1 hunks)crates/template_lib_types/src/serde_helpers.rs(1 hunks)crates/template_test_tooling/src/support/confidential.rs(5 hunks)crates/template_test_tooling/src/support/stealth.rs(3 hunks)crates/wallet/crypto/Cargo.toml(0 hunks)crates/wallet/crypto/src/confidential.rs(2 hunks)crates/wallet/crypto/src/encrypted_data.rs(6 hunks)crates/wallet/crypto/src/error.rs(1 hunks)crates/wallet/crypto/src/stealth.rs(2 hunks)crates/wallet/crypto/src/unblinded_statement.rs(3 hunks)crates/wallet/crypto/tests/output_statement.rs(3 hunks)crates/wallet/crypto/tests/viewable_balance_proof.rs(2 hunks)crates/wallet/sdk/src/apis/confidential_crypto.rs(3 hunks)crates/wallet/sdk/src/apis/confidential_outputs.rs(3 hunks)crates/wallet/sdk/src/apis/confidential_transfer.rs(8 hunks)crates/wallet/sdk/src/apis/stealth_crypto.rs(3 hunks)crates/wallet/sdk/src/apis/stealth_outputs.rs(5 hunks)crates/wallet/sdk/src/apis/stealth_transfer.rs(9 hunks)crates/wallet/sdk/src/models/confidential_output.rs(2 hunks)crates/wallet/sdk/src/models/stealth_output.rs(2 hunks)crates/wallet/sdk/tests/base_layer_compat.rs(1 hunks)crates/wallet/sdk/tests/confidential_output_api.rs(2 hunks)crates/wallet/sdk/tests/crypto_api.rs(2 hunks)crates/wallet/sdk/tests/support/harness.rs(2 hunks)crates/wallet/storage_sqlite/migrations/2023-02-08-122514_initial/up.sql(2 hunks)crates/wallet/storage_sqlite/src/models/confidential_output.rs(3 hunks)crates/wallet/storage_sqlite/src/models/stealth_output.rs(3 hunks)crates/wallet/storage_sqlite/src/schema.rs(3 hunks)crates/wallet/storage_sqlite/src/writer.rs(4 hunks)integration_tests/src/wallet_daemon_client.rs(2 hunks)integration_tests/tests/steps/wallet.rs(1 hunks)
💤 Files with no reviewable changes (2)
- crates/wallet/crypto/Cargo.toml
- crates/template_lib/src/models/mod.rs
🧰 Additional context used
🧬 Code graph analysis (46)
bindings/src/types/Memo.ts (1)
applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)
bindings/src/types/wallet-daemon-client/ProofsGenerateRequest.ts (2)
applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)
crates/wallet/sdk/tests/support/harness.rs (1)
bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)
bindings/src/types/wallet-daemon-client/ConfidentialTransferRequest.ts (2)
applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)
integration_tests/tests/steps/wallet.rs (1)
bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)
crates/wallet/crypto/src/unblinded_statement.rs (7)
bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)bindings/src/types/Amount.ts (1)
Amount(12-12)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)crates/p2p/src/conversions/transaction.rs (2)
value(807-811)value(865-869)crates/wallet/sdk/src/apis/stealth_transfer.rs (1)
value(949-951)
crates/wallet/sdk/tests/base_layer_compat.rs (1)
crates/template_lib_types/src/memo.rs (2)
decode_from(100-132)new_bytes(47-51)
crates/template_lib/src/models/unspent_output.rs (5)
bindings/src/types/PedersenCommitmentBytes.ts (1)
PedersenCommitmentBytes(6-6)bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)bindings/src/types/ViewableBalanceProof.ts (1)
ViewableBalanceProof(27-62)
applications/tari_walletd/web_ui/src/services/api/hooks/useAccounts.ts (2)
applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)
crates/wallet/storage_sqlite/src/schema.rs (2)
crates/wallet/storage_sqlite/src/writer.rs (14)
stealth_outputs(1065-1081)stealth_outputs(1151-1151)stealth_outputs(1152-1152)stealth_outputs(1181-1181)stealth_outputs(1182-1182)stealth_outputs(1194-1194)stealth_outputs(1195-1195)stealth_outputs(1214-1214)stealth_outputs(1215-1215)accounts(590-594)accounts(632-635)accounts(897-900)accounts(960-963)accounts(1059-1062)crates/wallet/storage_sqlite/src/reader.rs (15)
stealth_outputs(838-845)stealth_outputs(868-878)stealth_outputs(891-893)stealth_outputs(927-930)accounts(380-382)accounts(402-405)accounts(418-420)accounts(435-437)accounts(446-448)accounts(477-479)accounts(498-501)accounts(528-531)accounts(563-566)accounts(597-600)utxo_process_queue(1286-1291)
crates/wallet/crypto/tests/viewable_balance_proof.rs (2)
bindings/src/types/Amount.ts (1)
Amount(12-12)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)
crates/p2p/src/conversions/transaction.rs (1)
bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)
crates/wallet/crypto/src/confidential.rs (2)
bindings/src/types/Amount.ts (1)
Amount(12-12)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)
crates/template_test_tooling/src/support/confidential.rs (4)
bindings/src/types/ConfidentialOutputStatement.ts (1)
ConfidentialOutputStatement(10-32)bindings/src/types/ConfidentialWithdrawProof.ts (1)
ConfidentialWithdrawProof(19-30)bindings/src/types/Amount.ts (1)
Amount(12-12)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)
crates/template_lib_types/src/serde_helpers.rs (3)
crates/template_lib_types/src/encrypted_data.rs (2)
len(33-35)is_empty(37-39)crates/template_lib_types/src/memo.rs (2)
len(60-65)is_empty(67-69)crates/template_lib/src/args/freeze_flags.rs (1)
is_empty(20-22)
crates/wallet/sdk/src/models/confidential_output.rs (3)
bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)
applications/tari_wallet_cli/src/command/transaction.rs (3)
applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)crates/template_lib_types/src/memo.rs (1)
new_message(41-45)
bindings/src/types/wallet-daemon-client/UtxoInfo.ts (2)
applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)
crates/wallet/sdk/src/apis/stealth_crypto.rs (4)
crates/wallet/crypto/src/encrypted_data.rs (2)
encrypt_data(54-63)unblind_output(30-52)crates/wallet/sdk/src/apis/confidential_crypto.rs (1)
unblind_output(104-120)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)bindings/src/types/Memo.ts (1)
Memo(3-3)
crates/wallet/sdk/src/apis/confidential_outputs.rs (1)
crates/wallet/crypto/src/unblinded_statement.rs (2)
value(59-61)memo(67-69)
crates/wallet/storage_sqlite/src/models/stealth_output.rs (5)
bindings/src/types/ComponentAddress.ts (1)
ComponentAddress(6-6)bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)crates/wallet/storage_sqlite/src/models/transaction.rs (1)
deserialize_json(46-46)
bindings/src/types/wallet-daemon-client/StealthTransferRequest.ts (2)
applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)
crates/wallet/crypto/src/encrypted_data.rs (4)
crates/wallet/sdk/src/apis/confidential_crypto.rs (2)
unblind_output(104-120)new(25-27)crates/wallet/crypto/src/kdfs.rs (1)
encrypted_data_dh_kdf_aead(32-44)crates/wallet/crypto/src/unblinded_statement.rs (4)
value(59-61)mask(63-65)memo(67-69)new(39-41)crates/template_lib_types/src/encrypted_data.rs (4)
max_size(29-31)min_size(25-27)len(33-35)as_bytes(41-43)
crates/engine_types/src/crypto/output.rs (2)
bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)
crates/wallet/sdk/src/apis/confidential_transfer.rs (3)
applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(67-69)
applications/tari_walletd/web_ui/src/components/Memo.tsx (2)
bindings/src/types/Memo.ts (1)
Memo(3-3)crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(67-69)
applications/tari_walletd/web_ui/src/routes/StealthUtxoList/StealthUtxoList.tsx (2)
applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)
applications/tari_swarm_daemon/src/process_manager/processes/minotari_wallet.rs (5)
bindings/src/types/PedersenCommitmentBytes.ts (1)
PedersenCommitmentBytes(6-6)bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)bindings/src/types/Scalar32Bytes.ts (1)
Scalar32Bytes(3-3)bindings/src/types/SchnorrSignatureBytes.ts (1)
SchnorrSignatureBytes(5-5)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)
applications/tari_walletd/src/handlers/accounts.rs (2)
bindings/src/types/Memo.ts (1)
Memo(3-3)crates/template_lib_types/src/memo.rs (1)
new_message(41-45)
crates/template_lib_types/src/encrypted_data.rs (1)
bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)
crates/wallet/sdk/src/apis/stealth_transfer.rs (3)
applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(67-69)
crates/wallet/crypto/src/stealth.rs (3)
bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)bindings/src/types/Amount.ts (1)
Amount(12-12)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)
crates/template_lib_types/src/memo.rs (6)
crates/wallet/crypto/src/unblinded_statement.rs (2)
new(39-41)memo(67-69)applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)crates/template_lib_types/src/max_bytes.rs (1)
new_checked(26-33)crates/template_lib_types/src/max_string.rs (1)
new_checked(20-27)crates/template_lib_types/src/encrypted_data.rs (5)
as_bytes(41-43)len(33-35)is_empty(37-39)max_size(29-31)min_size(25-27)
crates/wallet/crypto/tests/output_statement.rs (4)
bindings/src/types/Amount.ts (1)
Amount(12-12)bindings/src/types/UtxoTag.ts (1)
UtxoTag(8-8)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)crates/template_lib_types/src/encrypted_data.rs (1)
min_size(25-27)
crates/wallet/sdk/tests/confidential_output_api.rs (1)
bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)
crates/template_lib_types/src/max_bytes.rs (2)
crates/template_lib_types/src/hex.rs (2)
bytes_from_hex(20-31)bytes_to_hex(33-37)crates/template_lib_types/src/serde_helpers.rs (6)
as_ref(56-61)serialize(100-107)serialize(150-157)deserialize(109-143)deserialize(159-182)len(19-24)
crates/engine_types/src/confidential/claim.rs (1)
bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)
crates/template_lib_types/src/lib.rs (1)
crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(67-69)
crates/wallet/sdk/tests/crypto_api.rs (4)
crates/wallet/crypto/src/unblinded_statement.rs (3)
memo(67-69)mask(63-65)value(59-61)crates/template_lib_types/src/memo.rs (2)
new_message(41-45)as_bytes(53-58)crates/engine_types/src/crypto/helpers.rs (1)
get_commitment_factory(50-52)crates/template_lib_types/src/encrypted_data.rs (1)
as_bytes(41-43)
crates/wallet/sdk/src/apis/stealth_outputs.rs (1)
crates/wallet/crypto/src/unblinded_statement.rs (2)
value(59-61)memo(67-69)
crates/template_lib_types/src/max_string.rs (1)
crates/template_lib_types/src/max_bytes.rs (11)
deref(20-22)new_checked(26-33)as_ref(41-43)deref_mut(47-50)serialize(76-84)deserialize(88-111)it_returns_some_if_data_le_size(122-127)it_returns_none_if_data_gt_size(130-134)tari_bor(171-171)it_fails_to_deserialize_if_length_is_too_large(162-173)serde_json(164-164)
clients/wallet_daemon_client/src/types.rs (3)
bindings/src/types/Amount.ts (1)
Amount(12-12)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)bindings/src/types/Memo.ts (1)
Memo(3-3)
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(5-13)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)
crates/wallet/sdk/src/models/stealth_output.rs (3)
bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)
crates/wallet/storage_sqlite/src/writer.rs (2)
bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)crates/wallet/storage_sqlite/src/reader.rs (4)
confidential_outputs(687-691)confidential_outputs(704-706)confidential_outputs(756-766)confidential_outputs(804-807)
crates/wallet/sdk/src/apis/confidential_crypto.rs (2)
crates/wallet/crypto/src/encrypted_data.rs (3)
decrypt_data(65-79)encrypt_data(54-63)unblind_output(30-52)crates/wallet/crypto/src/kdfs.rs (1)
encrypted_data_dh_kdf_aead(32-44)
🪛 Biome (2.1.2)
applications/tari_walletd/web_ui/src/components/Memo.tsx
[error] 9-9: Shouldn't redeclare 'Memo'. Consider to delete it or rename it.
'Memo' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (95)
applications/tari_swarm_daemon/src/process_manager/processes/minotari_wallet.rs (1)
11-15: LGTM! Clean import reorganization.The import consolidation is correct and improves readability. The relocation of
EncryptedDatafromtemplate_lib_modelstotari_template_lib_typesaligns with the PR's broader refactoring to support memo functionality.Cargo.toml (1)
239-239: Confirm thethiserror2.x upgrade impact.Jumping from
thiserror1.0.59 to 2.0.17 is a major release; it likely raises the MSRV and tweaks derive macro behaviors. Please confirm our supported toolchain still builds and no crates rely on pre‑2.0 semantics (e.g., transparent/source handling). A quickcargo tree -d thiserrorplus acargo checkacross the workspace on the target toolchain would help ensure we’re not missing follow‑up changes.crates/wallet/storage_sqlite/src/models/confidential_output.rs (2)
5-8: LGTM: Import path reorganization is consistent.The move of
EncryptedDatafrommodelstotypesmodule aligns with the broader refactoring visible across the codebase.
26-26: LGTM: Memo field addition and deserialization follow established patterns.The
memo_jsonfield and its deserialization logic follow the same pattern as other optional fields in this file (e.g.,owner_key_idon lines 72-73). The use ofas_ref().map(deserialize_json).transpose()?correctly handles the Option-Result composition.Also applies to: 89-89
crates/wallet/storage_sqlite/src/schema.rs (2)
53-53: LGTM: Schema definitions match migration.The
memo_jsoncolumn definitions correctly reflect the migration changes as nullable text fields in bothconfidential_outputsandstealth_outputstables.Also applies to: 163-163
259-260: LGTM: Joinable declarations are correct.The new joinable relationships properly link:
stealth_outputs.owner_account_id→accounts.idutxo_process_queue.account_id→accounts.idBoth foreign key relationships are valid and align with the table schemas defined earlier in the file.
crates/wallet/storage_sqlite/src/models/stealth_output.rs (2)
6-12: LGTM: Import reorganization is consistent.The import path changes align with the broader refactoring moving
EncryptedDatato thetypesmodule, matching the changes inconfidential_output.rs.
37-37: LGTM: Memo field handling mirrors confidential outputs.The
memo_jsonfield addition and deserialization logic are identical to the pattern used inconfidential_output.rs, ensuring consistency across output types.Also applies to: 83-83
crates/wallet/storage_sqlite/src/writer.rs (3)
48-54: LGTM: Import path change is consistent.The move of
EncryptedDatato thetypesmodule matches the changes in the model files and maintains consistency across the storage layer.
951-951: LGTM: Memo deserialization in lock path is correct.The deserialization logic follows the same pattern used in the model conversion methods, ensuring consistency across all read paths.
983-983: LGTM: Memo serialization is symmetric and consistent.Both confidential and stealth output insert paths correctly serialize the
memofield tomemo_jsonusing the same pattern. The serialization is the inverse of the deserialization operations in the read paths, ensuring data integrity.Also applies to: 1127-1127
crates/p2p/src/conversions/transaction.rs (1)
47-47: LGTM! Import path update aligns with type reorganization.The relocation of
EncryptedDatatotari_template_lib::typesis consistent with the broader refactoring to centralize crypto data types intari_template_lib_types.applications/tari_walletd/src/handlers/accounts.rs (6)
37-37: LGTM! Memo type import supports new memo functionality.The import is correctly added to the existing
use tari_template_lib::typesstatement.
458-469: Verify the skip_memo flag is intentionally set to true.The
skip_memoparameter is hardcoded totruewhen decrypting the claim burn output. This means any memo from the L1 burn transaction will be ignored. If this is intentional (e.g., L1 burns don't include memos or they're not needed here), this is fine. Otherwise, consider whether the memo should be preserved and potentially logged or stored.
494-500: LGTM! Passing None for memo maintains L1 burn format compatibility.The encryption here creates an output with the same encrypted data format used on L1, so omitting the memo is appropriate.
913-913: LGTM! Memo correctly propagated through confidential transfer flow.The memo from the request is properly passed through to the
ConfidentialTransferParams.
956-956: LGTM! Output memo correctly propagated through stealth transfer flow.The output memo from the request is properly passed through to the
StealthTransferParams.
521-521: Remove outdated check;Memo::new_messageis implemented and available.crates/wallet/sdk/src/apis/confidential_crypto.rs (4)
8-8: LGTM! Imports support the refactored encryption/decryption API.The new imports (
DecryptedData,Memo, and the updatedencrypted_datafunctions) are necessary for the memo feature and the unified decryption return type.Also applies to: 11-11, 19-19
66-77: LGTM! Memo parameter correctly added to encryption API.The method signature cleanly extends to accept an optional memo, and properly delegates to the underlying
encrypt_datafunction with the memo parameter.
79-88: LGTM! Decryption API refactored to support memos.The method rename from
extract_value_and_masktodecrypt_output_dataimproves clarity, and the new return typeDecryptedDataunifies the mask/value and optional memo into a single structure. Theskip_memoparameter provides flexibility for callers who don't need memo data.
104-120: LGTM! Unblinding API updated consistently with decryption changes.The addition of the
skip_memoparameter and theDecryptedDatareturn type maintain consistency with the other decryption methods in this API.clients/wallet_daemon_client/src/types.rs (5)
57-57: LGTM! Import additions support memo functionality.The imports of
ConfidentialOutputStatementandMemoare necessary for the new memo fields in the request/response types.Also applies to: 59-59
494-495: LGTM! Optional memo field added with proper serialization attributes.The
memofield is correctly marked as optional with#[serde(default, skip_serializing_if = "Option::is_none")], ensuring backward compatibility with existing clients that don't provide a memo.
549-550: LGTM! Consistent memo field addition to ConfidentialTransferRequest.The optional
memofield follows the same pattern asProofsGenerateRequest, maintaining API consistency.
1067-1068: LGTM! output_memo field clearly distinguishes memo purpose.The field is appropriately named
output_memoto clarify that this memo is attached to the transfer output, which is clearer than a genericmemofield for stealth transfers.
1109-1109: LGTM! Memo field added to UTXO information.The optional
memofield allows the API to expose memo data associated with UTXOs when available.integration_tests/tests/steps/wallet.rs (1)
17-17: LGTM! EncryptedData import path updated consistently.The import path change from
models::EncryptedDatatotypes::EncryptedDataaligns with the type reorganization across the codebase.crates/wallet/sdk/src/models/stealth_output.rs (2)
5-7: LGTM: Import path updates correctly reflect the type reorganization.The import changes align with the project-wide migration of
EncryptedDataand introduction ofMemointari_template_lib::types.
25-25: LGTM: Optional memo field added to support UTXO memos.The
Option<Memo>field correctly supports the new optional memo feature without breaking existing code.crates/wallet/crypto/src/stealth.rs (2)
164-164: LGTM: Import path updated for consistency.The import of
EncryptedDatafromtari_template_lib::typesaligns with the project-wide type reorganization.
180-180: LGTM: Test data updated for new memo field.The test correctly initializes the
memofield asNone, maintaining test validity with the updatedUnblindedOutputStatementstructure.bindings/src/index.ts (1)
66-66: LGTM: Memo type properly exported.The export makes the
Memotype publicly available and is correctly positioned in the alphabetical export list.crates/wallet/crypto/src/confidential.rs (2)
143-143: LGTM: Import paths updated for type reorganization.The import of
AmountandEncryptedDatafromtari_template_lib::typesaligns with the project-wide refactoring.
157-157: LGTM: Test data updated for memo field.The test correctly initializes the
memofield asNoneto maintain test validity with the updated structure.crates/engine_types/src/confidential/claim.rs (1)
10-10: LGTM: Import path updated for EncryptedData.The import path change from
tari_template_lib::modelstotari_template_lib::typesis consistent with the project-wide type reorganization.crates/wallet/crypto/tests/viewable_balance_proof.rs (2)
15-15: LGTM: Import paths updated for type reorganization.The import of
AmountandEncryptedDatafromtari_template_lib::typesaligns with the project-wide refactoring.
28-28: LGTM: Test helper updated for memo field.The test helper function correctly initializes the
memofield asNone, maintaining test validity with the updatedUnblindedOutputStatementstructure.applications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/steps/FormStep.tsx (2)
44-44: LGTM!The memo field addition to the SendMoneyFormState interface is straightforward and correctly typed.
178-188: Verify UTF-8 byte length constraint.The
maxLength: 253attribute limits JavaScript string length (UTF-16 code units), not UTF-8 byte length. Multi-byte characters (e.g., emoji, certain Unicode) can exceed 253 bytes when UTF-8 encoded, even if the string length is under 253.Consider adding UTF-8 byte length validation:
{transferFormState.outputToConfidential ? ( <TextField name="memo" label="Memo message (optional, max 253 characters)" - inputProps={{ maxLength: 253 }} value={transferFormState.memo} onChange={onFormValueChange} style={{ flexGrow: 1 }} disabled={disabled} + error={new TextEncoder().encode(transferFormState.memo).length > 253} + helperText={ + new TextEncoder().encode(transferFormState.memo).length > 253 + ? "Memo exceeds 253 bytes when UTF-8 encoded" + : undefined + } /> ) : null}bindings/src/types/Memo.ts (1)
1-3: LGTM!The Memo type definition is correct and properly structured as a union type matching the Rust enum pattern. The generated file warning is appropriately included.
crates/wallet/sdk/tests/support/harness.rs (2)
28-28: LGTM!The import path update for
EncryptedDatafromtari_template_lib::modelstotari_template_lib::typesis correct and aligns with the broader refactoring mentioned in the PR summary.
110-110: LGTM!The addition of
memo: Noneto the test harness is appropriate, as test outputs don't require memos.applications/tari_walletd/web_ui/src/services/api/hooks/useAccounts.ts (3)
49-49: LGTM!The Memo import is correctly added to support the new output_memo field.
129-129: LGTM!The optional
output_memofield addition to TransferParams is correctly typed.
148-148: LGTM!The
output_memofield is correctly propagated to both confidential and stealth transfer requests, with the|| nullpattern appropriately converting undefined to null for optional fields.Also applies to: 161-161
crates/template_lib/src/prelude.rs (1)
41-42: LGTM!The public re-exports of
MaxBytesandMaxStringappropriately expand the prelude's public API surface. These bounded data types align well with the memo functionality being introduced.integration_tests/src/wallet_daemon_client.rs (2)
129-129: LGTM!The
output_memo: Noneinitialization is appropriate for integration tests where memos are not being tested.
885-885: LGTM!The
memo: Noneinitialization is appropriate for integration tests where memos are not being tested.applications/tari_walletd/src/handlers/stealth_utxos.rs (1)
49-49: LGTM!The
memofield mapping toUtxoInfois correctly implemented, consistent with other field mappings in the response.bindings/src/types/wallet-daemon-client/StealthTransferRequest.ts (2)
4-4: LGTM!The Memo import is correctly added to support the new optional field.
17-17: LGTM!The
output_memofield is correctly typed as optional and nullable, consistent with the pattern used for other optional fields in the request.bindings/src/types/wallet-daemon-client/ConfidentialTransferRequest.ts (1)
4-18: LGTM!The addition of the optional
memofield is consistent with the Memo type definition and follows the existing nullable pattern used in other fields.crates/template_test_tooling/src/support/confidential.rs (5)
14-14: LGTM!The import path change for
EncryptedDatafrommodelstotypesaligns with the broader refactoring to consolidate types in thetari_template_lib::typesmodule.
38-46: LGTM!The addition of
memo: NonetoUnblindedOutputStatementis consistent with the new memo field support across the codebase.
49-57: LGTM!Consistent memo field initialization.
154-162: LGTM!Consistent memo field initialization.
163-171: LGTM!Consistent memo field initialization.
applications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/components/SendMoney.tsx (2)
193-193: LGTM!The conditional memo inclusion using
{ Message: transferFormState.memo }is consistent with the Memo type shape and follows the same pattern used in the actual transfer path.
257-257: LGTM!Consistent memo handling in the actual transfer path, matching the fee estimation logic.
crates/wallet/crypto/tests/output_statement.rs (2)
19-19: LGTM!The import path simplification removes the top-level
EncryptedDataimport since it's now imported within thestealth_testsmodule where it's actually used.
31-31: LGTM!The consolidated import brings both
UtxoTagandEncryptedDatafrom thetypesmodule, aligning with the broader reorganization of these types.applications/tari_walletd/web_ui/src/routes/StealthUtxoList/StealthUtxoList.tsx (3)
34-34: LGTM!Import of the Memo component for rendering memo values.
66-97: LGTM!The column width adjustments and the addition of the Memo column header are well-integrated into the existing table structure.
114-116: LGTM!Consistent usage of the Memo component to render
utxo.memovalues in the table.crates/wallet/sdk/tests/confidential_output_api.rs (2)
12-12: LGTM!The import path change for
EncryptedDataaligns with its relocation to thetypesmodule.
85-99: LGTM!The addition of
memo: Noneto theConfidentialOutputModelinitialization is consistent with the new optional memo field support.crates/template_test_tooling/src/support/stealth.rs (3)
21-27: LGTM!The import reorganization consolidates model types under one import and moves
EncryptedDatato thetypesmodule, aligning with the broader type system refactoring.
90-98: LGTM!Consistent addition of
memo: Noneto theUnblindedOutputStatementinitialization.
178-187: LGTM!Consistent memo field initialization in the transfer data generation.
crates/wallet/sdk/tests/base_layer_compat.rs (1)
9-18: Clarify MemoField truncation/metadata behaviorThe test feeds 254 bytes yet
MAX_BYTES_LENGTHis 253 and the assertion slicesmemo_bytes[1..=253]. Confirm thatnew_open_from_stringprepends a metadata byte and truncates the payload toMAX_BYTES_LENGTH, and add a code comment explaining this behavior.crates/wallet/sdk/src/apis/confidential_outputs.rs (1)
182-191: Nice reuse of decrypt_output_dataGreat to see the mask resolution path using the shared decrypt_output_data call—this keeps memo-aware parsing aligned across the stack.
applications/tari_wallet_cli/src/command/transaction.rs (1)
156-158: CLI memo option wired cleanlyThe optional memo flag plumbs through neatly, and the length validation via Memo::new_message keeps it safe for users.
Also applies to: 436-439
crates/wallet/sdk/src/apis/stealth_outputs.rs (2)
226-233: LGTM! Skip memo optimization for spending.The logic correctly skips memo decryption during spending operations since only the mask and value are needed. The comment clearly explains the rationale, and the API usage aligns with the new
DecryptedDatareturn type.
519-596: LGTM! Memo propagation in UTXO validation.The validation flow correctly:
- Decrypts memos when validating UTXOs (
skip_memo=false)- Consistently extracts and propagates memo across all status branches (Unspent/Invalid)
- Includes memo in the final
StealthOutputModelconstructionThe implementation maintains existing ownership validation logic while seamlessly adding memo support.
applications/tari_walletd/src/handlers/confidential.rs (2)
100-125: LGTM! Memo support in transfer proofs.The implementation correctly threads the memo from the request through encryption and into the
UnblindedOutputStatement. The API usage aligns with the updated signatures.
150-183: LGTM! Change output memo handling.Change outputs correctly use
Nonefor memos at all stages (encryption, model, statement). This is the expected behavior since change returns to the sender and doesn't require a memo.crates/wallet/sdk/src/apis/stealth_crypto.rs (3)
13-27: LGTM! Import updates for memo support.The import changes correctly bring in the new types (
DecryptedData,Memo,EncryptedData) and updated function signatures needed for memo support. The imports are well-organized and align with the API changes.
102-113: LGTM! Encryption API extended for memo support.The
encrypt_value_and_maskmethod correctly extends its signature to accept an optional memo and delegates toencrypt_data. The API design is clean and maintains backward compatibility through theOptiontype.
129-145: LGTM! Decryption API updated for memo support.The
decrypt_value_and_maskmethod correctly:
- Adds the
skip_memoparameter to enable optimization- Changes return type to
DecryptedDatato include memo information- Delegates to
unblind_outputwith proper parameter passingThe API maintains consistency with the broader memo support implementation.
crates/wallet/crypto/src/encrypted_data.rs (6)
30-52: LGTM! Unblinding updated with memo support.The
unblind_outputfunction correctly:
- Extends the API with
skip_memoparameter andDecryptedDatareturn type- Preserves critical commitment validation logic
- Maintains proper error handling with
CommitmentMismatchDecryptedDataThe security-critical commitment check remains intact.
54-63: LGTM! Encryption API with memo support.The
encrypt_datafunction cleanly extends the encryption API to accept an optional memo and delegates to the inner implementation. The commitment is computed before encryption, maintaining the correct order of operations.
65-79: LGTM! Decryption API with memo support.The
decrypt_datafunction correctly extends the decryption API withskip_memoparameter and constructs theDecryptedDataresult from the decrypted components.
166-226: LGTM! Decryption with memo support.The
decrypt_innerfunction correctly:
- Extracts and validates tag, nonce, and payload with proper error handling
- Decodes value and mask as before
- Conditionally decodes memo based on
skip_memoflag and memo presence- Handles empty memo bytes correctly
- Maps memo decoding errors appropriately
The implementation is thorough and handles edge cases well.
228-304: LGTM! Comprehensive test coverage.The test suite covers:
- Basic encryption/decryption without memo
- Various memo types (message, bytes, empty, maximum length)
skip_memofunctionality- Round-trip encoding/decoding
The tests provide good coverage of the new memo functionality and edge cases.
126-158: EncryptedData::try_from enforces both min_size() and max_size() bounds, ensuring any payload length between those values—including variable-length memo encodings—is accepted and rejected outside that range.crates/template_lib_types/src/max_string.rs (4)
6-17: LGTM! Well-designed bounded string type.The
MaxStringtype uses:
- Generic const parameter for compile-time length bounds
Box<str>for efficient storage without extra capacityDerefto enable transparent string operationsThe design is clean and memory-efficient.
19-32: LGTM! Safe construction with validation.The
new_checkedconstructor properly validates the length constraint and returnsOptionto handle validation failures. Theinto_stringmethod provides a way to convert back toString. No unsafe constructor is exposed, preventing invariant violations.
47-62: LGTM! Serde implementation with validation.The serde implementation correctly:
- Serializes as a plain string
- Validates length during deserialization using
new_checked- Provides clear error messages including the actual length
The pattern is consistent with the
MaxBytesimplementation shown in the snippets.
64-110: LGTM! Comprehensive test coverage.The test suite covers:
- Valid length construction
- Length overflow rejection
- Serde round-trip with
tari_bor- Deserialization failures with both JSON and binary formats
- Error message verification
The tests provide good coverage and follow patterns consistent with
MaxBytes.crates/template_lib_types/src/encrypted_data.rs (1)
62-66: Serde/TS shape: confirm MaxBytes serializes to a stringYou removed serde(dynamic_hex) and switched to MaxBytes<…> with ts(type = "string"), while bindings map EncryptedData to string (bindings/src/types/EncryptedData.ts). Ensure MaxBytes serializes/deserializes as a string (hex/base64) and not as a JSON array. If not, UI/SDK bindings will break.
Please confirm MaxBytes serde impl returns a string and specify which encoding (hex/base64). If it’s not a string, either:
- add serde(with = "...") to enforce string encoding, or
- update bindings/UI accordingly.
Based on learnings
Also applies to: 71-81
crates/template_lib_types/src/memo.rs (2)
27-35: Public API looks solid; TS/serde alignment okayMemo enum with MaxString/MaxBytes capped at 253 bytes is clear, derives serde and ts union matches bindings. Good introduction.
53-69: Double-check serde representation for Bytes in bindings/UIBytes variant maps to string in TS. Ensure serde for MaxBytes encodes to the same string format the UI expects. The web UI does Buffer.from(memo.Bytes).toString("hex") without specifying the input encoding.
- If serde emits base64: UI should use Buffer.from(memo.Bytes, "base64").
- If serde emits hex: UI is fine.
Please confirm and align one way across server and UI.Based on learnings
crates/wallet/sdk/src/apis/confidential_transfer.rs (1)
16-17: Memo plumbing looks correct; ensure encryption API and storage match
- Passing params.memo.clone() into create_confidential_proof_statement and forwarding memo.as_ref() to encrypt_value_and_mask is correct.
- Persisting memo: None for change outputs is sensible.
Please verify:
- encrypt_value_and_mask accepts Option<&Memo> and includes the memo into EncryptedData as per the new format.
- DB/storage layer schema for ConfidentialOutputModel includes memo and matches serde/TS representation.
Also applies to: 321-323, 433-434, 457-458, 467-468, 495-499
crates/wallet/sdk/src/apis/stealth_transfer.rs (1)
33-34: End-to-end memo support in stealth path is consistent
- Threading output_memo through create_output_statement and storing memo is consistent.
- Fee/change outputs use None as expected.
Please confirm:
- Stealth encrypt_value_and_mask handles Option<&Memo> identically to confidential path.
- StealthOutputModel schema and migrations include memo (nullable) and serialization shape is consistent with bindings/UI.
Also applies to: 459-460, 477-479, 749-755, 788-789, 804-805, 828-829, 845-846, 876-878
| impl<const N: usize> DerefMut for MaxString<N> { | ||
| fn deref_mut(&mut self) -> &mut Self::Target { | ||
| // Mutable but not resizeable | ||
| &mut self.s | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify DerefMut safety for bounded string.
The DerefMut implementation returns &mut str, which allows in-place mutation. While the comment states "Mutable but not resizeable", certain operations on &mut str could potentially violate the length invariant.
Consider the following:
&mut strallows methods likemake_ascii_uppercase()which modify bytes in place (safe)- However, user code with
&mut strcould use unsafe operations or create confusing APIs
Verify that this design is intentional and document any safety considerations. Check if similar bounded types in the codebase use DerefMut:
🏁 Script executed:
#!/bin/bash
# Search for similar patterns in MaxBytes and other bounded types
ast-grep --pattern $'impl$_DerefMut for $_ {
fn deref_mut(&mut self) -> &mut $_ {
$$$
}
}'
# Check if MaxBytes has similar DerefMut
rg -n 'impl.*DerefMut.*MaxBytes' --type rustLength of output: 93
Document DerefMut safety for MaxString
The DerefMut impl on MaxString mirrors MaxBytes, returning &mut str which enforces only length-preserving, in-place edits. Add a doc-comment clarifying that resizing isn’t supported and only safe, length-preserving mutations are allowed.
🤖 Prompt for AI Agents
In crates/template_lib_types/src/max_string.rs around lines 40 to 45, the
DerefMut impl lacks documentation about safety constraints; add a doc-comment
above the impl explaining that the returned &mut str permits only in-place,
length-preserving mutations (no resizing, truncation, or growth) and that
callers must not change the string length because MaxString enforces a fixed
capacity—mirror the wording from MaxBytes to clarify the safety contract.
Test Results (CI)136 tests - 291 135 ✅ - 292 25m 6s ⏱️ - 26m 12s For more details on these failures, see this check. Results for commit f6a2dee. ± Comparison against base commit 70f1f28. This pull request removes 291 tests. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
crates/wallet/crypto/src/encrypted_data.rs (2)
173-195: Consider improving error context for malformed encrypted data.The error handling for extracting tag, nonce, and payload slices uses generic error messages. If the encrypted data is malformed (e.g., too short), the errors don't indicate which specific field failed to parse.
Consider adding more specific error context:
let tag = Tag::from_slice( encrypted_data .tag_slice() .ok_or_else(|| WalletCryptoError::FailedDecryptData { - details: "Failed to get tag slice".to_string(), + details: format!("EncryptedData too short for tag (length: {})", encrypted_data.len()), })?, ); let nonce = XNonce::from_slice( encrypted_data .nonce_slice() .ok_or_else(|| WalletCryptoError::FailedDecryptData { - details: "Failed to get nonce slice".to_string(), + details: format!("EncryptedData too short for nonce (length: {})", encrypted_data.len()), })?, ); let mut bytes = Zeroizing::new( encrypted_data .payload_slice() .ok_or_else(|| WalletCryptoError::FailedDecryptData { - details: "Failed to get payload slice".to_string(), + details: format!("EncryptedData too short for payload (length: {})", encrypted_data.len()), })? .to_vec(), );This aids debugging when malformed data is encountered.
216-224: Validate trailing bytes after memo decode
Memo::decode_fromonly consumes the tag, length, and payload bytes—leaving extra bytes in the reader, which are then silently dropped by the wrapper in encrypted_data.rs (lines 216–224). To avoid hidden data loss, consider:
- Asserting
memo_bytes.is_empty()post‐decode and returning an error if not.- Or preserving leftover bytes for future memo extensions.
crates/wallet/sdk/src/apis/stealth_transfer.rs (1)
799-861: Align UnblindedOutputStatement initializer fields to match its declaration order.
encrypted_dataandminimum_value_promiseare swapped; reordering them to mirror the struct’s field sequence improves readability.crates/wallet/sdk/src/apis/stealth_crypto.rs (1)
130-146: Naming nit: consider aligning with “decrypt_output_data”/“unblind_output”This method performs unblinding and returns DecryptedData. Consider renaming to unblind_output or decrypt_output_data for parity with ConfidentialCryptoApi.
crates/wallet/crypto/src/memo.rs (1)
160-239: Tests are comprehensiveRound-trips, bounds, unknown-tag fallback, and size ceilings are covered. Minor typo in comment (“THis”), but not blocking.
crates/wallet/sdk/src/apis/confidential_crypto.rs (1)
105-121: Unblind with skip_memo: OK. Consider naming consistency across modulesThis mirrors the stealth API; consider standardizing method names across Confidential/Stealth for easier SDK consumption.
crates/wallet/crypto/Cargo.toml (1)
17-27: Argon2 usage confirmed; align subtle with workspace
- argon2 is required in
crates/wallet/crypto/src/encryption.rs.- subtle is used; if it’s declared in the workspace, switch to
workspace = truehere for consistency.applications/tari_walletd/src/handlers/accounts.rs (1)
459-465: Avoid magic boolean in decrypt call; name the intentThe trailing true is opaque. Prefer an inline named comment or local named var for clarity.
- true, + /* verify_commitment */ true,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
applications/tari_wallet_cli/src/command/transaction.rs(4 hunks)applications/tari_walletd/src/handlers/accounts.rs(7 hunks)bindings/src/types/wallet-daemon-client/UtxoInfo.ts(1 hunks)clients/wallet_daemon_client/src/types.rs(6 hunks)crates/template_abi/src/rust.rs(0 hunks)crates/template_lib/src/prelude.rs(1 hunks)crates/template_lib_types/src/lib.rs(1 hunks)crates/wallet/crypto/Cargo.toml(1 hunks)crates/wallet/crypto/src/encrypted_data.rs(6 hunks)crates/wallet/crypto/src/lib.rs(1 hunks)crates/wallet/crypto/src/memo.rs(1 hunks)crates/wallet/crypto/src/unblinded_statement.rs(3 hunks)crates/wallet/sdk/Cargo.toml(1 hunks)crates/wallet/sdk/src/apis/confidential_crypto.rs(3 hunks)crates/wallet/sdk/src/apis/confidential_transfer.rs(8 hunks)crates/wallet/sdk/src/apis/stealth_crypto.rs(3 hunks)crates/wallet/sdk/src/apis/stealth_transfer.rs(9 hunks)crates/wallet/sdk/src/models/confidential_output.rs(2 hunks)crates/wallet/sdk/src/models/stealth_output.rs(2 hunks)crates/wallet/sdk/tests/base_layer_compat.rs(1 hunks)crates/wallet/sdk/tests/crypto_api.rs(2 hunks)
💤 Files with no reviewable changes (1)
- crates/template_abi/src/rust.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- crates/template_lib_types/src/lib.rs
- bindings/src/types/wallet-daemon-client/UtxoInfo.ts
- crates/wallet/sdk/src/apis/confidential_transfer.rs
- applications/tari_wallet_cli/src/command/transaction.rs
- clients/wallet_daemon_client/src/types.rs
🧰 Additional context used
🧬 Code graph analysis (12)
crates/wallet/sdk/tests/base_layer_compat.rs (2)
crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(69-71)crates/wallet/crypto/src/memo.rs (2)
decode_from(101-133)new_bytes(48-52)
crates/wallet/crypto/src/unblinded_statement.rs (3)
applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)crates/wallet/sdk/src/apis/stealth_transfer.rs (1)
value(950-952)
applications/tari_walletd/src/handlers/accounts.rs (4)
crates/wallet/crypto/src/unblinded_statement.rs (2)
memo(69-71)mask(65-67)applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)crates/wallet/crypto/src/memo.rs (1)
new_message(42-46)
crates/wallet/sdk/src/models/stealth_output.rs (3)
crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(69-71)applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)
crates/wallet/crypto/src/lib.rs (1)
crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(69-71)
crates/wallet/sdk/src/apis/stealth_crypto.rs (3)
crates/wallet/crypto/src/encrypted_data.rs (2)
encrypt_data(54-63)unblind_output(30-52)crates/wallet/sdk/src/apis/confidential_crypto.rs (1)
unblind_output(105-121)crates/wallet/crypto/src/kdfs.rs (1)
encrypted_data_dh_kdf_aead(32-44)
crates/wallet/sdk/tests/crypto_api.rs (2)
crates/wallet/crypto/src/unblinded_statement.rs (3)
memo(69-71)mask(65-67)value(61-63)crates/wallet/crypto/src/memo.rs (2)
new_message(42-46)as_bytes(54-59)
crates/wallet/sdk/src/apis/confidential_crypto.rs (3)
crates/wallet/crypto/src/encrypted_data.rs (3)
decrypt_data(65-79)encrypt_data(54-63)unblind_output(30-52)crates/wallet/crypto/src/unblinded_statement.rs (2)
memo(69-71)mask(65-67)crates/wallet/crypto/src/kdfs.rs (1)
encrypted_data_dh_kdf_aead(32-44)
crates/wallet/crypto/src/memo.rs (2)
crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(69-71)crates/template_lib_types/src/encrypted_data.rs (2)
max_size(30-32)min_size(26-28)
crates/wallet/sdk/src/apis/stealth_transfer.rs (1)
crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(69-71)
crates/wallet/crypto/src/encrypted_data.rs (8)
bindings/src/types/PedersenCommitmentBytes.ts (1)
PedersenCommitmentBytes(6-6)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)crates/wallet/crypto/src/unblinded_statement.rs (4)
memo(69-71)value(61-63)mask(65-67)new(41-43)bindings/src/types/Memo.ts (1)
Memo(3-3)crates/wallet/sdk/src/apis/confidential_crypto.rs (2)
unblind_output(105-121)new(26-28)crates/wallet/crypto/src/kdfs.rs (1)
encrypted_data_dh_kdf_aead(32-44)crates/template_lib_types/src/encrypted_data.rs (4)
max_size(30-32)min_size(26-28)len(34-36)as_bytes(42-44)crates/wallet/crypto/src/memo.rs (5)
decode_from(101-133)len(61-66)new_message(42-46)as_bytes(54-59)new_bytes(48-52)
crates/wallet/sdk/src/models/confidential_output.rs (3)
crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(69-71)applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: machete
- GitHub Check: test
- GitHub Check: check nightly
- GitHub Check: check stable
- GitHub Check: clippy
- GitHub Check: fmt
🔇 Additional comments (28)
crates/template_lib/src/prelude.rs (1)
41-42: LGTM MaxBytes and MaxString are defined intari_template_lib_typesand in use within the memo implementation.crates/wallet/crypto/src/encrypted_data.rs (3)
30-52: LGTM! Good error handling for commitment mismatch.The refactored
unblind_outputcorrectly:
- Accepts the new
skip_memoparameter to control memo decryption- Returns
DecryptedDatacontaining both the value/mask and optional memo- Validates the commitment matches the decrypted data
- Provides clear error messages for invariant violations
The commitment verification at lines 47-51 is critical for security and correctly prevents acceptance of invalid decrypted data.
95-164: Review the memo encoding and buffer allocation strategy.The
encrypt_data_innerfunction allocates the fullEncryptedData::max_size()when a memo is present (lines 126-130), even if the memo is smaller than the maximum. This means:
- Buffer size: Every encrypted output with a memo will be
max_size()bytes, regardless of the actual memo length.- On-chain footprint: This could significantly increase transaction sizes if most memos are shorter than the maximum.
- Alternative approach: Consider allocating only the required size:
min_size() + actual_memo_encoded_lengthto reduce on-chain bloat.The current approach is simpler and avoids variable-length parsing complexity, but comes at the cost of larger payloads. This trade-off should be intentional.
Is the fixed-size allocation for memos intentional? If optimizing for smaller payloads is desired, consider dynamic sizing:
let mut bytes = vec![ 0; - memo.map(|_| EncryptedData::max_size()) + memo.as_ref().map(|m| { + let mut temp = Vec::new(); + m.encode_into(&mut temp).ok()?; + Some(EncryptedData::min_size() + temp.len()) + }).flatten() .unwrap_or(EncryptedData::min_size()) ];However, this adds complexity and requires two passes (one to measure, one to encode). The current fixed-size approach may be preferred for simplicity.
236-304: LGTM! Comprehensive test coverage.The test suite thoroughly covers:
- Basic encryption/decryption without memo
- Message memo encryption/decryption
- Byte memo encryption/decryption
- Empty memo handling
- Maximum-length memo handling
skip_memoflag behaviorThe tests validate both correctness and the edge cases of the memo implementation.
crates/wallet/sdk/src/apis/stealth_transfer.rs (2)
460-461: LGTM! Correct memo propagation for different output types.The implementation correctly differentiates memo handling:
- Line 460: Primary transfer output includes the user-provided
params.output_memo.clone()- Line 479: Fee change output explicitly sets memo to
None(appropriate, as this is an internal output)- Lines 750-756: Change output explicitly sets memo to
None(appropriate, as this returns funds to the sender)This design ensures memos are only attached to the intended recipient's output, not internal bookkeeping outputs.
Also applies to: 479-479, 750-756
877-878: Good documentation for the memo field.The documentation clearly explains the purpose of the
output_memofield: "Optional memo to include a memo in the output. This memo is encrypted and can only be read by the recipient."This is helpful for API users understanding the feature.
crates/wallet/crypto/src/lib.rs (1)
16-16: LGTM! Memo module correctly exposed.The addition of
pub mod memo;properly exposes the memo module as part of the public API, consistent with the other public modules in the crate.crates/wallet/sdk/Cargo.toml (1)
16-16: LGTM! Serde feature appropriately enabled.Adding the
serdefeature fortari_ootle_wallet_cryptois necessary to support serialization/deserialization of theMemotype in the SDK models and APIs.crates/wallet/sdk/src/models/confidential_output.rs (2)
26-26: LGTM! Memo field added to model.The addition of
pub memo: Option<Memo>toConfidentialOutputModelis consistent with the stealth output model and allows storing memo data for confidential outputs.
6-11: Approve code changes
No remaining stale imports ofEncryptedDatadetected after search.crates/wallet/sdk/src/models/stealth_output.rs (2)
4-9: Consistent import path updates.The
EncryptedDataimport path change fromtari_template_lib::modelstotari_template_lib::typesis consistent with the refactoring seen inconfidential_output.rs.
26-26: LGTM! Memo field added to stealth output model.The addition of
pub memo: Option<Memo>toStealthOutputModelallows storing memo data for stealth outputs, completing the memo support across both output types.crates/wallet/sdk/tests/crypto_api.rs (1)
84-94: LGTM! Test properly updated for memo support.The test correctly:
- Creates a memo using
Memo::new_message("Hello, world!")- Passes it to the encryption function with
Some(&memo)- Specifies
skip_memo: falseduring decryption- Uses accessor methods (
value(),mask(),memo()) instead of direct field access- Verifies the memo is correctly decrypted
The use of accessor methods is good practice and provides better encapsulation.
crates/wallet/sdk/tests/base_layer_compat.rs (1)
9-18: Re-validate test payload length and slice
LocalMemo::MAX_BYTES_LENGTH= 253, but the test constructs a 254-byte string. The slicememo_bytes[1..=Memo::MAX_BYTES_LENGTH]also includes the length prefix byte, not just the raw payload. Verify the externalMemoField::to_bytes()encoding (tag + length) and adjust the test to use ≤253 bytes or drop the length byte.crates/wallet/crypto/src/unblinded_statement.rs (2)
50-76: DecryptedData API looks solidAccessors, ownership (into_mask_and_value), and commitment derivation are correct and consistent with MaskAndValue.
18-18: Adding memo to UnblindedOutputStatementField addition is straightforward and keeps memo optional; no issues spotted.
crates/wallet/sdk/src/apis/stealth_crypto.rs (1)
103-114: Encrypt with memo: correct KDF and parameter threadingUses encrypted_data_dh_kdf_aead(secret, public_key) and passes memo to encrypt_data; looks correct.
crates/wallet/crypto/Cargo.toml (1)
33-35: Feature flags look goodOptional serde/ts-rs gating is clean and non-invasive.
crates/wallet/sdk/src/apis/confidential_crypto.rs (2)
67-77: Encrypt with memo: correct usageKDF and memo threading into encrypt_data look correct.
80-89: Decrypt API parityReturning DecryptedData and exposing skip_memo is consistent with wallet_crypto; looks good.
crates/wallet/crypto/src/memo.rs (2)
27-36:Bytesvariant serializes as hex string in JSON —MaxBytes<MAX_BYTES_LENGTH>’sserde::Serializeimpl emits a hex string whenserializer.is_human_readable(), aligning with the UI binding.
101-113: decode_from already receives a bounded memo reader
In decrypt_inner, we slice the decrypted payload to just the memo bytes and pass that subslice toMemo::decode_from, so it cannot read past the memo segment.applications/tari_walletd/src/handlers/accounts.rs (6)
20-24: Import looks correct for the type actually used by UnblindedOutputStatementUsing Memo from tari_ootle_wallet_crypto aligns with UnblindedOutputStatement’s Memo. No issues.
469-472: Final amount calc is correct and safeUses decrypted.value() with checked_sub_positive; good overflow/underflow handling.
531-534: Input statement creation looks correctUsing decrypted.into_mask_and_value() is appropriate for ownership spend.
904-916: Memo propagation enabled for confidential transfersGood threading of req.memo. Please ensure boundary tests (<=253 bytes) and rejection on overflow exist in SDK layer.
950-959: Memo propagation enabled for stealth transfersoutput_memo wiring looks correct. Recommend tests covering both Message and Bytes variants and dry-run behavior.
542-547: Ensure encrypted_data consistency in claim flow
ClaimBurnOutputDatais built from the originalclaimed_encrypted_data, yetgenerate_transfer_statementuses a freshly re-encrypted value inoutput_statement; unify to a single source—either passoutput_statement.statement.encrypted_dataintoClaimBurnOutputDataor confirm both encryptions are identical.
b85c7a2 to
2af48e4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
crates/wallet/crypto/src/memo.rs (1)
136-158: Consider clarifying the helper function's purpose.The
read_until_len_or_eoffunction is used only for the unknown variant fallback case. Consider adding a doc comment explaining this specialized use case to aid future maintainers.+/// Reads up to `max_len` bytes from reader, stopping at EOF if reached earlier. +/// Used for unknown memo variant fallback to preserve forward compatibility. fn read_until_len_or_eof<R: io::Read>(mut buf: &mut [u8], reader: &mut R, max_len: usize) -> io::Result<usize> {crates/wallet/crypto/src/encrypted_data.rs (3)
126-130: Consider the privacy implications of variable-size encrypted data.The current implementation allocates
max_sizefor any memo (regardless of actual size) andmin_sizefor no memo. This leaks information about memo presence but not its actual size.Consider whether constant-size encrypted data (always
max_size) would better protect privacy by preventing memo presence detection.
157-157: Clarify the invariant in the expect message.The expect message could be more specific about what invariant is being checked.
Apply this diff:
- Ok(EncryptedData::try_from(bytes).expect("bytes length <= EncryptedData::max_size()")) + Ok(EncryptedData::try_from(bytes).expect("invariant violation: bytes length exceeds EncryptedData::max_size()"))
173-195: Consider simplifying error handling.The repeated
ok_or_elsecalls with closure-constructed error messages could be simplified using a helper or by chaining with?and mapping errors at a higher level. However, the current approach is clear and functional.crates/wallet/sdk/src/apis/stealth_transfer.rs (1)
883-884: Consider adding a documentation comment for the output_memo field.While the field is correctly added, a doc comment would help clarify its purpose and constraints (e.g., max size, encryption, recipient-only readability).
Example:
+ /// Optional memo to include in the output. This memo is encrypted and can only be read by the recipient. + /// Maximum size is determined by the Memo type (253 bytes). pub output_memo: Option<Memo>,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
applications/tari_wallet_cli/src/command/transaction.rs(4 hunks)applications/tari_walletd/src/handlers/accounts.rs(6 hunks)applications/tari_walletd/src/handlers/confidential.rs(4 hunks)bindings/src/types/wallet-daemon-client/UtxoInfo.ts(1 hunks)clients/wallet_daemon_client/src/types.rs(6 hunks)crates/template_abi/src/rust.rs(0 hunks)crates/template_lib/src/prelude.rs(1 hunks)crates/template_lib_types/src/lib.rs(1 hunks)crates/template_test_tooling/src/support/confidential.rs(1 hunks)crates/template_test_tooling/src/support/stealth.rs(1 hunks)crates/wallet/crypto/Cargo.toml(1 hunks)crates/wallet/crypto/src/confidential.rs(1 hunks)crates/wallet/crypto/src/encrypted_data.rs(6 hunks)crates/wallet/crypto/src/lib.rs(1 hunks)crates/wallet/crypto/src/memo.rs(1 hunks)crates/wallet/crypto/src/stealth.rs(1 hunks)crates/wallet/crypto/src/unblinded_statement.rs(2 hunks)crates/wallet/crypto/tests/output_statement.rs(3 hunks)crates/wallet/crypto/tests/viewable_balance_proof.rs(1 hunks)crates/wallet/sdk/Cargo.toml(1 hunks)crates/wallet/sdk/src/apis/confidential_crypto.rs(3 hunks)crates/wallet/sdk/src/apis/confidential_transfer.rs(7 hunks)crates/wallet/sdk/src/apis/stealth_crypto.rs(3 hunks)crates/wallet/sdk/src/apis/stealth_transfer.rs(10 hunks)crates/wallet/sdk/src/models/confidential_output.rs(2 hunks)crates/wallet/sdk/src/models/stealth_output.rs(2 hunks)crates/wallet/sdk/tests/base_layer_compat.rs(1 hunks)crates/wallet/sdk/tests/crypto_api.rs(2 hunks)
💤 Files with no reviewable changes (1)
- crates/template_abi/src/rust.rs
🚧 Files skipped from review as they are similar to previous changes (15)
- crates/wallet/crypto/src/confidential.rs
- crates/wallet/crypto/src/lib.rs
- crates/template_lib/src/prelude.rs
- crates/wallet/sdk/Cargo.toml
- crates/wallet/sdk/tests/crypto_api.rs
- crates/wallet/crypto/src/stealth.rs
- crates/wallet/crypto/src/unblinded_statement.rs
- crates/template_test_tooling/src/support/stealth.rs
- applications/tari_wallet_cli/src/command/transaction.rs
- crates/wallet/crypto/tests/output_statement.rs
- crates/template_lib_types/src/lib.rs
- crates/wallet/sdk/tests/base_layer_compat.rs
- applications/tari_walletd/src/handlers/confidential.rs
- applications/tari_walletd/src/handlers/accounts.rs
- crates/template_test_tooling/src/support/confidential.rs
🧰 Additional context used
🧬 Code graph analysis (11)
bindings/src/types/wallet-daemon-client/UtxoInfo.ts (1)
bindings/src/types/Memo.ts (1)
Memo(3-3)
crates/wallet/crypto/tests/viewable_balance_proof.rs (2)
bindings/src/types/Amount.ts (1)
Amount(12-12)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)
crates/wallet/crypto/src/encrypted_data.rs (5)
crates/wallet/crypto/src/unblinded_statement.rs (4)
memo(68-70)value(60-62)mask(64-66)new(40-42)crates/wallet/sdk/src/apis/confidential_crypto.rs (2)
unblind_output(105-121)new(26-28)crates/wallet/crypto/src/kdfs.rs (1)
encrypted_data_dh_kdf_aead(32-44)crates/template_lib_types/src/encrypted_data.rs (4)
max_size(30-32)min_size(26-28)len(34-36)as_bytes(42-44)crates/wallet/crypto/src/memo.rs (5)
decode_from(101-133)len(61-66)new_message(42-46)as_bytes(54-59)new_bytes(48-52)
crates/wallet/crypto/src/memo.rs (2)
crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(68-70)crates/template_lib_types/src/encrypted_data.rs (2)
max_size(30-32)min_size(26-28)
crates/wallet/sdk/src/models/confidential_output.rs (3)
crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(68-70)applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)
crates/wallet/sdk/src/apis/stealth_transfer.rs (2)
crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(68-70)bindings/src/types/Memo.ts (1)
Memo(3-3)
crates/wallet/sdk/src/apis/confidential_crypto.rs (2)
crates/wallet/crypto/src/encrypted_data.rs (3)
decrypt_data(65-79)encrypt_data(54-63)unblind_output(30-52)crates/wallet/crypto/src/kdfs.rs (1)
encrypted_data_dh_kdf_aead(32-44)
crates/wallet/sdk/src/apis/confidential_transfer.rs (1)
crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(68-70)
crates/wallet/sdk/src/apis/stealth_crypto.rs (2)
crates/wallet/crypto/src/encrypted_data.rs (2)
encrypt_data(54-63)unblind_output(30-52)crates/wallet/sdk/src/apis/confidential_crypto.rs (1)
unblind_output(105-121)
crates/wallet/sdk/src/models/stealth_output.rs (3)
crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(68-70)applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)
clients/wallet_daemon_client/src/types.rs (4)
crates/wallet/crypto/src/unblinded_statement.rs (1)
memo(68-70)applications/tari_walletd/web_ui/src/components/Memo.tsx (1)
Memo(9-22)bindings/src/types/Memo.ts (1)
Memo(3-3)bindings/src/types/EncryptedData.ts (1)
EncryptedData(7-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: check nightly
- GitHub Check: machete
- GitHub Check: clippy
- GitHub Check: test
- GitHub Check: check stable
🔇 Additional comments (21)
crates/wallet/crypto/Cargo.toml (1)
17-35: Dependencies and feature gating look solid.argon2 addition and the new
serde/tsoptional deps with matching feature flags are consistent with the memo serialization work. Nice tidy integration.crates/wallet/crypto/tests/viewable_balance_proof.rs (1)
15-15: LGTM! Import path refactored correctly.The import of
EncryptedDataandAmountfromtari_template_lib::typesaligns with the reorganization described in the PR. Usage at line 26 remains unchanged and appropriate for these balance proof tests.bindings/src/types/wallet-daemon-client/UtxoInfo.ts (2)
3-3: LGTM!The type-only import for
Memois correctly structured and follows TypeScript best practices for importing types in a bindings file.
11-11: Approve memo field addition; 253-byte limit validated in Rust
The Rust code definesMAX_BYTES_LENGTH = 253incrates/wallet/crypto/src/memo.rsand existing tests cover this logic.clients/wallet_daemon_client/src/types.rs (1)
43-43: LGTM! Consistent memo field additions across public request/response types.The optional memo fields have been added consistently to
ProofsGenerateRequest,ConfidentialTransferRequest,StealthTransferRequest, andUtxoInfo. The serde attributes (default,skip_serializing_if) ensure backward compatibility and clean JSON serialization.Also applies to: 495-496, 550-551, 1068-1069, 1110-1110
crates/wallet/sdk/src/models/confidential_output.rs (1)
6-10: LGTM! Model updated with memo field and aligned imports.The addition of the
memofield toConfidentialOutputModelis consistent with the PR's objectives. The import reorganization (movingEncryptedDatafrommodelstotypes) correctly reflects the refactoring mentioned in the AI summary.Also applies to: 26-26
crates/wallet/sdk/src/models/stealth_output.rs (1)
4-8: LGTM! Stealth output model updated consistently.The changes mirror those in
confidential_output.rs, maintaining consistency across output models. Thememofield addition and import path updates are appropriate.Also applies to: 26-26
crates/wallet/sdk/src/apis/stealth_crypto.rs (1)
13-18: LGTM! Crypto API extended with memo support.The API changes correctly thread memo through encryption (
encrypt_value_and_mask) and addskip_memocontrol to decryption (decrypt_value_and_mask). The return type change fromMaskAndValuetoDecryptedDataappropriately encapsulates the additional memo information.Also applies to: 103-114, 130-146
crates/wallet/sdk/src/apis/confidential_transfer.rs (1)
13-13: LGTM! Memo properly threaded through confidential transfer flow.The memo parameter is correctly propagated from
ConfidentialTransferParamsthroughcreate_confidential_proof_statementto the encryption layer. The intentional use ofNonefor change outputs (Line 341, 360) is appropriate since change is internal and doesn't need a recipient-visible memo.Also applies to: 317-322, 337-342, 356-360, 428-458, 494-496
crates/wallet/crypto/src/memo.rs (1)
1-239: LGTM! Well-designed memo implementation with appropriate constraints.The implementation correctly enforces the 253-byte limit and provides a compact encoding format to maximize payload space within the 255-byte
EncryptedDataconstraint. Key strengths:
- Custom encoding (tag + 1-byte length + data) saves 3 bytes vs borsh's u32 length prefix
- UTF-8 validation for
Messagevariant ensures text safety- Forward compatibility via unknown variant fallback to
Bytes- Comprehensive test coverage including edge cases (max size, empty, invalid length, unknown tags)
- Doc comment (Lines 38-40) correctly states the calculation
crates/wallet/sdk/src/apis/confidential_crypto.rs (1)
8-20: LGTM! Confidential crypto API updated consistently with memo support.The API changes mirror those in
stealth_crypto.rs, maintaining consistency across the crypto layer. Key updates:
encrypt_value_and_masknow acceptsmemo: Option<&Memo>(Line 73)extract_value_and_maskrenamed todecrypt_output_datawithskip_memoparameter andDecryptedDatareturn type (Lines 80-89)unblind_outputextended withskip_memoparameter andDecryptedDatareturn type (Lines 111-121)The symmetry between confidential and stealth crypto APIs is well-maintained.
Also applies to: 67-78, 80-89, 105-121
crates/wallet/crypto/src/encrypted_data.rs (5)
30-52: LGTM!The function correctly threads through the
skip_memoparameter and properly validates the decrypted commitment against the output commitment. The error handling for overflow and commitment mismatch is appropriate.
54-63: LGTM!Clean delegation to
encrypt_data_innerwith proper memo parameter threading.
65-79: LGTM!Proper conversion of the decryption result into the
DecryptedDatastructure.
216-224: LGTM!The memo decoding logic correctly handles both the
skip_memoflag and empty memo bytes. The helpful comment at line 219 clarifies that remaining bytes are discarded.
236-304: LGTM!Excellent test coverage including:
- Encryption/decryption without memo
- Various memo types (message, bytes, empty, max-size)
- skip_memo flag behavior
- Verification that plaintext is not present in ciphertext
crates/wallet/sdk/src/apis/stealth_transfer.rs (5)
451-463: LGTM!Correctly passes
params.output_memo.as_ref()to the primary output statement, enabling recipients to read the sender's memo.
477-494: LGTM!Fee change outputs correctly use
Nonefor memo, as fee-related outputs don't require recipient messages.Also applies to: 504-510
756-767: LGTM!Change outputs correctly use
Nonefor memo since they're returned to the sender, not a recipient.
769-804: LGTM!The function signature correctly accepts the
memoparameter and properly stores it in theStealthOutputModelat line 796.
806-867: LGTM!The
memoparameter is correctly threaded through toencrypt_value_and_maskat line 836, enabling encryption of the memo alongside the value and mask.
2 participants
Description
feat!: adds UTXO memos to encrypted data and wallet
feat(wallet/webui): adds support for sending a memo message when performing stealth and confidential transfers
Motivation and Context
An optional UTXO memo is encrypted in the existing encrypted data for a UTXO. This allows senders to include an arbitrary 253 byte message (invoice number, saying hello etc) for the recipient.
How Has This Been Tested?
Manually, additional low-level unit tests
What process can a PR reviewer use to test or verify this change?
Breaking Changes
Summary by CodeRabbit
New Features
Chores