diff --git a/crates/example-types/src/storage_types.rs b/crates/example-types/src/storage_types.rs index 51b894ca56..3d9c59c422 100644 --- 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 = HashMap< @@ -84,7 +86,17 @@ impl Storage for TestStorage { async fn update_high_qc( &self, - _qc: hotshot_types::simple_certificate::QuorumCertificate, + _high_qc: hotshot_types::simple_certificate::QuorumCertificate, + ) -> Result<()> { + if self.should_return_err { + bail!("Failed to update high qc to storage"); + } + Ok(()) + } + async fn update_undecided_state( + &self, + _leafs: CommitmentMap>, + _state: BTreeMap>, ) -> 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 index 6286294268..78bea4ce58 100644 --- a/crates/hotshot/src/lib.rs +++ b/crates/hotshot/src/lib.rs @@ -198,10 +198,16 @@ impl> SystemContext { }, }, ); + 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 = match payload.encode() { // TODO (Keyao) [VALIDATED_STATE] - Avoid collect/copy on the encoded transaction bytes. @@ -229,7 +235,7 @@ impl> SystemContext { // 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 { /// 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, + /// 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>, + /// Not yet decided state + undecided_state: BTreeMap>, } impl HotShotInitializer { @@ -651,6 +666,9 @@ impl HotShotInitializer { 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,6 +684,9 @@ impl HotShotInitializer { instance_state: TYPES::InstanceState, validated_state: Option>, start_view: TYPES::Time, + high_qc: QuorumCertificate, + undecided_leafs: Vec>, + undecided_state: BTreeMap>, ) -> Self { Self { inner: anchor_leaf, @@ -673,6 +694,9 @@ impl HotShotInitializer { 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 index 6362aa148d..df5102fe35 100644 --- a/crates/task-impls/src/consensus.rs +++ b/crates/task-impls/src/consensus.rs @@ -513,6 +513,19 @@ impl, A: ConsensusApi + } }; + 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, A: ConsensusApi + ); 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, A: ConsensusApi + }, ); 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, A: ConsensusApi + } } 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 index 1bc98b63a4..69501c1e2a 100644 --- 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 index 3268c145f4..af80bcb65d 100644 --- 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> { pub(crate) latest_view: Option, /// Last decided leaf that can be used as the anchor leaf to initialize the node. pub(crate) last_decided_leaf: Leaf, + /// Highest qc seen in the test for restarting nodes + pub(crate) high_qc: QuorumCertificate, } impl> TaskState for SpinningTask { @@ -83,13 +87,24 @@ where _id: usize, task: &mut hotshot_task::task::TestTask, ) -> Option { - 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 index 9153dabf0e..620b297804 100644 --- 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>::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 index 666461c708..52a92d3c45 100644 --- 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, T>` -type CommitmentMap = HashMap, T>; +pub type CommitmentMap = HashMap, T>; /// A type alias for `BTreeMap>>>` type VidShares = BTreeMap< diff --git a/crates/types/src/traits/storage.rs b/crates/types/src/traits/storage.rs index eae0514852..bb62857b66 100644 --- 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: Send + Sync + Clone { async fn append_vid(&self, proposal: &Proposal>) -> Result<()>; async fn append_da(&self, proposal: &Proposal>) -> Result<()>; async fn record_action(&self, view: TYPES::Time, action: HotShotAction) -> Result<()>; - async fn update_high_qc(&self, qc: QuorumCertificate) -> Result<()>; + async fn update_high_qc(&self, high_qc: QuorumCertificate) -> 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>, + state: BTreeMap>, + ) -> Result<()>; } diff --git a/crates/types/src/utils.rs b/crates/types/src/utils.rs index 9cce5bb31b..663b676e75 100644 --- a/crates/types/src/utils.rs +++ b/crates/types/src/utils.rs @@ -44,7 +44,21 @@ pub enum ViewInner { /// Leaf has failed Failed, } - +impl Clone for ViewInner { + 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 = Commitment>; @@ -117,7 +131,7 @@ impl Deref for View { } /// This exists so we can perform state transitions mutably -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct View { /// The view data. Wrapped in a struct so we can mutate pub view_inner: ViewInner,