dao: make use of user_data from money_transfer (desc below for more info)

We weren't enforcing that dao_bulla we are operating on is the same as
the user data from the inputs we are spending from. So add those checks
into the ZK circuits.

Since we cannot loop over arbitrary inputs in the ZK circuit, make sure
all inputs have the same user_data_enc - that is they use the same
blind. This is safe since the data being blinded is also the same.

Lastly a minor simplification was made. Instead of storing input value
commits inside the DAO params, we can calculate it from the money
transfer params. This avoids needing to check they match which makes the
code simpler.

src/contract/dao/proof/dao-exec.zk

@@ -40,6 +40,9 @@ witness "DaoExec" {
 	Base dao_spend_hook,
 	Base user_spend_hook,
 	Base user_data,
+
+    # Check input user_data_enc encodes the same DAO bulla
+	Base input_user_data_blind,
 }
 
 circuit "DaoExec" {
@@ -139,6 +142,11 @@ circuit "DaoExec" {
 	# Create the input value commit
 	# Create the value commits
 
+    # The coin we are spending should have the encrypted DAO bulla
+    # Make sure it is the same as the DAO we are operating on.
+	input_user_data_enc = poseidon_hash(dao_bulla, input_user_data_blind);
+	constrain_instance(input_user_data_enc);
+
 	# NOTE: There is a vulnerability here where someone can create the exec
 	# transaction with a bad note so it cannot be decrypted by the receiver
 	# TODO: Research verifiable encryption inside ZK

src/contract/dao/src/client/exec.rs

@@ -27,7 +27,7 @@ use rand::rngs::OsRng;
 
 use darkfi::{
     zk::{Proof, ProvingKey, Witness, ZkCircuit},
-    zkas::ZkBinary,
+    zkas::{util::export_witness_json, ZkBinary},
     Result,
 };
 
@@ -45,6 +45,7 @@ pub struct DaoExecCall {
     pub dao_serial: pallas::Base,
     pub input_value: u64,
     pub input_value_blind: pallas::Scalar,
+    pub input_user_data_blind: pallas::Base,
     pub hook_dao_exec: pallas::Base,
     pub signature_secret: SecretKey,
 }
@@ -153,8 +154,13 @@ impl DaoExecCall {
             Witness::Base(Value::known(self.hook_dao_exec)),
             Witness::Base(Value::known(user_spend_hook)),
             Witness::Base(Value::known(user_data)),
+            // DAO bulla spend check
+            Witness::Base(Value::known(self.input_user_data_blind)),
         ];
 
+        let input_user_data_enc = poseidon_hash([dao_bulla, self.input_user_data_blind]);
+        debug!(target: "dao", "input_user_data_enc: {:?}", input_user_data_enc);
+
         debug!(target: "dao", "proposal_bulla: {:?}", proposal_bulla);
         let public_inputs = vec![
             proposal_bulla.inner(),
@@ -169,7 +175,9 @@ impl DaoExecCall {
             self.hook_dao_exec,
             user_spend_hook,
             user_data,
+            input_user_data_enc,
         ];
+        export_witness_json("witness.json", &prover_witnesses, &public_inputs);
 
         let circuit = ZkCircuit::new(prover_witnesses, exec_zkbin);
         let input_proof = Proof::create(exec_pk, &[circuit], &public_inputs, &mut OsRng)
@@ -178,10 +186,7 @@ impl DaoExecCall {
 
         let params = DaoExecParams {
             proposal: proposal_bulla,
-            coin_0: coin_0.into(),
-            coin_1: coin_1.into(),
             blind_total_vote: DaoBlindAggregateVote { yes_vote_commit, all_vote_commit },
-            input_value_commit,
         };
 
         Ok((params, proofs))

src/contract/dao/src/entrypoint/exec.rs

@@ -39,29 +39,40 @@ pub(crate) fn dao_exec_get_metadata(
     call_idx: u32,
     calls: Vec<ContractCall>,
 ) -> Result<Vec<u8>, ContractError> {
-    let self_ = &calls[call_idx as usize];
-    let params: DaoExecParams = deserialize(&self_.data[1..])?;
+    assert_eq!(call_idx, 1);
+    assert_eq!(calls.len(), 2);
+
+    let money_call = &calls[0];
+    let money_xfer_params: MoneyTransferParamsV1 = deserialize(&money_call.data[1..])?;
+
+    let dao_call = &calls[1];
+    let dao_exec_params: DaoExecParams = deserialize(&dao_call.data[1..])?;
 
     // Public inputs for the ZK proofs we have to verify
     let mut zk_public_inputs: Vec<(String, Vec<pallas::Base>)> = vec![];
     // Public keys for the transaction signatures we have to verify
     let signature_pubkeys: Vec<PublicKey> = vec![];
 
-    let blind_vote = params.blind_total_vote;
+    let blind_vote = dao_exec_params.blind_total_vote;
     let yes_vote_coords = blind_vote.yes_vote_commit.to_affine().coordinates().unwrap();
     let all_vote_coords = blind_vote.all_vote_commit.to_affine().coordinates().unwrap();
-    let input_value_coords = params.input_value_commit.to_affine().coordinates().unwrap();
 
-    // TODO (CRITICAL): deserialize MoneyTransfer data, and pass user_data_enc below.
-    //                  This should be enforced inside the ZK contract.
-    //                  When decrypted it should match dao_bulla.
+    let mut input_valcoms = pallas::Point::identity();
+    for input in &money_xfer_params.inputs {
+        input_valcoms += input.value_commit;
+    }
+    let input_value_coords = input_valcoms.to_affine().coordinates().unwrap();
+
+    assert!(money_xfer_params.inputs.len() > 0);
+    // This value should be the same for all inputs, as enforced in process_instruction() below.
+    let input_user_data_enc = money_xfer_params.inputs[0].user_data_enc;
 
     zk_public_inputs.push((
         DAO_CONTRACT_ZKAS_DAO_EXEC_NS.to_string(),
         vec![
-            params.proposal.inner(),
-            params.coin_0.inner(),
-            params.coin_1.inner(),
+            dao_exec_params.proposal.inner(),
+            money_xfer_params.outputs[1].coin.inner(),
+            money_xfer_params.outputs[0].coin.inner(),
             *yes_vote_coords.x(),
             *yes_vote_coords.y(),
             *all_vote_coords.x(),
@@ -71,6 +82,7 @@ pub(crate) fn dao_exec_get_metadata(
             cid.inner(),
             pallas::Base::ZERO,
             pallas::Base::ZERO,
+            input_user_data_enc,
         ],
     ));
 
@@ -103,39 +115,30 @@ pub(crate) fn dao_exec_process_instruction(
         return Err(DaoError::ExecCallInvalidFormat.into())
     }
 
-    // MoneyTransfer should have exactly 2 outputs
     let mt_params: MoneyTransferParamsV1 = deserialize(&calls[0].data[1..])?;
-    if mt_params.outputs.len() != 2 {
-        msg!("[Dao::Exec] Error: Money outputs != 2");
-        return Err(DaoError::ExecCallInvalidFormat.into())
+
+    // MoneyTransfer should all have the same user_data set.
+    // We check this by ensuring that user_data_enc is also the same for all inputs.
+    // This means using the same blinding factor for all input's user_data.
+    assert!(mt_params.inputs.len() > 0);
+    let user_data_enc = mt_params.inputs[0].user_data_enc;
+    for input in &mt_params.inputs[1..] {
+        if input.user_data_enc != user_data_enc {
+            msg!("[Dao::Exec] Error: Money inputs unmatched user_data_enc");
+            return Err(DaoError::ExecCallInvalidFormat.into())
+        }
     }
 
     // ======
     // Checks
     // ======
-    // 1. Check coins in MoneyTransfer are the same as our coin 0 and coin 1
-    // * outputs[0] is the change returned to DAO
-    // * outputs[1] is the value being sent to the recipient
-    // (This is how it's done in the client API of Money::Transfer)
-    if mt_params.outputs[0].coin != params.coin_1 ||
-        mt_params.outputs[1].coin != params.coin_0 ||
-        mt_params.outputs.len() != 2
-    {
-        msg!("[Dao::Exec] Error: Coin commitments mismatch");
-        return Err(DaoError::ExecCallOutputsMismatch.into())
+    // MoneyTransfer should have exactly 2 outputs
+    if mt_params.outputs.len() != 2 {
+        msg!("[Dao::Exec] Error: Money outputs != 2");
+        return Err(DaoError::ExecCallOutputsLenNot2.into())
     }
 
-    // 2. Sum of MoneyTransfer input value commits == our input value commit
-    let mut input_valcoms = pallas::Point::identity();
-    for input in &mt_params.inputs {
-        input_valcoms += input.value_commit;
-    }
-    if input_valcoms != params.input_value_commit {
-        msg!("[Dao::Exec] Error: Value commitments mismatch");
-        return Err(DaoError::ExecCallValueMismatch.into())
-    }
-
-    // 3. Get the ProposalVote from DAO state
+    // 2. Get the ProposalVote from DAO state
     let proposal_db = db_lookup(cid, DAO_CONTRACT_DB_PROPOSAL_BULLAS)?;
     let Some(data) = db_get(proposal_db, &serialize(&params.proposal))? else {
         msg!("[Dao::Exec] Error: Proposal {:?} not found", params.proposal);
@@ -148,7 +151,7 @@ pub(crate) fn dao_exec_process_instruction(
         return Err(DaoError::ProposalEnded.into())
     }
 
-    // 4. Check yes_vote commit and all_vote_commit are the same as in BlindAggregateVote
+    // 3. Check yes_vote commit and all_vote_commit are the same as in BlindAggregateVote
     if proposal.vote_aggregate.yes_vote_commit != params.blind_total_vote.yes_vote_commit ||
         proposal.vote_aggregate.all_vote_commit != params.blind_total_vote.all_vote_commit
     {

src/contract/dao/src/error.rs

@@ -56,8 +56,8 @@ pub enum DaoError {
     #[error("Exec call has invalid tx format")]
     ExecCallInvalidFormat,
 
-    #[error("Exec call mismatched outputs")]
-    ExecCallOutputsMismatch,
+    #[error("Exec call outputs.len() should be 2")]
+    ExecCallOutputsLenNot2,
 
     #[error("Exec call value commitment mismatch")]
     ExecCallValueMismatch,
@@ -81,7 +81,7 @@ impl From<DaoError> for ContractError {
             DaoError::CoinAlreadySpent => Self::Custom(10),
             DaoError::DoubleVote => Self::Custom(11),
             DaoError::ExecCallInvalidFormat => Self::Custom(12),
-            DaoError::ExecCallOutputsMismatch => Self::Custom(13),
+            DaoError::ExecCallOutputsLenNot2 => Self::Custom(13),
             DaoError::ExecCallValueMismatch => Self::Custom(14),
             DaoError::VoteCommitMismatch => Self::Custom(15),
         }

src/contract/dao/src/model.rs

@@ -18,7 +18,6 @@
 
 use core::str::FromStr;
 
-use darkfi_money_contract::model::Coin;
 use darkfi_sdk::{
     crypto::{note::AeadEncryptedNote, pasta_prelude::*, MerkleNode, Nullifier, PublicKey},
     error::ContractError,
@@ -236,14 +235,8 @@ impl Default for DaoBlindAggregateVote {
 pub struct DaoExecParams {
     /// The proposal bulla
     pub proposal: DaoProposalBulla,
-    /// The output coin for the proposal recipient
-    pub coin_0: Coin,
-    /// The output coin for the change returned to DAO
-    pub coin_1: Coin,
     /// Aggregated blinds for the vote commitments
     pub blind_total_vote: DaoBlindAggregateVote,
-    /// Value commitment for all the inputs
-    pub input_value_commit: pallas::Point,
 }
 
 /// State update for `Dao::Exec`

src/contract/money/src/client/transfer_v1.rs

@@ -145,6 +145,7 @@ pub struct TransferCallBuilder {
     /// User data for the change output
     pub change_user_data: pallas::Base,
     /// User data blind for the change output
+    // TODO (CRITICAL): this is wrongly labelled and used over several inputs
     pub change_user_data_blind: pallas::Base,
     /// Set of `OwnCoin` we're given to use in this builder
     pub coins: Vec<OwnCoin>,

src/contract/test-harness/src/dao_exec.rs

@@ -72,7 +72,8 @@ impl TestHarness {
 
         let change_spend_hook = DAO_CONTRACT_ID.inner();
         let change_user_data = dao_bulla.inner();
-        let change_user_data_blind = pallas::Base::random(&mut OsRng);
+
+        let input_user_data_blind = pallas::Base::random(&mut OsRng);
 
         let coins: Vec<OwnCoin> = dao_wallet
             .unspent_money_coins
@@ -92,7 +93,8 @@ impl TestHarness {
             rcpt_user_data_blind,
             change_spend_hook,
             change_user_data,
-            change_user_data_blind,
+            // TODO (ERROR): incorrectly named
+            change_user_data_blind: input_user_data_blind,
             coins,
             tree,
             mint_zkbin: mint_zkbin.clone(),
@@ -131,6 +133,7 @@ impl TestHarness {
             dao_serial,
             input_value,
             input_value_blind,
+            input_user_data_blind,
             hook_dao_exec: DAO_CONTRACT_ID.inner(),
             signature_secret: exec_signature_secret,
         };