auth_xfer: check output coins match proposal

src/contract/dao/src/client/propose.rs

@@ -16,7 +16,7 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
-use darkfi_money_contract::model::CoinParams;
+use darkfi_money_contract::model::CoinAttributes;
 use darkfi_sdk::{
     bridgetree,
     bridgetree::Hashable,
@@ -96,7 +96,7 @@ impl DaoProposeCall {
             ];
 
             let public_key = PublicKey::from_secret(input.secret);
-            let coin = CoinParams {
+            let coin = CoinAttributes {
                 public_key,
                 value: note.value,
                 token_id: note.token_id,

src/contract/dao/src/client/vote.rs

@@ -16,7 +16,7 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
-use darkfi_money_contract::model::CoinParams;
+use darkfi_money_contract::model::CoinAttributes;
 use darkfi_sdk::{
     bridgetree,
     bridgetree::Hashable,
@@ -109,7 +109,7 @@ impl DaoVoteCall {
             ];
 
             let public_key = PublicKey::from_secret(input.secret);
-            let coin = CoinParams {
+            let coin = CoinAttributes {
                 public_key,
                 value: note.value,
                 token_id: note.token_id,

src/contract/dao/src/entrypoint/auth_xfer.rs

@@ -16,7 +16,10 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
-use darkfi_money_contract::{model::MoneyTransferParamsV1, MoneyFunction};
+use darkfi_money_contract::{
+    model::{Coin, MoneyTransferParamsV1},
+    MoneyFunction,
+};
 use darkfi_sdk::{
     crypto::{contract_id::MONEY_CONTRACT_ID, pasta_prelude::*, ContractId, PublicKey},
     dark_tree::DarkLeaf,
@@ -122,9 +125,19 @@ pub(crate) fn dao_authxfer_process_instruction(
         return Err(DaoError::AuthXferCallNotFoundInParent.into())
     }
 
-    // Read the proposal_data which should be Vec<CoinParams>
-    // Deserialize the proposal_data
+    // Read the proposal auth data which should be Vec<CoinAttributes>
+    let proposal_coins: Vec<Coin> = deserialize(&auth_call.unwrap().auth_data[..])?;
+
     // Check all the outputs except the last match
+    let send_outs = xfer_params.outputs.split_last().unwrap().1;
+    if send_outs.len() != proposal_coins.len() {
+        return Err(DaoError::AuthXferWrongNumberOutputs.into())
+    }
+    for (output, coin) in send_outs.iter().zip(proposal_coins.iter()) {
+        if output.coin != *coin {
+            return Err(DaoError::AuthXferWrongOutputCoin.into())
+        }
+    }
 
     ///////////////////////////////////////////////////
     // 4. Change belongs to the DAO

src/contract/dao/src/error.rs

@@ -79,6 +79,12 @@ pub enum DaoError {
 
     #[error("Auth call not found in parent")]
     AuthXferCallNotFoundInParent,
+
+    #[error("Wrong number of outputs")]
+    AuthXferWrongNumberOutputs,
+
+    #[error("Wrong output coin")]
+    AuthXferWrongOutputCoin,
 }
 
 impl From<DaoError> for ContractError {
@@ -104,6 +110,8 @@ impl From<DaoError> for ContractError {
             DaoError::AuthXferSiblingWrongFunctionCode => Self::Custom(18),
             DaoError::AuthXferNonMatchingEncInputUserData => Self::Custom(19),
             DaoError::AuthXferCallNotFoundInParent => Self::Custom(20),
+            DaoError::AuthXferWrongNumberOutputs => Self::Custom(21),
+            DaoError::AuthXferWrongOutputCoin => Self::Custom(22),
         }
     }
 }

src/contract/dao/tests/integration.rs

@@ -22,7 +22,7 @@ use darkfi_dao_contract::{
     client::DaoVoteNote,
     model::{Dao, DaoBlindAggregateVote},
 };
-use darkfi_money_contract::model::CoinParams;
+use darkfi_money_contract::model::CoinAttributes;
 use darkfi_sdk::{
     crypto::{
         pasta_prelude::Field, pedersen_commitment_u64, poseidon_hash, DAO_CONTRACT_ID,
@@ -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 proposal_coins = vec![CoinParams {
+        let proposal_coinattrs = vec![CoinAttributes {
             public_key: th.holders.get(&Holder::Rachel).unwrap().keypair.public,
             value: PROPOSAL_AMOUNT,
             token_id: drk_token_id,
@@ -197,7 +197,7 @@ fn integration_test() -> Result<()> {
 
         let (propose_tx, propose_params, propose_info) = th.dao_propose(
             &Holder::Alice,
-            proposal_coins.clone(),
+            &proposal_coinattrs,
             user_data,
             &dao,
             &dao_mint_params.dao_bulla,
@@ -318,7 +318,7 @@ fn integration_test() -> Result<()> {
             &dao,
             &dao_mint_params.dao_bulla,
             &propose_info,
-            &proposal_coins,
+            &proposal_coinattrs,
             total_yes_vote_value,
             total_all_vote_value,
             total_yes_vote_blind,

src/contract/money/src/client/genesis_mint_v1.rs

@@ -91,9 +91,10 @@ impl GenesisMintCallBuilder {
         };
 
         let output = TransferCallOutput {
+            public_key: self.keypair.public,
             value: self.amount,
             token_id,
-            public_key: self.keypair.public,
+            serial: pallas::Base::random(&mut OsRng),
             spend_hook: pallas::Base::ZERO,
             user_data: pallas::Base::ZERO,
         };

src/contract/money/src/client/mod.rs

@@ -101,6 +101,7 @@ pub const MONEY_ALIASES_COL_TOKEN_ID: &str = "token_id";
 /// `MoneyNote` holds the inner attributes of a `Coin`.
 #[derive(Debug, Clone, Eq, PartialEq, SerialEncodable, SerialDecodable)]
 pub struct MoneyNote {
+    // TODO: replace these fields with CoinAttributes
     /// Serial number of the coin, used for the nullifier
     pub serial: pallas::Base,
     /// Value of the coin
@@ -112,6 +113,7 @@ pub struct MoneyNote {
     pub spend_hook: pallas::Base,
     /// User data used by protocol when spend hook is enabled
     pub user_data: pallas::Base,
+    // end todo
     /// Blinding factor for the value pedersen commitment
     pub value_blind: pallas::Scalar,
     /// Blinding factor for the token ID pedersen commitment

src/contract/money/src/client/pow_reward_v1.rs

@@ -99,9 +99,10 @@ impl PoWRewardCallBuilder {
         let input = TransferCallClearInput { value, token_id, signature_secret: self.secret };
 
         let output = TransferCallOutput {
+            public_key: self.recipient,
             value,
             token_id,
-            public_key: self.recipient,
+            serial: pallas::Base::random(&mut OsRng),
             spend_hook: pallas::Base::ZERO,
             user_data: pallas::Base::ZERO,
         };

src/contract/money/src/client/swap_v1.rs

@@ -119,9 +119,10 @@ impl SwapCallBuilder {
         };
 
         let output = TransferCallOutput {
+            public_key: self.pubkey,
             value: self.value_recv,
             token_id: self.token_id_recv,
-            public_key: self.pubkey,
+            serial: pallas::Base::random(&mut OsRng),
             spend_hook: pallas::Base::ZERO,
             user_data: pallas::Base::ZERO,
         };

src/contract/money/src/client/token_mint_v1.rs

@@ -105,9 +105,10 @@ impl TokenMintCallBuilder {
         };
 
         let output = TransferCallOutput {
+            public_key: self.recipient,
             value: self.amount,
             token_id,
-            public_key: self.recipient,
+            serial: pallas::Base::random(&mut OsRng),
             spend_hook: pallas::Base::ZERO,
             user_data: pallas::Base::ZERO,
         };

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

@@ -34,7 +34,7 @@ use rand::rngs::OsRng;
 use super::proof::{create_transfer_burn_proof, create_transfer_mint_proof};
 use crate::{
     client::{MoneyNote, OwnCoin},
-    model::{ClearInput, Input, MoneyTransferParamsV1, Output},
+    model::{ClearInput, CoinAttributes, Input, MoneyTransferParamsV1, Output},
 };
 
 /// Struct holding necessary information to build a `Money::TransferV1` contract call.
@@ -71,13 +71,7 @@ pub struct TransferCallInput {
     pub user_data_blind: pallas::Base,
 }
 
-pub struct TransferCallOutput {
-    pub value: u64,
-    pub token_id: TokenId,
-    pub public_key: PublicKey,
-    pub spend_hook: pallas::Base,
-    pub user_data: pallas::Base,
-}
+pub type TransferCallOutput = CoinAttributes;
 
 impl TransferCallBuilder {
     fn compute_remainder_blind(
@@ -174,8 +168,6 @@ impl TransferCallBuilder {
 
             output_blinds.push(value_blind);
 
-            let serial = pallas::Base::random(&mut OsRng);
-
             info!("Creating transfer mint proof for output {}", i);
             let (proof, public_inputs) = create_transfer_mint_proof(
                 &self.mint_zkbin,
@@ -183,7 +175,7 @@ impl TransferCallBuilder {
                 output,
                 value_blind,
                 token_blind,
-                serial,
+                output.serial,
                 output.spend_hook,
                 output.user_data,
             )?;
@@ -192,7 +184,7 @@ impl TransferCallBuilder {
 
             // Encrypted note
             let note = MoneyNote {
-                serial,
+                serial: output.serial,
                 value: output.value,
                 token_id: output.token_id,
                 spend_hook: output.spend_hook,

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

@@ -123,18 +123,20 @@ pub fn make_transfer_call(
     debug!("Selected inputs");
 
     outputs.push(TransferCallOutput {
+        public_key: recipient,
         value,
         token_id,
-        public_key: recipient,
+        serial: pallas::Base::random(&mut OsRng),
         spend_hook: pallas::Base::ZERO,
         user_data: pallas::Base::ZERO,
     });
 
     if change_value > 0 {
         outputs.push(TransferCallOutput {
+            public_key: keypair.public,
             value: change_value,
             token_id,
-            public_key: keypair.public,
+            serial: pallas::Base::random(&mut OsRng),
             spend_hook: pallas::Base::ZERO,
             user_data: pallas::Base::ZERO,
         });

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

@@ -33,7 +33,7 @@ use log::debug;
 use rand::rngs::OsRng;
 
 use super::{TransferCallInput, TransferCallOutput};
-use crate::model::{Coin, CoinParams};
+use crate::model::{Coin, CoinAttributes};
 
 pub struct TransferMintRevealed {
     pub coin: Coin,
@@ -95,7 +95,7 @@ pub fn create_transfer_burn_proof(
 
     let signature_public = PublicKey::from_secret(signature_secret);
 
-    let coin = CoinParams {
+    let coin = CoinAttributes {
         public_key,
         value: input.note.value,
         token_id: input.note.token_id,
@@ -169,7 +169,7 @@ pub fn create_transfer_mint_proof(
     let token_commit = poseidon_hash([output.token_id.inner(), token_blind]);
     let (pub_x, pub_y) = output.public_key.xy();
 
-    let coin = CoinParams {
+    let coin = CoinAttributes {
         public_key: output.public_key,
         value: output.value,
         token_id: output.token_id,

src/contract/money/src/model.rs

@@ -57,7 +57,7 @@ impl Coin {
 }
 
 #[derive(Debug, Clone, SerialEncodable, SerialDecodable)]
-pub struct CoinParams {
+pub struct CoinAttributes {
     pub public_key: PublicKey,
     pub value: u64,
     pub token_id: TokenId,
@@ -66,7 +66,7 @@ pub struct CoinParams {
     pub user_data: pallas::Base,
 }
 
-impl CoinParams {
+impl CoinAttributes {
     pub fn to_coin(&self) -> Coin {
         let (pub_x, pub_y) = self.public_key.xy();
         let coin = poseidon_hash([

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

@@ -29,7 +29,7 @@ use darkfi_dao_contract::{
 };
 use darkfi_money_contract::{
     client::transfer_v1 as xfer,
-    model::{CoinParams, MoneyTransferParamsV1},
+    model::{CoinAttributes, MoneyTransferParamsV1},
     MoneyFunction, MONEY_CONTRACT_ZKAS_BURN_NS_V1, MONEY_CONTRACT_ZKAS_MINT_NS_V1,
 };
 use darkfi_sdk::{
@@ -53,7 +53,7 @@ impl TestHarness {
         dao: &Dao,
         dao_bulla: &DaoBulla,
         proposal: &DaoProposal,
-        proposal_coins: &Vec<CoinParams>,
+        proposal_coinattrs: &Vec<CoinAttributes>,
         yes_vote_value: u64,
         all_vote_value: u64,
         yes_vote_blind: pallas::Scalar,
@@ -75,10 +75,10 @@ impl TestHarness {
         // TODO: FIXME: This is not checked anywhere!
         let exec_signature_secret = SecretKey::random(&mut OsRng);
 
-        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();
+        assert!(proposal_coinattrs.len() > 0);
+        let proposal_token_id = proposal_coinattrs[0].token_id;
+        assert!(proposal_coinattrs.iter().all(|c| c.token_id == proposal_token_id));
+        let proposal_amount = proposal_coinattrs.iter().map(|c| c.value).sum();
 
         let dao_coins = dao_wallet
             .unspent_money_coins
@@ -104,19 +104,22 @@ impl TestHarness {
         }
 
         let mut outputs = vec![];
-        for coin in proposal_coins {
+        for coin in proposal_coinattrs {
+            assert_eq!(proposal_token_id, coin.token_id);
             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,
+                value: coin.value,
+                token_id: proposal_token_id,
+                serial: coin.serial,
+                spend_hook: coin.spend_hook,
+                user_data: coin.user_data,
             });
         }
         outputs.push(xfer::TransferCallOutput {
+            public_key: dao_wallet.keypair.public,
             value: change_value,
             token_id: proposal_token_id,
-            public_key: dao_wallet.keypair.public,
+            serial: pallas::Base::random(&mut OsRng),
             spend_hook: DAO_CONTRACT_ID.inner(),
             user_data: dao_bulla.inner(),
         });

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

@@ -27,7 +27,7 @@ use darkfi_dao_contract::{
     model::{Dao, DaoAuthCall, DaoBulla, DaoProposal, DaoProposeParams},
     DaoFunction, DAO_CONTRACT_ZKAS_DAO_PROPOSE_BURN_NS, DAO_CONTRACT_ZKAS_DAO_PROPOSE_MAIN_NS,
 };
-use darkfi_money_contract::{client::OwnCoin, model::CoinParams, MoneyFunction};
+use darkfi_money_contract::{client::OwnCoin, model::CoinAttributes, MoneyFunction};
 use darkfi_sdk::{
     crypto::{
         pasta_prelude::Field, MerkleNode, SecretKey, TokenId, DAO_CONTRACT_ID, MONEY_CONTRACT_ID,
@@ -44,7 +44,7 @@ impl TestHarness {
     pub fn dao_propose(
         &mut self,
         proposer: &Holder,
-        proposal_coins: Vec<CoinParams>,
+        proposal_coinattrs: &Vec<CoinAttributes>,
         user_data: pallas::Base,
         dao: &Dao,
         dao_bulla: &DaoBulla,
@@ -78,6 +78,11 @@ impl TestHarness {
             signature_secret,
         };
 
+        // Convert coin_params to actual coins
+        let mut proposal_coins = vec![];
+        for coin_params in proposal_coinattrs {
+            proposal_coins.push(coin_params.to_coin());
+        }
         let mut proposal_data = vec![];
         proposal_coins.encode(&mut proposal_data).unwrap();
 

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

@@ -69,9 +69,10 @@ impl TestHarness {
             }],
             inputs: vec![],
             outputs: vec![xfer::TransferCallOutput {
+                public_key: recipient,
                 value,
                 token_id: *DARK_TOKEN_ID,
-                public_key: recipient,
+                serial: pallas::Base::random(&mut OsRng),
                 spend_hook: rcpt_spend_hook.unwrap_or(pallas::Base::ZERO),
                 user_data: rcpt_user_data.unwrap_or(pallas::Base::ZERO),
             }],