blockchain: include total_stake in Epoch (#23)

Having cached total stake value in Epoch structure helps with rewards
calculation where we need give validators proportional payouts for
signing blocks.

common/blockchain/src/candidates.rs

@@ -99,26 +99,19 @@ impl<PK: PubKey> Candidates<PK> {
             .fold(0, |sum, c| sum.checked_add(c.stake.get()).unwrap())
     }
 
-    /// Returns top validators together with their total stake if changed since
-    /// last call.
-    pub fn maybe_get_head(&mut self) -> Option<(Vec<Validator<PK>>, u128)> {
+    /// Returns top validators if changed since last call.
+    pub fn maybe_get_head(&mut self) -> Option<Vec<Validator<PK>>> {
         if !self.changed {
             return None;
         }
-        let mut total: u128 = 0;
         let validators = self
             .candidates
             .iter()
             .take(self.max_validators())
-            .map(|candidate| {
-                total = total.checked_add(candidate.stake.get())?;
-                Some(Validator::from(candidate))
-            })
-            .collect::<Option<Vec<_>>>()
-            .unwrap();
+            .map(Validator::from)
+            .collect::<Vec<_>>();
         self.changed = false;
-        self.debug_verify_state();
-        Some((validators, total))
+        Some(validators)
     }
 
     /// Adds a new candidates or updates existing candidate’s stake.

common/blockchain/src/epoch.rs

@@ -1,16 +1,16 @@
 use alloc::vec::Vec;
 use core::num::NonZeroU128;
 
-use crate::validators::{PubKey, Validator};
+use borsh::maybestd::io;
+
+use crate::validators::Validator;
 
 /// An epoch describing configuration applying to all blocks within an epoch.
 ///
 /// An epoch is identified by hash of the block it was introduced in.  As such,
 /// epoch’s identifier is unknown until block which defines it in
 /// [`crate::block::Block::next_blok`] field is created.
-#[derive(
-    Clone, Debug, PartialEq, Eq, borsh::BorshSerialize, borsh::BorshDeserialize,
-)]
+#[derive(Clone, Debug, PartialEq, Eq, borsh::BorshSerialize)]
 pub struct Epoch<PK> {
     /// Version of the structure.  Used to support forward-compatibility.  At
     /// the moment this is always zero.
@@ -20,10 +20,29 @@ pub struct Epoch<PK> {
     validators: Vec<Validator<PK>>,
 
     /// Minimum stake to consider block signed.
+    ///
+    /// Always no more than `total_stake`.
     quorum_stake: NonZeroU128,
+
+    /// Total stake.
+    ///
+    /// This is always `sum(v.stake for v in validators)`.
+    // We don’t serialise it because we calculate it when deserializing to make
+    // sure that it’s always a correct value.
+    #[borsh_skip]
+    total_stake: NonZeroU128,
+}
+
+impl<PK: borsh::BorshDeserialize> borsh::BorshDeserialize for Epoch<PK> {
+    fn deserialize_reader<R: io::Read>(reader: &mut R) -> io::Result<Self> {
+        let _ = crate::common::VersionZero::deserialize_reader(reader)?;
+        let (validators, quorum_stake) = <_>::deserialize_reader(reader)?;
+        Self::new(validators, quorum_stake)
+            .ok_or_else(|| io::ErrorKind::InvalidData.into())
+    }
 }
 
-impl<PK: PubKey> Epoch<PK> {
+impl<PK> Epoch<PK> {
     /// Creates a new epoch.
     ///
     /// Returns `None` if the epoch is invalid, i.e. if quorum stake is greater
@@ -34,37 +53,32 @@ impl<PK: PubKey> Epoch<PK> {
         validators: Vec<Validator<PK>>,
         quorum_stake: NonZeroU128,
     ) -> Option<Self> {
-        let version = crate::common::VersionZero;
-        let this = Self { version, validators, quorum_stake };
-        Some(this).filter(Self::is_valid)
+        Self::new_with(validators, |_| quorum_stake)
     }
 
-    /// Creates a new epoch without checking whether it’s valid.
+    /// Creates a new epoch with function determining quorum.
     ///
-    /// It’s caller’s responsibility to guarantee that total stake of all
-    /// validators is no more than quorum stake.
-    ///
-    /// In debug builds panics if the result is an invalid epoch.
-    pub(crate) fn new_unchecked(
+    /// The callback function is invoked with the total stake of all the
+    /// validators and must return positive number no greater than the argument.
+    /// If the returned value is greater, the epoch would be invalid and this
+    /// constructor returns `None`.  Also returns `None` when total stake is
+    /// zero.
+    pub fn new_with(
         validators: Vec<Validator<PK>>,
-        quorum_stake: NonZeroU128,
-    ) -> Self {
-        let version = crate::common::VersionZero;
-        let this = Self { version, validators, quorum_stake };
-        debug_assert!(this.is_valid());
-        this
-    }
-
-    /// Checks whether the epoch is valid.
-    fn is_valid(&self) -> bool {
-        let mut left = self.quorum_stake.get();
-        for validator in self.validators.iter() {
-            left = left.saturating_sub(validator.stake().get());
-            if left == 0 {
-                return true;
-            }
+        quorum_stake: impl FnOnce(NonZeroU128) -> NonZeroU128,
+    ) -> Option<Self> {
+        let mut total: u128 = 0;
+        for validator in validators.iter() {
+            total = total.checked_add(validator.stake().get())?;
+        }
+        let total_stake = NonZeroU128::new(total)?;
+        let quorum_stake = quorum_stake(total_stake);
+        if quorum_stake <= total_stake {
+            let version = crate::common::VersionZero;
+            Some(Self { version, validators, quorum_stake, total_stake })
+        } else {
+            None
         }
-        false
     }
 
     /// Returns list of all validators in the epoch.
@@ -74,7 +88,10 @@ impl<PK: PubKey> Epoch<PK> {
     pub fn quorum_stake(&self) -> NonZeroU128 { self.quorum_stake }
 
     /// Finds a validator by their public key.
-    pub fn validator(&self, pk: &PK) -> Option<&Validator<PK>> {
+    pub fn validator(&self, pk: &PK) -> Option<&Validator<PK>>
+    where
+        PK: Eq,
+    {
         self.validators.iter().find(|validator| validator.pubkey() == pk)
     }
 }
@@ -94,7 +111,10 @@ impl Epoch<crate::validators::MockPubKey> {
                 Validator::new(pk.into(), NonZeroU128::new(stake).unwrap())
             })
             .collect();
-        Self::new(validators, NonZeroU128::new(total / 2 + 1).unwrap()).unwrap()
+        Self::new_with(validators, |total| {
+            NonZeroU128::new(total.get() / 2 + 1).unwrap()
+        })
+        .unwrap()
     }
 }
 
@@ -118,3 +138,80 @@ fn test_creation() {
     assert_eq!(Some(&validators[0]), epoch.validator(&MockPubKey(0)));
     assert_eq!(None, epoch.validator(&MockPubKey(2)));
 }
+
+#[test]
+fn test_borsh_success() {
+    let epoch = Epoch::test(&[(0, 10), (1, 10)]);
+    let encoded = borsh::to_vec(&epoch).unwrap();
+    #[rustfmt::skip]
+    assert_eq!(&[
+        /* version: */ 0,
+        /* length:  */ 2, 0, 0, 0,
+        /* v[0].version: */ 0,
+        /* v[0].pubkey: */ 0, 0, 0, 0,
+        /* v[0].stake: */ 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        /* v[1].version: */ 0,
+        /* v[1].pubkey: */ 1, 0, 0, 0,
+        /* v[1].stake: */ 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        /* quorum: */ 11, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    ], encoded.as_slice());
+
+    let got = borsh::BorshDeserialize::try_from_slice(encoded.as_slice());
+    assert_eq!(epoch, got.unwrap());
+}
+
+#[test]
+#[rustfmt::skip]
+fn test_borsh_failures() {
+    fn test(bytes: &[u8]) {
+        use borsh::BorshDeserialize;
+        let got = Epoch::<crate::validators::MockPubKey>::try_from_slice(bytes);
+        got.unwrap_err();
+    }
+
+    // No validators
+    test(&[
+        /* version: */ 0,
+        /* length:  */ 0, 0, 0, 0,
+        /* quorum: */ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    ]);
+
+    // Validator with no stake.
+    test(&[
+        /* version: */ 0,
+        /* length:  */ 2, 0, 0, 0,
+        /* v[0].version: */ 0,
+        /* v[0].pubkey: */ 0, 0, 0, 0,
+        /* v[0].stake: */ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        /* v[1].version: */ 0,
+        /* v[1].pubkey: */ 1, 0, 0, 0,
+        /* v[1].stake: */ 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        /* quorum: */ 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    ]);
+
+    // Zero quorum
+    test(&[
+        /* version: */ 0,
+        /* length:  */ 2, 0, 0, 0,
+        /* v[0].version: */ 0,
+        /* v[0].pubkey: */ 0, 0, 0, 0,
+        /* v[0].stake: */ 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        /* v[1].version: */ 0,
+        /* v[1].pubkey: */ 1, 0, 0, 0,
+        /* v[1].stake: */ 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        /* quorum: */ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    ]);
+
+    // Quorum over total
+    test(&[
+        /* version: */ 0,
+        /* length:  */ 2, 0, 0, 0,
+        /* v[0].version: */ 0,
+        /* v[0].pubkey: */ 0, 0, 0, 0,
+        /* v[0].stake: */ 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        /* v[1].version: */ 0,
+        /* v[1].pubkey: */ 1, 0, 0, 0,
+        /* v[1].stake: */ 10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+        /* quorum: */ 21, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+    ]);
+}

common/blockchain/src/manager.rs

@@ -168,16 +168,16 @@ impl<PK: PubKey> ChainManager<PK> {
         {
             return None;
         }
-        let (validators, total) = self.candidates.maybe_get_head()?;
-        // 1. We validate that genesis has a valid epoch (at least 1 stake).
-        // 2. We never allow fewer than config.min_validators candidates.
-        // 3. We never allow candidates with zero stake.
-        // Therefore, total should always be positive.
-        let total = NonZeroU128::new(total).unwrap();
-        // SAFETY: anything_unsigned + 1 > 0
-        let quorum = unsafe { NonZeroU128::new_unchecked(total.get() / 2 + 1) }
-            .clamp(self.config.min_quorum_stake, total);
-        Some(epoch::Epoch::new_unchecked(validators, quorum))
+        epoch::Epoch::new_with(self.candidates.maybe_get_head()?, |total| {
+            // SAFETY: 1. ‘total / 2 ≥ 0’ thus ‘total / 2 + 1 > 0’.
+            // 2. ‘total / 2 <= u128::MAX / 2’ thus ‘total / 2 + 1 < u128::MAX’.
+            let quorum =
+                unsafe { NonZeroU128::new_unchecked(total.get() / 2 + 1) };
+            // min_quorum_stake may be greater than total_stake so we’re not
+            // using .clamp to make sure we never return value higher than
+            // total_stake.
+            quorum.max(self.config.min_quorum_stake).min(total)
+        })
     }
 
     /// Adds a signature to pending block.

common/blockchain/src/validators.rs

@@ -45,7 +45,7 @@ pub struct Validator<PK> {
     stake: NonZeroU128,
 }
 
-impl<PK: PubKey> Validator<PK> {
+impl<PK> Validator<PK> {
     pub fn new(pubkey: PK, stake: NonZeroU128) -> Self {
         Self { version: crate::common::VersionZero, pubkey, stake }
     }