📋 Code Owners Summary

7 file(s) changed, 7 with assigned owners

2 team(s) affected: @brave/crypto-wallets-core, @brave/crypto-wallets-front-end

Owners and Their Files

@brave/crypto-wallets-core — 6 file(s)

• components/brave_wallet/browser/internal/orchard_bundle_manager.cc
• components/brave_wallet/browser/zcash/zcash_complete_transaction_task.cc
• components/brave_wallet/browser/zcash/zcash_tx_manager.cc
• components/brave_wallet/browser/zcash/zcash_wallet_service.cc
• components/brave_wallet/browser/zcash/zcash_wallet_service.h

@brave/crypto-wallets-front-end — 1 file(s)

• components/brave_wallet_ui/page/screens/send/components/select_address_modal/select_address_modal.tsx

A Storybook has been deployed to preview UI for the latest push

UI Logic Change: The UI change inverts the disabled logic for account selection (!selectedAsset?.isShielded instead of !!selectedAsset?.isShielded). This means transparent assets can no longer be sent to the same account, which may be unintended.

Is this intentional or has it been tested to make sure we're not breaking it unintentionally?

UI Logic Change: The UI change inverts the disabled logic for account selection (!selectedAsset?.isShielded instead of !!selectedAsset?.isShielded). This means transparent assets can no longer be sent to the same account, which may be unintended.

Is this intentional or has it been tested to make sure we're not breaking it unintentionally?

I am still able to send t->s \ s->t inside an account.

Cool thanks for double checking, probably an AI hallucination then.

[puLL-Merge] - brave/brave-core@32580

Description

This PR adds support for "unshielding" ZCash transactions, which allows users to transfer funds from their shielded (private) Orchard addresses back to transparent (public) addresses. The implementation mirrors the existing shielding functionality but operates in reverse. Key changes include:

• Added new transaction type kUnshielding to differentiate unshielding from regular Orchard-to-transparent transfers
• Created ZCashCreateOrchardToTransparentTransactionTask for handling the transaction creation
• Updated UI components to display appropriate labels, buttons, and alerts for unshielding operations
• Modified transaction validation logic to detect when sending to a transparent address from a shielded account
• Added localized strings for all unshielding-related UI elements

Possible Issues

1. Duplicate key in localization: Line 1718 in brave_wallet_constants.h has a duplicate key "braveWalletShielding" that's assigned different values (IDS_BRAVE_WALLET_SHIELDING and IDS_BRAVE_WALLET_UNSHIELDING). The second one should be "braveWalletUnshielding".

2. Missing file: The PR references ZCashCreateOrchardToTransparentTransactionTask but doesn't include the implementation file. This suggests incomplete PR or missing files.

3. UI logic complexity: The send screen now has three possible states (normal send, shielding, unshielding) which increases complexity. The condition checking in multiple places could lead to maintenance issues.

Security Hotspots

1. Address validation bypass (Medium Risk): In zcash_wallet_service.cc lines 442-448, when sending from a shielded pool, the code first checks for Orchard addresses, then iterates through all accounts looking for matching transparent addresses. If an attacker can predict or manipulate account enumeration, they might influence transaction routing.

2. Transaction type detection (Low-Medium Risk): The logic distinguishing between kUnshielding (to own transparent address) and kOrchardToTransparent (to external address) relies on matching against known account addresses. If account enumeration is incomplete or stale, funds could be misclassified and potentially sent without proper user awareness.

Privacy Hotspots

1. Address enumeration (High Privacy Risk): Lines 448-457 in zcash_wallet_service.cc iterate through all account information to check if the destination address matches any known transparent addresses. This creates a correlation between transparent addresses across all accounts, reducing privacy by linking them together in the validation logic.

2. Transaction metadata (Medium Privacy Risk): The new kUnshielding transaction type explicitly tags transactions that move from shielded to transparent pools, creating metadata that identifies the privacy characteristics of the transaction. While this is necessary for UI purposes, it does create an additional privacy fingerprint.

sequenceDiagram
    participant User
    participant UI
    participant ZCashTxManager
    participant ZCashWalletService
    participant CreateTask as CreateOrchardToTransparent<br/>TransactionTask
    participant OrchardBundleManager
    participant ZCashRpc

    User->>UI: Selects shielded asset & transparent address
    UI->>ZCashWalletService: GetTransactionType(account, address)
    ZCashWalletService->>ZCashWalletService: Check if address is Orchard
    ZCashWalletService->>ZCashWalletService: Check if address matches known accounts
    alt Matches own transparent address
        ZCashWalletService-->>UI: kUnshielding
    else External transparent address
        ZCashWalletService-->>UI: kOrchardToTransparent
    end
    
    User->>UI: Confirms transaction
    UI->>ZCashTxManager: AddUnapprovedZCashTransaction()
    ZCashTxManager->>ZCashWalletService: CreateOrchardToTransparentTransaction()
    ZCashWalletService->>ZCashWalletService: Validate transparent address
    ZCashWalletService->>CreateTask: Start()
    CreateTask->>OrchardSyncState: GetSpendableNotes()
    OrchardSyncState-->>CreateTask: Shielded notes
    CreateTask->>OrchardSyncState: CalculateWitnessForCheckpoint()
    OrchardSyncState-->>CreateTask: Notes with witnesses
    CreateTask->>ZCashRpc: GetTreeState()
    ZCashRpc-->>CreateTask: Tree state
    CreateTask->>OrchardBundleManager: Create(inputs, outputs=[])
    OrchardBundleManager-->>CreateTask: Unsigned bundle
    CreateTask-->>ZCashWalletService: ZCashTransaction
    ZCashWalletService-->>ZCashTxManager: Transaction
    ZCashTxManager-->>UI: Transaction created
    
    User->>UI: Signs & sends
    UI->>ZCashWalletService: SignAndPostTransaction()
    ZCashWalletService->>OrchardBundleManager: Sign()
    OrchardBundleManager-->>ZCashWalletService: Signed transaction data
    ZCashWalletService->>ZCashRpc: SendTransaction()
    ZCashRpc-->>ZCashWalletService: Success
    ZCashWalletService-->>UI: Transaction submitted
    UI->>User: Shows "Amount has been unshielded"

Found 1 lines longer than 80 characters (first 5 shown).

Items:

components/brave_wallet_ui/page/screens/send/components/select_address_modal/select_address_modal.tsx, line 623, 88 chars

Released in v1.87.85

Verification passed on

• Verified steps from PR description
• Verified able to Unsheild ZEC in send transaction
• Verified Unshield transaction shows correct set of inputs/outputs
• Verified Unshield transaction generates correct transaction fee value
• Verified once transaction is completed, the transparent address has the correct balance