fix: address PR review feedback

- Add SealingPublicKey/SealingSecretKey type aliases for seal_for/unseal
- Generalize PrivateGroupPda to PrivatePda with pre-resolved keys
- Rename group_pda_spender to private_pda_spender
- Rename group_pda_accounts to pda_accounts with serde alias
- Remove unused storage_mut()
- Remove stale group_pda_router.bin artifact
This commit is contained in:
Moudy 2026-04-30 09:11:08 +02:00
parent c327f592bf
commit f375a35929
10 changed files with 60 additions and 59 deletions

Binary file not shown.

View File

@ -1,14 +1,24 @@
use aes_gcm::{Aes256Gcm, KeyInit as _, aead::Aead as _};
use nssa_core::{
SharedSecretKey,
encryption::{Scalar, ViewingPublicKey, shared_key_derivation::Secp256k1Point},
encryption::{Scalar, shared_key_derivation::Secp256k1Point},
program::PdaSeed,
};
use rand::{RngCore as _, rngs::OsRng};
use serde::{Deserialize, Serialize};
use sha2::{Digest as _, digest::FixedOutput as _};
use super::secret_holders::{PrivateKeyHolder, SecretSpendingKey, ViewingSecretKey};
use super::secret_holders::{PrivateKeyHolder, SecretSpendingKey};
/// Public key used to seal a `GroupKeyHolder` for distribution to a recipient.
///
/// Structurally identical to `ViewingPublicKey` (both are secp256k1 points), but given
/// a distinct alias to clarify intent: viewing keys encrypt account state, sealing keys
/// encrypt the GMS for off-chain distribution.
pub type SealingPublicKey = Secp256k1Point;
/// Secret key used to unseal a `GroupKeyHolder` received from another member.
pub type SealingSecretKey = Scalar;
/// Manages shared viewing keys for a group of controllers owning private PDAs.
///
@ -19,7 +29,7 @@ use super::secret_holders::{PrivateKeyHolder, SecretSpendingKey, ViewingSecretKe
/// # Distribution
///
/// The GMS is a long-term secret and must never cross a trust boundary in raw form.
/// Controllers share it off-chain by sealing it under each recipient's [`ViewingPublicKey`]
/// Controllers share it off-chain by sealing it under each recipient's [`SealingPublicKey`]
/// (see `seal_for` / `unseal`). Wallets persisting a `GroupKeyHolder` must encrypt it at
/// rest; the raw bytes are exposed only via [`GroupKeyHolder::dangerous_raw_gms`], which
/// is intended for the sealing path exclusively.
@ -130,7 +140,7 @@ impl GroupKeyHolder {
.produce_private_key_holder(None)
}
/// Encrypts this holder's GMS and epoch under the recipient's [`ViewingPublicKey`].
/// Encrypts this holder's GMS and epoch under the recipient's [`SealingPublicKey`].
///
/// Uses an ephemeral ECDH key exchange to derive a shared secret, then AES-256-GCM
/// to encrypt the payload. The returned bytes are
@ -139,11 +149,11 @@ impl GroupKeyHolder {
/// Each call generates a fresh ephemeral key, so two seals of the same holder produce
/// different ciphertexts.
#[must_use]
pub fn seal_for(&self, recipient_vpk: &ViewingPublicKey) -> Vec<u8> {
pub fn seal_for(&self, recipient_key: &SealingPublicKey) -> Vec<u8> {
let mut ephemeral_scalar: Scalar = [0_u8; 32];
OsRng.fill_bytes(&mut ephemeral_scalar);
let ephemeral_pubkey = ViewingPublicKey::from_scalar(ephemeral_scalar);
let shared = SharedSecretKey::new(&ephemeral_scalar, recipient_vpk);
let ephemeral_pubkey = Secp256k1Point::from_scalar(ephemeral_scalar);
let shared = SharedSecretKey::new(&ephemeral_scalar, recipient_key);
let aes_key = Self::seal_kdf(&shared);
let cipher = Aes256Gcm::new(&aes_key.into());
@ -170,11 +180,11 @@ impl GroupKeyHolder {
out
}
/// Decrypts a sealed `GroupKeyHolder` using the recipient's `ViewingSecretKey`.
/// Decrypts a sealed `GroupKeyHolder` using the recipient's [`SealingSecretKey`].
///
/// Returns `Err` if the ciphertext is too short, the ECDH point is invalid, or the
/// AES-GCM authentication tag doesn't verify (wrong key or tampered data).
pub fn unseal(sealed: &[u8], own_vsk: &ViewingSecretKey) -> Result<Self, SealError> {
pub fn unseal(sealed: &[u8], own_key: &SealingSecretKey) -> Result<Self, SealError> {
const HEADER_LEN: usize = 33 + 12;
const MIN_LEN: usize = HEADER_LEN + 16;
if sealed.len() < MIN_LEN {
@ -185,7 +195,7 @@ impl GroupKeyHolder {
let nonce = aes_gcm::Nonce::from_slice(&sealed[33..HEADER_LEN]);
let ciphertext = &sealed[HEADER_LEN..];
let shared = SharedSecretKey::new(own_vsk, &ephemeral_pubkey);
let shared = SharedSecretKey::new(own_key, &ephemeral_pubkey);
let aes_key = Self::seal_kdf(&shared);
let cipher = Aes256Gcm::new(&aes_key.into());
@ -560,7 +570,7 @@ mod tests {
/// Sealed payload is too short.
#[test]
fn unseal_too_short_fails() {
let vsk: ViewingSecretKey = [7_u8; 32];
let vsk: SealingSecretKey = [7_u8; 32];
let result = GroupKeyHolder::unseal(&[0_u8; 10], &vsk);
assert!(matches!(result, Err(super::SealError::TooShort)));
}

View File

@ -29,12 +29,12 @@ pub struct NSSAUserData {
/// An older wallet binary that re-serializes this struct will drop the field.
#[serde(default)]
pub group_key_holders: BTreeMap<String, GroupKeyHolder>,
/// Cached plaintext state of group private PDA accounts, keyed by `AccountId`.
/// Updated after each group PDA transaction by decrypting the circuit output.
/// Cached plaintext state of private PDA accounts, keyed by `AccountId`.
/// Updated after each private PDA transaction by decrypting the circuit output.
/// The sequencer only stores encrypted commitments, so this local cache is the
/// only source of plaintext state for group PDAs.
#[serde(default)]
pub group_pda_accounts: BTreeMap<nssa::AccountId, nssa_core::account::Account>,
/// only source of plaintext state for private PDAs.
#[serde(default, alias = "group_pda_accounts")]
pub pda_accounts: BTreeMap<nssa::AccountId, nssa_core::account::Account>,
}
impl NSSAUserData {
@ -94,7 +94,7 @@ impl NSSAUserData {
public_key_tree,
private_key_tree,
group_key_holders: BTreeMap::new(),
group_pda_accounts: BTreeMap::new(),
pda_accounts: BTreeMap::new(),
})
}

View File

@ -420,10 +420,10 @@ mod tests {
}
/// Group PDA deposit: creates a new PDA and transfers balance from the
/// counterparty. Both accounts owned by `group_pda_spender`.
/// counterparty. Both accounts owned by `private_pda_spender`.
#[test]
fn group_pda_deposit() {
let program = Program::group_pda_spender();
let program = Program::private_pda_spender();
let noop = Program::noop();
let keys = test_private_account_keys_1();
let npk = keys.npk();
@ -475,7 +475,7 @@ mod tests {
/// testing with a pre-funded PDA requires a two-tx sequence with membership proofs.
#[test]
fn group_pda_spend_binding() {
let program = Program::group_pda_spender();
let program = Program::private_pda_spender();
let noop = Program::noop();
let keys = test_private_account_keys_1();
let npk = keys.npk();

View File

@ -313,12 +313,12 @@ mod tests {
}
#[must_use]
pub fn group_pda_spender() -> Self {
use test_program_methods::{GROUP_PDA_SPENDER_ELF, GROUP_PDA_SPENDER_ID};
pub fn private_pda_spender() -> Self {
use test_program_methods::{PRIVATE_PDA_SPENDER_ELF, PRIVATE_PDA_SPENDER_ID};
Self {
id: GROUP_PDA_SPENDER_ID,
elf: GROUP_PDA_SPENDER_ELF.to_vec(),
id: PRIVATE_PDA_SPENDER_ID,
elf: PRIVATE_PDA_SPENDER_ELF.to_vec(),
}
}

View File

@ -201,12 +201,6 @@ impl WalletCore {
&self.storage
}
/// Get mutable storage (e.g. for inserting group key holders).
#[must_use]
pub const fn storage_mut(&mut self) -> &mut WalletChainStore {
&mut self.storage
}
/// Restore storage from an existing mnemonic phrase.
pub fn restore_storage(&mut self, mnemonic: &Mnemonic, password: &str) -> Result<()> {
self.storage = WalletChainStore::restore_storage(

View File

@ -18,11 +18,13 @@ pub enum PrivacyPreservingAccount {
npk: NullifierPublicKey,
vpk: ViewingPublicKey,
},
/// A private PDA owned by a group. The wallet derives keys from the
/// `GroupKeyHolder` stored under `group_label`, then computes the
/// `AccountId` via `AccountId::for_private_pda(program_id, seed, npk)`.
PrivateGroupPda {
group_label: String,
/// A private PDA with externally-provided keys. The caller resolves the keys
/// (e.g. via `GroupKeyHolder::derive_keys_for_pda`) before constructing this variant.
/// The wallet computes the `AccountId` via `AccountId::for_private_pda(program_id, seed, npk)`.
PrivatePda {
nsk: NullifierSecretKey,
npk: NullifierPublicKey,
vpk: ViewingPublicKey,
program_id: ProgramId,
seed: PdaSeed,
},
@ -40,7 +42,7 @@ impl PrivacyPreservingAccount {
&self,
Self::PrivateOwned(_)
| Self::PrivateForeign { npk: _, vpk: _ }
| Self::PrivateGroupPda { .. }
| Self::PrivatePda { .. }
)
}
}
@ -105,13 +107,15 @@ impl AccountManager {
(State::Private(pre), 2)
}
PrivacyPreservingAccount::PrivateGroupPda {
group_label,
PrivacyPreservingAccount::PrivatePda {
nsk,
npk,
vpk,
program_id,
seed,
} => {
let pre =
group_pda_preparation(wallet, &group_label, &program_id, &seed).await?;
private_pda_preparation(wallet, nsk, npk, vpk, &program_id, &seed).await?;
(State::Private(pre), 3)
}
@ -227,22 +231,14 @@ struct AccountPreparedData {
proof: Option<MembershipProof>,
}
async fn group_pda_preparation(
async fn private_pda_preparation(
wallet: &WalletCore,
group_label: &str,
nsk: NullifierSecretKey,
npk: NullifierPublicKey,
vpk: ViewingPublicKey,
program_id: &ProgramId,
seed: &PdaSeed,
) -> Result<AccountPreparedData, ExecutionFailureKind> {
let holder = wallet
.storage
.user_data
.group_key_holder(group_label)
.ok_or(ExecutionFailureKind::KeyNotFoundError)?;
let keys = holder.derive_keys_for_pda(seed);
let npk = keys.generate_nullifier_public_key();
let vpk = keys.generate_viewing_public_key();
let nsk = keys.nullifier_secret_key;
let account_id = nssa::AccountId::for_private_pda(program_id, seed, &npk);
// Check local cache first (private PDA state is encrypted on-chain, the sequencer
@ -250,7 +246,7 @@ async fn group_pda_preparation(
let acc = wallet
.storage
.user_data
.group_pda_accounts
.pda_accounts
.get(&account_id)
.cloned()
.unwrap_or_default();
@ -260,8 +256,7 @@ async fn group_pda_preparation(
// is_authorized tracks whether the account existed on-chain before this tx.
// NSK is only provided for existing accounts: the circuit consumes NSKs sequentially
// from an iterator and asserts none are left over, so supplying an NSK for a new
// (unauthorized) account would trigger the over-supply assertion. This matches the
// PrivateForeign path (nsk: None for unauthorized accounts).
// (unauthorized) account would trigger the over-supply assertion.
let pre_state = AccountWithMetadata::new(acc, exists, account_id);
let proof = if exists {
@ -324,11 +319,13 @@ mod tests {
use super::*;
#[test]
fn private_group_pda_is_private() {
let acc = PrivacyPreservingAccount::PrivateGroupPda {
group_label: String::from("test"),
program_id: [1; 8],
seed: PdaSeed::new([2; 32]),
fn private_pda_is_private() {
let acc = PrivacyPreservingAccount::PrivatePda {
nsk: [0; 32],
npk: NullifierPublicKey([1; 32]),
vpk: ViewingPublicKey::from_scalar([2; 32]),
program_id: [3; 8],
seed: PdaSeed::new([4; 32]),
};
assert!(acc.is_private());
assert!(!acc.is_public());