From 53ee11777d2203402b486abbc825db947958c461 Mon Sep 17 00:00:00 2001 From: x Date: Wed, 20 Dec 2023 12:51:30 +0000 Subject: [PATCH] dao: remove money::transfer specific checks in prep for move to auth module --- src/contract/dao/proof/dao-exec.zk | 68 ------------ src/contract/dao/src/client/exec.rs | 60 ----------- src/contract/dao/src/client/vote.rs | 1 - src/contract/dao/src/entrypoint.rs | 105 ++++++------------- src/contract/dao/src/entrypoint/exec.rs | 8 -- src/contract/dao/src/lib.rs | 2 + src/contract/dao/src/model.rs | 9 +- src/contract/dao/tests/integration.rs | 5 +- src/contract/test-harness/src/dao_exec.rs | 52 +++++---- src/contract/test-harness/src/dao_propose.rs | 4 - 10 files changed, 71 insertions(+), 243 deletions(-) diff --git a/src/contract/dao/proof/dao-exec.zk b/src/contract/dao/proof/dao-exec.zk index ed08011f9..341ec1f30 100644 --- a/src/contract/dao/proof/dao-exec.zk +++ b/src/contract/dao/proof/dao-exec.zk @@ -8,12 +8,6 @@ constant "DaoExec" { witness "DaoExec" { # Proposal parameters - # delet [ - Base proposal_dest_x, - Base proposal_dest_y, - Base proposal_amount, - Base proposal_token_id, - # ] Base proposal_content_commit, Base proposal_auth_contract_id, Base proposal_auth_function_id, @@ -34,20 +28,6 @@ witness "DaoExec" { Base all_vote_value, Scalar yes_vote_blind, Scalar all_vote_blind, - - # Outputs + Inputs - Base user_serial, - Base dao_serial, - Base input_value, - Scalar input_value_blind, - - # Miscellaneous - 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" { @@ -74,30 +54,6 @@ circuit "DaoExec" { ); constrain_instance(proposal_bulla); - coin_0 = poseidon_hash( - proposal_dest_x, - proposal_dest_y, - proposal_amount, - proposal_token_id, - user_serial, - user_spend_hook, - user_data, - ); - constrain_instance(coin_0); - - change = base_sub(input_value, proposal_amount); - - coin_1 = poseidon_hash( - dao_public_x, - dao_public_y, - change, - proposal_token_id, - dao_serial, - dao_spend_hook, - dao_bulla, - ); - constrain_instance(coin_1); - # Create Pedersen commitments for win_votes and total_votes, and # constrain the commitments' coordinates. yes_vote_value_c = ec_mul_short(yes_vote_value, VALUE_COMMIT_VALUE); @@ -112,17 +68,6 @@ circuit "DaoExec" { constrain_instance(ec_get_x(all_vote_commit)); constrain_instance(ec_get_y(all_vote_commit)); - # Create Pedersen commitment for input_value and make public - input_value_v = ec_mul_short(input_value, VALUE_COMMIT_VALUE); - input_value_r = ec_mul(input_value_blind, VALUE_COMMIT_RANDOM); - input_value_commit = ec_add(input_value_v, input_value_r); - constrain_instance(ec_get_x(input_value_commit)); - constrain_instance(ec_get_y(input_value_commit)); - - constrain_instance(dao_spend_hook); - constrain_instance(user_spend_hook); - constrain_instance(user_data); - # Check that dao_quorum is less than or equal to all_vote_value one = witness_base(1); all_vote_value_1 = base_add(all_vote_value, one); @@ -138,19 +83,6 @@ circuit "DaoExec" { rhs_1 = base_add(rhs, one); less_than_strict(lhs, rhs_1); - # Create coin 0 - # Create coin 1 - # Check values of coin 0 + coin 1 == input_value - # Check value of coin 0 == proposal_amount - # Check public key matches too - # 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 753758d40..0fa68ee03 100644 --- a/src/contract/dao/src/client/exec.rs +++ b/src/contract/dao/src/client/exec.rs @@ -58,10 +58,6 @@ impl DaoExecCall { debug!(target: "dao", "build()"); let mut proofs = vec![]; - let (proposal_dest_x, proposal_dest_y) = self.proposal.dest.xy(); - - let proposal_amount = pallas::Base::from(self.proposal.amount); - let dao_proposer_limit = pallas::Base::from(self.dao.proposer_limit); let dao_quorum = pallas::Base::from(self.dao.quorum); let dao_approval_ratio_quot = pallas::Base::from(self.dao.approval_ratio_quot); @@ -69,52 +65,18 @@ impl DaoExecCall { let (dao_pub_x, dao_pub_y) = self.dao.public_key.xy(); - let user_spend_hook = pallas::Base::from(0); - let user_data = pallas::Base::from(0); - let input_value = pallas::Base::from(self.input_value); - let change = self.input_value - self.proposal.amount; - let dao_bulla = self.dao.to_bulla(); assert_eq!(dao_bulla, self.proposal.dao_bulla); let proposal_bulla = self.proposal.to_bulla(); - let coin_0 = CoinParams { - public_key: self.proposal.dest, - value: self.proposal.amount, - token_id: self.proposal.token_id, - serial: self.user_serial, - spend_hook: user_spend_hook, - user_data, - } - .to_coin(); - debug!("created coin_0 {:?}", coin_0); - - let coin_1 = CoinParams { - public_key: self.dao.public_key, - value: change, - token_id: self.proposal.token_id, - serial: self.dao_serial, - spend_hook: self.hook_dao_exec, - user_data: dao_bulla.inner(), - } - .to_coin(); - debug!("created coin_1 {:?}", coin_1); - let yes_vote_commit = pedersen_commitment_u64(self.yes_vote_value, self.yes_vote_blind); let yes_vote_commit_coords = yes_vote_commit.to_affine().coordinates().unwrap(); let all_vote_commit = pedersen_commitment_u64(self.all_vote_value, self.all_vote_blind); let all_vote_commit_coords = all_vote_commit.to_affine().coordinates().unwrap(); - let input_value_commit = pedersen_commitment_u64(self.input_value, self.input_value_blind); - let input_value_commit_coords = input_value_commit.to_affine().coordinates().unwrap(); - let prover_witnesses = vec![ // proposal params - Witness::Base(Value::known(proposal_dest_x)), - Witness::Base(Value::known(proposal_dest_y)), - Witness::Base(Value::known(proposal_amount)), - Witness::Base(Value::known(self.proposal.token_id.inner())), Witness::Base(Value::known(self.proposal.content_commit)), Witness::Base(Value::known(self.proposal.auth_contract_id)), Witness::Base(Value::known(self.proposal.auth_function_id)), @@ -133,37 +95,15 @@ impl DaoExecCall { Witness::Base(Value::known(pallas::Base::from(self.all_vote_value))), Witness::Scalar(Value::known(self.yes_vote_blind)), Witness::Scalar(Value::known(self.all_vote_blind)), - // outputs + inputs - Witness::Base(Value::known(self.user_serial)), - Witness::Base(Value::known(self.dao_serial)), - Witness::Base(Value::known(input_value)), - Witness::Scalar(Value::known(self.input_value_blind)), - // misc - 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.inner(), 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(), - coin_0.inner(), - coin_1.inner(), *yes_vote_commit_coords.x(), *yes_vote_commit_coords.y(), *all_vote_commit_coords.x(), *all_vote_commit_coords.y(), - *input_value_commit_coords.x(), - *input_value_commit_coords.y(), - self.hook_dao_exec, - user_spend_hook, - user_data, - input_user_data_enc, ]; //export_witness_json("witness.json", &prover_witnesses, &public_inputs); diff --git a/src/contract/dao/src/client/vote.rs b/src/contract/dao/src/client/vote.rs index bf4d15fec..00b86b0bf 100644 --- a/src/contract/dao/src/client/vote.rs +++ b/src/contract/dao/src/client/vote.rs @@ -217,7 +217,6 @@ impl DaoVoteCall { let public_inputs = vec![ token_commit, proposal_bulla.inner(), - // this should be a value commit?? *yes_vote_commit_coords.x(), *yes_vote_commit_coords.y(), *all_vote_commit_coords.x(), diff --git a/src/contract/dao/src/entrypoint.rs b/src/contract/dao/src/entrypoint.rs index 708d40e9b..fad8905f2 100644 --- a/src/contract/dao/src/entrypoint.rs +++ b/src/contract/dao/src/entrypoint.rs @@ -55,6 +55,9 @@ use vote::{dao_vote_get_metadata, dao_vote_process_instruction, dao_vote_process mod exec; use exec::{dao_exec_get_metadata, dao_exec_process_instruction, dao_exec_process_update}; +mod auth_xfer; +use auth_xfer::{dao_authxfer_get_metadata, dao_authxfer_process_instruction}; + darkfi_sdk::define_contract!( init: init_contract, exec: process_instruction, @@ -69,12 +72,13 @@ darkfi_sdk::define_contract!( fn init_contract(cid: ContractId, _ix: &[u8]) -> ContractResult { // The zkas circuits can simply be embedded in the wasm and set up by // the initialization. - zkas_db_set(&include_bytes!("../proof/dao-exec.zk.bin")[..])?; zkas_db_set(&include_bytes!("../proof/dao-mint.zk.bin")[..])?; - zkas_db_set(&include_bytes!("../proof/dao-vote-burn.zk.bin")[..])?; - zkas_db_set(&include_bytes!("../proof/dao-vote-main.zk.bin")[..])?; zkas_db_set(&include_bytes!("../proof/dao-propose-burn.zk.bin")[..])?; zkas_db_set(&include_bytes!("../proof/dao-propose-main.zk.bin")[..])?; + zkas_db_set(&include_bytes!("../proof/dao-vote-burn.zk.bin")[..])?; + zkas_db_set(&include_bytes!("../proof/dao-vote-main.zk.bin")[..])?; + zkas_db_set(&include_bytes!("../proof/dao-exec.zk.bin")[..])?; + //zkas_db_set(&include_bytes!("../proof/dao-auth-money-transfer.zk.bin")[..])?; // Set up db for general info let dao_info_db = match db_lookup(cid, DAO_CONTRACT_DB_INFO_TREE) { @@ -140,91 +144,39 @@ fn init_contract(cid: ContractId, _ix: &[u8]) -> ContractResult { /// contract calls in the transaction. fn get_metadata(cid: ContractId, ix: &[u8]) -> ContractResult { let (call_idx, calls): (u32, Vec>) = deserialize(ix)?; - if call_idx >= calls.len() as u32 { - msg!("[DAO:get_metadata()] Error: call_idx >= calls.len()"); - return Err(ContractError::Internal) - } + let self_ = &calls[call_idx as usize].data; + let func = DaoFunction::try_from(self_.data[0])?; - match DaoFunction::try_from(calls[call_idx as usize].data.data[0])? { - DaoFunction::Mint => { - let metadata = dao_mint_get_metadata(cid, call_idx, calls)?; - Ok(set_return_data(&metadata)?) - } - - DaoFunction::Propose => { - let metadata = dao_propose_get_metadata(cid, call_idx, calls)?; - Ok(set_return_data(&metadata)?) - } - - DaoFunction::Vote => { - let metadata = dao_vote_get_metadata(cid, call_idx, calls)?; - Ok(set_return_data(&metadata)?) - } - - DaoFunction::Exec => { - let metadata = dao_exec_get_metadata(cid, call_idx, calls)?; - Ok(set_return_data(&metadata)?) - } - } + let metadata = match func { + DaoFunction::Mint => dao_mint_get_metadata(cid, call_idx, calls)?, + DaoFunction::Propose => dao_propose_get_metadata(cid, call_idx, calls)?, + DaoFunction::Vote => dao_vote_get_metadata(cid, call_idx, calls)?, + DaoFunction::Exec => dao_exec_get_metadata(cid, call_idx, calls)?, + DaoFunction::AuthMoneyTransfer => dao_authxfer_get_metadata(cid, call_idx, calls)?, + }; + Ok(set_return_data(&metadata)?) } /// This function verifies a state transition and produces a state update /// if everything is successful. fn process_instruction(cid: ContractId, ix: &[u8]) -> ContractResult { let (call_idx, calls): (u32, Vec>) = deserialize(ix)?; - if call_idx >= calls.len() as u32 { - msg!("[DAO::process_instruction()] Error: call_idx >= calls.len()"); - return Err(ContractError::Internal) - } - let self_ = &calls[call_idx as usize].data; let func = DaoFunction::try_from(self_.data[0])?; - if calls.len() != 1 { - // Enforce a strict structure for our tx - if calls.len() != 2 || call_idx != 1 { - msg!("[Dao] Error: No more than 2 calls allowed, and DAO call must be last"); - return Err(DaoError::InvalidCalls.into()) - } - - // We can unpack user_data and check the function call is correct. - // But in this contract, only DAO::exec() can be invoked by other ones. - // So just check the function call is correct. - - // NOTE: we may wish to improve this since it cripples user composability. - - if func != DaoFunction::Exec { - msg!("[Dao] Error: Only DAO::exec() can be invoked"); - return Err(DaoError::InvalidCalls.into()) - } - } - - match func { - DaoFunction::Mint => { - let update_data = dao_mint_process_instruction(cid, call_idx, calls)?; - Ok(set_return_data(&update_data)?) - } - - DaoFunction::Propose => { - let update_data = dao_propose_process_instruction(cid, call_idx, calls)?; - Ok(set_return_data(&update_data)?) - } - - DaoFunction::Vote => { - let update_data = dao_vote_process_instruction(cid, call_idx, calls)?; - Ok(set_return_data(&update_data)?) - } - - DaoFunction::Exec => { - let update_data = dao_exec_process_instruction(cid, call_idx, calls)?; - Ok(set_return_data(&update_data)?) - } - } + let update_data = match func { + DaoFunction::Mint => dao_mint_process_instruction(cid, call_idx, calls)?, + DaoFunction::Propose => dao_propose_process_instruction(cid, call_idx, calls)?, + DaoFunction::Vote => dao_vote_process_instruction(cid, call_idx, calls)?, + DaoFunction::Exec => dao_exec_process_instruction(cid, call_idx, calls)?, + DaoFunction::AuthMoneyTransfer => dao_authxfer_process_instruction(cid, call_idx, calls)?, + }; + Ok(set_return_data(&update_data)?) } /// This function attempts to write a given state update provided the previous /// steps of the contract call execution were successful. The payload given to -/// the functioon is the update data retrieved from `process_instruction()`. +/// the function is the update data retrieved from `process_instruction()`. fn process_update(cid: ContractId, update_data: &[u8]) -> ContractResult { match DaoFunction::try_from(update_data[0])? { DaoFunction::Mint => { @@ -246,5 +198,10 @@ fn process_update(cid: ContractId, update_data: &[u8]) -> ContractResult { let update: DaoExecUpdate = deserialize(&update_data[1..])?; Ok(dao_exec_process_update(cid, update)?) } + + DaoFunction::AuthMoneyTransfer => { + // Does nothing, just verifies the other calls are correct + Ok(()) + } } } diff --git a/src/contract/dao/src/entrypoint/exec.rs b/src/contract/dao/src/entrypoint/exec.rs index 6d2f3c823..2aba54192 100644 --- a/src/contract/dao/src/entrypoint/exec.rs +++ b/src/contract/dao/src/entrypoint/exec.rs @@ -72,18 +72,10 @@ pub(crate) fn dao_exec_get_metadata( DAO_CONTRACT_ZKAS_DAO_EXEC_NS.to_string(), vec![ dao_exec_params.proposal.inner(), - money_xfer_params.outputs[0].coin.inner(), - money_xfer_params.outputs[1].coin.inner(), *yes_vote_coords.x(), *yes_vote_coords.y(), *all_vote_coords.x(), *all_vote_coords.y(), - *input_value_coords.x(), - *input_value_coords.y(), - cid.inner(), - pallas::Base::ZERO, - pallas::Base::ZERO, - input_user_data_enc, ], )); diff --git a/src/contract/dao/src/lib.rs b/src/contract/dao/src/lib.rs index bace1a504..c000ae2c4 100644 --- a/src/contract/dao/src/lib.rs +++ b/src/contract/dao/src/lib.rs @@ -28,6 +28,7 @@ pub enum DaoFunction { Propose = 0x01, Vote = 0x02, Exec = 0x03, + AuthMoneyTransfer = 0x04, } impl TryFrom for DaoFunction { @@ -39,6 +40,7 @@ impl TryFrom for DaoFunction { 0x01 => Ok(DaoFunction::Propose), 0x02 => Ok(DaoFunction::Vote), 0x03 => Ok(DaoFunction::Exec), + 0x04 => Ok(DaoFunction::AuthMoneyTransfer), _ => Err(ContractError::InvalidFunction), } } diff --git a/src/contract/dao/src/model.rs b/src/contract/dao/src/model.rs index 293e8da10..471d086f5 100644 --- a/src/contract/dao/src/model.rs +++ b/src/contract/dao/src/model.rs @@ -116,11 +116,6 @@ impl TryInto for ShareAddress { #[derive(Debug, Clone, SerialEncodable, SerialDecodable)] pub struct DaoProposal { - // delet [ - pub dest: PublicKey, - pub amount: u64, - pub token_id: TokenId, - // ] pub content_commit: pallas::Base, pub auth_contract_id: pallas::Base, pub auth_function_id: pallas::Base, @@ -319,3 +314,7 @@ pub struct DaoExecUpdate { /// The proposal bulla pub proposal: DaoProposalBulla, } + +/// Parameters for `Dao::AuthMoneyTransfer` +#[derive(Debug, Copy, Clone, SerialEncodable, SerialDecodable)] +pub struct DaoAuthMoneyTransferParams {} diff --git a/src/contract/dao/tests/integration.rs b/src/contract/dao/tests/integration.rs index 564ba8300..89075863a 100644 --- a/src/contract/dao/tests/integration.rs +++ b/src/contract/dao/tests/integration.rs @@ -183,7 +183,7 @@ fn integration_test() -> Result<()> { // These coins are passed around to all DAO members who verify its validity // They also check hashing them equals the proposal_commit - let coins = vec![CoinParams { + let proposal_coins = vec![CoinParams { public_key: th.holders.get(&Holder::Rachel).unwrap().keypair.public, value: PROPOSAL_AMOUNT, token_id: drk_token_id, @@ -193,7 +193,7 @@ fn integration_test() -> Result<()> { }]; // We can add whatever we want in here, even arbitrary text // It's up to the auth module to decide what to do with it. - let content_commit = poseidon_hash([coins[0].to_coin().inner()]); + let content_commit = poseidon_hash([proposal_coins[0].to_coin().inner()]); let auth_contract_id = pallas::Base::ZERO; let auth_function_id = pallas::Base::ZERO; @@ -324,6 +324,7 @@ fn integration_test() -> Result<()> { &dao, &dao_mint_params.dao_bulla, &propose_info, + &proposal_coins, total_yes_vote_value, total_all_vote_value, total_yes_vote_blind, diff --git a/src/contract/test-harness/src/dao_exec.rs b/src/contract/test-harness/src/dao_exec.rs index 9eae646ce..4f5052807 100644 --- a/src/contract/test-harness/src/dao_exec.rs +++ b/src/contract/test-harness/src/dao_exec.rs @@ -28,8 +28,9 @@ use darkfi_dao_contract::{ DaoFunction, DAO_CONTRACT_ZKAS_DAO_EXEC_NS, }; use darkfi_money_contract::{ - client::transfer_v1 as xfer, model::MoneyTransferParamsV1, MoneyFunction, - MONEY_CONTRACT_ZKAS_BURN_NS_V1, MONEY_CONTRACT_ZKAS_MINT_NS_V1, + client::transfer_v1 as xfer, + model::{CoinParams, MoneyTransferParamsV1}, + MoneyFunction, MONEY_CONTRACT_ZKAS_BURN_NS_V1, MONEY_CONTRACT_ZKAS_MINT_NS_V1, }; use darkfi_sdk::{ crypto::{ @@ -51,6 +52,7 @@ impl TestHarness { dao: &Dao, dao_bulla: &DaoBulla, proposal: &DaoProposal, + proposal_coins: &Vec, yes_vote_value: u64, all_vote_value: u64, yes_vote_blind: pallas::Scalar, @@ -72,13 +74,18 @@ impl TestHarness { // TODO: FIXME: This is not checked anywhere! let exec_signature_secret = SecretKey::random(&mut OsRng); - let coins = dao_wallet + assert!(proposal_coins.len() > 0); + let proposal_token_id = proposal_coins[0].token_id; + assert!(proposal_coins.iter().all(|c| c.token_id == proposal_token_id)); + let proposal_amount = proposal_coins.iter().map(|c| c.value).sum(); + + let dao_coins = dao_wallet .unspent_money_coins .iter() - .filter(|x| x.note.token_id == proposal.token_id) + .filter(|x| x.note.token_id == proposal_token_id) .cloned() .collect(); - let (spent_coins, change_value) = xfer::select_coins(coins, proposal.amount)?; + let (spent_coins, change_value) = xfer::select_coins(dao_coins, proposal_amount)?; let tree = dao_wallet.money_merkle_tree.clone(); let mut inputs = vec![]; @@ -95,25 +102,28 @@ impl TestHarness { }); } + let mut outputs = vec![]; + for coin in proposal_coins { + outputs.push(xfer::TransferCallOutput { + value: proposal_amount, + token_id: proposal_token_id, + public_key: coin.public_key, + spend_hook: pallas::Base::ZERO, + user_data: pallas::Base::ZERO, + }); + } + outputs.push(xfer::TransferCallOutput { + value: change_value, + token_id: proposal_token_id, + public_key: dao_wallet.keypair.public, + spend_hook: DAO_CONTRACT_ID.inner(), + user_data: dao_bulla.inner(), + }); + let xfer_builder = xfer::TransferCallBuilder { clear_inputs: vec![], inputs, - outputs: vec![ - xfer::TransferCallOutput { - value: proposal.amount, - token_id: proposal.token_id, - public_key: proposal.dest, - spend_hook: pallas::Base::ZERO, - user_data: pallas::Base::ZERO, - }, - xfer::TransferCallOutput { - value: change_value, - token_id: proposal.token_id, - public_key: dao_wallet.keypair.public, - spend_hook: DAO_CONTRACT_ID.inner(), - user_data: dao_bulla.inner(), - }, - ], + outputs, mint_zkbin: mint_zkbin.clone(), mint_pk: mint_pk.clone(), burn_zkbin: burn_zkbin.clone(), diff --git a/src/contract/test-harness/src/dao_propose.rs b/src/contract/test-harness/src/dao_propose.rs index 81587a7a1..dd0c518a2 100644 --- a/src/contract/test-harness/src/dao_propose.rs +++ b/src/contract/test-harness/src/dao_propose.rs @@ -81,13 +81,9 @@ impl TestHarness { }; let proposal = DaoProposal { - dest: self.holders.get(recipient).unwrap().keypair.public, - amount, - token_id: tx_token_id, content_commit, auth_contract_id, auth_function_id, - // TODO: pass proposal in directly dao_bulla: dao.to_bulla(), blind: pallas::Base::random(&mut OsRng), };