fix: limit possible underflows/overflows

src/beacon/drand.rs

@@ -87,8 +87,7 @@ impl BeaconSchedule {
             let (pb_epoch, _) = self.beacon_for_epoch(parent_epoch)?;
             if cb_epoch != pb_epoch {
                 // Fork logic, take entries from the last two rounds of the new beacon.
-                let round = curr_beacon.max_beacon_round_for_epoch(network_version, epoch);
-
+                let round = curr_beacon.max_beacon_round_for_epoch(network_version, epoch)?;
                 let out = vec![
                     curr_beacon.entry(round - 1).await?,
                     curr_beacon.entry(round).await?,
@@ -97,7 +96,7 @@ impl BeaconSchedule {
             }
         }
 
-        let max_round = curr_beacon.max_beacon_round_for_epoch(network_version, epoch);
+        let max_round = curr_beacon.max_beacon_round_for_epoch(network_version, epoch)?;
         // We don't expect this to ever be the case
         if max_round == prev.round() {
             tracing::warn!(
@@ -117,18 +116,19 @@ impl BeaconSchedule {
 
         let mut out = Vec::with_capacity(2);
         if curr_beacon.network().is_unchained() {
-            for covered_epoch in (parent_epoch + 1)..=epoch {
-                let round = curr_beacon.max_beacon_round_for_epoch(network_version, covered_epoch);
+            // Newest-first, so a large gap fails on its first unavailable round:
+            // <https://github.com/filecoin-project/lotus/blob/v1.35.1/chain/beacon/beacon.go#L152>
+            for covered_epoch in (parent_epoch + 1..=epoch).rev() {
+                let round =
+                    curr_beacon.max_beacon_round_for_epoch(network_version, covered_epoch)?;
                 out.push(curr_beacon.entry(round).await?);
             }
+            out.reverse();
             Ok(out)
         } else {
-            let mut cur = max_round;
-            while cur > prev_round {
-                // Push all entries from rounds elapsed since the last chain epoch.
-                let entry = curr_beacon.entry(cur).await?;
-                cur = entry.round() - 1;
-                out.push(entry);
+            // Rounds elapsed since the last chain epoch, newest-first as above.
+            for round in (prev_round + 1..=max_round).rev() {
+                out.push(curr_beacon.entry(round).await?);
             }
             out.reverse();
             Ok(out)
@@ -187,7 +187,7 @@ pub trait Beacon {
         &self,
         network_version: NetworkVersion,
         fil_epoch: ChainEpoch,
-    ) -> u64;
+    ) -> anyhow::Result<u64>;
 }
 
 #[derive(SerdeDeserialize, SerdeSerialize, Debug, Clone, PartialEq, Eq, Default)]
@@ -370,15 +370,24 @@ impl Beacon for DrandBeacon {
                         anyhow::Ok(server.join(&format!("{}/public/{round}", self.hash))?)
                     })
                     .try_collect()?;
-                Ok((|| fetch_entry(urls.iter().cloned()))
+                let entry = (|| fetch_entry(urls.iter().cloned()))
                     .retry(ExponentialBuilder::default())
                     .notify(|err, dur| {
                         debug!(
                             "retrying fetch_entry after {}: {err:#}",
                             humantime::format_duration(dur)
                         );
                     })
-                    .await?)
+                    .await?;
+                // Callers assume the entry is for the round they asked for. Round 0 is served
+                // as "latest", so it answers with a different round by design:
+                // <https://github.com/drand/drand/blob/v2.1.6/handler/http/server.go#L367>
+                anyhow::ensure!(
+                    round == 0 || entry.round() == round,
+                    "drand returned round {} for round {round}",
+                    entry.round()
+                );
+                Ok(entry)
             }
         }
     }
@@ -387,23 +396,33 @@ impl Beacon for DrandBeacon {
         &self,
         network_version: NetworkVersion,
         fil_epoch: ChainEpoch,
-    ) -> u64 {
-        let latest_ts =
-            ((fil_epoch as u64 * self.fil_round_time) + self.fil_gen_time) - self.fil_round_time;
+    ) -> anyhow::Result<u64> {
+        // Lotus wraps and returns a garbage round instead:
+        // <https://github.com/filecoin-project/lotus/blob/v1.35.1/chain/beacon/drand/drand.go#L227>
+        let out_of_range = || anyhow::anyhow!("epoch {fil_epoch} has no drand round");
+        let latest_ts = u64::try_from(fil_epoch)
+            .ok()
+            .and_then(|epoch| epoch.checked_mul(self.fil_round_time))
+            .and_then(|ts| ts.checked_add(self.fil_gen_time))
+            .and_then(|ts| ts.checked_sub(self.fil_round_time))
+            .ok_or_else(out_of_range)?;
         if network_version <= NetworkVersion::V15 {
             // Algorithm for nv15 and below
-            (latest_ts - self.drand_gen_time) / self.interval
+            Ok(latest_ts
+                .checked_sub(self.drand_gen_time)
+                .ok_or_else(out_of_range)?
+                / self.interval)
         } else {
             // Algorithm for nv16 and above
             if latest_ts < self.drand_gen_time {
-                return 1;
+                return Ok(1);
             }
 
             let from_genesis = latest_ts - self.drand_gen_time;
             // we take the time from genesis divided by the periods in seconds, that
             // gives us the number of periods since genesis.  We also add +1 because
             // round 1 starts at genesis time.
-            from_genesis / self.interval + 1
+            Ok(from_genesis / self.interval + 1)
         }
     }
 }

src/beacon/mock_beacon.rs

@@ -50,7 +50,7 @@ impl Beacon for MockBeacon {
         &self,
         _network_version: NetworkVersion,
         fil_epoch: ChainEpoch,
-    ) -> u64 {
-        fil_epoch as u64
+    ) -> anyhow::Result<u64> {
+        Ok(u64::try_from(fil_epoch)?)
     }
 }

src/beacon/tests/drand.rs

@@ -4,13 +4,17 @@
 use itertools::Itertools;
 
 use crate::{
+    beacon::mock_beacon::MockBeacon,
     beacon::{
         Beacon, BeaconEntry, BeaconPoint, BeaconSchedule, ChainInfo, DrandBeacon, DrandConfig,
         DrandNetwork,
     },
-    shim::version::NetworkVersion,
+    shim::{clock::ChainEpoch, version::NetworkVersion},
 };
+use quickcheck_macros::quickcheck;
+use rstest::rstest;
 use std::borrow::Cow;
+use std::sync::LazyLock;
 
 fn new_beacon_mainnet() -> DrandBeacon {
     DrandBeacon::new(
@@ -76,6 +80,9 @@ pub fn new_beacon_quicknet() -> DrandBeacon {
     )
 }
 
+static MAINNET: LazyLock<DrandBeacon> = LazyLock::new(new_beacon_mainnet);
+static QUICKNET: LazyLock<DrandBeacon> = LazyLock::new(new_beacon_quicknet);
+
 #[test]
 fn construct_drand_beacon_mainnet() {
     new_beacon_mainnet();
@@ -139,14 +146,87 @@ async fn ask_and_verify_quicknet_beacon_entry_success_2() {
     assert!(beacon.verify_entries(&[e3, e2], &e1).unwrap());
 }
 
+#[quickcheck]
+fn max_beacon_round_for_epoch_no_panic(fil_epoch: ChainEpoch) {
+    for nv in [NetworkVersion::V15, NetworkVersion::V16] {
+        let _ = QUICKNET.max_beacon_round_for_epoch(nv, fil_epoch);
+    }
+}
+
+/// Expected rounds derived from FIP-0063 timings.
+#[rstest]
+#[case(0, 95844, 95845)]
+#[case(1, 95845, 95846)]
+#[case(100, 95944, 95945)]
+fn max_beacon_round_for_epoch_mainnet(
+    #[case] epoch: ChainEpoch,
+    #[case] chained: u64,
+    #[case] unchained: u64,
+) {
+    let round = |nv| MAINNET.max_beacon_round_for_epoch(nv, epoch).unwrap();
+    assert_eq!(round(NetworkVersion::V15), chained);
+    assert_eq!(round(NetworkVersion::V16), unchained);
+}
+
+#[rstest]
+// Quicknet genesis postdates these epochs, so the first round stands in.
+#[case(0, 1)]
+#[case(3149899, 1)]
+// First epoch at or after quicknet genesis, then the next: 10 drand rounds per 30s epoch.
+#[case(3149900, 2)]
+#[case(3149901, 12)]
+// Also asserted against the live network by `beacon_entries_for_block_covers_null_rounds_quicknet`.
+#[case(6216200, 30663002)]
 // https://github.com/filecoin-project/FIPs/pull/914/files#diff-fa537e813e7b41bd21980a06cf452f13e1b40e8a74f47a9f4bc4dd47c1df43b0L76
-#[test]
-fn test_max_beacon_round_for_epoch_quicknet() {
-    let beacon = new_beacon_quicknet();
-    let round = beacon.max_beacon_round_for_epoch(NetworkVersion::V21, 3547000);
+#[case(3547000, 3971002)]
+fn max_beacon_round_for_epoch_quicknet(#[case] epoch: ChainEpoch, #[case] expected: u64) {
+    let round = QUICKNET
+        .max_beacon_round_for_epoch(NetworkVersion::V22, epoch)
+        .unwrap();
+    assert_eq!(round, expected);
+}
+
+#[rstest]
+#[case(i64::MIN)]
+#[case(i64::MAX)]
+fn max_beacon_round_for_epoch_rejects_out_of_range_epochs(#[case] epoch: ChainEpoch) {
+    assert!(
+        QUICKNET
+            .max_beacon_round_for_epoch(NetworkVersion::V21, epoch)
+            .is_err()
+    );
+}
+
+/// `MockBeacon` is chained and serves entries locally, so the chained paths need no drand server.
+#[tokio::test]
+async fn beacon_entries_for_block_chained_walks_elapsed_rounds() {
+    let schedule = BeaconSchedule(vec![BeaconPoint::new(0, MockBeacon::default())]);
+    let prev = BeaconEntry::new(3, vec![]);
+
+    let entries = schedule
+        .beacon_entries_for_block(NetworkVersion::V15, 5, 3, &prev)
+        .await
+        .unwrap();
+
+    assert_eq!(entries.iter().map(BeaconEntry::round).collect_vec(), [4, 5]);
+}
+
+#[tokio::test]
+async fn beacon_entries_for_block_takes_two_entries_at_a_beacon_fork() {
+    let schedule = BeaconSchedule(vec![
+        BeaconPoint::new(0, MockBeacon::default()),
+        BeaconPoint::new(10, MockBeacon::default()),
+    ]);
+    let prev = BeaconEntry::new(9, vec![]);
+
+    let entries = schedule
+        .beacon_entries_for_block(NetworkVersion::V15, 10, 9, &prev)
+        .await
+        .unwrap();
+
     assert_eq!(
-        round,
-        ((1598306400 + 3547000 * 30) - 1692803367 - 30) / 3 + 1
+        entries.iter().map(BeaconEntry::round).collect_vec(),
+        [9, 10]
     );
 }
 

src/blocks/header.rs

@@ -138,7 +138,9 @@ impl RawBlockHeader {
             }
         }
 
-        let max_round = curr_beacon.max_beacon_round_for_epoch(network_version, self.epoch);
+        let max_round = curr_beacon
+            .max_beacon_round_for_epoch(network_version, self.epoch)
+            .map_err(|e| Error::Validation(format!("{e:#}").into()))?;
         // We don't expect to ever actually meet this condition
         if max_round == prev_entry.round() {
             if !self.beacon_entries.is_empty() {
@@ -185,8 +187,9 @@ impl RawBlockHeader {
             for (idx, beacon_entry) in self.beacon_entries.iter().enumerate() {
                 let lookup_epoch = parent_epoch + 1 + idx as i64;
 
-                let expected_round =
-                    curr_beacon.max_beacon_round_for_epoch(network_version, lookup_epoch);
+                let expected_round = curr_beacon
+                    .max_beacon_round_for_epoch(network_version, lookup_epoch)
+                    .map_err(|e| Error::Validation(format!("{e:#}").into()))?;
                 if beacon_entry.round() != expected_round {
                     return Err(Error::Validation(
                         format!(

src/chain/mod.rs

@@ -63,6 +63,16 @@ pub struct ExportResult<D: Digest> {
     pub tipset_lookup: Option<anyhow::Result<Hamt<IndexMapBlockstore, TipsetKey, ChainEpoch>>>,
 }
 
+/// Oldest epoch whose state roots an export walks.
+fn lookup_epoch_limit(
+    tipset_epoch: ChainEpoch,
+    lookup_depth: ChainEpoch,
+) -> anyhow::Result<ChainEpoch> {
+    tipset_epoch
+        .checked_sub(lookup_depth)
+        .with_context(|| format!("recent roots depth {lookup_depth} is out of range"))
+}
+
 /// Exports a Filecoin snapshot in v1 format
 /// See <https://github.com/filecoin-project/FIPs/blob/98e33b9fa306959aa0131519eb4cc155522b2081/FRCs/frc-0108.md#v1-specification>
 pub async fn export<D: Digest, S: CidHashSetLike + Send + Sync + 'static>(
@@ -170,7 +180,7 @@ async fn export_to_forest_car<D: Digest, S: CidHashSetLike + Send + Sync + 'stat
         prefix_data_frames.as_ref().map(|v| v.len()).unwrap_or(0)
     );
 
-    let stateroot_lookup_limit = tipset.epoch() - lookup_depth;
+    let stateroot_lookup_limit = lookup_epoch_limit(tipset.epoch(), lookup_depth)?;
 
     // Wrap writer in optional checksum calculator
     let mut writer = AsyncWriterWithChecksum::<D, _>::new(BufWriter::new(writer), !skip_checksum);
@@ -274,7 +284,7 @@ pub async fn export_receipts_events_to_forest_car(
         tipset.epoch(),
     );
 
-    let min_lookup_epoch_exclusive = tipset.epoch() - lookup_depth;
+    let min_lookup_epoch_exclusive = lookup_epoch_limit(tipset.epoch(), lookup_depth)?;
     let ipld_roots = tokio::task::spawn_blocking({
         let tipset = tipset.shallow_clone();
         let db = db.shallow_clone();

src/chain/store/chain_store.rs

@@ -514,7 +514,9 @@ impl ChainStore {
         } else {
             chain_config.policy.chain_finality
         };
-        let lbr = (round - lb).max(0);
+        // The subtraction, not the result, is what must be guarded, as in Lotus:
+        // <https://github.com/filecoin-project/lotus/blob/v1.35.1/chain/stmgr/utils.go#L167>
+        let lbr = if round > lb { round - lb } else { 0 };
 
         // More null blocks than our lookback
         if lbr >= heaviest_tipset.epoch() {

src/chain/store/errors.rs

@@ -37,6 +37,8 @@ pub enum Error {
     /// Lotus-compatible message, so this internal phrasing is intentionally distinct.
     #[error("null round at epoch {0}")]
     NullRound(ChainEpoch),
+    #[error("height {0} is negative")]
+    NegativeHeight(ChainEpoch),
     #[error("lookback height {lookback_height} is at or after base height {base_height}")]
     LookbackHeightOverflow {
         lookback_height: ChainEpoch,

src/chain/store/index.rs

@@ -130,7 +130,8 @@ impl ChainIndex {
     ///
     /// Returns `Ok(Some(tipset))` when epoch `to` resolves. Returns `Ok(None)` if the ancestor
     /// walk completes without resolving `to` (for example missing parent tipsets). Returns `Err`
-    /// if `to` is greater than `from.epoch()` or genesis lookup fails when `to` is zero.
+    /// if `to` is negative, greater than `from.epoch()`, or genesis lookup fails when `to` is
+    /// zero.
     ///
     /// # Why pass in the `from` argument?
     ///
@@ -177,6 +178,11 @@ impl ChainIndex {
 
         crate::def_is_env_truthy!(lookup_table_disabled, "FOREST_TIPSET_LOOKUP_TABLE_DISABLED");
 
+        // Lotus parity: <https://github.com/filecoin-project/lotus/blob/v1.35.1/chain/store/store.go#L1267>
+        if to < 0 {
+            return Err(Error::NegativeHeight(to));
+        }
+
         if to == 0 {
             return Ok(Some(self.genesis.shallow_clone()));
         }
@@ -452,6 +458,7 @@ pub mod tests {
     use crate::shim::address::Address;
     use crate::test_utils::dummy_ticket;
     use crate::utils::db::CborStoreExt;
+    use rstest::rstest;
     use std::sync::{
         Arc,
         atomic::{AtomicU64, Ordering},
@@ -514,6 +521,32 @@ pub mod tests {
         );
     }
 
+    fn genesis_index() -> (Arc<MemoryDB>, Tipset, ChainIndex) {
+        let db = Arc::new(MemoryDB::default());
+        let genesis = genesis_tipset();
+        persist_tipset(&genesis, &db);
+        let index = ChainIndex::new(db.clone(), genesis.shallow_clone());
+        (db, genesis, index)
+    }
+
+    fn persisted_child(db: &Arc<MemoryDB>, genesis: &Tipset, epoch: ChainEpoch) -> Tipset {
+        let child = tipset_child(genesis, epoch);
+        persist_tipset(&child, db);
+        child
+    }
+
+    #[rstest]
+    #[case(i64::MIN)]
+    #[case(-1)]
+    fn tipset_by_height_rejects_negative_height(#[case] height: ChainEpoch) {
+        let (db, genesis, index) = genesis_index();
+        let child = persisted_child(&db, &genesis, 1);
+        let err = index
+            .tipset_by_height_blocking(height, child, ResolveNullTipset::TakeOlder)
+            .expect_err("negative height is rejected");
+        assert!(matches!(err, Error::NegativeHeight(h) if h == height));
+    }
+
     #[test]
     fn get_different_branches() {
         let db = Arc::new(MemoryDB::default());