Merge pull request #121 from vacp2p/schouhy/fix-non-uniqueness-of-private-account-ids

Proposal for enforcing uniqueness of private account ids

nssa/core/src/circuit_io.rs

@@ -82,7 +82,7 @@ mod tests {
                 &Account::default(),
             )],
             new_nullifiers: vec![(
-                Nullifier::new(
+                Nullifier::for_account_update(
                     &Commitment::new(&NullifierPublicKey::from(&[2; 32]), &Account::default()),
                     &[1; 32],
                 ),

nssa/core/src/commitment.rs

@@ -7,9 +7,33 @@ use crate::{NullifierPublicKey, account::Account};
 #[cfg_attr(any(feature = "host", test), derive(Debug, Clone, PartialEq, Eq, Hash))]
 pub struct Commitment(pub(super) [u8; 32]);
 
+/// A commitment to all zero data.
+/// ```python
+/// from hashlib import sha256
+/// hasher = sha256()
+/// hasher.update(bytes([0] * 32 + [0] * 32 + [0] * 16 + [0] * 16 + list(sha256().digest())))
+/// DUMMY_COMMITMENT = hasher.digest()
+/// ```
+pub const DUMMY_COMMITMENT: Commitment = Commitment([
+    130, 75, 48, 230, 171, 101, 121, 141, 159, 118, 21, 74, 135, 248, 16, 255, 238, 156, 61, 24,
+    165, 33, 34, 172, 227, 30, 215, 20, 85, 47, 230, 29,
+]);
+
+/// The hash of the dummy commitment
+/// ```python
+/// from hashlib import sha256
+/// hasher = sha256()
+/// hasher.update(DUMMY_COMMITMENT)
+/// DUMMY_COMMITMENT_HASH = hasher.digest()
+/// ```
+pub const DUMMY_COMMITMENT_HASH: [u8; 32] = [
+    170, 10, 217, 228, 20, 35, 189, 177, 238, 235, 97, 129, 132, 89, 96, 247, 86, 91, 222, 214, 38,
+    194, 216, 67, 56, 251, 208, 226, 0, 117, 149, 39,
+];
+
 impl Commitment {
     /// Generates the commitment to a private account owned by user for npk:
-    /// SHA256(npk || program_owner || balance || nonce || data)
+    /// SHA256(npk || program_owner || balance || nonce || SHA256(data))
     pub fn new(npk: &NullifierPublicKey, account: &Account) -> Self {
         let mut bytes = Vec::new();
         bytes.extend_from_slice(&npk.to_byte_array());
@@ -64,3 +88,30 @@ pub fn compute_digest_for_path(
     }
     result
 }
+
+#[cfg(test)]
+mod tests {
+    use risc0_zkvm::sha::{Impl, Sha256};
+
+    use crate::{
+        Commitment, DUMMY_COMMITMENT, DUMMY_COMMITMENT_HASH, NullifierPublicKey, account::Account,
+    };
+
+    #[test]
+    fn test_nothing_up_my_sleeve_dummy_commitment() {
+        let default_account = Account::default();
+        let npk_null = NullifierPublicKey([0; 32]);
+        let expected_dummy_commitment = Commitment::new(&npk_null, &default_account);
+        assert_eq!(DUMMY_COMMITMENT, expected_dummy_commitment);
+    }
+
+    #[test]
+    fn test_nothing_up_my_sleeve_dummy_commitment_hash() {
+        let expected_dummy_commitment_hash: [u8; 32] =
+            Impl::hash_bytes(&DUMMY_COMMITMENT.to_byte_array())
+                .as_bytes()
+                .try_into()
+                .unwrap();
+        assert_eq!(DUMMY_COMMITMENT_HASH, expected_dummy_commitment_hash);
+    }
+}

nssa/core/src/lib.rs

@@ -9,7 +9,10 @@ pub mod program;
 pub mod address;
 
 pub use circuit_io::{PrivacyPreservingCircuitInput, PrivacyPreservingCircuitOutput};
-pub use commitment::{Commitment, CommitmentSetDigest, MembershipProof, compute_digest_for_path};
+pub use commitment::{
+    Commitment, CommitmentSetDigest, DUMMY_COMMITMENT, DUMMY_COMMITMENT_HASH, MembershipProof,
+    compute_digest_for_path,
+};
 pub use encryption::{EncryptionScheme, SharedSecretKey};
 pub use nullifier::{Nullifier, NullifierPublicKey, NullifierSecretKey};
 

nssa/core/src/nullifier.rs

@@ -45,12 +45,20 @@ pub type NullifierSecretKey = [u8; 32];
 pub struct Nullifier(pub(super) [u8; 32]);
 
 impl Nullifier {
-    pub fn new(commitment: &Commitment, nsk: &NullifierSecretKey) -> Self {
+    pub fn for_account_update(commitment: &Commitment, nsk: &NullifierSecretKey) -> Self {
-        let mut bytes = Vec::new();
+        const UPDATE_PREFIX: &[u8; 32] = b"/NSSA/v0.1/Nullifier/Update/\x00\x00\x00\x00";
+        let mut bytes = UPDATE_PREFIX.to_vec();
         bytes.extend_from_slice(&commitment.to_byte_array());
         bytes.extend_from_slice(nsk);
         Self(Impl::hash_bytes(&bytes).as_bytes().try_into().unwrap())
     }
+
+    pub fn for_account_initialization(npk: &NullifierPublicKey) -> Self {
+        const INIT_PREFIX: &[u8; 32] = b"/NSSA/v0.1/Nullifier/Initialize/";
+        let mut bytes = INIT_PREFIX.to_vec();
+        bytes.extend_from_slice(&npk.to_byte_array());
+        Self(Impl::hash_bytes(&bytes).as_bytes().try_into().unwrap())
+    }
 }
 
 #[cfg(test)]
@@ -58,14 +66,28 @@ mod tests {
     use super::*;
 
     #[test]
-    fn test_constructor() {
+    fn test_constructor_for_account_update() {
         let commitment = Commitment((0..32u8).collect::<Vec<_>>().try_into().unwrap());
         let nsk = [0x42; 32];
         let expected_nullifier = Nullifier([
-            97, 87, 111, 191, 0, 44, 125, 145, 237, 104, 31, 230, 203, 254, 68, 176, 126, 17, 240,
+            235, 128, 185, 229, 74, 74, 83, 13, 165, 48, 239, 24, 48, 101, 71, 251, 253, 92, 88,
-            205, 249, 143, 11, 43, 15, 198, 189, 219, 191, 49, 36, 61,
+            201, 103, 43, 250, 135, 193, 54, 175, 82, 245, 171, 90, 135,
         ]);
-        let nullifier = Nullifier::new(&commitment, &nsk);
+        let nullifier = Nullifier::for_account_update(&commitment, &nsk);
+        assert_eq!(nullifier, expected_nullifier);
+    }
+
+    #[test]
+    fn test_constructor_for_account_initialization() {
+        let npk = NullifierPublicKey([
+            112, 188, 193, 129, 150, 55, 228, 67, 88, 168, 29, 151, 5, 92, 23, 190, 17, 162, 164,
+            255, 29, 105, 42, 186, 43, 11, 157, 168, 132, 225, 17, 163,
+        ]);
+        let expected_nullifier = Nullifier([
+            96, 99, 33, 1, 116, 84, 169, 18, 85, 201, 17, 243, 123, 240, 242, 34, 116, 233, 92,
+            203, 247, 92, 161, 162, 135, 66, 127, 108, 230, 149, 105, 157,
+        ]);
+        let nullifier = Nullifier::for_account_initialization(&npk);
         assert_eq!(nullifier, expected_nullifier);
     }
 

nssa/program_methods/guest/src/bin/privacy_preserving_circuit.rs

@@ -3,8 +3,8 @@ use std::collections::HashSet;
 use risc0_zkvm::{guest::env, serde::to_vec};
 
 use nssa_core::{
-    Commitment, CommitmentSetDigest, EncryptionScheme, Nullifier, NullifierPublicKey,
+    Commitment, CommitmentSetDigest, DUMMY_COMMITMENT_HASH, EncryptionScheme,
-    PrivacyPreservingCircuitInput, PrivacyPreservingCircuitOutput,
+    Nullifier, NullifierPublicKey, PrivacyPreservingCircuitInput, PrivacyPreservingCircuitOutput,
     account::{Account, AccountId, AccountWithMetadata},
     compute_digest_for_path,
     encryption::Ciphertext,
@@ -105,8 +105,8 @@ fn main() {
                         panic!("Pre-state not authorized");
                     }
 
-                    // Compute nullifier
+                    // Compute update nullifier
-                    let nullifier = Nullifier::new(&commitment_pre, nsk);
+                    let nullifier = Nullifier::for_account_update(&commitment_pre, nsk);
                     new_nullifiers.push((nullifier, set_digest));
                 } else {
                     if pre_states[i].account != Account::default() {
@@ -116,6 +116,10 @@ fn main() {
                     if pre_states[i].is_authorized {
                         panic!("Found new private account marked as authorized.");
                     }
+
+                    // Compute initialization nullifier
+                    let nullifier = Nullifier::for_account_initialization(npk);
+                    new_nullifiers.push((nullifier, DUMMY_COMMITMENT_HASH));
                 }
 
                 // Update post-state with new nonce

nssa/src/privacy_preserving_transaction/circuit.rs

@@ -91,7 +91,7 @@ impl Proof {
 #[cfg(test)]
 mod tests {
     use nssa_core::{
-        Commitment, EncryptionScheme, Nullifier,
+        Commitment, DUMMY_COMMITMENT_HASH, EncryptionScheme, Nullifier,
         account::{Account, AccountId, AccountWithMetadata},
     };
 
@@ -165,7 +165,7 @@ mod tests {
         assert_eq!(sender_pre, expected_sender_pre);
         assert_eq!(sender_post, expected_sender_post);
         assert_eq!(output.new_commitments.len(), 1);
-        assert_eq!(output.new_nullifiers.len(), 0);
+        assert_eq!(output.new_nullifiers.len(), 1);
         assert_eq!(output.ciphertexts.len(), 1);
 
         let recipient_post = EncryptionScheme::decrypt(
@@ -206,10 +206,16 @@ mod tests {
         let mut commitment_set = CommitmentSet::with_capacity(2);
         commitment_set.extend(std::slice::from_ref(&commitment_sender));
 
-        let expected_new_nullifiers = vec![(
+        let expected_new_nullifiers = vec![
-            Nullifier::new(&commitment_sender, &sender_keys.nsk),
+            (
-            commitment_set.digest(),
+                Nullifier::for_account_update(&commitment_sender, &sender_keys.nsk),
-        )];
+                commitment_set.digest(),
+            ),
+            (
+                Nullifier::for_account_initialization(&recipient_keys.npk()),
+                DUMMY_COMMITMENT_HASH,
+            ),
+        ];
 
         let program = Program::authenticated_transfer_program();
 

nssa/src/privacy_preserving_transaction/message.rs

@@ -125,7 +125,10 @@ pub mod tests {
         let new_commitments = vec![Commitment::new(&npk2, &account2)];
 
         let old_commitment = Commitment::new(&npk1, &account1);
-        let new_nullifiers = vec![(Nullifier::new(&old_commitment, &nsk1), [0; 32])];
+        let new_nullifiers = vec![(
+            Nullifier::for_account_update(&old_commitment, &nsk1),
+            [0; 32],
+        )];
 
         Message {
             public_addresses: public_addresses.clone(),

nssa/src/state.rs

@@ -4,7 +4,7 @@ use crate::{
     public_transaction::PublicTransaction,
 };
 use nssa_core::{
-    Commitment, CommitmentSetDigest, MembershipProof, Nullifier,
+    Commitment, CommitmentSetDigest, DUMMY_COMMITMENT, MembershipProof, Nullifier,
     account::Account,
     address::Address,
     program::{DEFAULT_PROGRAM_ID, ProgramId},
@@ -84,6 +84,7 @@ impl V01State {
             .collect();
 
         let mut private_state = CommitmentSet::with_capacity(32);
+        private_state.extend(&[DUMMY_COMMITMENT]);
         private_state.extend(initial_commitments);
 
         let mut this = Self {
@@ -1026,7 +1027,8 @@ pub mod tests {
         );
 
         let sender_pre_commitment = Commitment::new(&sender_keys.npk(), &sender_private_account);
-        let expected_new_nullifier = Nullifier::new(&sender_pre_commitment, &sender_keys.nsk);
+        let expected_new_nullifier =
+            Nullifier::for_account_update(&sender_pre_commitment, &sender_keys.nsk);
 
         let expected_new_commitment_2 = Commitment::new(
             &recipient_keys.npk(),
@@ -1100,7 +1102,8 @@ pub mod tests {
         );
 
         let sender_pre_commitment = Commitment::new(&sender_keys.npk(), &sender_private_account);
-        let expected_new_nullifier = Nullifier::new(&sender_pre_commitment, &sender_keys.nsk);
+        let expected_new_nullifier =
+            Nullifier::for_account_update(&sender_pre_commitment, &sender_keys.nsk);
 
         assert!(state.private_state.0.contains(&sender_pre_commitment));
         assert!(!state.private_state.0.contains(&expected_new_commitment));
@@ -1942,6 +1945,61 @@ pub mod tests {
         assert!(matches!(result, Err(NssaError::CircuitProvingError(_))));
     }
 
+    #[test]
+    fn test_private_accounts_can_only_be_initialized_once() {
+        let sender_keys = test_private_account_keys_1();
+        let sender_private_account = Account {
+            program_owner: Program::authenticated_transfer_program().id(),
+            balance: 100,
+            nonce: 0xdeadbeef,
+            data: vec![],
+        };
+        let recipient_keys = test_private_account_keys_2();
+
+        let mut state = V01State::new_with_genesis_accounts(&[], &[])
+            .with_private_account(&sender_keys, &sender_private_account);
+
+        let balance_to_move = 37;
+
+        let tx = private_balance_transfer_for_tests(
+            &sender_keys,
+            &sender_private_account,
+            &recipient_keys,
+            balance_to_move,
+            [0xcafecafe, 0xfecafeca],
+            &state,
+        );
+
+        state
+            .transition_from_privacy_preserving_transaction(&tx)
+            .unwrap();
+
+        let sender_private_account = Account {
+            program_owner: Program::authenticated_transfer_program().id(),
+            balance: 100 - balance_to_move,
+            nonce: 0xcafecafe,
+            data: vec![],
+        };
+
+        let tx = private_balance_transfer_for_tests(
+            &sender_keys,
+            &sender_private_account,
+            &recipient_keys,
+            balance_to_move,
+            [0x1234, 0x5678],
+            &state,
+        );
+
+        let result = state.transition_from_privacy_preserving_transaction(&tx);
+
+        assert!(matches!(result, Err(NssaError::InvalidInput(_))));
+        let NssaError::InvalidInput(error_message) = result.err().unwrap() else {
+            panic!("Incorrect message error");
+        };
+        let expected_error_message = "Nullifier already seen".to_string();
+        assert_eq!(error_message, expected_error_message);
+    }
+
     #[test]
     fn test_circuit_should_fail_if_there_are_repeated_ids() {
         let program = Program::simple_balance_transfer();