blockchain: remove state root from chain manager

common/blockchain/Cargo.toml

@@ -11,7 +11,7 @@ derive_more.workspace = true
 lib = { workspace = true, features = ["borsh"] }
 
 [dev-dependencies]
-lib = { workspace = true, features = ["borsh", "test_utils"] }
+lib = { workspace = true, features = ["test_utils"] }
 rand.workspace = true
 stdx.workspace = true
 

common/blockchain/src/candidates.rs

@@ -99,32 +99,51 @@ impl<PK: PubKey> Candidates<PK> {
             .fold(0, |sum, c| sum.checked_add(c.stake.get()).unwrap())
     }
 
-    /// 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 validators = self
-            .candidates
-            .iter()
-            .take(self.max_validators())
-            .map(Validator::from)
-            .collect::<Vec<_>>();
-        self.changed = false;
-        Some(validators)
+    /// Returns top validators if changed since last time changed flag was
+    /// cleared.
+    ///
+    /// To clear changed flag, use [`Self::clear_changed_flag`].
+    pub fn maybe_get_head(&self) -> Option<Vec<Validator<PK>>> {
+        self.changed.then(|| {
+            self.candidates
+                .iter()
+                .take(self.max_validators())
+                .map(Validator::from)
+                .collect::<Vec<_>>()
+        })
     }
 
+    /// Clears the changed flag.
+    ///
+    /// Changed flag is set automatically whenever head of the candidates list
+    /// is modified (note that changes outside of the head of candidates list do
+    /// not affect the flag).
+    pub fn clear_changed_flag(&mut self) { self.changed = false; }
+
     /// Adds a new candidates or updates existing candidate’s stake.
+    ///
+    /// If `stake` is zero, removes the candidate from the set.
     pub fn update(
         &mut self,
         cfg: &chain::Config,
         pubkey: PK,
         stake: u128,
     ) -> Result<(), UpdateCandidateError> {
-        let stake = NonZeroU128::new(stake)
-            .filter(|stake| *stake >= cfg.min_validator_stake)
-            .ok_or(UpdateCandidateError::NotEnoughValidatorStake)?;
-        let candidate = Candidate { pubkey, stake };
+        match NonZeroU128::new(stake) {
+            None => self.do_remove(cfg, &pubkey),
+            Some(stake) if stake < cfg.min_validator_stake => {
+                Err(UpdateCandidateError::NotEnoughValidatorStake)
+            }
+            Some(stake) => self.do_update(cfg, Candidate { pubkey, stake }),
+        }
+    }
+
+    /// Adds a new candidates or updates existing candidate’s stake.
+    fn do_update(
+        &mut self,
+        cfg: &chain::Config,
+        candidate: Candidate<PK>,
+    ) -> Result<(), UpdateCandidateError> {
         let old_pos =
             self.candidates.iter().position(|el| el.pubkey == candidate.pubkey);
         let mut new_pos =
@@ -143,7 +162,7 @@ impl<PK: PubKey> Candidates<PK> {
     }
 
     /// Removes an existing candidate.
-    pub fn remove(
+    fn do_remove(
         &mut self,
         cfg: &chain::Config,
         pubkey: &PK,

common/blockchain/src/candidates/tests.rs

@@ -121,23 +121,23 @@ fn test_candidates_0() {
     // Check minimum total stake and count are checked
     assert_eq!(
         Err(NotEnoughTotalStake),
-        candidates.remove(&cfg_with_min_total_stake(10), &pk('E')),
+        candidates.update(&cfg_with_min_total_stake(10), pk('E'), 0),
     );
     assert_eq!(
         Err(NotEnoughValidators),
-        candidates.remove(&cfg_with_min_validators(5), &pk('E')),
+        candidates.update(&cfg_with_min_validators(5), pk('E'), 0),
     );
 
     // Removal is idempotent
     for _ in 0..2 {
-        candidates.remove(&cfg_with_min_validators(2), &pk('E')).unwrap();
+        candidates.update(&cfg_with_min_validators(2), pk('E'), 0).unwrap();
         check([('D', 4), ('C', 3), ('B', 2), ('A', 1)], &candidates);
     }
 
     // Go below max_validators of candidates.
-    candidates.remove(&cfg_with_min_validators(1), &pk('C')).unwrap();
-    candidates.remove(&cfg_with_min_validators(1), &pk('B')).unwrap();
-    candidates.remove(&cfg_with_min_validators(1), &pk('A')).unwrap();
+    candidates.update(&cfg_with_min_validators(1), pk('C'), 0).unwrap();
+    candidates.update(&cfg_with_min_validators(1), pk('B'), 0).unwrap();
+    candidates.update(&cfg_with_min_validators(1), pk('A'), 0).unwrap();
     check([('D', 4)], &candidates);
 
     // Minimum validator stake is checked
@@ -261,7 +261,7 @@ impl TestCtx {
         let count = self.candidates.candidates.len();
         let head_stake = self.candidates.head_stake;
 
-        let res = self.candidates.remove(&self.config, &pubkey);
+        let res = self.candidates.update(&self.config, pubkey.clone(), 0);
         self.check();
 
         if let Err(err) = res {
@@ -375,38 +375,30 @@ impl TestCtx {
         );
     }
 
-    /// Performs a random test.  `data` must be a three-element slice.  The
-    /// random test is determined from values in the slice.
-    fn test(&mut self, data: &[u8]) {
+    /// Performs a random test.  `data` must be a two-element slice.  The random
+    /// test is determined from values in the slice.
+    fn test(&mut self, pubkey: u8, stake: u8) {
         let old_state = self.candidates.clone();
-        let pubkey = MockPubKey((data[0]).into());
-        let op = if data[2] % 2 == 0 {
-            Ok(pubkey)
-        } else {
-            Err((pubkey, u128::from(data[1])))
-        };
+        let pubkey = MockPubKey(u32::from(pubkey));
+        let stake = u128::from(stake);
 
         let this = self as *mut TestCtx;
         let res = std::panic::catch_unwind(|| {
             // SAFETY: It’s test code.  I don’t care. ;)  It’s probably safe but
             // self.candidates may be in inconsistent state.  This is fine since
             // we’re panicking anyway.
             let this = unsafe { &mut *this };
-            match &op {
-                Ok(pubkey) => this.test_remove(pubkey.clone()),
-                Err((pubkey, stake)) => {
-                    this.test_update(pubkey.clone(), *stake)
-                }
+            match u128::from(stake) {
+                0 => this.test_remove(pubkey),
+                _ => this.test_update(pubkey.clone(), stake),
             }
         });
 
         if let Err(err) = res {
             std::eprintln!("{:?}", old_state);
-            match op {
-                Ok(pubkey) => std::eprintln!("  Remove {pubkey:?}"),
-                Err((pubkey, stake)) => {
-                    std::eprintln!(" Update {pubkey:?} staking {stake}")
-                }
+            match stake {
+                0 => std::eprintln!(" Remove {pubkey:?}"),
+                _ => std::eprintln!(" Update {pubkey:?} staking {stake}"),
             }
             std::eprintln!("{:?}", self.candidates);
             std::panic::resume_unwind(err);
@@ -421,11 +413,11 @@ fn stress_test() {
     let mut rng = rand::thread_rng();
     let mut ctx = TestCtx::new(&mut rng);
     let mut n = lib::test_utils::get_iteration_count(1);
-    let mut buf = [0u8; 3 * 1024];
+    let mut buf = [0u8; 2 * 1024];
     while n > 0 {
         rng.fill(&mut buf[..]);
-        for data in buf.chunks_exact(3).take(n) {
-            ctx.test(data);
+        for data in buf.chunks_exact(2).take(n) {
+            ctx.test(data[0], data[1]);
         }
         n = n.saturating_sub(1024);
     }