[Catchup] Store High QC When it is Updated (#2807)

* add more action types * integrate new storage type * fix build, tie up stragglers, modifying tasks next * merge latest stable tabs * merge latest tags and fix method signatures * working towards adding action to storage * Store high qc whenever we update it * fix lint * adding high_qc to initializer and tests * typos and if let instead of match * better comment * adding state to storage * add state to initializer * split high qc and undecided state storage * fix new clippy lints --------- Co-authored-by: Jarred Parr <[email protected]>
EspressoSystems · Mar 25, 2024 · db3fcfa · db3fcfa
1 parent ddde957
commit db3fcfa
Show file tree

Hide file tree

Showing 9 changed files with 139 additions and 14 deletions.
diff --git a/crates/example-types/src/storage_types.rs b/crates/example-types/src/storage_types.rs
@@ -2,11 +2,13 @@ use anyhow::{bail, Result};
 use async_lock::RwLock;
 use async_trait::async_trait;
 use hotshot_types::{
-    data::{DAProposal, VidDisperseShare},
+    consensus::CommitmentMap,
+    data::{DAProposal, Leaf, VidDisperseShare},
     message::Proposal,
     traits::{node_implementation::NodeType, storage::Storage},
+    utils::View,
 };
-use std::collections::HashMap;
+use std::collections::{BTreeMap, HashMap};
 use std::sync::Arc;
 
 type VidShares<TYPES> = HashMap<
@@ -84,7 +86,17 @@ impl<TYPES: NodeType> Storage<TYPES> for TestStorage<TYPES> {
 
     async fn update_high_qc(
         &self,
-        _qc: hotshot_types::simple_certificate::QuorumCertificate<TYPES>,
+        _high_qc: hotshot_types::simple_certificate::QuorumCertificate<TYPES>,
+    ) -> Result<()> {
+        if self.should_return_err {
+            bail!("Failed to update high qc to storage");
+        }
+        Ok(())
+    }
+    async fn update_undecided_state(
+        &self,
+        _leafs: CommitmentMap<Leaf<TYPES>>,
+        _state: BTreeMap<TYPES::Time, View<TYPES>>,
     ) -> Result<()> {
         if self.should_return_err {
             bail!("Failed to update high qc to storage");

diff --git a/crates/hotshot/src/lib.rs b/crates/hotshot/src/lib.rs
@@ -198,10 +198,16 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>> SystemContext<TYPES, I> {
                 },
             },
         );
+        for (view_num, inner) in initializer.undecided_state {
+            validated_state_map.insert(view_num, inner);
+        }
 
         let mut saved_leaves = HashMap::new();
         let mut saved_payloads = BTreeMap::new();
         saved_leaves.insert(anchored_leaf.commit(), anchored_leaf.clone());
+        for leaf in initializer.undecided_leafs {
+            saved_leaves.insert(leaf.commit(), leaf.clone());
+        }
         if let Some(payload) = anchored_leaf.get_block_payload() {
             let encoded_txns: Vec<u8> = match payload.encode() {
                 // TODO (Keyao) [VALIDATED_STATE] - Avoid collect/copy on the encoded transaction bytes.
@@ -229,7 +235,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>> SystemContext<TYPES, I> {
             // TODO this is incorrect
             // https://github.com/EspressoSystems/HotShot/issues/560
             locked_view: anchored_leaf.get_view_number(),
-            high_qc: anchored_leaf.get_justify_qc(),
+            high_qc: initializer.high_qc,
             metrics: consensus_metrics.clone(),
         };
         let consensus = Arc::new(RwLock::new(consensus));
@@ -637,6 +643,15 @@ pub struct HotShotInitializer<TYPES: NodeType> {
 
     /// Starting view number that we are confident won't lead to a double vote after restart.
     start_view: TYPES::Time,
+    /// Highest QC that was seen, for genesis it's the genesis QC.  It should be for a view greater
+    /// than `inner`s view number for the non genesis case because we must have seen higher QCs
+    /// to decide on the leaf.
+    high_qc: QuorumCertificate<TYPES>,
+    /// Undecided leafs that were seen, but not yet decided on.  These allow a restarting node
+    /// to vote and propose right away if they didn't miss anything while down.
+    undecided_leafs: Vec<Leaf<TYPES>>,
+    /// Not yet decided state
+    undecided_state: BTreeMap<TYPES::Time, View<TYPES>>,
 }
 
 impl<TYPES: NodeType> HotShotInitializer<TYPES> {
@@ -651,6 +666,9 @@ impl<TYPES: NodeType> HotShotInitializer<TYPES> {
             validated_state: Some(Arc::new(validated_state)),
             state_delta: Some(Arc::new(state_delta)),
             start_view: TYPES::Time::new(0),
+            high_qc: QuorumCertificate::genesis(),
+            undecided_leafs: Vec::new(),
+            undecided_state: BTreeMap::new(),
         })
     }
 
@@ -666,13 +684,19 @@ impl<TYPES: NodeType> HotShotInitializer<TYPES> {
         instance_state: TYPES::InstanceState,
         validated_state: Option<Arc<TYPES::ValidatedState>>,
         start_view: TYPES::Time,
+        high_qc: QuorumCertificate<TYPES>,
+        undecided_leafs: Vec<Leaf<TYPES>>,
+        undecided_state: BTreeMap<TYPES::Time, View<TYPES>>,
     ) -> Self {
         Self {
             inner: anchor_leaf,
             instance_state,
             validated_state,
             state_delta: None,
             start_view,
+            high_qc,
+            undecided_leafs,
+            undecided_state,
         }
     }
 }
diff --git a/crates/task-impls/src/consensus.rs b/crates/task-impls/src/consensus.rs
@@ -513,6 +513,19 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                     }
                 };
 
+                if justify_qc.get_view_number() > consensus.high_qc.view_number {
+                    if let Err(e) = self
+                        .storage
+                        .write()
+                        .await
+                        .update_high_qc(justify_qc.clone())
+                        .await
+                    {
+                        warn!("Failed to store High QC not voting. Error: {:?}", e);
+                        return;
+                    }
+                }
+
                 let mut consensus = RwLockUpgradableReadGuard::upgrade(consensus).await;
 
                 if justify_qc.get_view_number() > consensus.high_qc.view_number {
@@ -551,6 +564,19 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                     );
                     consensus.saved_leaves.insert(leaf.commit(), leaf.clone());
 
+                    if let Err(e) = self
+                        .storage
+                        .write()
+                        .await
+                        .update_undecided_state(
+                            consensus.saved_leaves.clone(),
+                            consensus.validated_state_map.clone(),
+                        )
+                        .await
+                    {
+                        warn!("Couldn't store undecided state.  Error: {:?}", e);
+                    }
+
                     // If we are missing the parent from storage, the safety check will fail.  But we can
                     // still vote if the liveness check succeeds.
                     let liveness_check = justify_qc.get_view_number() > consensus.locked_view;
@@ -782,6 +808,20 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                     },
                 );
                 consensus.saved_leaves.insert(leaf.commit(), leaf.clone());
+
+                if let Err(e) = self
+                    .storage
+                    .write()
+                    .await
+                    .update_undecided_state(
+                        consensus.saved_leaves.clone(),
+                        consensus.validated_state_map.clone(),
+                    )
+                    .await
+                {
+                    warn!("Couldn't store undecided state.  Error: {:?}", e);
+                }
+
                 if new_commit_reached {
                     consensus.locked_view = new_locked_view;
                 }
@@ -969,6 +1009,10 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
                     }
                 }
                 if let either::Left(qc) = cert {
+                    if let Err(e) = self.storage.write().await.update_high_qc(qc.clone()).await {
+                        warn!("Failed to store High QC of QC we formed. Error: {:?}", e);
+                    }
+
                     let mut consensus = self.consensus.write().await;
                     consensus.high_qc = qc.clone();
 

diff --git a/crates/task-impls/src/vote_collection.rs b/crates/task-impls/src/vote_collection.rs
@@ -67,6 +67,7 @@ impl<
 {
     /// Take one vote and accumultate it. Returns either the cert or the updated state
     /// after the vote is accumulated
+    #[allow(clippy::question_mark)]
     pub async fn accumulate_vote(
         &mut self,
         vote: &VOTE,
@@ -85,6 +86,7 @@ impl<
             );
             return None;
         }
+
         let accumulator = self.accumulator.as_mut()?;
         match accumulator.accumulate(vote, &self.membership) {
             Either::Left(()) => None,

diff --git a/crates/testing/src/spinning_task.rs b/crates/testing/src/spinning_task.rs
@@ -3,10 +3,12 @@ use std::collections::HashMap;
 use crate::test_runner::HotShotTaskCompleted;
 use crate::test_runner::{LateStartNode, Node, TestRunner};
 use either::{Left, Right};
+use hotshot::types::EventType;
 use hotshot::{traits::TestableNodeImplementation, HotShotInitializer};
 use hotshot_example_types::state_types::TestInstanceState;
 use hotshot_example_types::storage_types::TestStorage;
 use hotshot_task::task::{Task, TaskState, TestTaskState};
+use hotshot_types::simple_certificate::QuorumCertificate;
 use hotshot_types::{data::Leaf, ValidatorConfig};
 use hotshot_types::{
     event::Event,
@@ -39,6 +41,8 @@ pub struct SpinningTask<TYPES: NodeType, I: TestableNodeImplementation<TYPES>> {
     pub(crate) latest_view: Option<TYPES::Time>,
     /// Last decided leaf that can be used as the anchor leaf to initialize the node.
     pub(crate) last_decided_leaf: Leaf<TYPES>,
+    /// Highest qc seen in the test for restarting nodes
+    pub(crate) high_qc: QuorumCertificate<TYPES>,
 }
 
 impl<TYPES: NodeType, I: TestableNodeImplementation<TYPES>> TaskState for SpinningTask<TYPES, I> {
@@ -83,13 +87,24 @@ where
         _id: usize,
         task: &mut hotshot_task::task::TestTask<Self::State, Self>,
     ) -> Option<Self::Output> {
-        let Event {
-            view_number,
-            event: _,
-        } = message;
+        let Event { view_number, event } = message;
 
         let state = &mut task.state_mut();
 
+        if let EventType::Decide {
+            leaf_chain,
+            qc: _,
+            block_size: _,
+        } = event
+        {
+            state.last_decided_leaf = leaf_chain.first().unwrap().leaf.clone();
+        } else if let EventType::QuorumProposal {
+            proposal,
+            sender: _,
+        } = event
+        {
+            state.high_qc = proposal.data.justify_qc;
+        }
         // if we have not seen this view before
         if state.latest_view.is_none() || view_number > state.latest_view.unwrap() {
             // perform operations on the nodes
@@ -111,6 +126,9 @@ where
                                             TestInstanceState {},
                                             None,
                                             view_number,
+                                            state.high_qc.clone(),
+                                            Vec::new(),
+                                            BTreeMap::new(),
                                         );
                                         // We assign node's public key and stake value rather than read from config file since it's a test
                                         let validator_config =

diff --git a/crates/testing/src/test_runner.rs b/crates/testing/src/test_runner.rs
@@ -20,7 +20,6 @@ use hotshot_example_types::{state_types::TestInstanceState, storage_types::TestS
 use hotshot::{traits::TestableNodeImplementation, HotShotInitializer, SystemContext};
 
 use hotshot_task::task::{Task, TaskRegistry, TestTask};
-use hotshot_types::constants::EVENT_CHANNEL_SIZE;
 use hotshot_types::{
     consensus::ConsensusMetricsValue,
     data::Leaf,
@@ -30,6 +29,7 @@ use hotshot_types::{
     },
     HotShotConfig, ValidatorConfig,
 };
+use hotshot_types::{constants::EVENT_CHANNEL_SIZE, simple_certificate::QuorumCertificate};
 use hotshot_types::{
     message::Message,
     traits::{network::ConnectedNetwork, node_implementation::NodeImplementation},
@@ -219,6 +219,7 @@ where
             latest_view: None,
             changes,
             last_decided_leaf: Leaf::genesis(&TestInstanceState {}),
+            high_qc: QuorumCertificate::genesis(),
         };
         let spinning_task = TestTask::<SpinningTask<TYPES, I>, SpinningTask<TYPES, I>>::new(
             Task::new(tx.clone(), rx.clone(), reg.clone(), spinning_task_state),

diff --git a/crates/types/src/consensus.rs b/crates/types/src/consensus.rs
@@ -24,7 +24,7 @@ use std::{
 use tracing::error;
 
 /// A type alias for `HashMap<Commitment<T>, T>`
-type CommitmentMap<T> = HashMap<Commitment<T>, T>;
+pub type CommitmentMap<T> = HashMap<Commitment<T>, T>;
 
 /// A type alias for `BTreeMap<T::Time, HashMap<T::SignatureKey, Proposal<T, VidDisperseShare<T>>>>`
 type VidShares<TYPES> = BTreeMap<

diff --git a/crates/types/src/traits/storage.rs b/crates/types/src/traits/storage.rs
@@ -3,11 +3,14 @@
 //! This modules provides the [`Storage`] trait.
 //!
 
+use std::collections::BTreeMap;
+
 use anyhow::Result;
 use async_trait::async_trait;
 
 use crate::{
-    data::{DAProposal, VidDisperseShare},
+    consensus::{CommitmentMap, View},
+    data::{DAProposal, Leaf, VidDisperseShare},
     event::HotShotAction,
     message::Proposal,
     simple_certificate::QuorumCertificate,
@@ -21,5 +24,12 @@ pub trait Storage<TYPES: NodeType>: Send + Sync + Clone {
     async fn append_vid(&self, proposal: &Proposal<TYPES, VidDisperseShare<TYPES>>) -> Result<()>;
     async fn append_da(&self, proposal: &Proposal<TYPES, DAProposal<TYPES>>) -> Result<()>;
     async fn record_action(&self, view: TYPES::Time, action: HotShotAction) -> Result<()>;
-    async fn update_high_qc(&self, qc: QuorumCertificate<TYPES>) -> Result<()>;
+    async fn update_high_qc(&self, high_qc: QuorumCertificate<TYPES>) -> Result<()>;
+    /// Update the currently undecided state of consensus.  This includes the undecided leaf chain,
+    /// and the undecided state.
+    async fn update_undecided_state(
+        &self,
+        leafs: CommitmentMap<Leaf<TYPES>>,
+        state: BTreeMap<TYPES::Time, View<TYPES>>,
+    ) -> Result<()>;
 }
diff --git a/crates/types/src/utils.rs b/crates/types/src/utils.rs
@@ -44,7 +44,21 @@ pub enum ViewInner<TYPES: NodeType> {
     /// Leaf has failed
     Failed,
 }
-
+impl<TYPES: NodeType> Clone for ViewInner<TYPES> {
+    fn clone(&self) -> Self {
+        match self {
+            Self::DA { payload_commitment } => Self::DA {
+                payload_commitment: *payload_commitment,
+            },
+            Self::Leaf { leaf, state, delta } => Self::Leaf {
+                leaf: *leaf,
+                state: state.clone(),
+                delta: delta.clone(),
+            },
+            Self::Failed => Self::Failed,
+        }
+    }
+}
 /// The hash of a leaf.
 type LeafCommitment<TYPES> = Commitment<Leaf<TYPES>>;
 
@@ -117,7 +131,7 @@ impl<TYPES: NodeType> Deref for View<TYPES> {
 }
 
 /// This exists so we can perform state transitions mutably
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct View<TYPES: NodeType> {
     /// The view data. Wrapped in a struct so we can mutate
     pub view_inner: ViewInner<TYPES>,