PR comments

data_structures/src/error.rs

@@ -278,14 +278,17 @@ pub enum TransactionError {
         max_weight: u32,
         dr_output: Box<DataRequestOutput>,
     },
-    /// Stake weight limit exceeded
+    /// Stake amount below minimum
     #[fail(
-        display = "Stake ({}) doesn't reach the minimum amount ({})",
+        display = "The amount of coins in stake ({}) is less than the minimum allowed ({})",
         min_stake, stake
     )]
-    MinStakeNotReached { min_stake: u64, stake: u64 },
-    /// An stake output with zero value does not make sense
-    #[fail(display = "Transaction {} has a zero value stake output", tx_hash)]
+    StakeBelowMinimum { min_stake: u64, stake: u64 },
+    /// A stake output with zero value does not make sense
+    #[fail(
+        display = "Transaction {} contains a stake output with zero value",
+        tx_hash
+    )]
     ZeroValueStakeOutput { tx_hash: Hash },
     #[fail(
         display = "The reward-to-collateral ratio for this data request is {}, but must be equal or less than {}",
@@ -410,15 +413,18 @@ pub enum BlockError {
         weight, max_weight
     )]
     TotalDataRequestWeightLimitExceeded { weight: u32, max_weight: u32 },
-    /// Stake weight limit exceeded
+    /// Stake weight limit exceeded by a block candidate
     #[fail(
         display = "Total weight of Stake Transactions in a block ({}) exceeds the limit ({})",
         weight, max_weight
     )]
     TotalStakeWeightLimitExceeded { weight: u32, max_weight: u32 },
     /// Repeated operator Stake
-    #[fail(display = "A single operator is staking more than once: ({}) ", pkh)]
-    RepeatedStakeOperator { pkh: Hash },
+    #[fail(
+        display = "A single operator is receiving stake more than once in a block: ({}) ",
+        pkh
+    )]
+    RepeatedStakeOperator { pkh: PublicKeyHash },
     /// Missing expected tallies
     #[fail(
         display = "{} expected tally transactions are missing in block candidate {}",

data_structures/src/transaction.rs

@@ -697,15 +697,16 @@ pub struct StakeTransaction {
     pub body: StakeTransactionBody,
     pub signatures: Vec<KeyedSignature>,
 }
+
 impl StakeTransaction {
     // Creates a new stake transaction.
     pub fn new(body: StakeTransactionBody, signatures: Vec<KeyedSignature>) -> Self {
         StakeTransaction { body, signatures }
     }
 
     /// Returns the weight of a stake transaction.
-    /// This is the weight that will be used to calculate
-    /// how many transactions can fit inside one block
+    /// This is the weight that will be used to calculate how many transactions can fit inside one
+    /// block
     pub fn weight(&self) -> u32 {
         self.body.weight()
     }
@@ -724,20 +725,6 @@ pub struct StakeTransactionBody {
 }
 
 impl StakeTransactionBody {
-    /// Creates a new stake transaction body.
-    pub fn new(
-        inputs: Vec<Input>,
-        output: StakeOutput,
-        change: Option<ValueTransferOutput>,
-    ) -> Self {
-        StakeTransactionBody {
-            inputs,
-            output,
-            change,
-            hash: MemoHash::new(),
-        }
-    }
-
     /// Stake transaction weight. It is calculated as:
     ///
     /// ```text

data_structures/src/transaction_factory.rs

@@ -537,9 +537,12 @@ pub fn transaction_outputs_sum(outputs: &[ValueTransferOutput]) -> Result<u64, T
     Ok(total_value)
 }
 
+/// Build stake transaction with the given inputs, stake output and change.
 pub fn build_st() -> Result<StakeTransactionBody, TransactionError> {
+    // TODO: add stake transaction factory logic here
     !unimplemented!()
 }
+
 #[cfg(test)]
 mod tests {
     use std::{

schemas/witnet/witnet.proto

@@ -239,7 +239,7 @@ message StakeOutput {
 message StakeTransactionBody {
     repeated Input inputs = 1;
     StakeOutput output = 2;
-    ValueTransferOutput change = 3;
+    optional ValueTransferOutput change = 3;
 }
 
 message StakeTransaction {

validations/Cargo.toml

@@ -8,7 +8,7 @@ workspace = ".."
 
 [dependencies]
 failure = "0.1.8"
-itertools = "0.8.2"
+itertools = "0.11.0"
 log = "0.4.8"
 url = "2.2.2"
 

validations/src/tests/mod.rs

@@ -8457,20 +8457,14 @@ fn st_no_inputs() {
     let block_number = 0;
     let utxo_diff = UtxoDiff::new(&utxo_set, block_number);
 
-    // Try to create an stake tx with no inputs
+    // Try to create a stake tx with no inputs
     let st_output = StakeOutput {
         value: MIN_STAKE_NANOWITS + 1,
         authorization: KeyedSignature::default(),
     };
-    // let vto0 = ValueTransferOutput {
-    //     pkh,
-    //     value: 1000,
-    //     time_lock: 0,
-    // };
+
     let st_body = StakeTransactionBody::new(Vec::new(), st_output, None);
     let st_tx = StakeTransaction::new(st_body, vec![]);
-    // let vt_body = VTTransactionBody::new(vec![], vec![vto0]);
-    // let vt_tx = VTTransaction::new(vt_body, vec![]);
     let x = validate_stake_transaction(
         &st_tx,
         &utxo_diff,
@@ -8523,7 +8517,7 @@ fn st_one_input_but_no_signature() {
 }
 
 #[test]
-fn st_min_stake_not_reach() {
+fn st_below_min_stake() {
     let mut signatures_to_verify = vec![];
     let utxo_set = UnspentOutputsPool::default();
     let block_number = 0;
@@ -8551,7 +8545,7 @@ fn st_min_stake_not_reach() {
     );
     assert_eq!(
         x.unwrap_err().downcast::<TransactionError>().unwrap(),
-        TransactionError::MinStakeNotReached {
+        TransactionError::StakeBelowMinimum {
             min_stake: MIN_STAKE_NANOWITS,
             stake: 1
         }

validations/src/validations.rs

@@ -101,20 +101,23 @@ pub fn dr_transaction_fee(
 
 /// Returns the fee of a stake transaction.
 ///
-/// The fee is the difference between the output and the inputs
-/// of the transaction. The pool parameter is used to find the
-/// outputs pointed by the inputs and that contain the actual
-/// their value.
+/// The fee is the difference between the output and the inputs of the transaction.
 pub fn st_transaction_fee(
     st_tx: &StakeTransaction,
     utxo_diff: &UtxoDiff<'_>,
     epoch: Epoch,
     epoch_constants: EpochConstants,
 ) -> Result<u64, failure::Error> {
     let in_value = transaction_inputs_sum(&st_tx.body.inputs, utxo_diff, epoch, epoch_constants)?;
-    let out_value = &st_tx.body.output.value;
+    let out_value = &st_tx.body.output.value
+        - &st_tx
+            .body
+            .change
+            .clone()
+            .unwrap_or(Default::default())
+            .value;
 
-    if out_value > &in_value {
+    if out_value > in_value {
         Err(TransactionError::NegativeFee.into())
     } else {
         Ok(in_value - out_value)
@@ -1113,7 +1116,7 @@ pub fn validate_tally_transaction<'a>(
     Ok((ta_tx.outputs.iter().collect(), tally_extra_fee))
 }
 
-/// Function to validate a stake transaction
+/// Function to validate a stake transaction.
 pub fn validate_stake_transaction<'a>(
     st_tx: &'a StakeTransaction,
     utxo_diff: &UtxoDiff<'_>,
@@ -1130,9 +1133,9 @@ pub fn validate_stake_transaction<'a>(
     ),
     failure::Error,
 > {
-    // Check that the stake is greater than the min allowed
+    // Check that the amount of coins to stake is equal or greater than the minimum allowed
     if st_tx.body.output.value < MIN_STAKE_NANOWITS {
-        return Err(TransactionError::MinStakeNotReached {
+        return Err(TransactionError::StakeBelowMinimum {
             min_stake: MIN_STAKE_NANOWITS,
             stake: st_tx.body.output.value,
         }
@@ -1759,28 +1762,20 @@ pub fn validate_block_transactions(
     let mut st_weight: u32 = 0;
 
     // Check if the block contains more than one stake tx from the same operator
-    for i in 1..block.txns.stake_txns.len() {
-        let found = block.txns.stake_txns[i..].iter().find(|stake_tx| {
-            let stake_tx_aux = &block.txns.stake_txns[i - 1];
-
-            stake_tx.body.output.authorization.public_key
-                == stake_tx_aux.body.output.authorization.public_key
-        });
+    let duplicate = block
+        .txns
+        .stake_txns
+        .clone()
+        .into_iter()
+        .map(|stake_tx| stake_tx.body.output.authorization.public_key)
+        .duplicates()
+        .next();
 
-        // TODO: refactor
-        if found.is_some() {
-            return Err(BlockError::RepeatedStakeOperator {
-                pkh: found
-                    .unwrap()
-                    .body
-                    .output
-                    .authorization
-                    .public_key
-                    .pkh()
-                    .hash(),
-            }
-            .into());
+    if duplicate.is_some() {
+        return Err(BlockError::RepeatedStakeOperator {
+            pkh: duplicate.unwrap().pkh(),
         }
+        .into());
     }
 
     for transaction in &block.txns.stake_txns {
@@ -1805,20 +1800,11 @@ pub fn validate_block_transactions(
         }
         st_weight = acc_weight;
 
-        // TODO: refactor
-        if change.is_some() {
-            let mut outputs: Vec<&ValueTransferOutput> = Vec::new();
-            let val = change.clone().unwrap();
-            outputs.push(&val);
-            update_utxo_diff(&mut utxo_diff, inputs, outputs, transaction.hash());
-        } else {
-            update_utxo_diff(&mut utxo_diff, inputs, Vec::new(), transaction.hash());
-        }
+        let outputs = change.into_iter().collect_vec();
+        update_utxo_diff(&mut utxo_diff, inputs, outputs, transaction.hash());
 
         // Add new hash to merkle tree
-        let txn_hash = transaction.hash();
-        let Hash::SHA256(sha) = txn_hash;
-        st_mt.push(Sha256(sha));
+        st_mt.push(transaction.hash().into());
 
         // TODO: Move validations to a visitor
         // // Execute visitor