Lr/epochs more tests (#4005)

* Add more epoch tests

* Do not fail if the new locked view is the same as the stored locked view

* Add catchup test for epochs and retrieve stored VIDs after restart

* Make sure that the decided view is greater than the old one when GCing

* Fix test_with_failures_half_f

* Revert "Do not fail if the new locked view is the same as the stored locked view"

This reverts commit f4f37ef.

* Revert "Make sure that the decided view is greater than the old one when GCing"

This reverts commit 91af3ab.

* Make sure the new view is greater than the old one before GCing

* Return None instead of exiting with Error if no VID

* Move epoch tests to a separate file

crates/example-types/src/state_types.rs

@@ -23,7 +23,7 @@ use rand::Rng;
 use serde::{Deserialize, Serialize};
 use vbs::version::Version;
 
-pub use crate::node_types::TestTypes;
+pub use crate::node_types::{TestTwoStakeTablesTypes, TestTypes};
 use crate::{
     block_types::{TestBlockPayload, TestTransaction},
     testable_delay::{DelayConfig, SupportedTraitTypesForAsyncDelay, TestableDelay},

crates/example-types/src/storage_types.rs

@@ -33,11 +33,11 @@ use jf_vid::VidScheme;
 
 use crate::testable_delay::{DelayConfig, SupportedTraitTypesForAsyncDelay, TestableDelay};
 
-type VidShares<TYPES> = HashMap<
+type VidShares<TYPES> = BTreeMap<
     <TYPES as NodeType>::View,
     HashMap<<TYPES as NodeType>::SignatureKey, Proposal<TYPES, VidDisperseShare<TYPES>>>,
 >;
-type VidShares2<TYPES> = HashMap<
+type VidShares2<TYPES> = BTreeMap<
     <TYPES as NodeType>::View,
     HashMap<<TYPES as NodeType>::SignatureKey, Proposal<TYPES, VidDisperseShare2<TYPES>>>,
 >;
@@ -61,8 +61,8 @@ pub struct TestStorageState<TYPES: NodeType> {
 impl<TYPES: NodeType> Default for TestStorageState<TYPES> {
     fn default() -> Self {
         Self {
-            vids: HashMap::new(),
-            vid2: HashMap::new(),
+            vids: BTreeMap::new(),
+            vid2: BTreeMap::new(),
             das: HashMap::new(),
             da2s: HashMap::new(),
             proposals: BTreeMap::new(),
@@ -127,6 +127,9 @@ impl<TYPES: NodeType> TestStorage<TYPES> {
     pub async fn last_actioned_epoch(&self) -> TYPES::Epoch {
         self.inner.read().await.epoch
     }
+    pub async fn vids_cloned(&self) -> VidShares2<TYPES> {
+        self.inner.read().await.vid2.clone()
+    }
 }
 
 #[async_trait]

crates/hotshot/src/lib.rs

@@ -47,7 +47,7 @@ use hotshot_task_impls::{events::HotShotEvent, helpers::broadcast_event};
 /// Reexport error type
 pub use hotshot_types::error::HotShotError;
 use hotshot_types::{
-    consensus::{Consensus, ConsensusMetricsValue, OuterConsensus, View, ViewInner},
+    consensus::{Consensus, ConsensusMetricsValue, OuterConsensus, VidShares, View, ViewInner},
     constants::{EVENT_CHANNEL_SIZE, EXTERNAL_EVENT_CHANNEL_SIZE},
     data::{Leaf2, QuorumProposal, QuorumProposal2},
     event::{EventType, LeafInfo},
@@ -69,7 +69,6 @@ use hotshot_types::{
 pub use rand;
 use tokio::{spawn, time::sleep};
 use tracing::{debug, instrument, trace};
-
 // -- Rexports
 // External
 use crate::{
@@ -332,6 +331,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> SystemContext<T
         };
         let consensus = Consensus::new(
             validated_state_map,
+            initializer.saved_vid_shares,
             anchored_leaf.view_number(),
             anchored_epoch,
             anchored_leaf.view_number(),
@@ -1022,6 +1022,8 @@ pub struct HotShotInitializer<TYPES: NodeType> {
     undecided_state: BTreeMap<TYPES::View, View<TYPES>>,
     /// Proposals we have sent out to provide to others for catchup
     saved_proposals: BTreeMap<TYPES::View, Proposal<TYPES, QuorumProposal2<TYPES>>>,
+    /// Saved VID shares
+    saved_vid_shares: Option<VidShares<TYPES>>,
 }
 
 impl<TYPES: NodeType> HotShotInitializer<TYPES> {
@@ -1048,6 +1050,7 @@ impl<TYPES: NodeType> HotShotInitializer<TYPES> {
             undecided_leaves: Vec::new(),
             undecided_state: BTreeMap::new(),
             instance_state,
+            saved_vid_shares: None,
         })
     }
 
@@ -1072,6 +1075,7 @@ impl<TYPES: NodeType> HotShotInitializer<TYPES> {
         decided_upgrade_certificate: Option<UpgradeCertificate<TYPES>>,
         undecided_leaves: Vec<Leaf2<TYPES>>,
         undecided_state: BTreeMap<TYPES::View, View<TYPES>>,
+        saved_vid_shares: Option<VidShares<TYPES>>,
     ) -> Self {
         Self {
             inner: anchor_leaf,
@@ -1087,6 +1091,7 @@ impl<TYPES: NodeType> HotShotInitializer<TYPES> {
             decided_upgrade_certificate,
             undecided_leaves,
             undecided_state,
+            saved_vid_shares,
         }
     }
 }

crates/testing/src/spinning_task.rs

@@ -166,6 +166,7 @@ where
                                             None,
                                             Vec::new(),
                                             BTreeMap::new(),
+                                            None,
                                         );
                                         // We assign node's public key and stake value rather than read from config file since it's a test
                                         let validator_config =
@@ -256,6 +257,7 @@ where
                                     read_storage.decided_upgrade_certificate().await,
                                     Vec::new(),
                                     BTreeMap::new(),
+                                    Some(read_storage.vids_cloned().await),
                                 );
                                 // We assign node's public key and stake value rather than read from config file since it's a test
                                 let validator_config = ValidatorConfig::generated_from_seed_indexed(

crates/testing/tests/tests_1/test_with_failures_2.rs

@@ -74,50 +74,6 @@ cross_tests!(
     }
 );
 
-cross_tests!(
-    TestName: test_with_failures_2_with_epochs,
-    Impls: [Libp2pImpl, PushCdnImpl, CombinedImpl],
-    Types: [TestTwoStakeTablesTypes],
-    Versions: [EpochsTestVersions],
-    Ignore: false,
-    Metadata: {
-        let mut metadata = TestDescription::default_more_nodes();
-        metadata.num_nodes_with_stake = 12;
-        metadata.da_staked_committee_size = 12;
-        metadata.start_nodes = 12;
-        metadata.epoch_height = 10;
-        let dead_nodes = vec![
-            ChangeNode {
-                idx: 10,
-                updown: NodeAction::Down,
-            },
-            ChangeNode {
-                idx: 11,
-                updown: NodeAction::Down,
-            },
-        ];
-
-        metadata.spinning_properties = SpinningTaskDescription {
-            node_changes: vec![(5, dead_nodes)]
-        };
-
-        // 2 nodes fail triggering view sync, expect no other timeouts
-        metadata.overall_safety_properties.num_failed_views = 6;
-        // Make sure we keep committing rounds after the bad leaders, but not the full 50 because of the numerous timeouts
-        metadata.overall_safety_properties.num_successful_views = 20;
-        metadata.overall_safety_properties.expected_views_to_fail = HashMap::from([
-            (ViewNumber::new(5), false),
-            (ViewNumber::new(11), false),
-            (ViewNumber::new(17), false),
-            (ViewNumber::new(23), false),
-            (ViewNumber::new(29), false),
-            (ViewNumber::new(35), false),
-        ]);
-
-        metadata
-    }
-);
-
 cross_tests!(
     TestName: test_with_double_leader_failures,
     Impls: [MemoryImpl, Libp2pImpl, PushCdnImpl],

crates/testing/tests/tests_3/test_with_failures_half_f.rs

@@ -5,7 +5,7 @@
 // along with the HotShot repository. If not, see <https://mit-license.org/>.
 
 use hotshot_example_types::{
-    node_types::{EpochsTestVersions, Libp2pImpl, MemoryImpl, PushCdnImpl, TestVersions},
+    node_types::{Libp2pImpl, MemoryImpl, PushCdnImpl, TestVersions},
     state_types::TestTypes,
 };
 use hotshot_macros::cross_tests;
@@ -53,42 +53,3 @@ cross_tests!(
         metadata
     }
 );
-cross_tests!(
-    TestName: test_with_failures_half_f_epochs,
-    Impls: [MemoryImpl, Libp2pImpl, PushCdnImpl],
-    Types: [TestTypes],
-    Versions: [EpochsTestVersions],
-    Ignore: false,
-    Metadata: {
-        let mut metadata = TestDescription::default_more_nodes();
-        metadata.epoch_height = 0;
-        metadata.num_bootstrap_nodes = 17;
-        metadata.epoch_height = 10;
-        // The first 14 (i.e., 20 - f) nodes are in the DA committee and we may shutdown the
-        // remaining 6 (i.e., f) nodes. We could remove this restriction after fixing the
-        // following issue.
-        let dead_nodes = vec![
-            ChangeNode {
-                idx: 17,
-                updown: NodeAction::Down,
-            },
-            ChangeNode {
-                idx: 18,
-                updown: NodeAction::Down,
-            },
-            ChangeNode {
-                idx: 19,
-                updown: NodeAction::Down,
-            },
-        ];
-
-        metadata.spinning_properties = SpinningTaskDescription {
-            node_changes: vec![(5, dead_nodes)]
-        };
-
-        metadata.overall_safety_properties.num_failed_views = 3;
-        // Make sure we keep committing rounds after the bad leaders, but not the full 50 because of the numerous timeouts
-        metadata.overall_safety_properties.num_successful_views = 22;
-        metadata
-    }
-);