Skip to content

feat(shielded-pool)!: publish an OVK blob per private transfer, and tag the pool per nullifier - #129

Closed
nol4lej wants to merge 4 commits into
mainfrom
feat/ovk-outgoing-viewing-keys
Closed

feat(shielded-pool)!: publish an OVK blob per private transfer, and tag the pool per nullifier#129
nol4lej wants to merge 4 commits into
mainfrom
feat/ovk-outgoing-viewing-keys

Conversation

@nol4lej

@nol4lej nol4lej commented Aug 11, 2026

Copy link
Copy Markdown
Member

Summary

Adds outgoing viewing keys (OVK) to private_transfer, so a sender who loses their vault can recover what they sent. Along the way this fixes two security issues found while auditing the change — one of them in code that predates it.

Ships as spec 9 / transaction_version 3. Breaking in both directions: wallet and runtime must be deployed together.


Why OVK

A note's memo is sealed toward the recipient, so its own author cannot reopen it. Restore a wallet from its seed and the incoming notes come back — but the record of what you sent does not, because it never existed anywhere except the lost device.

Each private_transfer now publishes a 56-byte blob wrapping that memo's shared secret under the sender's ovk:

nonce_suffix(8) | ciphertext(32) | MAC(16)   =  56 bytes
ock = HKDF(ikm=ovk, salt=commitment ‖ ephPk, info="orbinum-outgoing-cipher-v1")

Unwrapping yields the same sharedSecret the recipient derives by ECDH, feeding the same decrypt-and-verify routine — one note-decryption path in the system, not two. The sender recovers the exact amount and the recipient's one-time key, including for exact-amount transfers that leave no change note and were previously unrecoverable.

The ovk derives from the master seed as a sibling of the incoming viewing key, not from it, so the two capabilities stay separately delegable.

The chain validates only the length. It holds no key, so any 56 bytes are accepted — zeros included. A sender opting out publishes 56 random bytes: zeros would be greppable, permanently marking the opt-out and making everyone who chose it a trivially identifiable set.

The call emits OutgoingBlobPublished { commitment, blob }, bound to commitments[0]. It is a separate event because CommitmentsInserted is shared with shield, shield_batch and unshield, none of which carry a blob.


Security fixes

1. Mempool tags did not enforce mutual exclusion

and_provides contributes exactly one tag, so passing it a Vec encoded the whole nullifier set plus the relayer into a single blob. Three consequences, all free to exploit since the fee is only charged on execution:

  • reordering the two inputs minted a second admissible pool entry for the same spend;
  • two transfers sharing only one note (A+B and A+C) did not collide at all, letting one note back unboundedly many entries;
  • private_transfer and unshield used different tag prefixes, so the same note could back one of each simultaneously.

Every variant propagates and is revalidated network-wide while at most one can ever execute.

Fix: one tag per nullifier under a shared ShieldedPoolSpend prefix. relayer deliberately leaves the tag — binding it made a fee-recipient-swapped copy a separate entry, so anyone could rebroadcast another user's spend pointed at their own account. Keyed on the nullifier the two are mutually exclusive, so taking the fee requires out-bidding, which means paying it.

Admission policy, not state transition — consensus is unaffected. Nodes on the old logic keep accepting duplicates, so the mitigation completes as the network updates.

2. Relay fallback selectors were stale (ME-8)

The runtime's fallback selector list — used when the Runtime API call fails — held 0x47fc44a2 and 0x8c0f5d24. Derived by keccak, these turn out to be real selectors from signatures two versions old, predating circuit_version. Falling back would have rejected every relay call as "unsupported selector", silently, because that is indistinguishable from a legitimate rejection.

The same stale literals were hardcoded in ts-tests/tests/test-relay-rpc.ts, so the tests stayed green while testing nothing.

Fix: both now derive from pallet_evm_precompile_shielded_pool::selectors::{UNSHIELD, PRIVATE_TRANSFER} / the ABI signature, so they cannot drift again.

Related: source_pk — the chain's side of a leak fixed in the SDK

A separate privacy leak surfaced during this work: the recipient's memo carried the spent note's owner_pk unconditionally, which for a shield note is the sender's permanent key. That fix lives in the SDK, since the memo is built there — the chain treats it as opaque ciphertext and holds no key to inspect it.

What belongs in this PR is proving the chain stays out of it: attack_decoder_is_blind_to_the_sourcepk_region_of_the_memo asserts that two calls differing only in those bytes are treated identically. A decoder that could tell them apart would mean the field was not actually encrypted.


Breaking changes

Before After
private_transfer 9 args 10 — trailing ovk_blob: OvkBlob
privateTransfer selector 0x66ed2cd4 0x1ec439cf
ABI head 8 slots (256 B) 9 slots (288 B)
transaction_version 2 3
spec_version 8 9

Extrinsics encoded for tx 2 no longer decode; callers on the old selector are rejected as unsupported. No storage migrationovk_blob is additive and read only at dispatch.


Weights

shielded-pool was marked STALE since OVK landed without re-measuring. Re-benchmarked: +1.9% mean, +9% worst case. The marginal cost per output moved 910M → 989M ps and stays parameterised by output count, so private_transfer_weight_scales_with_outputs still guards against under-pricing the second leaf insert.

Run: 2026-08-11, AMD EPYC-Genoa, steps 50 / repeat 20, --wasm-execution=compiled.


Testing

Suite Result
pallet-shielded-pool 356 passed
pallet-evm-precompile-shielded-pool 93 passed
fc-rpc (relay) 45 passed
cargo test --release --lib --all workspace clean
CI parity cargo fmt --all, taplo fmt --check, clippy --release --all-targets with runtime-benchmarks,skip-proof-verification,try-runtime -D warnings

Adversarial batteries were added rather than only happy-path tests — 27 attacks across the pallet, ABI decoder and relay RPC, plus 45 000 fuzz rounds over the unauthenticated relay surface and the calldata decoder. These cover double-spend within one extrinsic, non-canonical field elements, forged Merkle roots, blob transplantation onto another note, mempool tag collisions, and tree-rollover invariants.

Verified end-to-end against a live dev node and indexer: e2e-ovk (15 checks) and e2e-provenance (32 checks), covering shield → transfer with a real Groth16 proof, recipient scan from the indexer, sender-side OVK recovery, payment-slip regeneration, and selective disclosure.


Also in this PR

  • Decoder documentation. All four precompile call decoders (shield, private_transfer, unshield, claim_shielded_fees) now carry numbered steps matched to their ABI-layout tables, plus named HEAD_SIZE constants and explicit notes on why certain values come from the caller rather than calldata.
  • counterparty_pksource_pk. The old name asserted something false — it does not identify the sender. Renamed in comments and docs across the node; the unused orbinum-encrypted-memo crate keeps the old spelling. Byte offsets are unchanged.
  • README correction. primitives/encrypted-memo/README.md declared a 116-byte plaintext / 176-byte wire format, missing circuit_version — actually 120 / 180.

@nol4lej nol4lej closed this Aug 12, 2026
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