PR comments

data_structures/src/chain/mod.rs

@@ -518,6 +518,7 @@ impl Block {
     pub fn weight(&self) -> u32 {
         self.dr_weight() + self.vt_weight() + self.st_weight()
     }
+
 }
 
 impl BlockTransactions {
@@ -2054,6 +2055,9 @@ pub struct TransactionsPool {
     required_reward_collateral_ratio: u64,
     // Map for unconfirmed transactions
     unconfirmed_transactions: UnconfirmedTransactions,
+    // TODO: refactor to use Rc<WriteLock<PrioritizedStakeTransaction>> or
+    // Arc<Mutex<PrioritizedStakeTransaction>> to prevent the current indirect lookup (having to
+    // first query the index for the hash, and then using the hash to find the actual data)
     st_transactions: HashMap<Hash, PrioritizedStakeTransaction>,
     sorted_st_index: BTreeSet<PrioritizedHash>,
     // Minimum fee required to include a Stake Transaction into a block. We check for this fee in the
@@ -2405,8 +2409,7 @@ impl TransactionsPool {
             .unwrap_or(Ok(false))
     }
 
-    /// Returns `true` if the pool contains a stake
-    /// transaction for the specified hash.
+    /// Returns `true` if the pool contains a stake transaction for the specified hash.
     ///
     /// The `key` may be any borrowed form of the hash, but `Hash` and
     /// `Eq` on the borrowed form must match those for the key type.
@@ -2563,6 +2566,7 @@ impl TransactionsPool {
                 for hash in hashes.iter() {
                     self.vt_remove_inner(hash, false);
                     self.dr_remove_inner(hash, false);
+                    self.st_remove_inner(hash, false);
                 }
             }
         }
@@ -2642,7 +2646,7 @@ impl TransactionsPool {
     /// that may try to spend the same UTXOs are also removed.
     /// This should be used to remove transactions that got included in a consolidated block.
     ///
-    /// Returns an `Option` with the value transfer transaction for the specified hash or `None` if not exist.
+    /// Returns an `Option` with the stake transaction for the specified hash or `None` if not exist.
     ///
     /// The `key` may be any borrowed form of the hash, but `Hash` and
     /// `Eq` on the borrowed form must match those for the key type.
@@ -2757,13 +2761,12 @@ impl TransactionsPool {
     /// Returns a list of all the removed transactions.
     fn remove_transactions_for_size_limit(&mut self) -> Vec<Transaction> {
         let mut removed_transactions = vec![];
-        // TODO: Don't we have a method that make the following sum??
-        while self.total_vt_weight + self.total_dr_weight + self.total_st_weight > self.weight_limit
+        while self.total_transactions_weight() > self.weight_limit
         {
             // Try to split the memory between value transfer and data requests using the same
             // ratio as the one used in blocks
-            // The ratio of vt to dr in blocks is currently 4:1
-            // TODO: What the criteria to delete st?
+            // The ratio of vt to dr in blocks is currently 1:4
+            // TODO: What the criteria to delete st? It should be 1:8
             #[allow(clippy::cast_precision_loss)]
             let more_vtts_than_drs =
                 self.total_vt_weight as f64 >= self.total_dr_weight as f64 * self.vt_to_dr_factor;
@@ -2898,7 +2901,7 @@ impl TransactionsPool {
                     for input in &st_tx.body.inputs {
                         self.output_pointer_map
                             .entry(input.output_pointer)
-                            .or_insert_with(Vec::new)
+                            .or_default()
                             .push(st_tx.hash());
                     }
 
@@ -3103,6 +3106,10 @@ impl TransactionsPool {
 
         v
     }
+
+    pub fn total_transactions_weight (&self) -> u64 {
+        self.total_vt_weight + self.total_dr_weight + self.total_st_weight 
+    }
 }
 
 /// Unspent output data structure (equivalent of Bitcoin's UTXO)

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
         }