Merge pull request #91 from datachainlab/S2

S2

Author: yoshidan

light-client/src/errors.rs

@@ -141,6 +141,7 @@ pub enum Error {
     UnexpectedMissingForkSpecInCurrentEpochCalculation(BlockNumber, alloc::boxed::Box<Error>),
     UnexpectedMissingForkSpecInPreviousEpochCalculation(BlockNumber, alloc::boxed::Box<Error>),
     UnexpectedPreviousEpochInCalculatingNextEpoch(BlockNumber, BlockNumber, BlockNumber),
+    UnexpectedEpochLength(u64, u64),
     MustBeEpoch(BlockNumber, ForkSpec),
     MustNotBeEpoch(BlockNumber, ForkSpec),
 
@@ -508,6 +509,9 @@ impl core::fmt::Display for Error {
             Error::UnexpectedEpochInfo(e1, e2) => {
                 write!(f, "UnexpectedEpochInfo : {} {}", e1, e2)
             }
+            Error::UnexpectedEpochLength(e1, e2) => {
+                write!(f, "UnexpectedEpochLength : {} {}", e1, e2)
+            }
             Error::MustBeEpoch(e1, e2) => {
                 write!(f, "MustBeEpoch : {} {:?}", e1, e2)
             }

light-client/src/fork_spec.rs

@@ -3,6 +3,8 @@ use crate::misc::BlockNumber;
 use parlia_ibc_proto::ibc::lightclients::parlia::v1::fork_spec::HeightOrTimestamp as RawHeightOrTimestamp;
 use parlia_ibc_proto::ibc::lightclients::parlia::v1::ForkSpec as RawForkSpec;
 
+const DEFAULT_EPOCH_LENGTH: u64 = 200;
+
 #[derive(Clone, Debug, PartialEq, serde::Serialize, serde::Deserialize)]
 pub enum HeightOrTimestamp {
     Height(u64),
@@ -67,34 +69,44 @@ impl ForkSpec {
     /// current_first = 2000
     pub fn boundary_epochs(&self, prev_fork_spec: &ForkSpec) -> Result<BoundaryEpochs, Error> {
         if let HeightOrTimestamp::Height(height) = self.height_or_timestamp {
-            let prev_last = height - (height % prev_fork_spec.epoch_length);
-            let current_first = (height..).find(|&h| h % self.epoch_length == 0).unwrap();
+            if self.epoch_length == 0 || prev_fork_spec.epoch_length == 0 {
+                return Err(Error::UnexpectedEpochLength(
+                    self.epoch_length,
+                    prev_fork_spec.epoch_length,
+                ));
+            }
+            let previous_last = height - (height % prev_fork_spec.epoch_length);
+
+            let current_first = if height % self.epoch_length == 0 {
+                height
+            } else {
+                height + (self.epoch_length - height % self.epoch_length)
+            };
             let mut intermediates = vec![];
 
             // starts 0, 200, 400...epoch_length
-            if prev_last == 0 {
-                const DEFAULT_EPOCH_LENGTH: u64 = 200;
+            if previous_last == 0 {
                 let additive: u64 = if prev_fork_spec.epoch_length > DEFAULT_EPOCH_LENGTH {
                     DEFAULT_EPOCH_LENGTH
                 } else {
                     prev_fork_spec.epoch_length
                 };
-                let mut mid = prev_last + additive;
+                let mut mid = previous_last + additive;
                 while mid < prev_fork_spec.epoch_length {
                     intermediates.push(mid);
                     mid += additive;
                 }
             }
-            let mut mid = prev_last + prev_fork_spec.epoch_length;
+            let mut mid = previous_last + prev_fork_spec.epoch_length;
             while mid < current_first {
                 intermediates.push(mid);
                 mid += prev_fork_spec.epoch_length;
             }
             return Ok(BoundaryEpochs {
                 previous_fork_spec: prev_fork_spec.clone(),
                 current_fork_spec: self.clone(),
+                previous_last,
                 current_first,
-                prev_last,
                 intermediates,
             });
         }
@@ -106,7 +118,7 @@ impl ForkSpec {
 pub struct BoundaryEpochs {
     previous_fork_spec: ForkSpec,
     current_fork_spec: ForkSpec,
-    prev_last: BlockNumber,
+    previous_last: BlockNumber,
     current_first: BlockNumber,
     intermediates: alloc::vec::Vec<BlockNumber>,
 }
@@ -147,14 +159,14 @@ impl BoundaryEpochs {
             return 0;
         }
         // Before HF
-        if current_epoch_block_number <= self.prev_last {
+        if current_epoch_block_number <= self.previous_last {
             return current_epoch_block_number - self.previous_fork_spec.epoch_length;
         }
 
         for (i, mid) in self.intermediates.iter().enumerate() {
             if current_epoch_block_number == *mid {
                 if i == 0 {
-                    return self.prev_last;
+                    return self.previous_last;
                 } else {
                     return self.intermediates[i - 1];
                 }
@@ -163,7 +175,7 @@ impl BoundaryEpochs {
 
         if current_epoch_block_number == self.current_first {
             if self.intermediates.is_empty() {
-                return self.prev_last;
+                return self.previous_last;
             }
             return *self.intermediates.last().unwrap();
         }
@@ -600,36 +612,76 @@ mod test {
         }
     }
 
+    #[test]
+    fn test_error_boundary_epochs_current_epoch_length_zero() {
+        let current = ForkSpec {
+            height_or_timestamp: HeightOrTimestamp::Height(0),
+            additional_header_item_count: 1,
+            epoch_length: 0,
+            max_turn_length: 64,
+        };
+        match current
+            .boundary_epochs(&fork_spec_after_pascal())
+            .unwrap_err()
+        {
+            Error::UnexpectedEpochLength(e1, e2) => {
+                assert_eq!(current.epoch_length, e1);
+                assert_ne!(current.epoch_length, e2);
+            }
+            _ => unreachable!("unexpected error"),
+        }
+    }
+
+    #[test]
+    fn test_error_boundary_epochs_previous_epoch_length_zero() {
+        let previous = ForkSpec {
+            height_or_timestamp: HeightOrTimestamp::Height(0),
+            additional_header_item_count: 1,
+            epoch_length: 0,
+            max_turn_length: 64,
+        };
+        match fork_spec_after_pascal()
+            .boundary_epochs(&previous)
+            .unwrap_err()
+        {
+            Error::UnexpectedEpochLength(e1, e2) => {
+                assert_ne!(previous.epoch_length, e1);
+                assert_eq!(previous.epoch_length, e2);
+            }
+            _ => unreachable!("unexpected error"),
+        }
+    }
+
     #[test]
     fn test_success_boundary_epochs() {
         let mut f1 = fork_spec_after_lorentz().clone();
         f1.height_or_timestamp = HeightOrTimestamp::Height(1501);
         let be = f1.boundary_epochs(&fork_spec_after_pascal()).unwrap();
-        assert_eq!(be.prev_last, 1400);
+        assert_eq!(be.previous_last, 1400);
         assert_eq!(be.intermediates, vec![1600, 1800]);
         assert_eq!(be.current_first, 2000);
 
         f1.height_or_timestamp = HeightOrTimestamp::Height(1600);
         let be = f1.boundary_epochs(&fork_spec_after_pascal()).unwrap();
-        assert_eq!(be.prev_last, 1600);
+        assert_eq!(be.previous_last, 1600);
         assert_eq!(be.intermediates, vec![1800]);
         assert_eq!(be.current_first, 2000);
 
         f1.height_or_timestamp = HeightOrTimestamp::Height(1601);
         let be = f1.boundary_epochs(&fork_spec_after_pascal()).unwrap();
-        assert_eq!(be.prev_last, 1600);
+        assert_eq!(be.previous_last, 1600);
         assert_eq!(be.intermediates, vec![1800]);
         assert_eq!(be.current_first, 2000);
 
         f1.height_or_timestamp = HeightOrTimestamp::Height(1800);
         let be = f1.boundary_epochs(&fork_spec_after_pascal()).unwrap();
-        assert_eq!(be.prev_last, 1800);
+        assert_eq!(be.previous_last, 1800);
         assert_eq!(be.intermediates, vec![]);
         assert_eq!(be.current_first, 2000);
 
         f1.height_or_timestamp = HeightOrTimestamp::Height(2000);
         let be = f1.boundary_epochs(&fork_spec_after_pascal()).unwrap();
-        assert_eq!(be.prev_last, 2000);
+        assert_eq!(be.previous_last, 2000);
         assert_eq!(be.intermediates, vec![]);
         assert_eq!(be.current_first, 2000);
     }
@@ -639,7 +691,7 @@ mod test {
         let be = fork_spec_after_lorentz()
             .boundary_epochs(&fork_spec_after_pascal())
             .unwrap();
-        assert_eq!(be.prev_last, 0);
+        assert_eq!(be.previous_last, 0);
         assert_eq!(be.intermediates[0], 200);
         assert_eq!(be.intermediates[1], 400);
         assert_eq!(be.current_first, 500);
@@ -670,7 +722,7 @@ mod test {
         let be = fork_spec_after_maxwell()
             .boundary_epochs(&fork_spec_after_lorentz())
             .unwrap();
-        assert_eq!(be.prev_last, 0);
+        assert_eq!(be.previous_last, 0);
         assert_eq!(be.intermediates[0], 200);
         assert_eq!(be.intermediates[1], 400);
         assert_eq!(be.intermediates[2], 500);