Description
Found while auditing every arm of Drive::state_transition_proof_binds_execution for #4760. This one is not about shielded pools and is reached from released protocol versions, so it is filed rather than fixed there.
The arm
packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs:577-585:
DocumentTransition::Delete(delete_transition) => {
if document.is_some() {
return Err(Error::Proof(ProofError::IncorrectProof(format!(
"proof of state transition execution contained document after delete with id {}", …))));
}
Ok((root_hash, BTreeMap::from([(delete_transition.base().id(), None)])))
}The only thing checked is that the document is absent. There is no nonce, no revision, and no check that the document ever existed. The classifier then tags any non-indexOnly document transition as ExecutionProved.
What a client is told
Ask "was my delete of document X executed?" for an id that never existed. The proof shows absence, the branch accepts it, and the outcome is ExecutionProved — the strong guarantee, meaning "a valid proof binds the execution of this specific transition". Nothing executed. A wallet that treats that as confirmation will show a delete that never happened, and will do so for an arbitrary id the user never created.
Absence is the same evidence for "deleted" and "never existed". That is the same shape as the shielded findings in #4760 — evidence that authenticates state rather than the request — except here the evidence does not even distinguish whether anything happened at all.
Why no test caught it
The delete test at v0/mod.rs:4520-4540 inserts a document and then deletes it, so it only exercises the happy path. test_unexecuted_delete_proof_is_refused is the opposite case and uses an indexOnly type, which classifies as AffectedState anyway. The never-existed case is pinned nowhere.
More generally, classifier_tags_snapshot_only_families_as_affected_state — the only test that calls the classifier — cannot cover an ExecutionProved arm: its contract lookup returns Ok(None) and both Batch sub-branches ok_or(UnknownContract)?, so a Batch entry would make the test's own expect panic. Every arm claiming the strong guarantee is therefore unpinned.
Scope
- decide what a delete proof must bind: prior existence, the owner's nonce, or a tombstone
- reclassify until it binds it
- a test for a delete of an id that never existed
- give the snapshot test a contract lookup that returns a contract, so
ExecutionProvedarms can be pinned at all
Secondary, same area
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/contract_user_moderation/tests.rs:649 declares fn assert_execution_proved whose body asserts the negative (!is_execution_proved(), message "expected AffectedState"), with nine callers, and two sibling helpers share the mismatch. It reads as if those cases are execution-proved when the test asserts the opposite — consistent with that family having been reclassified without renaming the helpers. Worth renaming before someone trusts the name.
🤖 Generated with Claude Code
Activity
fix(drive)!: a document proof does not prove which shielded payment p…