From 6ed80950f2d3925d02400180b725d48c498f3662 Mon Sep 17 00:00:00 2001 From: rfxfxfx Date: Mon, 31 Aug 2026 13:42:48 +0800 Subject: [PATCH] docs: correct doc comments that describe behaviour the code does not have Four comments that a reader would reasonably act on, each contradicted by the code immediately below it. - pallet-validator-set documents a bond it never takes. The module header attributes spam resistance partly to a bond, `ApprovedValidators` says approval happens "after bond", and `PendingValidators` describes its entries as having a "bond locked". `register_validator` locks nothing and there is no currency dependency in the pallet's Config. Removed the bond references and stated what actually bounds the queue, including the point that `register_relayer` is `ManageOrigin`-gated, so the relayer prerequisite is a stronger gate than the header implied. - The `change_encrypted_memo` comment on `unshield` lists a 116-byte plaintext and omits `circuit_version`. `MemoData` has been 120 bytes since `circuit_version` was added, so a client written against this comment would produce a memo the pallet accepts and no wallet can decrypt. Corrected, and added the 180-byte wire format for the same reason. - `NullifiersSpent` and `CommitmentsInserted` both claim the split between them prevents graph correlation. It does not: both are deposited by the same extrinsic and carry the same `Phase::ApplyExtrinsic` index, so joining them is a one-line query. The split is still worth keeping, but the guarantee comes from the commitments and nullifiers being opaque. Reworded so the comment does not imply a property the runtime cannot provide. - The mainnet genesis preset passes an all-zero sudo key, empty validator and session-key lists, and no verification keys. A chain built from it cannot produce a block and has no governance origin to repair itself. Added a doc comment saying so, since nothing in the file marks it as a placeholder. Comments only. No executable code, signature, or type is changed. Co-Authored-By: Claude Opus 5 --- frame/shielded-pool/src/lib.rs | 16 +++++++++++++--- frame/validator-set/src/lib.rs | 10 ++++++---- .../runtime/src/genesis_config_preset/mainnet.rs | 14 ++++++++++++++ 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/frame/shielded-pool/src/lib.rs b/frame/shielded-pool/src/lib.rs index 00f2bbfe..4fd59e0a 100644 --- a/frame/shielded-pool/src/lib.rs +++ b/frame/shielded-pool/src/lib.rs @@ -541,14 +541,21 @@ pub mod pallet { }, /// Input nullifiers were spent in a private transfer. - /// Emitted independently of CommitmentsInserted to prevent graph correlation. + /// + /// Emitted separately from `CommitmentsInserted` so the two carry no shared + /// field. This does NOT unlink them: both are deposited by the same + /// extrinsic and therefore share a `Phase::ApplyExtrinsic` index, which any + /// indexer can join on. Unlinkability comes from the commitments and + /// nullifiers being opaque, not from the event split. NullifiersSpent { /// Input nullifiers consumed — max 2. nullifiers: BoundedVec>, }, /// Output commitments were inserted into the Merkle tree in a private transfer. - /// Emitted independently of NullifiersSpent to prevent graph correlation. + /// + /// Emitted separately from `NullifiersSpent`; see the note there on what that + /// separation does and does not buy. CommitmentsInserted { /// New commitments created — max 2. commitments: BoundedVec>, @@ -880,7 +887,10 @@ pub mod pallet { // For partial unshield, must equal NoteCommitment(change_value, asset_id, change_owner_pk, change_blinding). change_commitment: Hash, // Encrypted memo for the change note. Must be [0u8; 0] for total unshield. - // For partial unshield, contains encrypted plaintext: [value_lo(8), value_hi(8), owner_pk(32), blinding(32), asset_id(4), counterparty_pk(32)]. + // For partial unshield, contains the encrypted 120-byte `MemoData` plaintext: + // [value_lo(8), value_hi(8), owner_pk(32), blinding(32), asset_id(4), + // counterparty_pk(32), circuit_version(4)]. The wire format wraps it as + // nonce(12) | ciphertext(120) | MAC(16) | ephPk(32) = 180 bytes. change_encrypted_memo: FrameEncryptedMemo, // EVM address of the relay node that signed the tx (from precompile caller); None for direct Substrate. relayer: Option, diff --git a/frame/validator-set/src/lib.rs b/frame/validator-set/src/lib.rs index 1b0ce779..6e0ec797 100644 --- a/frame/validator-set/src/lib.rs +++ b/frame/validator-set/src/lib.rs @@ -24,8 +24,10 @@ //! ## Security //! //! - A non-approved account **never** enters the active validator set. -//! - Spam is bounded by `MaxPendingValidators` and by the session-key and relayer -//! prerequisites, both of which cost a transaction to satisfy. +//! - Registration takes no bond. Spam is bounded by `MaxPendingValidators` and by +//! the session-key and relayer prerequisites. Note that `register_relayer` is +//! itself `ManageOrigin`-gated, so in practice only accounts governance has +//! already touched can enter the pending queue at all. #![cfg_attr(not(feature = "std"), no_std)] @@ -96,13 +98,13 @@ pub mod pallet { /// The approved (active) set of validator account IDs. /// /// Included as block producers at every session rotation. - /// Updated by `add_validator` (sudo) and `approve_validator` (sudo after bond). + /// Updated by `add_validator` (sudo) and `approve_validator` (sudo). #[pallet::storage] #[pallet::getter(fn approved_validators)] pub type ApprovedValidators = StorageValue<_, BoundedVec, ValueQuery>; - /// Accounts that have self-registered (bond locked) and are awaiting governance approval. + /// Accounts that have self-registered and are awaiting governance approval. /// /// Entries here are **not** included in the active session — they only become validators /// after `AddRemoveOrigin` calls `approve_validator`. diff --git a/template/runtime/src/genesis_config_preset/mainnet.rs b/template/runtime/src/genesis_config_preset/mainnet.rs index 9ceb3830..335c21e4 100644 --- a/template/runtime/src/genesis_config_preset/mainnet.rs +++ b/template/runtime/src/genesis_config_preset/mainnet.rs @@ -2,6 +2,20 @@ use crate::genesis_config_preset::build_genesis; use crate::AccountId; use sp_std::vec; +/// PLACEHOLDER. This preset does not describe a launchable chain. +/// +/// Three of its arguments are unset, and each is individually fatal: +/// +/// - the sudo key is the all-zero `AccountId32`, which nobody holds the secret +/// for, so the chain would have no governance origin and no way to register +/// verification keys with `pallet-zk-verifier`; +/// - the validator and session-key lists are empty, so `pallet-session` would +/// hand Aura an empty authority set and no block would ever be produced; +/// - no genesis verification keys are supplied, so every shielded operation +/// would fail even if the first two were fixed. +/// +/// Fill all three in before this is used for anything. Left in place so the +/// preset id resolves and `get_preset` stays total. pub fn mainnet() -> serde_json::Value { build_genesis( AccountId::from([0u8; 32]),