Repository navigation
Shielded credit pool proofs may classify spent nullifiers as proof of a particular transition #5243
Description
Drive::state_transition_proof_binds_execution decides whether a valid proof binds the execution of the requested state transition (ExecutionProved) or only authenticates the state that transition affects (AffectedState). Three credit pool arms claim the former on evidence that may not carry it.
packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs:
StateTransition::Unshield(_) => true,
StateTransition::ShieldedTransfer(_) => true,
StateTransition::ShieldedWithdrawal(_) => true,Why this is a problem
An Orchard spend derives its nullifier from the note it consumes and the viewing key, never from the outputs that replace it. A note owner can therefore authorize two transitions spending the same notes with different outputs, and with no dummy padding their nullifier sets are identical. Whichever executes leaves evidence that fits the other equally well.
Where the verification shows only that the named nullifiers are spent, ExecutionProved tells a client that their transition executed when a different one did — paying someone else, or moving a different amount. That is a false positive in a client-facing guarantee, not a consensus fault: validators agree on what happened, the client is told the wrong thing about which request caused it.
The equivalent token pool arms were corrected in #4760 (TokenTransition::ShieldedTransfer, TokenShieldedTransferWithShieldedFee now report affected state, and TokenTransition::BurnFromPool is classified by whether it is grouped, since a group action's recorded entry binds the action id and that id derives from the digest of its actions).
These three are not uniform — judge them separately
This is the part worth knowing before anyone changes them as a set. What each branch actually verifies differs:
| arm | what the verification returns | reading |
|---|---|---|
ShieldedWithdrawal |
VerifiedShieldedNullifiersWithWithdrawalDocument |
a withdrawal document records its destination and amount, so the specific withdrawal may genuinely be bound — true may be correct |
Unshield |
VerifiedShieldedNullifiersWithAddressInfos |
address info is a snapshot of the address, not evidence that this unshield paid it |
ShieldedTransfer |
VerifiedShieldedNullifiers |
nullifiers alone, the same shape the token transfer had |
So ShieldedWithdrawal should not be changed without first establishing whether the withdrawal document is bound to the request, and Unshield needs the same question asked of its address infos.
Why this was not fixed in #4760
These arms are reached from protocol version 12 and are live, including on mainnet. Changing what a client is told about a released protocol version is a behaviour change for SDK consumers, and deserves its own analysis rather than riding along with an unreleased feature.
No test pins any of these classifications today, which is why the token-side defect went unnoticed. Whatever is decided, each arm wants a test asserting its classification.
Scope
- establish whether the withdrawal document binds the requested withdrawal
- establish whether the address infos bind the requested unshield
- reclassify what is not bound
- a test per arm, since none exists
Found while addressing a review finding on #4760.
🤖 Generated with Claude Code
Activity
shumkov commentedon Oct 2, 2026
Worked through the three arms against the code. Two need changing, one does not — and that one was listed here as a candidate on my part, which was wrong.
ShieldedWithdrawal— leave it aloneThe destination is bound. The branch derives the withdrawal document id deterministically:
So two withdrawals of the same notes to different destinations have different document ids, and proving the document exists at the requested id proves a withdrawal with that destination consumed those notes.
ExecutionProvedis defensible.One residual, much narrower than the transfer case: the entropy covers the nullifier and the output script but not the amount, and the branch returns the document rather than comparing its amount to the request. Two withdrawals of the same notes to the same destination differing only in amount would share an id, so a caller that does not check the returned amount could be misled. Worth a look, not worth reclassifying.
Unshield— not boundThe branch verifies the nullifier spend statuses and, in the same strict merged query, the balance of
st.output_address()viaDrive::balances_for_clear_addresses_query. That is a plain balance read: it shows what the address holds at the committed block, not that this unshield credited it. Two unshields of the same notes to different addresses share their nullifiers, so the evidence fits both and the address balance adds nothing that separates them.ShieldedTransfer— not boundNullifiers alone, the same shape the token transfer had before #4760 reclassified it.
Revised scope
UnshieldandShieldedTransferas affected state; leaveShieldedWithdrawalas execution provedThis is a client-visible change on protocol versions 12 and up, so it stays out of #4760 and wants its own PR.
🤖 Generated with Claude Code
fix(drive)!: a document proof does not prove which shielded payment p…
shumkov commentedon Oct 7, 2026
Closing as not planned per maintainer decision following closure of #5302 without merging. The proposed downgrade of shielded credit-pool proof outcomes and the resulting strict-wait behavior change are not being pursued. This issue is deliberately cancelled, not marked fixed.