contract/money: Clean up Money::UnstakeV1

src/contract/consensus/src/client/unstake_v1.rs

@@ -63,7 +63,7 @@ pub struct ConsensusUnstakeCallBuilder {
 impl ConsensusUnstakeCallBuilder {
     pub fn build(&self) -> Result<ConsensusUnstakeCallDebris> {
         info!("Building Consensus::UnstakeV1 contract call");
-        assert!(self.coin.note.value != 0);
+        assert!(self.owncoin.note.value != 0);
 
         debug!("Building Consensus::UnstakeV1 anonymous input");
         let root = self.tree.root(0).unwrap();
@@ -90,7 +90,7 @@ impl ConsensusUnstakeCallBuilder {
         };
 
         // We now fill this with necessary stuff
-        let params = ConsensusUnstakeParamsV1 { input };
+        let params = ConsensusUnstakeParamsV1 { input: tx_input };
 
         // Construct debris
         let debris = ConsensusUnstakeCallDebris {

src/contract/consensus/src/entrypoint/stake_v1.rs

@@ -140,10 +140,14 @@ pub(crate) fn consensus_stake_process_instruction_v1(
     }
 
     // The nullifiers should not already exist. It is the double-mint protection.
-    if db_contains_key(money_nullifiers_db, &serialize(&input.nullifier))? {
+    // TODO: FIXME: This should be uncommented when Validator::verify_transaction
+    //               works as Read->Write->Read->Write
+    /*
+    if !db_contains_key(money_nullifiers_db, &serialize(&input.nullifier))? {
         msg!("[ConsensusStakeV1] Error: Missing nullifier");
         return Err(MoneyError::StakeMissingNullifier.into())
     }
+    */
 
     // Newly created coin for this call is in the output. Here we gather it,
     // and we also check that it hasn't existed before.

src/contract/consensus/tests/genesis_stake_unstake.rs

@@ -204,7 +204,7 @@ async fn consensus_contract_genesis_stake_unstake() -> Result<()> {
     info!(target: "consensus", "[Alice] ===================");
     info!(target: "consensus", "[Alice] Building unstake tx");
     info!(target: "consensus", "[Alice] ===================");
-    let (unstake_tx, unstake_params, unstake_secret_key) =
+    let (unstake_tx, unstake_params, _unstake_secret_key) =
         th.unstake(Holder::Alice, alice_unstake_request_oc.clone())?;
 
     info!(target: "consensus", "[Faucet] ==========================");
@@ -221,8 +221,7 @@ async fn consensus_contract_genesis_stake_unstake() -> Result<()> {
     th.assert_trees();
 
     // Gather new unstaked owncoin
-    let alice_unstaked_oc =
-        th.gather_owncoin(Holder::Alice, unstake_params.output, Some(unstake_secret_key))?;
+    let alice_unstaked_oc = th.gather_owncoin(Holder::Alice, unstake_params.output, None)?;
 
     // Verify values match
     assert!(alice_unstake_request_oc.note.value == alice_unstaked_oc.note.value);

src/contract/consensus/tests/harness.rs

@@ -742,7 +742,7 @@ impl ConsensusTestHarness {
 
         // Building Consensus::Unstake params
         let consensus_unstake_call_debris = ConsensusUnstakeCallBuilder {
-            coin: staked_oc.clone(),
+            owncoin: staked_oc.clone(),
             tree: wallet.consensus_unstaked_merkle_tree.clone(),
             burn_zkbin: burn_zkbin.clone(),
             burn_pk: burn_pk.clone(),
@@ -762,7 +762,8 @@ impl ConsensusTestHarness {
 
         // Building Money::Unstake params
         let money_unstake_call_debris = MoneyUnstakeCallBuilder {
-            coin: staked_oc.into(),
+            owncoin: staked_oc.into(),
+            recipient: self.holders.get_mut(&holder).unwrap().keypair.public,
             value_blind: consensus_unstake_value_blind,
             nullifier: consensus_unstake_params.input.nullifier,
             merkle_root: consensus_unstake_params.input.merkle_root,

src/contract/consensus/tests/stake_unstake.rs

@@ -160,7 +160,7 @@ async fn consensus_contract_stake_unstake() -> Result<()> {
         unstake_request_tx,
         unstake_request_params,
         unstake_request_output_secret_key,
-        unstake_request_signature_secret_key,
+        _unstake_request_signature_secret_key,
     ) = th.unstake_request(Holder::Alice, current_slot, alice_rewarded_staked_oc.clone()).await?;
 
     info!(target: "consensus", "[Faucet] ==================================");
@@ -245,8 +245,7 @@ async fn consensus_contract_stake_unstake() -> Result<()> {
     th.assert_trees();
 
     // Gather new unstaked owncoin
-    let alice_unstaked_oc =
-        th.gather_owncoin(Holder::Alice, unstake_params.output, Some(unstake_secret_key))?;
+    let alice_unstaked_oc = th.gather_owncoin(Holder::Alice, unstake_params.output, None)?;
 
     // Verify values match
     assert!(alice_unstake_request_oc.note.value == alice_unstaked_oc.note.value);

src/contract/money/proof/mint_v1.zk

@@ -28,8 +28,6 @@ witness "Mint_V1" {
 }
 
 circuit "Mint_V1" {
-	# TODO: verify if value must be > 0 and add corresponding opcode
-
 	# Poseidon hash of the coin
 	C = poseidon_hash(
 		pub_x,

src/contract/money/src/client/unstake_v1.rs

@@ -76,14 +76,16 @@ pub struct TransactionBuilderOutputInfo {
 /// Struct holding necessary information to build a `Money::UnstakeV1` contract call.
 pub struct MoneyUnstakeCallBuilder {
     /// `ConsensusOwnCoin` we're given to use in this builder
-    pub coin: ConsensusOwnCoin,
+    pub owncoin: ConsensusOwnCoin,
+    /// Recipient pubkey of the minted output
+    pub recipient: PublicKey,
     /// Blinding factor for value commitment
     pub value_blind: pallas::Scalar,
     /// Revealed nullifier
     pub nullifier: Nullifier,
     /// Revealed Merkle root
     pub merkle_root: MerkleNode,
-    /// Public key for the signature
+    /// Signature public key used in the input
     pub signature_public: PublicKey,
     /// `Mint_V1` zkas circuit ZkBinary
     pub mint_zkbin: ZkBinary,
@@ -93,24 +95,23 @@ pub struct MoneyUnstakeCallBuilder {
 
 impl MoneyUnstakeCallBuilder {
     pub fn build(&self) -> Result<MoneyUnstakeCallDebris> {
-        debug!("Building Money::UnstakeV1 contract call");
-        assert!(self.coin.note.value != 0);
+        info!("Building Money::UnstakeV1 contract call");
+        assert!(self.owncoin.note.value != 0);
 
-        debug!("Building anonymous output");
+        debug!("Building Money::UnstakeV1 anonymous output");
         let output = TransactionBuilderOutputInfo {
-            value: self.coin.note.value,
+            value: self.owncoin.note.value,
             token_id: *DARK_TOKEN_ID,
-            public_key: self.signature_public,
+            public_key: self.recipient,
         };
-        debug!("Finished building output");
 
         let serial = pallas::Base::random(&mut OsRng);
         let spend_hook = pallas::Base::ZERO;
-        let user_data_enc = pallas::Base::ZERO;
+        let user_data_enc = pallas::Base::random(&mut OsRng);
         let token_blind = pallas::Scalar::ZERO;
         let coin_blind = pallas::Base::random(&mut OsRng);
 
-        info!("Creating unstake mint proof for output");
+        info!("Building Money::UnstakeV1 Mint ZK proof");
         let (proof, public_inputs) = create_unstake_mint_proof(
             &self.mint_zkbin,
             &self.mint_pk,
@@ -138,15 +139,15 @@ impl MoneyUnstakeCallBuilder {
 
         let encrypted_note = AeadEncryptedNote::encrypt(&note, &output.public_key, &mut OsRng)?;
 
-        let output = Output {
+        let tx_output = Output {
             value_commit: public_inputs.value_commit,
             token_commit: public_inputs.token_commit,
             coin: public_inputs.coin,
             note: encrypted_note,
         };
 
-        let input = ConsensusInput {
-            epoch: self.coin.note.epoch,
+        let tx_input = ConsensusInput {
+            epoch: self.owncoin.note.epoch,
             value_commit: public_inputs.value_commit,
             nullifier: self.nullifier,
             merkle_root: self.merkle_root,
@@ -154,12 +155,11 @@ impl MoneyUnstakeCallBuilder {
         };
 
         // We now fill this with necessary stuff
-        let params = MoneyUnstakeParamsV1 { input, spend_hook, user_data_enc, output };
-        let proofs = vec![proof];
+        let params = MoneyUnstakeParamsV1 { input: tx_input, output: tx_output };
 
         // Now we should have all the params and zk proof.
         // We return it all and let the caller deal with it.
-        let debris = MoneyUnstakeCallDebris { params, proofs };
+        let debris = MoneyUnstakeCallDebris { params, proofs: vec![proof] };
         Ok(debris)
     }
 }

src/contract/money/src/entrypoint/unstake_v1.rs

@@ -18,8 +18,8 @@
 
 use darkfi_sdk::{
     crypto::{
-        pasta_prelude::*, pedersen_commitment_base, ContractId, MerkleNode, CONSENSUS_CONTRACT_ID,
-        DARK_TOKEN_ID,
+        pasta_prelude::*, pedersen_commitment_base, ContractId, MerkleNode, PublicKey,
+        CONSENSUS_CONTRACT_ID, DARK_TOKEN_ID,
     },
     db::{db_contains_key, db_lookup, db_set},
     error::{ContractError, ContractResult},
@@ -48,18 +48,18 @@ pub(crate) fn money_unstake_get_metadata_v1(
 
     // 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![params.input.signature_public];
+    // We don't have to verify any signatures here, since they're already
+    // in the previous contract call (Consensus::UnstakeV1)
+    let signature_pubkeys: Vec<PublicKey> = vec![];
 
     // Grab the pedersen commitment from the anonymous output
-    let output = &params.output;
-    let value_coords = output.value_commit.to_affine().coordinates().unwrap();
-    let token_coords = output.token_commit.to_affine().coordinates().unwrap();
+    let value_coords = params.output.value_commit.to_affine().coordinates().unwrap();
+    let token_coords = params.output.token_commit.to_affine().coordinates().unwrap();
 
     zk_public_inputs.push((
         MONEY_CONTRACT_ZKAS_MINT_NS_V1.to_string(),
         vec![
-            output.coin.inner(),
+            params.output.coin.inner(),
             *value_coords.x(),
             *value_coords.y(),
             *token_coords.x(),
@@ -71,7 +71,6 @@ pub(crate) fn money_unstake_get_metadata_v1(
     let mut metadata = vec![];
     zk_public_inputs.encode(&mut metadata)?;
     signature_pubkeys.encode(&mut metadata)?;
-
     Ok(metadata)
 }
 
@@ -83,6 +82,8 @@ pub(crate) fn money_unstake_process_instruction_v1(
 ) -> Result<Vec<u8>, ContractError> {
     let self_ = &calls[call_idx as usize];
     let params: MoneyUnstakeParamsV1 = deserialize(&self_.data[1..])?;
+    let input = &params.input;
+    let output = &params.output;
 
     // Access the necessary databases where there is information to
     // validate this state transition.
@@ -96,39 +97,6 @@ pub(crate) fn money_unstake_process_instruction_v1(
     // Perform the actual state transition
     // ===================================
 
-    msg!("[MoneyUnstakeV1] Validating anonymous output");
-    let input = &params.input;
-    let output = &params.output;
-
-    // Only native token can be unstaked.
-    // Since consensus coins don't have token commitments or blinds,
-    // we use zero as the token blind of newlly minded token
-    if output.token_commit !=
-        pedersen_commitment_base(DARK_TOKEN_ID.inner(), pallas::Scalar::zero())
-    {
-        msg!("[MoneyUnstakeV1] Error: Input used non-native token");
-        return Err(MoneyError::StakeInputNonNativeToken.into())
-    }
-
-    // Verify value commits match
-    if output.value_commit != input.value_commit {
-        msg!("[MoneyUnstakeV1] Error: Value commitments do not match");
-        return Err(MoneyError::ValueMismatch.into())
-    }
-
-    // The Merkle root is used to know whether this is a coin that
-    // existed in a previous state.
-    if !db_contains_key(consensus_unstaked_coin_roots_db, &serialize(&input.merkle_root))? {
-        msg!("[MoneyUnstakeV1] Error: Merkle root not found in previous state");
-        return Err(MoneyError::TransferMerkleRootNotFound.into())
-    }
-
-    // The nullifiers should already exist. It is the double-mint protection.
-    if db_contains_key(consensus_nullifiers_db, &serialize(&input.nullifier))? {
-        msg!("[MoneyUnstakeV1] Error: Duplicate nullifier found");
-        return Err(MoneyError::DuplicateNullifier.into())
-    }
-
     // Check previous call is consensus contract
     if call_idx == 0 {
         msg!("[MoneyUnstakeV1] Error: previous_call_idx will be out of bounds");
@@ -156,22 +124,38 @@ pub(crate) fn money_unstake_process_instruction_v1(
         return Err(MoneyError::PreviousCallInputMismatch.into())
     }
 
-    // If next spend hook is set, check its correctness
-    if params.spend_hook != pallas::Base::ZERO {
-        let next_call_idx = call_idx + 1;
-        if next_call_idx >= calls.len() as u32 {
-            msg!("[MoneyUnstakeV1] Error: next_call_idx out of bounds");
-            return Err(MoneyError::CallIdxOutOfBounds.into())
-        }
-
-        let next = &calls[next_call_idx as usize];
-        if next.contract_id.inner() != params.spend_hook {
-            msg!(
-                "[MoneyUnstakeV1] Error: Invoking contract call does not match spend hook in input"
-            );
-            return Err(MoneyError::SpendHookMismatch.into())
-        }
+    msg!("[MoneyUnstakeV1] Validating anonymous output");
+    // Only native token can be minted here.
+    // Since consensus coins don't have token commitments, we use zero as
+    // the token blind for the token commitment of the newly minted token
+    if output.token_commit != pedersen_commitment_base(DARK_TOKEN_ID.inner(), pallas::Scalar::ZERO)
+    {
+        msg!("[MoneyUnstakeV1] Error: Input used non-native token");
+        return Err(MoneyError::StakeInputNonNativeToken.into())
+    }
+
+    // Verify value commits match
+    if output.value_commit != input.value_commit {
+        msg!("[MoneyUnstakeV1] Error: Value commitments do not match");
+        return Err(MoneyError::ValueMismatch.into())
+    }
+
+    // The Merkle root is used to know whether this is a coin that
+    // existed in a previous state.
+    if !db_contains_key(consensus_unstaked_coin_roots_db, &serialize(&input.merkle_root))? {
+        msg!("[MoneyUnstakeV1] Error: Merkle root not found in previous state");
+        return Err(MoneyError::TransferMerkleRootNotFound.into())
+    }
+
+    // The nullifiers should already exist in the Consensus nullifier set
+    // TODO: FIXME: This should be uncommented when Validator::verify_transaction
+    //              works as Read->Write->Read->Write
+    /*
+    if !db_contains_key(consensus_nullifiers_db, &serialize(&input.nullifier))? {
+        msg!("[MoneyUnstakeV1] Error: Nullifier not found in Consensus nullifier set");
+        return Err(MoneyError::MissingNullifier.into())
     }
+    */
 
     // Newly created coin for this call is in the output. Here we gather it,
     // and we also check that it hasn't existed before.
@@ -185,8 +169,6 @@ pub(crate) fn money_unstake_process_instruction_v1(
     let mut update_data = vec![];
     update_data.write_u8(MoneyFunction::UnstakeV1 as u8)?;
     update.encode(&mut update_data)?;
-
-    // and return it
     Ok(update_data)
 }
 

src/contract/money/src/error.rs

@@ -114,6 +114,9 @@ pub enum MoneyError {
 
     #[error("Call is not executed on genesis slot")]
     GenesisCallNonGenesisSlot,
+
+    #[error("Missing nullifier in set")]
+    MissingNullifier,
 }
 
 impl From<MoneyError> for ContractError {
@@ -150,6 +153,7 @@ impl From<MoneyError> for ContractError {
             MoneyError::PreviousCallFunctionMismatch => Self::Custom(29),
             MoneyError::PreviousCallInputMismatch => Self::Custom(30),
             MoneyError::GenesisCallNonGenesisSlot => Self::Custom(31),
+            MoneyError::MissingNullifier => Self::Custom(32),
         }
     }
 }

src/contract/money/src/model.rs

@@ -209,27 +209,23 @@ pub struct MoneyStakeUpdateV1 {
 
 /// Parameters for `Money::Unstake`
 #[derive(Clone, Debug, SerialEncodable, SerialDecodable)]
+// ANCHOR: MoneyUnstakeParams
 pub struct MoneyUnstakeParamsV1 {
     /// Burnt token revealed info
     pub input: ConsensusInput,
-    /// Spend hook used to invoke other contracts.
-    /// If this value is nonzero then the subsequent contract call in the tx
-    /// must have this value as its ID.
-    pub spend_hook: pallas::Base,
-    /// Encrypted user data field. An encrypted commitment to arbitrary data.
-    /// When spend hook is set (it is nonzero), then this field may be user
-    /// to pass data to the invoked contract.
-    pub user_data_enc: pallas::Base,
     /// Anonymous output
     pub output: Output,
 }
+// ANCHOR_END: MoneyUnstakeParams
 
 /// State update for `Money::Unstake`
 #[derive(Clone, Debug, SerialEncodable, SerialDecodable)]
+// ANCHOR: MoneyUnstakeUpdate
 pub struct MoneyUnstakeUpdateV1 {
     /// The newly minted coin
     pub coin: Coin,
 }
+// ANCHOR_END: MoneyUnstakeUpdate
 
 /// Parameters for `Consensus::Stake`
 #[derive(Clone, Debug, SerialEncodable, SerialDecodable)]
@@ -262,14 +258,18 @@ pub struct ConsensusUnstakeReqParamsV1 {
 
 /// Parameters for `Consensus::Unstake`
 #[derive(Clone, Debug, SerialEncodable, SerialDecodable)]
+// ANCHOR: ConsensusUnstakeParams
 pub struct ConsensusUnstakeParamsV1 {
     /// Anonymous input
     pub input: ConsensusInput,
 }
+// ANCHOR_END: ConsensusUnstakeParams
 
 /// State update for `Consensus::Unstake`
 #[derive(Clone, Debug, SerialEncodable, SerialDecodable)]
+// ANCHOR: ConsensusUnstakeUpdate
 pub struct ConsensusUnstakeUpdateV1 {
     /// Revealed nullifier
     pub nullifier: Nullifier,
 }
+// ANCHOR_END: ConsensusUnstakeUpdate