Merge pull request #29 from logos-co/goas/fix-tx-binding

goas: cryptography in zone

goas/atomic_asset_transfer/common/Cargo.toml

@@ -9,3 +9,7 @@ cl = { path = "../../cl/cl" }
 ledger_proof_statements = { path = "../../cl/ledger_proof_statements" }
 once_cell = "1"
 sha2 = "0.10"
+curve25519-dalek = { version = "4.1", features = ["serde", "digest", "rand_core"] }
+ed25519-dalek = { version = "2.1.1", features = ["rand_core"] }
+rand_core = "0.6.0"
+serde_arrays = "0.1.0"

goas/atomic_asset_transfer/common/src/lib.rs

@@ -1,5 +1,10 @@
-use cl::{balance::Unit, merkle, PartialTxInputWitness};
+use cl::{balance::Unit, merkle, NoteCommitment};
+use ed25519_dalek::{
+    ed25519::{signature::SignerMut, SignatureBytes},
+    Signature, SigningKey, VerifyingKey, PUBLIC_KEY_LENGTH,
+};
 use once_cell::sync::Lazy;
+use rand_core::CryptoRngCore;
 use serde::{Deserialize, Serialize};
 use sha2::{Digest, Sha256};
 use std::collections::BTreeMap;
@@ -13,11 +18,15 @@ pub const MAX_EVENTS: usize = 1 << 8;
 #[derive(Debug, PartialEq, Eq, Clone, Copy, Serialize, Deserialize)]
 pub struct StateCommitment(pub [u8; 32]);
 
-pub type AccountId = u32;
+pub type AccountId = [u8; PUBLIC_KEY_LENGTH];
 
 // PLACEHOLDER: this is probably going to be NMO?
 pub static ZONE_CL_FUNDS_UNIT: Lazy<Unit> = Lazy::new(|| cl::note::unit_point("NMO"));
 
+pub fn new_account(mut rng: impl CryptoRngCore) -> SigningKey {
+    SigningKey::generate(&mut rng)
+}
+
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
 pub struct ZoneMetadata {
     pub zone_vk: [u8; 32],
@@ -93,7 +102,7 @@ impl StateWitness {
     }
 
     pub fn included_tx_witness(&self, idx: usize) -> IncludedTxWitness {
-        let tx = self.included_txs.get(idx).unwrap().clone();
+        let tx = *self.included_txs.get(idx).unwrap();
         let path = merkle::path(self.included_tx_merkle_leaves(), idx);
         IncludedTxWitness { tx, path }
     }
@@ -101,7 +110,7 @@ impl StateWitness {
     pub fn balances_root(&self) -> [u8; 32] {
         let balance_bytes = Vec::from_iter(self.balances.iter().map(|(owner, balance)| {
             let mut bytes: Vec<u8> = vec![];
-            bytes.extend(owner.to_le_bytes());
+            bytes.extend(owner);
             bytes.extend(balance.to_le_bytes());
             bytes
         }));
@@ -136,10 +145,10 @@ pub struct Withdraw {
 }
 
 impl Withdraw {
-    pub fn to_bytes(&self) -> [u8; 12] {
+    pub fn to_bytes(&self) -> [u8; 40] {
-        let mut bytes = [0; 12];
+        let mut bytes = [0; 40];
-        bytes[0..4].copy_from_slice(&self.from.to_le_bytes());
+        bytes[0..PUBLIC_KEY_LENGTH].copy_from_slice(&self.from);
-        bytes[4..12].copy_from_slice(&self.amount.to_le_bytes());
+        bytes[PUBLIC_KEY_LENGTH..PUBLIC_KEY_LENGTH + 8].copy_from_slice(&self.amount.to_le_bytes());
         bytes
     }
 }
@@ -152,31 +161,58 @@ pub struct Deposit {
 }
 
 impl Deposit {
-    pub fn to_bytes(&self) -> [u8; 12] {
+    pub fn to_bytes(&self) -> [u8; 40] {
-        let mut bytes = [0; 12];
+        let mut bytes = [0; 40];
-        bytes[0..4].copy_from_slice(&self.to.to_le_bytes());
+        bytes[0..PUBLIC_KEY_LENGTH].copy_from_slice(&self.to);
-        bytes[4..12].copy_from_slice(&self.amount.to_le_bytes());
+        bytes[PUBLIC_KEY_LENGTH..PUBLIC_KEY_LENGTH + 8].copy_from_slice(&self.amount.to_le_bytes());
         bytes
     }
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
+pub struct SignedBoundTx {
+    pub bound_tx: BoundTx,
+    #[serde(with = "serde_arrays")]
+    pub sig: SignatureBytes,
+}
+
+impl SignedBoundTx {
+    pub fn sign(bound_tx: BoundTx, signing_key: &mut SigningKey) -> Self {
+        let msg = bound_tx.to_bytes();
+        let sig = signing_key.sign(&msg).to_bytes();
+
+        Self { bound_tx, sig }
+    }
+
+    pub fn verify_and_unwrap(&self) -> BoundTx {
+        let msg = self.bound_tx.to_bytes();
+
+        let sig = Signature::from_bytes(&self.sig);
+        let vk = self.bound_tx.tx.verifying_key();
+        vk.verify_strict(&msg, &sig).expect("Invalid tx signature");
+
+        self.bound_tx
+    }
+}
+
 /// A Tx that is executed in the zone if and only if the bind is
 /// present is the same partial transaction
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
 pub struct BoundTx {
     pub tx: Tx,
-    pub bind: PartialTxInputWitness,
+    pub bind: NoteCommitment,
 }
 
 impl BoundTx {
     pub fn to_bytes(&self) -> Vec<u8> {
-        let mut bytes = self.tx.to_bytes();
+        let mut bytes = Vec::new();
-        bytes.extend(self.bind.input.commit().to_bytes());
+        bytes.extend(self.tx.to_bytes());
+        bytes.extend(self.bind.as_bytes());
         bytes
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
 pub enum Tx {
     Withdraw(Withdraw),
     Deposit(Deposit),
@@ -189,6 +225,13 @@ impl Tx {
             Tx::Deposit(deposit) => deposit.to_bytes().to_vec(),
         }
     }
+
+    pub fn verifying_key(&self) -> VerifyingKey {
+        match self {
+            Tx::Withdraw(w) => VerifyingKey::from_bytes(&w.from).unwrap(),
+            Tx::Deposit(d) => VerifyingKey::from_bytes(&d.to).unwrap(),
+        }
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]

goas/atomic_asset_transfer/executor/src/lib.rs

@@ -1,6 +1,6 @@
 use std::collections::BTreeMap;
 
-use common::{BoundTx, StateWitness, Tx, ZoneMetadata};
+use common::{AccountId, SignedBoundTx, StateWitness, Tx, ZoneMetadata};
 use goas_proof_statements::{
     user_note::UserAtomicTransfer, zone_funds::SpendFundsPrivate, zone_state::ZoneStatePrivate,
 };
@@ -16,7 +16,7 @@ pub struct ZoneNotes {
 impl ZoneNotes {
     pub fn new_with_balances(
         zone_name: &str,
-        balances: BTreeMap<u32, u64>,
+        balances: BTreeMap<AccountId, u64>,
         mut rng: impl CryptoRngCore,
     ) -> Self {
         let state = StateWitness {
@@ -121,7 +121,7 @@ pub fn zone_metadata(zone_mnemonic: &str) -> ZoneMetadata {
 
 pub fn prove_zone_stf(
     state: StateWitness,
-    inputs: Vec<BoundTx>,
+    inputs: Vec<(SignedBoundTx, cl::PartialTxInputWitness)>,
     zone_in: cl::PartialTxInputWitness,
     zone_out: cl::PartialTxOutputWitness,
     funds_out: cl::PartialTxOutputWitness,

goas/atomic_asset_transfer/executor/tests/atomic_transfer.rs

@@ -1,7 +1,7 @@
 use std::collections::BTreeMap;
 
 use cl::{BundleWitness, NoteWitness, NullifierNonce};
-use common::{BoundTx, Deposit, Tx, Withdraw};
+use common::{new_account, BoundTx, Deposit, SignedBoundTx, Tx, Withdraw};
 use executor::ZoneNotes;
 use goas_proof_statements::user_note::{UserAtomicTransfer, UserIntent};
 
@@ -9,10 +9,11 @@ use goas_proof_statements::user_note::{UserAtomicTransfer, UserIntent};
 fn test_atomic_transfer() {
     let mut rng = rand::thread_rng();
 
-    let alice = 42;
+    let mut alice = new_account(&mut rng);
+    let alice_vk = alice.verifying_key().to_bytes();
 
     let zone_a_start =
-        ZoneNotes::new_with_balances("ZONE_A", BTreeMap::from_iter([(alice, 100)]), &mut rng);
+        ZoneNotes::new_with_balances("ZONE_A", BTreeMap::from_iter([(alice_vk, 100)]), &mut rng);
 
     let zone_b_start = ZoneNotes::new_with_balances("ZONE_B", BTreeMap::from_iter([]), &mut rng);
 
@@ -20,11 +21,11 @@ fn test_atomic_transfer() {
         zone_a_meta: zone_a_start.state.zone_metadata,
         zone_b_meta: zone_b_start.state.zone_metadata,
         withdraw: Withdraw {
-            from: alice,
+            from: alice_vk,
             amount: 75,
         },
         deposit: Deposit {
-            to: alice,
+            to: alice_vk,
             amount: 75,
         },
     };
@@ -69,6 +70,21 @@ fn test_atomic_transfer() {
         ],
     };
 
+    let signed_withdraw = SignedBoundTx::sign(
+        BoundTx {
+            tx: Tx::Withdraw(alice_intent.withdraw),
+            bind: alice_intent_in.note_commitment(),
+        },
+        &mut alice,
+    );
+    let signed_deposit = SignedBoundTx::sign(
+        BoundTx {
+            tx: Tx::Deposit(alice_intent.deposit),
+            bind: alice_intent_in.note_commitment(),
+        },
+        &mut alice,
+    );
+
     let death_proofs = BTreeMap::from_iter([
         (
             alice_intent_in.nullifier(),
@@ -87,13 +103,10 @@ fn test_atomic_transfer() {
             zone_a_start.state_input_witness().nullifier(),
             executor::prove_zone_stf(
                 zone_a_start.state.clone(),
-                vec![BoundTx {
+                vec![(signed_withdraw, atomic_transfer_ptx.input_witness(0))], // withdraw bound to input intent note
-                    tx: Tx::Withdraw(alice_intent.withdraw),
+                atomic_transfer_ptx.input_witness(1),                          // input state note
-                    bind: atomic_transfer_ptx.input_witness(0), // input intent note
+                atomic_transfer_ptx.output_witness(0),                         // output state note
-                }],
+                atomic_transfer_ptx.output_witness(1),                         // output funds note
-                atomic_transfer_ptx.input_witness(1), // input state note
-                atomic_transfer_ptx.output_witness(0), // output state note
-                atomic_transfer_ptx.output_witness(1), // output funds note
             ),
         ),
         (
@@ -108,13 +121,10 @@ fn test_atomic_transfer() {
             zone_b_start.state_input_witness().nullifier(),
             executor::prove_zone_stf(
                 zone_b_start.state.clone(),
-                vec![BoundTx {
+                vec![(signed_deposit, atomic_transfer_ptx.input_witness(0))], // deposit bound to input intent note
-                    tx: Tx::Deposit(alice_intent.deposit),
+                atomic_transfer_ptx.input_witness(3),                         // input state note
-                    bind: atomic_transfer_ptx.input_witness(0), // input intent note
+                atomic_transfer_ptx.output_witness(2),                        // output state note
-                }],
+                atomic_transfer_ptx.output_witness(3),                        // output funds note
-                atomic_transfer_ptx.input_witness(3), // input state note
-                atomic_transfer_ptx.output_witness(2), // output state note
-                atomic_transfer_ptx.output_witness(3), // output funds note
             ),
         ),
         (

goas/atomic_asset_transfer/executor/tests/deposit_ptx.rs

@@ -1,7 +1,7 @@
 use std::collections::BTreeMap;
 
 use cl::{NoteWitness, NullifierSecret};
-use common::{BoundTx, StateWitness, Tx, ZONE_CL_FUNDS_UNIT};
+use common::{new_account, BoundTx, SignedBoundTx, StateWitness, Tx, ZONE_CL_FUNDS_UNIT};
 use executor::ZoneNotes;
 use ledger::death_constraint::DeathProof;
 
@@ -9,13 +9,14 @@ use ledger::death_constraint::DeathProof;
 fn test_deposit() {
     let mut rng = rand::thread_rng();
 
-    let alice = 42;
+    let mut alice = new_account(&mut rng);
-    let alice_sk = NullifierSecret::random(&mut rng);
+    let alice_vk = alice.verifying_key().to_bytes();
+    let alice_cl_sk = NullifierSecret::random(&mut rng);
 
     let zone_start = ZoneNotes::new_with_balances("ZONE", BTreeMap::new(), &mut rng);
 
     let deposit = common::Deposit {
-        to: alice,
+        to: alice_vk,
         amount: 78,
     };
 
@@ -28,10 +29,10 @@ fn test_deposit() {
                 *ZONE_CL_FUNDS_UNIT,
                 DeathProof::nop_constraint(), // alice should demand a tx inclusion proof for the deposit
             ),
-            alice_sk.commit(),
+            alice_cl_sk.commit(),
             &mut rng,
         ),
-        alice_sk,
+        alice_cl_sk,
         &mut rng,
     );
 
@@ -40,16 +41,21 @@ fn test_deposit() {
         outputs: vec![zone_end.state_note, zone_end.fund_note],
     };
 
+    let signed_deposit = SignedBoundTx::sign(
+        BoundTx {
+            tx: Tx::Deposit(deposit),
+            bind: alice_deposit.note_commitment(),
+        },
+        &mut alice,
+    );
+
     let death_proofs = BTreeMap::from_iter([
         (
             zone_start.state_input_witness().nullifier(),
             executor::prove_zone_stf(
                 zone_start.state.clone(),
-                vec![BoundTx {
+                vec![(signed_deposit, deposit_ptx.input_witness(1))], // bind it to the deposit note)],
-                    tx: Tx::Deposit(deposit),
+                deposit_ptx.input_witness(0),                         // input state note (input #0)
-                    bind: deposit_ptx.input_witness(1), // bind it to the deposit note
-                }],
-                deposit_ptx.input_witness(0), // input state note (input #0)
                 deposit_ptx.output_witness(0), // output state note (output #0)
                 deposit_ptx.output_witness(1), // output funds note (output #1)
             ),
@@ -78,7 +84,7 @@ fn test_deposit() {
     assert_eq!(
         zone_end.state_note.note.state,
         StateWitness {
-            balances: BTreeMap::from_iter([(alice, 78)]),
+            balances: BTreeMap::from_iter([(alice_vk, 78)]),
             included_txs: vec![Tx::Deposit(deposit)],
             zone_metadata: zone_start.state.zone_metadata,
         }

goas/atomic_asset_transfer/executor/tests/withdraw_ptx.rs

@@ -1,7 +1,7 @@
 use std::collections::BTreeMap;
 
 use cl::{NoteWitness, NullifierSecret};
-use common::{BoundTx, StateWitness, Tx, ZONE_CL_FUNDS_UNIT};
+use common::{new_account, BoundTx, SignedBoundTx, StateWitness, Tx, ZONE_CL_FUNDS_UNIT};
 use executor::ZoneNotes;
 use ledger::death_constraint::DeathProof;
 
@@ -9,24 +9,25 @@ use ledger::death_constraint::DeathProof;
 fn test_withdrawal() {
     let mut rng = rand::thread_rng();
 
-    let alice = 42;
+    let mut alice = new_account(&mut rng);
-    let alice_sk = NullifierSecret::random(&mut rng);
+    let alice_vk = alice.verifying_key().to_bytes();
+    let alice_cl_sk = NullifierSecret::random(&mut rng);
 
     let zone_start =
-        ZoneNotes::new_with_balances("ZONE", BTreeMap::from_iter([(alice, 100)]), &mut rng);
+        ZoneNotes::new_with_balances("ZONE", BTreeMap::from_iter([(alice_vk, 100)]), &mut rng);
 
     let alice_intent = cl::InputWitness::random(
         cl::OutputWitness::random(
             NoteWitness::stateless(1, *ZONE_CL_FUNDS_UNIT, DeathProof::nop_constraint()), // TODO, intent should be in the death constraint
-            alice_sk.commit(),
+            alice_cl_sk.commit(),
             &mut rng,
         ),
-        alice_sk,
+        alice_cl_sk,
         &mut rng,
     );
 
     let withdraw = common::Withdraw {
-        from: alice,
+        from: alice_vk,
         amount: 78,
     };
 
@@ -38,7 +39,7 @@ fn test_withdrawal() {
             *ZONE_CL_FUNDS_UNIT,
             DeathProof::nop_constraint(),
         ),
-        alice_sk.commit(),
+        alice_cl_sk.commit(),
         &mut rng,
     );
 
@@ -51,15 +52,20 @@ fn test_withdrawal() {
         outputs: vec![zone_end.state_note, zone_end.fund_note, alice_withdrawal],
     };
 
+    let signed_withdraw = SignedBoundTx::sign(
+        BoundTx {
+            tx: Tx::Withdraw(withdraw),
+            bind: alice_intent.note_commitment(),
+        },
+        &mut alice,
+    );
+
     let death_proofs = BTreeMap::from_iter([
         (
             zone_start.state_input_witness().nullifier(),
             executor::prove_zone_stf(
                 zone_start.state.clone(),
-                vec![BoundTx {
+                vec![(signed_withdraw, withdraw_ptx.input_witness(2))],
-                    tx: Tx::Withdraw(withdraw),
-                    bind: withdraw_ptx.input_witness(2),
-                }],
                 withdraw_ptx.input_witness(0), // input state note (input #0)
                 withdraw_ptx.output_witness(0), // output state note (output #0)
                 withdraw_ptx.output_witness(1), // output funds note (output #1)
@@ -98,7 +104,7 @@ fn test_withdrawal() {
     assert_eq!(
         zone_end.state_note.note.state,
         StateWitness {
-            balances: BTreeMap::from_iter([(alice, 22)]),
+            balances: BTreeMap::from_iter([(alice_vk, 22)]),
             included_txs: vec![Tx::Withdraw(withdraw)],
             zone_metadata: zone_start.state.zone_metadata,
         }

goas/atomic_asset_transfer/proof_statements/src/zone_state.rs

@@ -1,10 +1,10 @@
-use common::{BoundTx, StateWitness};
+use common::{SignedBoundTx, StateWitness};
 use serde::{Deserialize, Serialize};
 
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub struct ZoneStatePrivate {
     pub state: StateWitness,
-    pub inputs: Vec<BoundTx>,
+    pub inputs: Vec<(SignedBoundTx, cl::PartialTxInputWitness)>,
     pub zone_in: cl::PartialTxInputWitness,
     pub zone_out: cl::PartialTxOutputWitness,
     /// While the absence of birth constraints does not guarantee uniqueness of a note that can be used as

goas/atomic_asset_transfer/risc0_proofs/zone_state/src/main.rs

@@ -78,9 +78,16 @@ fn main() {
 
     let in_state_cm = state.commit();
 
-    for BoundTx { tx, bind } in inputs {
+    for (signed_bound_tx, ptx_input_witness) in inputs {
-        assert_eq!(bind.input_root(), input_root);
+        // verify the signature
-        state = state.apply(tx)
+        let bound_tx = signed_bound_tx.verify_and_unwrap();
+
+        // ensure the note this tx is bound to is present in the ptx
+        assert_eq!(bound_tx.bind, ptx_input_witness.input.note_commitment());
+        assert_eq!(ptx_input_witness.input_root(), input_root);
+
+        // apply the ptx
+        state = state.apply(bound_tx.tx)
     }
 
     validate_zone_transition(zone_in, zone_out, funds_out, in_state_cm, state);