Skip to content

fix(platform): encrypt shielded outputs to the sender's outgoing viewing key#3839

Merged
QuantumExplorer merged 3 commits into
v3.1-devfrom
claude/nervous-yonath-458203
Jun 10, 2026
Merged

fix(platform): encrypt shielded outputs to the sender's outgoing viewing key#3839
QuantumExplorer merged 3 commits into
v3.1-devfrom
claude/nervous-yonath-458203

Conversation

@QuantumExplorer

@QuantumExplorer QuantumExplorer commented Jun 10, 2026

Copy link
Copy Markdown
Member

Issue being fixed or feature implemented

Every shielded builder passed ovk: None to the Orchard bundle builder, so each real output's out_ciphertext was encrypted to a per-output random cipher key. As a result, try_recover_outgoing_note — the wallet scanner's outgoing-payment recovery path — could never decrypt the wallet's own sends, and the outgoing-payments store (surfaced in the app as sent-payment history with memos) never populated, even though the sync path in sync.rs already tried every account's OVK and persisted hits.

What was done?

rs-dpp builders (client-side only — out_ciphertext is opaque to consensus validation, so no protocol impact):

  • build_output_only_bundle takes a new sender_ovk: Option<OutgoingViewingKey> parameter, exposed as a new parameter on build_shield_transition and both build_shield_from_asset_lock_transition variants (these signatures carry no FVK to derive it from).
  • Spend-side builders (build_spend_bundle — used by unshield, withdrawal, identity-create — and the inline transfer builder) derive fvk.to_ovk(Scope::External) internally, so transfer recipient outputs and change outputs are recoverable with no public-API change on the spend side.
  • Orchard's dummy/padding outputs keep random cipher keys, as before.

rs-platform-wallet wiring:

  • operations::shield passes the account's outgoing_viewing_key.
  • shielded_fund_from_asset_lock resolves the OVK from the bound account whose IVK recognizes the recipient address (falling back to the lowest bound account), on both the normal and the InstantLock→ChainLock retry broadcast paths.

How Has This Been Tested?

  • New dpp bundle-level test output_built_with_sender_ovk_recovers_under_that_ovk_only: exactly one action recovers (note, recipient, memo) under the sender's OVK via try_output_recovery_with_ovk, and a foreign OVK opens nothing.
  • New ovk_builder_roundtrip_tests::shield_built_note_ovk_recovers_and_persists_as_outgoing in rs-platform-wallet — the sender-side mirror of shield_decrypt_tests: builds a real Shield transition with the wallet's OVK, reassembles each action into the scanner's wire form, asserts try_recover_outgoing_note recovers exactly the recipient output with matching recipient/value/memo (this returned None before the fix), and persists it end-to-end via record_outgoing_note.
  • cargo test -p dpp --all-features shielded → 172 passed.
  • cargo test -p platform-wallet --features shielded --release → all green (lib: 218 passed).
  • cargo fmt --all clean. drive-abci shielded-transfer tests use unchanged signatures and assert only ciphertext sizes, so they are unaffected.

Breaking Changes

None at the protocol level. build_shield_transition and build_shield_from_asset_lock_transition(_with_signer) gained a sender_ovk parameter (pre-release client API; all in-repo call sites updated).

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added optional outgoing viewing key (OVK) support for shielded outputs so senders can enable recoverable sender-side data (recipient, amount, memo).
  • Tests

    • Added end-to-end OVK roundtrip tests and updated unit tests to validate OVK recoverability and non-recoverability with mismatched keys.
  • Documentation

    • Updated examples and docs to show passing the sender OVK.

…ing key

All shielded builders passed ovk: None to the Orchard builder, so every
real output's out_ciphertext was keyed to random bytes. The wallet's
try_recover_outgoing_note OVK scan could never open its own sends, and
the outgoing-payments store never populated.

Output-only builders (shield, shield-from-asset-lock) now take an
explicit sender OVK; spend-side builders (transfer, unshield,
withdrawal, identity-create) derive it from the full viewing key they
already receive, covering recipient and change outputs. Dummy/padding
outputs keep random keys. out_ciphertext is opaque to consensus, so
this is a client-side fix only.

platform-wallet passes the account OVK at shield call sites and adds a
sender-side round-trip test: build a Shield transition, recover the
recipient note via the scanner's OVK path, persist it as an outgoing
payment.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Extend shielded transaction builders to support optional outgoing viewing key (OVK) encryption so senders can recover their complete send history from chain data. Thread sender_ovk through bundle creation, shield builders, wallet operations, and add OVK recovery round-trip validation tests.

Changes

Outgoing Viewing Key (OVK) Support for Shielded Output Recovery

Layer / File(s) Summary
Core bundle builder contract and OVK wiring
packages/rs-dpp/src/shielded/builder/mod.rs
build_output_only_bundle now accepts sender_ovk: Option<OutgoingViewingKey> and uses it as the output out_ciphertext/OVK parameter; build_spend_bundle applies fvk.to_ovk(Scope::External) for added outputs. New unit test verifies OVK recovery under the same OVK and no recovery under a foreign OVK.
Shield builder signatures and wiring
packages/rs-dpp/src/shielded/builder/shield.rs, packages/rs-dpp/src/shielded/builder/shield_from_asset_lock.rs
Add sender_ovk parameter to build_shield_transition and build_shield_from_asset_lock_transition (plus async/signer variant), thread it through to build_output_only_bundle, and update inline docs. Unit tests updated to pass None where prior behavior is preserved.
Shielded transfer and withdrawal OVK application
packages/rs-dpp/src/shielded/builder/shielded_transfer.rs, packages/rs-dpp/src/shielded/builder/shielded_withdrawal.rs, packages/rs-dpp/src/shielded/builder/unshield.rs
build_shielded_transfer_transition now derives sender external-scope OVK and uses it for both recipient and change outputs; withdrawal/unshield docs updated to describe OVK-based memo recovery.
Wallet funding & shield operation wiring
packages/rs-platform-wallet/src/wallet/shielded/fund_from_asset_lock.rs, packages/rs-platform-wallet/src/wallet/shielded/operations.rs
Wallet derives/selects an optional sender_ovk from bound shielded keys (preferring a key whose IVK recognizes the recipient) and passes it into build_and_broadcast_shielded / build_shield_transition so outgoing notes are encrypted for OVK recovery.
OVK round-trip recovery validation & test updates
packages/rs-platform-wallet/src/wallet/shielded/sync/ovk_builder_roundtrip_tests.rs, packages/rs-platform-wallet/src/wallet/shielded/sync/shield_decrypt_tests.rs, packages/rs-platform-wallet/src/wallet/shielded/sync.rs
Add new ovk_builder_roundtrip_tests module that builds a shield transition with the wallet OVK, reconstructs encrypted notes, confirms try_recover_outgoing_note returns exactly the expected outgoing note under the same OVK and zero hits under a foreign OVK, persists the outgoing note, and asserts stored fields. Update existing tests to supply the wallet OVK where appropriate.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • dashpay/platform#3830: Related updates to shield decrypt tests and trial-decryption wiring that interact with OVK plumbing.
  • dashpay/platform#3753: Related signer-driven shield-from-asset-lock builder variant that this PR threads sender_ovk through.

Suggested labels

ready for final review

Suggested reviewers

  • shumkov

A rabbit weaves outgoing keys through shielded notes,
Where senders remember each note and memo they wrote—
OVK recovery peeks the chain and finds the tale,
From transfer to withdrawal, the sender's trail won't fail! 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: enabling sender OVK encryption for shielded outputs. It is concise and directly reflects the core fix across the rs-dpp and rs-platform-wallet changes.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/nervous-yonath-458203

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

QuantumExplorer and others added 2 commits June 10, 2026 11:36
…nath-458203

# Conflicts:
#	packages/rs-platform-wallet/src/wallet/shielded/sync.rs
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@QuantumExplorer QuantumExplorer merged commit 7e235c0 into v3.1-dev Jun 10, 2026
4 of 5 checks passed
@QuantumExplorer QuantumExplorer deleted the claude/nervous-yonath-458203 branch June 10, 2026 10:13
@QuantumExplorer

Copy link
Copy Markdown
Member Author

Reviewed

@QuantumExplorer QuantumExplorer added this to the v4.0.0 milestone Jun 10, 2026
@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

✅ DashSDKFFI.xcframework built for this PR.

SwiftPM (host the zip at a stable URL, then use):

.binaryTarget(
  name: "DashSDKFFI",
  url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
  checksum: "4f86b7c8c83bdad78e5a532b9d16b25b3f2ba0581bc6aae8d62a314267f7d9b6"
)

Xcode manual integration:

  • Download 'DashSDKFFI.xcframework' artifact from the run link above.
  • Drag it into your app target (Frameworks, Libraries & Embedded Content) and set Embed & Sign.
  • If using the Swift wrapper package, point its binaryTarget to the xcframework location or add the package and place the xcframework at the expected path.

@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.05%. Comparing base (af5611e) to head (e51b2e7).
⚠️ Report is 7 commits behind head on v3.1-dev.

Files with missing lines Patch % Lines
packages/rs-dpp/src/shielded/builder/mod.rs 94.54% 3 Missing ⚠️
...s/rs-dpp/src/shielded/builder/shielded_transfer.rs 0.00% 3 Missing ⚠️
...dpp/src/shielded/builder/shield_from_asset_lock.rs 71.42% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           v3.1-dev    #3839    +/-   ##
==========================================
  Coverage     87.04%   87.05%            
==========================================
  Files          2677     2679     +2     
  Lines        329918   330173   +255     
==========================================
+ Hits         287182   287427   +245     
- Misses        42736    42746    +10     
Components Coverage Δ
dpp 87.43% <93.19%> (+0.01%) ⬆️
drive 85.99% <ø> (ø)
drive-abci 89.29% <ø> (ø)
sdk ∅ <ø> (∅)
dapi-client ∅ <ø> (∅)
platform-version ∅ <ø> (∅)
platform-value 92.20% <ø> (ø)
platform-wallet ∅ <ø> (∅)
drive-proof-verifier 49.55% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant