From 9048b16359e480b002d3a2d6d2351ffbd9d5e3f3 Mon Sep 17 00:00:00 2001
From: Michal Nazarewicz <mina86@mina86.com>
Date: Mon, 2 Sep 2024 08:39:47 +0200
Subject: [PATCH] guestchain: clear candidates changed flag only if new epoch
 to start (#386)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The Candidates struct has a changed flag indicating whether the set of
candidates has changed (in such a way that it would affect the
validators set). This is used to determine whether a new epoch should be
started or not. Once next epoch is scheduled in the block, the flag
should be cleared.

Alas, the code clears the flag each time block is generated even if a
new epoch isn’t about to start. As a consequence, if set of candidates
changes in the middle of an epoch, the change will not be reflected once
the new epoch is ready to start.

Fix this by clearing the flag only if next_epoch is populated in
produced block.
---
 common/guestchain/src/manager.rs | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/common/guestchain/src/manager.rs b/common/guestchain/src/manager.rs
index 282b78a8..eb0a7412 100644
--- a/common/guestchain/src/manager.rs
+++ b/common/guestchain/src/manager.rs
@@ -198,13 +198,13 @@ impl<PK: crate::PubKey> ChainManager<PK> {
         host_height: crate::HostHeight,
         host_timestamp: NonZeroU64,
         state_root: CryptoHash,
-    ) -> Result<bool, GenerateError> {
+    ) -> Result<(), GenerateError> {
         let next_epoch = self.validate_generate_next(
             host_height,
             host_timestamp,
             &state_root,
         )?;
-        let epoch_ends = self.header.next_epoch_commitment.is_some();
+        let has_next_epoch = next_epoch.is_some();
         let next_block = self.header.generate_next(
             host_height,
             host_timestamp,
@@ -227,11 +227,21 @@ impl<PK: crate::PubKey> ChainManager<PK> {
             signers: Set::new(),
             signing_stake: 0,
         });
-        self.candidates.clear_changed_flag();
 
-        Ok(epoch_ends)
+        if has_next_epoch {
+            self.candidates.clear_changed_flag();
+        }
+
+        Ok(())
     }
 
+    /// Verifies whether new block can be generated.
+    ///
+    /// Like [`generate_next`] returns an error if the new block cannot be
+    /// generated.  If it can, returns an `Ok` value.
+    ///
+    /// If the new block should contain a next epoch commitment, returns `Some`
+    /// new epoch.  Otherwise returns `None`.
     pub fn validate_generate_next(
         &self,
         host_height: crate::HostHeight,