fix(wallet)!: non-xtr resource fixes, support for badges - #1615
Conversation
WalkthroughThis pull request introduces badge usage support for stealth transfers, implements eviction proposal configuration for the consensus layer, adds a decrypt UTXO balance UI feature, includes async blocking refactoring, and implements a new CLI command for viewable balance keys. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as SendMoney UI
participant Handler as Stealth Transfer Handler
participant SDK as Wallet SDK
participant Store as Wallet Store
User->>UI: Initiate stealth transfer with optional badge
UI->>UI: Construct badge_usage (Resource variant or None)
UI->>Handler: Send transfer request with badge_usage
Handler->>Handler: Extract badge from badge_usage
Handler->>SDK: Call stealth_transfer with badge_usage
SDK->>SDK: Parse badge_usage variant
alt Badge Usage Present
SDK->>Store: Fetch badge vault for resource
SDK->>SDK: Generate badge proof
SDK->>SDK: Add badge vault to inputs
else No Badge
SDK->>SDK: Skip badge handling
end
SDK->>SDK: Compute fee with fee inputs
SDK->>SDK: Collect transfer inputs/outputs
SDK->>SDK: Build and sign transfer statement
SDK->>Store: Submit transaction
SDK-->>Handler: Return result
Handler-->>UI: Return transfer response
UI-->>User: Display result
sequenceDiagram
participant Bootstrap
participant Config as Validator Config
participant Consensus as Consensus Module
participant Hotstuff as Hotstuff Engine
Bootstrap->>Config: Load validator node config
Config->>Config: Parse ConsensusConfig with enable_eviction_proposal
Bootstrap->>Consensus: spawn(..., consensus_config)
Consensus->>Hotstuff: Create HotstuffConfig with enable_eviction_proposal flag
loop Block Proposal
Hotstuff->>Hotstuff: Check enable_eviction_proposal flag
alt Flag Enabled
Hotstuff->>Hotstuff: Calculate evictions
Hotstuff->>Hotstuff: Add eviction nodes to proposal
else Flag Disabled
Hotstuff->>Hotstuff: Skip eviction calculations
end
Hotstuff->>Hotstuff: Finalize proposal
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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: 5
🧹 Nitpick comments (11)
crates/template_builtin/templates/account/src/lib.rs (1)
181-183: Consider emitting an event for consistency.The implementation is correct. However, for consistency with other proof creation methods in this impl block (lines 175-179, 185-196, 198-205), consider emitting an event before delegating:
pub fn create_proof_by_non_fungible(&mut self, nft: NonFungibleAddress) -> Proof { + emit_event("create_proof_by_non_fungible", [ + ("resource", nft.resource_address().to_string()), + ("id", nft.id().to_string()), + ]); self.create_proof_by_non_fungible_ids(*nft.resource_address(), vec![nft.id().clone()]) }This improves observability by clearly indicating which API entry point was invoked.
applications/tari_walletd/web_ui/src/routes/Settings/Components/DecryptUtxoBalance.tsx (3)
41-53: Type the response and handle BigInt/JSON-RPC errors.Avoid any; add proper type and error handling to prevent crashes on bad input.
- const [balance, setBalance] = useState<any>(null); + import type { StealthUtxosDecryptValueResponse } from "@tari-project/typescript-bindings"; + const [balance, setBalance] = useState<StealthUtxosDecryptValueResponse | null>(null); + const [error, setError] = useState<string | null>(null); @@ - const onViewBalanceClicked = async () => { - const resp = await stealthDecryptUtxoBalance({ + const onViewBalanceClicked = async () => { + try { + const resp = await stealthDecryptUtxoBalance({ resource_address: formState.resourceAddress!, ids: [formState.utxoId!], minimum_expected_value: formState.minimumExpectedValue ? BigInt(formState.minimumExpectedValue) : null, maximum_expected_value: formState.maximumExpectedValue ? BigInt(formState.maximumExpectedValue) : null, view_key_id: BigInt(formState.keyId), - } as StealthUtxosDecryptValueRequest); - - setBalance(resp); + } as StealthUtxosDecryptValueRequest); + setError(null); + setBalance(resp); + } catch (e: any) { + setBalance(null); + setError(e?.message ?? "Failed to decrypt UTXO balance"); + } };
55-66: Don’t treat 0 as failure; fix message.Use nullish coalescing and correct the text.
- {key}: {balance.balances[key] || "Failed not decrypt value"} + {key}: {balance.balances[key] ?? "Failed to decrypt value"}
67-72: Guard required fields before decrypt.Prevent null utxoId submission and accidental BigInt errors by gating the button.
- const onChange = (e: React.ChangeEvent<HTMLInputElement>) => { + const onChange = (e: React.ChangeEvent<HTMLInputElement>) => { setFormState({ ...formState, [e.target.name]: e.target.value, }); }; @@ - <Button variant="contained" onClick={onViewBalanceClicked} disabled={!formState.resourceAddress}> + <Button + variant="contained" + onClick={onViewBalanceClicked} + disabled={!formState.resourceAddress || !formState.utxoId} + >Also applies to: 101-104
applications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/components/SendMoney.tsx (3)
200-200: badge_usage construction: OK, but tighten types and avoid casts.
- Current: { Resource: transferFormState.badge } or "None" as BadgeUsage.
- Suggest: model badges as ResourceAddress[] and type transferFormState.badge as ResourceAddress to drop the cast and catch mismatches at compile time. Also consider supporting other BadgeUsage variants when UI exposes them.
Example diff (types only; adjust where defined):
- const badges = ... as string[]; + const badges: ResourceAddress[] = ... ... - badge_usage: transferFormState.badge ? { Resource: transferFormState.badge } : ("None" as BadgeUsage), + badge_usage: transferFormState.badge ? { Resource: transferFormState.badge } : "None",Also applies to: 275-277
211-216: Result discriminants: verify shapes and improve reason extraction.
- "Reject" key looks correct per new bindings, but confirm the exact casing.
- AcceptFeeRejectRest indexing assumes tuple shape; guard against object-shaped variants to avoid undefined messages.
Minimal defensive tweak:
- throw new Error(`Transaction rejected: ${rejectReasonToString(transactionResult.AcceptFeeRejectRest[1])}`); + const afr = transactionResult.AcceptFeeRejectRest; + const reason = Array.isArray(afr) ? afr[1] : (afr?.reason ?? afr?.Reject ?? null); + throw new Error(`Transaction rejected: ${rejectReasonToString(reason)}`);
178-206: Duplicate payload assembly between estimate and confirm.Build the transfer payload once to avoid drift (e.g., future BadgeUsage variants or flags). Extract a helper buildTransferPayload() used by both paths.
Also applies to: 265-284
crates/wallet/sdk/src/apis/stealth_transfer/api.rs (4)
30-31: OK to import block_in_place; verify runtime flavor.block_in_place requires the multi-threaded Tokio runtime. Confirm walletd runs with the multi-thread flavor to avoid stalls. If not guaranteed, prefer spawn_blocking or reduce the blocking section.
347-355: Scope of block_in_place is large; shrink the blocking critical section.Only wrap truly blocking CPU/IO work and leave pure assembly outside to improve scheduler fairness. Consider moving lock creation + UTXO/vault locking into block_in_place and keep statement assembly outside.
550-571: Inputs for badges and UTXOs: consider dedup and completeness.
- You add badge_vault.id via substate_inputs and badge resource/NFT via builder.add_input(...). Ensure no duplicate input entries and that resource substates required by create_proof_* are always included for all BadgeUsage variants. A small set-based dedup on inputs can prevent redundant entries.
Also applies to: 572-583
843-846: Extra add_input(XTR): verify necessity.The TODO suggests this is a workaround. If still required, document why (metadata lookup, template dependency) and consider moving to builder-level defaults to avoid leaking into callsites.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
applications/tari_app_utilities/config_presets/c_validator_node.toml(1 hunks)applications/tari_validator_node/src/bootstrap.rs(1 hunks)applications/tari_validator_node/src/config.rs(3 hunks)applications/tari_validator_node/src/consensus/mod.rs(2 hunks)applications/tari_walletd/src/handlers/accounts.rs(1 hunks)applications/tari_walletd/src/handlers/stealth_utxos.rs(2 hunks)applications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/components/SendMoney.tsx(4 hunks)applications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/steps/FormStep.tsx(0 hunks)applications/tari_walletd/web_ui/src/routes/Settings/Components/DecryptUtxoBalance.tsx(1 hunks)applications/tari_walletd/web_ui/src/routes/Settings/Settings.tsx(2 hunks)applications/tari_walletd/web_ui/src/services/api/hooks/useAccounts.ts(5 hunks)applications/tari_walletd/web_ui/src/utils/json_rpc.ts(2 hunks)bindings/src/types/tari-indexer-client/IndexerGetIdentityResponse.ts(1 hunks)bindings/src/types/wallet-daemon-client/BadgeUsage.ts(1 hunks)bindings/src/types/wallet-daemon-client/StealthTransferRequest.ts(1 hunks)bindings/src/wallet-daemon-client.ts(1 hunks)clients/wallet_daemon_client/src/types.rs(2 hunks)crates/consensus/src/hotstuff/config.rs(1 hunks)crates/consensus/src/hotstuff/on_propose.rs(1 hunks)crates/consensus_tests/src/support/harness.rs(1 hunks)crates/engine_types/src/crypto/elgamal.rs(1 hunks)crates/template_builtin/templates/account/src/lib.rs(1 hunks)crates/wallet/sdk/src/apis/stealth_transfer/api.rs(5 hunks)crates/wallet/sdk/src/apis/stealth_transfer/error.rs(2 hunks)crates/wallet/sdk/src/apis/stealth_transfer/params.rs(3 hunks)integration_tests/src/wallet_daemon_client.rs(2 hunks)
💤 Files with no reviewable changes (1)
- applications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/steps/FormStep.tsx
🧰 Additional context used
🧬 Code graph analysis (12)
integration_tests/src/wallet_daemon_client.rs (4)
crates/engine/src/runtime/mod.rs (1)
stealth_transfer(203-208)crates/engine/src/transaction/processor.rs (1)
stealth_transfer(373-396)crates/engine/src/runtime/impl.rs (1)
stealth_transfer(2760-2776)bindings/src/types/wallet-daemon-client/BadgeUsage.ts (1)
BadgeUsage(6-10)
crates/wallet/sdk/src/apis/stealth_transfer/error.rs (1)
bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)
applications/tari_walletd/web_ui/src/services/api/hooks/useAccounts.ts (1)
bindings/src/types/wallet-daemon-client/BadgeUsage.ts (1)
BadgeUsage(6-10)
applications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/components/SendMoney.tsx (2)
bindings/src/types/wallet-daemon-client/BadgeUsage.ts (1)
BadgeUsage(6-10)bindings/src/helpers/helpers.ts (1)
rejectReasonToString(121-164)
bindings/src/types/wallet-daemon-client/BadgeUsage.ts (3)
bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)bindings/src/types/NonFungibleAddress.ts (1)
NonFungibleAddress(7-7)bindings/src/types/Amount.ts (1)
Amount(12-12)
applications/tari_walletd/web_ui/src/routes/Settings/Components/DecryptUtxoBalance.tsx (2)
applications/tari_walletd/web_ui/src/utils/json_rpc.ts (1)
stealthDecryptUtxoBalance(313-315)bindings/src/types/wallet-daemon-client/StealthUtxosDecryptValueRequest.ts (1)
StealthUtxosDecryptValueRequest(5-11)
clients/wallet_daemon_client/src/types.rs (1)
bindings/src/types/wallet-daemon-client/BadgeUsage.ts (1)
BadgeUsage(6-10)
applications/tari_walletd/web_ui/src/utils/json_rpc.ts (2)
bindings/src/types/wallet-daemon-client/StealthUtxosDecryptValueRequest.ts (1)
StealthUtxosDecryptValueRequest(5-11)bindings/src/types/wallet-daemon-client/StealthUtxosDecryptValueResponse.ts (1)
StealthUtxosDecryptValueResponse(4-4)
bindings/src/types/wallet-daemon-client/StealthTransferRequest.ts (1)
bindings/src/types/wallet-daemon-client/BadgeUsage.ts (1)
BadgeUsage(6-10)
crates/wallet/sdk/src/apis/stealth_transfer/params.rs (4)
bindings/src/types/NonFungibleAddress.ts (1)
NonFungibleAddress(7-7)bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)bindings/src/types/Amount.ts (1)
Amount(12-12)bindings/src/types/wallet-daemon-client/BadgeUsage.ts (1)
BadgeUsage(6-10)
applications/tari_walletd/src/handlers/stealth_utxos.rs (1)
crates/engine_types/src/crypto/elgamal.rs (1)
lookup(281-286)
crates/wallet/sdk/src/apis/stealth_transfer/api.rs (3)
crates/wallet/sdk/src/apis/stealth_outputs.rs (1)
params(675-687)bindings/src/types/wallet-daemon-client/BadgeUsage.ts (1)
BadgeUsage(6-10)crates/transaction/src/builder/mod.rs (2)
None(199-199)None(227-227)
⏰ 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). (7)
- GitHub Check: test
- GitHub Check: check nightly
- GitHub Check: file licenses
- GitHub Check: machete
- GitHub Check: check stable
- GitHub Check: fmt
- GitHub Check: clippy
🔇 Additional comments (30)
crates/consensus/src/hotstuff/on_propose.rs (1)
527-546: LGTM!The eviction proposal guard is correctly implemented. When
enable_eviction_proposalis false, the filter returnsNone, bypassing the eviction node calculation and resulting in an empty list viaunwrap_or_default().crates/consensus/src/hotstuff/config.rs (1)
18-18: LGTM!The new configuration field is correctly added to
HotstuffConfig.crates/consensus_tests/src/support/harness.rs (1)
656-656: LGTM!Enabling eviction proposals by default in tests is appropriate to ensure the full consensus behavior is tested.
applications/tari_validator_node/src/consensus/mod.rs (1)
47-47: LGTM!The consensus configuration is correctly imported and wired through to the
HotstuffConfig.Also applies to: 53-53, 84-84
applications/tari_validator_node/src/bootstrap.rs (1)
332-332: LGTM!The consensus configuration is correctly passed to
consensus::spawn.applications/tari_validator_node/src/config.rs (2)
175-190: LGTM, but note the critical issue in the TOML preset.The
ConsensusConfigstruct is well-defined with clear documentation. The field name isenable_eviction_proposal(singular), which should match the TOML configuration key. However, the TOML preset inapplications/tari_app_utilities/config_presets/c_validator_node.tomluses the plural form, which will cause a deserialization error or silent failure.
110-110: LGTM!The consensus configuration field is correctly added to
ValidatorNodeConfigand initialized with sensible defaults.Also applies to: 164-164
bindings/src/types/tari-indexer-client/IndexerGetIdentityResponse.ts (1)
7-7: No action required — change resulted from ts-rs regeneration.Verification confirms this change was from regenerating TypeScript bindings, not manual editing. The commit b2d12be regenerated 4 files in
bindings/src/types/(including this one, plus BadgeUsage.ts and StealthTransferRequest.ts), which aligns with the commit's wallet/badge fixes. ts-rs v11.0 (confirmed in Cargo.toml) generated the more concisestring[]syntax as part of this automated batch regeneration.bindings/src/types/wallet-daemon-client/StealthTransferRequest.ts (1)
1-16: LGTM! BadgeUsage integration is clean.The generated binding correctly extends
StealthTransferRequestwith the newbadge_usagefield, enabling badge-based stealth transfers as described in the PR objectives.applications/tari_walletd/web_ui/src/utils/json_rpc.ts (2)
110-111: LGTM! Imports support the new decrypt UTXO balance feature.The new type imports are correctly placed and necessary for the
stealthDecryptUtxoBalancewrapper.
313-315: LGTM! RPC wrapper follows established patterns.The new
stealthDecryptUtxoBalancewrapper correctly delegates to the underlying client method and maintains consistency with other RPC wrappers in the file.bindings/src/types/wallet-daemon-client/BadgeUsage.ts (1)
1-10: LGTM! Well-structured discriminated union.The
BadgeUsagetype is cleanly defined with four variants covering different badge usage scenarios. The discriminated union structure enables type-safe handling in TypeScript.applications/tari_walletd/src/handlers/stealth_utxos.rs (4)
22-22: LGTM! Necessary import for async refactor.The
spawn_blockingimport is required for the improved async handling of CPU-intensive balance decryption operations.
102-103: Precomputation enables spawn_blocking migration.Collecting
elgamal_proofsand cloningsdkbefore the match statement avoids lifetime issues when moving intospawn_blockingclosures. Thesdk.clone()should be cheap if internally Arc-based.
106-120: Excellent refactor to spawn_blocking for I/O and CPU-intensive work.Moving file loading and balance brute-forcing into
spawn_blockingprevents blocking the async runtime. The error handling with.await??correctly unwraps both theJoinErrorand inneranyhow::Error.
123-131: LGTM! Consistent spawn_blocking pattern for CPU-bound work.The
Nonebranch correctly usesspawn_blockingfor the CPU-intensive balance brute-force withAlwaysMissLookupTable, maintaining consistency with theSome(file)branch.bindings/src/wallet-daemon-client.ts (1)
112-112: LGTM! Standard barrel export.The re-export of
BadgeUsagefollows the established pattern and makes the type available through the main bindings entry point.applications/tari_walletd/web_ui/src/routes/Settings/Settings.tsx (2)
32-32: LGTM! Import for new Decrypt UTXO component.The import follows the established pattern and conventions for this file.
67-71: LGTM! New settings tab for UTXO decryption.The menu item follows the established pattern and successfully integrates the new Decrypt UTXO Balance feature into the Settings page.
integration_tests/src/wallet_daemon_client.rs (2)
31-31: LGTM! Import for new BadgeUsage field.The import correctly adds
BadgeUsagefrom the stealth_transfer API module, enabling its use in the test payload.
126-126: LGTM! Test updated for API change.Setting
badge_usage: BadgeUsage::Nonecorrectly adapts the test to the new API while maintaining the original test intent (stealth transfer without badge requirements).crates/wallet/sdk/src/apis/stealth_transfer/error.rs (2)
5-5: LGTM! Import for new error variant.The
ResourceAddressimport is necessary for theBadgeVaultNotFounderror variant and follows the existing import patterns.
33-34: LGTM! Descriptive error variant for badge failures.The
BadgeVaultNotFounderror variant provides clear context by including theresource_address, enabling better error reporting and debugging for badge-related operations.applications/tari_walletd/src/handlers/accounts.rs (1)
971-987: Propagating badge_usage looks correct; consider tightening validation in SDK.Plumbing the new field is fine. Ensure StealthTransferParams::validate also rejects BadgeUsage::AmountOfResource with a negative amount to fail fast before building/signing a tx. See suggested SDK change in params.rs comment.
applications/tari_walletd/web_ui/src/services/api/hooks/useAccounts.ts (1)
128-131: BadgeUsage wiring looks good; ensure callers default to "None".Resource-only extraction for public/confidential is correct and passing through for stealth is expected. To avoid undefined at call sites, ensure TransferParams.badge_usage is initialized to "None".
Would you like a small helper to normalize UI inputs into a valid BadgeUsage ("None" | {Resource...} | ...), so all call sites can pass through safely?
Also applies to: 140-156, 158-174, 183-191
crates/wallet/sdk/src/apis/stealth_transfer/params.rs (1)
4-4: Add badge_usage.amount validation to the validate() method.The review correctly identifies a missing validation check:
badge_usage.amountis not validated for negative values, despite other amount fields being checked. However, the claim about serde trait bounds is unsupported by the codebase—StealthTransferParamsdoes not deriveSerialize, and no evidence of serde serialization for these types exists.Keep the validation addition (which is necessary and consistent with existing validation patterns for other amount fields), but remove the serde import change:
impl StealthTransferParams { pub fn validate(&self, network: Network) -> Result<(), StealthTransferApiError> { if self.outputs.is_empty() { return Err(StealthTransferApiError::InvalidParameter { param: "outputs", reason: "At least one output must be specified".to_string(), }); } + if let BadgeUsage::AmountOfResource { amount, .. } = &self.badge_usage { + if amount.is_negative() { + return Err(StealthTransferApiError::InvalidParameter { + param: "badge_usage.amount", + reason: "Badge amount must be non-negative".to_string(), + }); + } + }Likely an incorrect or invalid review comment.
applications/tari_walletd/web_ui/src/routes/AssetVault/Tokens/components/SendMoney.tsx (1)
30-34: Imports LGTM; keep versions in lockstep.Importing BadgeUsage and rejectReasonToString is correct for the new payload and messaging. Ensure the UI bindings package version matches the daemon to avoid enum drift.
crates/wallet/sdk/src/apis/stealth_transfer/api.rs (3)
356-399: Fee statement correctness: confirm “revealed vs stealth” split.You subtract max_fee from stealth to form fee_stealth_change_amt while setting output_revealed_amount = max_fee. This assumes revealed fee is always the revealed portion; if fee inputs are all-stealth, revealed_output_amount should be zero. Validate generate_transfer_statement semantics to ensure mixed inputs are handled as intended.
739-772: Proof lifecycle: good; confirm DropAllProofs only when proofs exist.Logic drops proofs only when a badge is used. If future variants introduce other proofs, keep this conditional close to the proof creation to avoid regressions. LGTM as-is.
Also applies to: 835-841
726-738: Fee path: withdraw + pay_fee_stealth flow LGTM.The conditional use of withdraw with input bucket when revealed fee > 0 matches builder semantics.
| }; | ||
| use tari_ootle_wallet_sdk::{ | ||
| apis::confidential_transfer::ConfidentialTransferInputSelection, | ||
| apis::{confidential_transfer::ConfidentialTransferInputSelection, stealth_transfer::BadgeUsage}, |
There was a problem hiding this comment.
🧩 Analysis chain
BadgeUsage must implement serde traits; otherwise this struct won’t serialize.
StealthTransferRequest derives serde Serialize/Deserialize. The SDK enum BadgeUsage (in crates/wallet/sdk/.../params.rs) currently derives tari_bor::{Deserialize, Serialize}, not serde. This will fail trait bounds here.
Fix in SDK by deriving serde instead of tari_bor (see params.rs comment and diff). Your skip_serializing_if = "BadgeUsage::is_none" will then work as intended.
Also applies to: 1064-1066
🏁 Script executed:
#!/bin/bash
# Verify BadgeUsage derives serde (not only tari_bor)
rg -n 'enum\s+BadgeUsage\b' crates/wallet/sdk/src/apis/stealth_transfer/params.rs -C2
rg -n 'use\s+tari_bor::\{Deserialize,\s*Serialize\}' crates/wallet/sdk/src/apis/stealth_transfer/params.rs
rg -n 'use\s+serde::\{Deserialize,\s*Serialize\}' crates/wallet/sdk/src/apis/stealth_transfer/params.rsLength of output: 278
[/skip_cloning]
---
🏁 Script executed:
```shell
#!/bin/bash
# Check all imports in params.rs to see if serde is also imported
rg -n '^use' crates/wallet/sdk/src/apis/stealth_transfer/params.rs | head -20
# Find StealthTransferRequest struct and check BadgeUsage usage
rg -n 'struct\s+StealthTransferRequest' clients/wallet_daemon_client/src/types.rs -A20 | head -30
# Check if BadgeUsage field exists in wallet_daemon_client's StealthTransferRequest
rg -n 'badge' clients/wallet_daemon_client/src/types.rs -B2 -A2
Length of output: 1915
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full StealthTransferRequest struct definition with derives
rg -n 'pub struct StealthTransferRequest' clients/wallet_daemon_client/src/types.rs -B5 -A15
# Get full BadgeUsage enum definition with all derives
rg -n 'pub enum BadgeUsage' crates/wallet/sdk/src/apis/stealth_transfer/params.rs -B3 -A8
Length of output: 1398
---
🏁 Script executed:
```shell
#!/bin/bash
# Check serde imports in wallet_daemon_client types.rs
rg -n '^use serde' clients/wallet_daemon_client/src/types.rs
# Check for any feature flags or cfg_attr on BadgeUsage
rg -n '#\[cfg' crates/wallet/sdk/src/apis/stealth_transfer/params.rs | grep -i badge -B2 -A2
# Broader check: are there any conditional derives on BadgeUsage?
rg -n 'derive.*BadgeUsage|BadgeUsage.*derive' crates/wallet/sdk/src/apis/stealth_transfer/params.rs
Length of output: 39
BadgeUsage must derive serde traits; current implementation blocks compilation.
StealthTransferRequest (line 1058) derives serde traits and uses #[serde(default, skip_serializing_if = "BadgeUsage::is_none")] on the badge_usage field. However, BadgeUsage in crates/wallet/sdk/src/apis/stealth_transfer/params.rs (line 143) currently derives only tari_bor traits, not serde. The serde derive macro will fail to serialize/deserialize this field.
Add serde import and derive in params.rs:
use serde::{Deserialize, Serialize};
...
#[derive(Debug, Clone, Deserialize, Serialize, Default)]
pub enum BadgeUsage { ... }🤖 Prompt for AI Agents
In clients/wallet_daemon_client/src/types.rs around line 42 and in
crates/wallet/sdk/src/apis/stealth_transfer/params.rs around line 143,
BadgeUsage is only deriving tari_bor traits but StealthTransferRequest in
types.rs derives serde and uses serde attributes on badge_usage which causes
compilation failure; update params.rs to import serde (use serde::{Deserialize,
Serialize};) and add serde derives to BadgeUsage (e.g., #[derive(Debug, Clone,
Deserialize, Serialize, Default)]) so the enum can be serialized/deserialized by
serde while preserving existing traits.
Test Results (CI)479 tests ±0 467 ✅ +2 1h 32m 37s ⏱️ + 1m 52s For more details on these failures, see this check. Results for commit b2d12be. ± Comparison against base commit d80aae4. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/wallet/sdk/src/apis/stealth_transfer/api.rs (2)
205-207: Don’t panic on negative input amounts; return a typed error.This
expectcan crash the daemon. Use InvariantViolation like elsewhere.- let total_confidential_spent = Amount::sum_from_positive(inputs.iter().map(|i| i.value)) - // The wallet has somehow stored a negative amount, which should not happen. - .expect("BUG: an unblinded input amount was negative"); + let total_confidential_spent = Amount::sum_from_positive(inputs.iter().map(|i| i.value)) + .ok_or_else(|| StealthTransferApiError::InvariantViolation { + details: "An unblinded input amount was negative".to_string(), + })?;
231-237: Lock misuse: shadowing provided lock_id creates a second lock that’s not released.PreferConfidential creates a new lock instead of using the caller’s lock_id. On errors, unlock_on_failure won’t release this inner lock.
- let lock_id = self.outputs_api.create_lock()?; let (inputs, blinded_amount_locked) = self.outputs_api.lock_outputs_until_partial_amount( owner_account_component_address, &resource_address, spend_amount, - lock_id, + lock_id, )?;
♻️ Duplicate comments (2)
crates/wallet/sdk/src/apis/stealth_transfer/api.rs (2)
388-399: Align spend_key_branch/id with the actual signer (fee statement).You select a Nonce signer when no revealed funds, but still pass Account/owner_key_id here. This risks verification/key mismatch.
- spend_key_branch: KeyBranch::Account, - spend_key_id: owner_key_id, + spend_key_branch: signing_key_branch, + spend_key_id: signing_key_id,
506-521: Align spend_key_branch/id with the actual signer (main statement).Same issue as fee statement; use the resolved main signing key.
- spend_key_branch: KeyBranch::Account, - spend_key_id: owner_key_id, + spend_key_branch: signing_key_branch, + spend_key_id: signing_key_id,
🧹 Nitpick comments (2)
crates/wallet/sdk/src/apis/stealth_transfer/error.rs (1)
51-55: Consider marking BadgeVaultNotFound as “not found” for transport layers.If higher layers map IsNotFoundError -> 404/Optional, extend is_not_found_error to include BadgeVaultNotFound.
impl IsNotFoundError for StealthTransferApiError { fn is_not_found_error(&self) -> bool { - matches!(self, Self::StoreError(e) if e.is_not_found_error() ) + matches!( + self, + Self::StoreError(e) if e.is_not_found_error() + | Self::BadgeVaultNotFound { .. } + ) } }applications/tari_walletd/src/main.rs (1)
156-175: Protect the secret key when writing to disk (0600) and avoid accidental logging.Set restrictive perms on the output file; keep stdout printing but ensure logs don’t capture the secret.
+#[cfg(unix)] +use std::os::unix::fs::OpenOptionsExt; @@ - Some(path) => { - let mut file = fs::File::options() + Some(path) => { + let mut file = fs::OpenOptions::new() .create(true) .write(true) .truncate(true) + // Unix: file mode 0600 to protect the secret + .apply(|opts| { + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + opts.mode(0o600); + } + opts + }) .open(path) .context("failed to open file for writing")?; serde_json::to_writer_pretty(&mut file, &json).context("failed to encode key json to file")?; println!("Key written to {}", path.display()); },Alternatively, call fs::set_permissions after create if you prefer to avoid cfgs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
applications/tari_app_utilities/config_presets/c_validator_node.toml(1 hunks)applications/tari_walletd/src/cli.rs(1 hunks)applications/tari_walletd/src/main.rs(1 hunks)crates/wallet/sdk/src/apis/stealth_transfer/api.rs(5 hunks)crates/wallet/sdk/src/apis/stealth_transfer/error.rs(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- applications/tari_app_utilities/config_presets/c_validator_node.toml
🧰 Additional context used
🧬 Code graph analysis (3)
applications/tari_walletd/src/main.rs (2)
applications/tari_walletd/src/lib.rs (2)
init_wallet_store(178-182)initialize_wallet_sdk(184-198)utilities/tariswap_test_bench/src/runner.rs (1)
initialize_wallet_sdk(113-125)
crates/wallet/sdk/src/apis/stealth_transfer/error.rs (1)
bindings/src/types/ResourceAddress.ts (1)
ResourceAddress(6-6)
crates/wallet/sdk/src/apis/stealth_transfer/api.rs (3)
bindings/src/types/wallet-daemon-client/BadgeUsage.ts (1)
BadgeUsage(6-10)crates/wallet/sdk/src/apis/stealth_transfer/types.rs (1)
total_amount(52-54)crates/wallet/sdk/src/apis/stealth_transfer/params.rs (2)
total_output_amount(98-100)total_output_amount(121-123)
⏰ 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). (7)
- GitHub Check: test
- GitHub Check: file licenses
- GitHub Check: clippy
- GitHub Check: fmt
- GitHub Check: check stable
- GitHub Check: check nightly
- GitHub Check: machete
🔇 Additional comments (4)
crates/wallet/sdk/src/apis/stealth_transfer/error.rs (2)
33-35: Good: specific error for missing badge vault.Clear variant and message will help upstream callers and UI.
47-49: Good: domain error for invariants.Preferable to panics; aligns with wallet-daemon stability.
crates/wallet/sdk/src/apis/stealth_transfer/api.rs (1)
571-581: Badge vault handling looks correct.Typed error on absence plus substate input inclusion is solid.
If AccountsApi can return stale vaults, consider guarding with an on-chain check in a follow-up.
applications/tari_walletd/src/cli.rs (1)
141-147: LGTM: clear, focused subcommand for viewable-balance keys.Names and flags mirror CreateAccount; discoverable and consistent.
2 participants
Description
fix(wallet)!: non-xtr resource fixes, support for badges
feat(consensus): setting to disable eviction proposals
fix(wallet): bug in transfers where the view key (Elgamal proof) was used with fee outputs (XTR) causing the transaction to be rejected.
fix: web ui fixes
feat: add page for utxo decryption with view key
Motivation and Context
The badge option for stealth transfers did nothing and was not supported by the API.
Fixes from testing with stable coin transfers.
How Has This Been Tested?
Manually
What process can a PR reviewer use to test or verify this change?
Breaking Changes
Summary by CodeRabbit
New Features
Improvements