diff --git a/src/contract/dao/proof/dao-exec.zk b/src/contract/dao/proof/dao-exec.zk index 04ca98577..e832de300 100644 --- a/src/contract/dao/proof/dao-exec.zk +++ b/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 diff --git a/src/contract/dao/src/client/exec.rs b/src/contract/dao/src/client/exec.rs index 8e262fdd9..f1d5f6596 100644 --- a/src/contract/dao/src/client/exec.rs +++ b/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)) diff --git a/src/contract/dao/src/entrypoint/exec.rs b/src/contract/dao/src/entrypoint/exec.rs index ec0aae46a..e9b48ee83 100644 --- a/src/contract/dao/src/entrypoint/exec.rs +++ b/src/contract/dao/src/entrypoint/exec.rs @@ -39,29 +39,40 @@ pub(crate) fn dao_exec_get_metadata( call_idx: u32, calls: Vec, ) -> Result, 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)> = vec![]; // Public keys for the transaction signatures we have to verify let signature_pubkeys: Vec = 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(¶ms.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 { diff --git a/src/contract/dao/src/error.rs b/src/contract/dao/src/error.rs index 54e83e2c4..bee5d5581 100644 --- a/src/contract/dao/src/error.rs +++ b/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 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), } diff --git a/src/contract/dao/src/model.rs b/src/contract/dao/src/model.rs index 1c1e990e2..9b2adc7c5 100644 --- a/src/contract/dao/src/model.rs +++ b/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` diff --git a/src/contract/money/src/client/transfer_v1.rs b/src/contract/money/src/client/transfer_v1.rs index bc624510c..2f24b62bc 100644 --- a/src/contract/money/src/client/transfer_v1.rs +++ b/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, diff --git a/src/contract/test-harness/src/dao_exec.rs b/src/contract/test-harness/src/dao_exec.rs index 360afdf0b..fb2458724 100644 --- a/src/contract/test-harness/src/dao_exec.rs +++ b/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 = 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, };