Skip to content

Per-kind obligations in the token pool paths default to doing nothing #5252

Closed as not planned

Description

@shumkov

Follow-up to #4760. Three places ask the author of a future transition kind to remember something, and the default when they forget is "no check" rather than "refuse". None is reachable today — every current kind is handled — so this is hardening against the next kind, not a defect.

The pattern was discussed while reviewing #4760 and deliberately deferred: a structural change across consensus paths does not belong at the end of a 53,000-line PR.

1. One obligation instead of three lists

The three identity-less kinds that touch a pool — TokenShieldedTransferWithShieldedFee, TokenUnshieldWithShieldedFee, TokenPurchaseFromShieldedPool — are enumerated by hand in three places:

  • processor/traits/is_allowed.rs, the protocol-version gate
  • process_raw_state_transitions/v0/mod.rs, token_shielded_pools_touched
  • the fee estimate

A new kind must be added to all three. The gate's detector also ends in _ => None, which cannot be made exhaustive because its arms carry if guards, so the compiler cannot help there at all.

A single trait method — "which pool do I touch, if any" — implemented per kind would collapse the three lists into one and make the compiler enforce it. For batched token transitions this already exists as shielded_pool_actions() on the token transition trait, and that half works correctly; it is the top-level kinds that are hand-listed.

2. The encrypted note length belongs in the type

SerializedAction::encrypted_note is a Vec<u8>. serialized_actions_digest concatenates the six fields of every action with no length prefixes, so the digest is unambiguous only because something else enforces that the note is exactly ENCRYPTED_NOTE_SIZE. That something is validate_encrypted_note_sizes, called from 26 structure-validation sites — and a new kind that forgets to call it makes two different action lists hashable to the same digest.

A fixed-width type for the field would make the invariant unforgettable and the digest unambiguous by construction. The digest layout is otherwise frozen once a protocol version ships it, so this is cheap only while protocol version 14 is unreleased.

3. VerifiedMasternodeVote and VerifiedNextDistribution share a TryInto conversion

derive_more::TryInto on StateTransitionProofResult writes one TryFrom per distinct field-type list and ORs the variants that match. Both of these variants are (Vote), so TryFrom<StateTransitionProofResult> for Vote accepts either, and a caller using the tuple type to tell them apart cannot. The two token pool pairs with the same problem carry #[try_into(ignore)]; this pair does not.

It is not exercised today — the wasm boundary matches on the variant, and nothing uses that conversion — and the pair predates #4760, which is why it was not changed there.

Scope

  • one per-kind obligation for "which pool do I touch", replacing the three hand-written lists
  • encrypted_note fixed-width, before protocol version 14 ships
  • #[try_into(ignore)] on one of the (Vote) variants, or a named conversion

See also #5243 (the credit pool proof classifiers) and #5251 (PreferContractOwner), both deferred from the same review.

🤖 Generated with Claude Code

Activity

  1. shumkov commented on Oct 6, 2026

    @shumkov
    CollaboratorAuthor

    Closing as not planned per maintainer decision. The remaining pool-obligation and fixed-width encrypted-note changes are preventive hardening; no currently reachable defect has been demonstrated here. The shared note type also belongs to shipped credit-transition formats, so changing it requires a separate compatibility case. #5301 was closed without merging. For precision, the Rust SDK does use the Vote conversion while waiting for masternode votes; the current verifier produces VerifiedMasternodeVote on that path, and no confusion with VerifiedNextDistribution was reproduced. These changes are deliberately not being implemented, rather than marked fixed.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions