diff --git a/src/beacon/drand.rs b/src/beacon/drand.rs index 61ebddbca410..75758c50ca9d 100644 --- a/src/beacon/drand.rs +++ b/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: + // + 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; } #[derive(SerdeDeserialize, SerdeSerialize, Debug, Clone, PartialEq, Eq, Default)] @@ -370,7 +370,7 @@ 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!( @@ -378,7 +378,16 @@ impl Beacon for DrandBeacon { 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: + // + 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 { + // Lotus wraps and returns a garbage round instead: + // + 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) } } } diff --git a/src/beacon/mock_beacon.rs b/src/beacon/mock_beacon.rs index 6b271294e548..ac38705ed50e 100644 --- a/src/beacon/mock_beacon.rs +++ b/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 { + Ok(u64::try_from(fil_epoch)?) } } diff --git a/src/beacon/tests/drand.rs b/src/beacon/tests/drand.rs index 98967a2eba67..c92c6715518c 100644 --- a/src/beacon/tests/drand.rs +++ b/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 = LazyLock::new(new_beacon_mainnet); +static QUICKNET: LazyLock = 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] ); } diff --git a/src/blocks/header.rs b/src/blocks/header.rs index d967cfee7d35..489f45d88431 100644 --- a/src/blocks/header.rs +++ b/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!( diff --git a/src/chain/mod.rs b/src/chain/mod.rs index cf30efeb91cf..3d394a3dff0f 100644 --- a/src/chain/mod.rs +++ b/src/chain/mod.rs @@ -63,6 +63,16 @@ pub struct ExportResult { pub tipset_lookup: Option>>, } +/// Oldest epoch whose state roots an export walks. +fn lookup_epoch_limit( + tipset_epoch: ChainEpoch, + lookup_depth: ChainEpoch, +) -> anyhow::Result { + 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 pub async fn export( @@ -170,7 +180,7 @@ async fn export_to_forest_car::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(); diff --git a/src/chain/store/chain_store.rs b/src/chain/store/chain_store.rs index b68ac96d37aa..98294e1234bc 100644 --- a/src/chain/store/chain_store.rs +++ b/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: + // + let lbr = if round > lb { round - lb } else { 0 }; // More null blocks than our lookback if lbr >= heaviest_tipset.epoch() { diff --git a/src/chain/store/errors.rs b/src/chain/store/errors.rs index 2b2430e21921..142effa9267b 100644 --- a/src/chain/store/errors.rs +++ b/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, diff --git a/src/chain/store/index.rs b/src/chain/store/index.rs index 5b4a84689ecd..dc9262910712 100644 --- a/src/chain/store/index.rs +++ b/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: + 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, 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, 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()); diff --git a/src/chain/tests.rs b/src/chain/tests.rs index 59dac1e85fdc..ea2b50e8921b 100644 --- a/src/chain/tests.rs +++ b/src/chain/tests.rs @@ -158,6 +158,26 @@ async fn test_export_inner( Ok(()) } +#[rstest] +#[case(1_000, 0, Some(1_000))] +#[case(1_000, 900, Some(100))] +#[case(1_000, 2_000, Some(-1_000))] +// A negative depth widens the range instead of underflowing; `ChainExport` rejects it at ingress. +#[case(1_000, -1, Some(1_001))] +#[case(0, i64::MIN, None)] +#[case(-1, i64::MAX, Some(i64::MIN))] +#[case(-2, i64::MAX, None)] +fn lookup_epoch_limit_errors_only_when_unrepresentable( + #[case] tipset_epoch: ChainEpoch, + #[case] lookup_depth: ChainEpoch, + #[case] expected: Option, +) { + assert_eq!( + lookup_epoch_limit(tipset_epoch, lookup_depth).ok(), + expected + ); +} + /// Regression tests for the "snapshot export stuck at `Exporting: 100.0%`" incidents: /// once the DAG walk reaches genesis (`epoch == 0`, progress pins at 100%), the remaining /// pipeline steps must not be able to wait forever on a stalled writer. diff --git a/src/chain_sync/validation.rs b/src/chain_sync/validation.rs index 43c352f987a4..6470003c05ea 100644 --- a/src/chain_sync/validation.rs +++ b/src/chain_sync/validation.rs @@ -304,8 +304,11 @@ impl<'a> GossipBlockValidator<'a> { let epoch = self.block.header.epoch; let timestamp = self.block.header.timestamp; // epoch is validated non-negative by validate_epoch_range before this - let expected = - genesis_tipset.min_timestamp() + (epoch as u64).saturating_mul(u64::from(block_delay)); + // Saturating would let a block claiming `u64::MAX` match the saturated expectation. + let expected = (epoch as u64) + .checked_mul(u64::from(block_delay)) + .and_then(|elapsed| genesis_tipset.min_timestamp().checked_add(elapsed)) + .ok_or(GossipBlockRejectReason::EpochTooFarAhead(epoch))?; if timestamp != expected { return Err(GossipBlockRejectReason::TimestampMismatch { timestamp, @@ -600,6 +603,33 @@ mod tests { )); } + /// A genesis timestamp ahead of the local clock makes `max_allowed_epoch` fall back to + /// `ChainEpoch::MAX`, so the epoch range check no longer bounds what reaches the timestamp + /// arithmetic. + #[test] + fn timestamp_check_survives_extreme_epoch_when_clock_is_behind_genesis() { + let genesis = Tipset::from(CachingBlockHeader::new(RawBlockHeader { + timestamp: u64::MAX, + ..Default::default() + })); + + // The second block claims the timestamp that saturating arithmetic would have computed + // as the expected one, so saturating would have accepted it. + for timestamp in [0, u64::MAX] { + let block = make_gossip_block_with(|h| { + h.epoch = i64::MAX; + h.timestamp = timestamp; + }); + let err = GossipBlockValidator::new(&block) + .validate_pre_fetch(&genesis, 30, 0, None, &SeenBlockCache::default()) + .unwrap_err(); + assert!( + matches!(err, GossipBlockRejectReason::EpochTooFarAhead(_)), + "timestamp {timestamp}: {err}" + ); + } + } + #[test] fn rejected_block_not_cached_as_seen() { // A block rejected for a transient reason (e.g., epoch too far ahead) diff --git a/src/daemon/db_util.rs b/src/daemon/db_util.rs index c32bf3b8cc2c..181ecf0a4f8d 100644 --- a/src/daemon/db_util.rs +++ b/src/daemon/db_util.rs @@ -654,12 +654,28 @@ async fn process_ts( Ok(ProcessOutcome::Indexed) } -#[derive(Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum RangeSpec { To(ChainEpoch), NumTipsets(usize), } +impl RangeSpec { + /// Both ingresses (`Filecoin.IndexBackfill` and `forest-tool index backfill`) parse the same + /// mutually exclusive pair. + pub fn new(to: Option, n_tipsets: Option) -> anyhow::Result { + match (to, n_tipsets) { + (Some(to), None) => { + anyhow::ensure!(to >= 0, "'to' must not be negative, got {to}."); + Ok(Self::To(to)) + } + (None, Some(n)) => Ok(Self::NumTipsets(n)), + (None, None) => anyhow::bail!("You must provide either 'to' or 'n_tipsets'."), + (Some(_), Some(_)) => anyhow::bail!("'to' and 'n_tipsets' are mutually exclusive."), + } + } +} + impl std::fmt::Display for RangeSpec { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { diff --git a/src/db/car/forest/index/mod.rs b/src/db/car/forest/index/mod.rs index 2cc37e5f0b07..c7fe9ce64029 100644 --- a/src/db/car/forest/index/mod.rs +++ b/src/db/car/forest/index/mod.rs @@ -146,10 +146,14 @@ where else { return Ok(smallvec![]); // empty table }; - let offset_in_table = - u64::try_from(hash::ideal_slot_ix(needle, initial_buckets)).unwrap() * RawSlot::LEN; - let mut haystack = - positioned_io::Cursor::new_pos(&self.inner, self.table_offset + offset_in_table); + // `initial_buckets` comes verbatim from a header a crafted file controls. + let slot_offset = (hash::ideal_slot_ix(needle, initial_buckets) as u64) + .checked_mul(RawSlot::LEN) + .and_then(|offset_in_table| self.table_offset.checked_add(offset_in_table)) + .ok_or_else(|| { + io::Error::new(io::ErrorKind::InvalidData, "index slot offset out of range") + })?; + let mut haystack = positioned_io::Cursor::new_pos(&self.inner, slot_offset); let mut limit = self.header.longest_distance; while let Slot::Occupied(OccupiedSlot { hash, frame_offset }) = @@ -799,6 +803,25 @@ mod tests { use tap::Tap as _; use tokio_test::block_on; + #[test] + fn lookup_with_out_of_range_bucket_count_errors() { + let cid = Cid::default(); + let r: ZstdSkipFramesEncodedDataReader> = + ZstdSkipFramesEncodedDataReader::new(write_to_vec(|v| { + let writer = Builder::from_iter([(hash::summary(&cid), 0)]).into_writer(); + block_on(async { writer.write_zstd_skip_frames_into(&mut *v).await })?; + Ok(()) + })); + let mut subject = Reader::new(r).unwrap(); + subject.header.initial_buckets = u64::MAX; + // A high hash lands in a high bucket, whose byte offset overflows. + assert!( + subject + .get_by_hash(NonMaximalU64::fit(u64::MAX - 1)) + .is_err() + ); + } + /// [`Reader`] should behave like a [`HashMap`], with a caveat for collisions. fn do_hashmap_of_cids(reference: HashMap>) { for multi_index_frame in [false, true] { diff --git a/src/fil_cns/weight.rs b/src/fil_cns/weight.rs index bbf8fd790357..614afd46240b 100644 --- a/src/fil_cns/weight.rs +++ b/src/fil_cns/weight.rs @@ -35,13 +35,15 @@ where ); }; - let mut total_j = 0; + // `win_count` is only bounded once `validate_winner_election` runs. + let mut total_j: i128 = 0; for b in ts.block_headers() { - total_j += b - .election_proof - .as_ref() - .ok_or("Block contained no election proof when calculating weight")? - .win_count; + total_j += i128::from( + b.election_proof + .as_ref() + .ok_or("Block contained no election proof when calculating weight")? + .win_count, + ); } let mut out = ts.weight().to_owned(); diff --git a/src/lotus_json/mod.rs b/src/lotus_json/mod.rs index 2667f7ce5be5..4af7ffd1f688 100644 --- a/src/lotus_json/mod.rs +++ b/src/lotus_json/mod.rs @@ -448,12 +448,7 @@ pub mod hexify { D: Deserializer<'de>, { let s = String::deserialize(deserializer)?; - #[allow(clippy::indexing_slicing)] - if s.len() > 2 && &s[..2] == "0x" { - T::from_str_radix(&s[2..], 16).map_err(serde::de::Error::custom) - } else { - Err(serde::de::Error::custom("Invalid hex")) - } + crate::utils::encoding::hex::parse_prefixed_int(&s).map_err(serde::de::Error::custom) } } @@ -689,7 +684,8 @@ mod tests { self::assert_eq!(de("0x2a").unwrap(), 42); self::assert_eq!(de("0x0").unwrap(), 0); - for invalid in ["", "0x", "2a", "0xzz", "cthulhu"] { + // "0é" is 3 bytes, so slicing a byte-length-checked prefix would panic here. + for invalid in ["", "0x", "2a", "0xzz", "cthulhu", "0é", "0x-1"] { assert!(de(invalid).is_err(), "{invalid:?} should be rejected"); } } diff --git a/src/rpc/methods/beacon.rs b/src/rpc/methods/beacon.rs index b4853dc27a1d..c2cdce58fbe5 100644 --- a/src/rpc/methods/beacon.rs +++ b/src/rpc/methods/beacon.rs @@ -28,8 +28,8 @@ impl RpcMethod<1> for BeaconGetEntry { _: &http::Extensions, ) -> Result { let (_, beacon) = ctx.beacon().beacon_for_epoch(first)?; - let rr = - beacon.max_beacon_round_for_epoch(ctx.state_manager.get_network_version(first), first); + let rr = beacon + .max_beacon_round_for_epoch(ctx.state_manager.get_network_version(first), first)?; let e = beacon.entry(rr).await?; Ok(e) } diff --git a/src/rpc/methods/chain.rs b/src/rpc/methods/chain.rs index 6472547dc706..963a3428fd4e 100644 --- a/src/rpc/methods/chain.rs +++ b/src/rpc/methods/chain.rs @@ -344,6 +344,11 @@ async fn export_chain_inner( dry_run, } = params; + anyhow::ensure!( + recent_roots >= 0, + "recentRoots must not be negative, got {recent_roots}." + ); + let head = ctx.chain_store().load_required_tipset_or_heaviest(&tsk)?; let start_ts = ctx .chain_index() @@ -602,7 +607,7 @@ impl RpcMethod<0> for ForestChainExportCancel { } /// Parameters for [`IndexBackfill`]. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct IndexBackfillParams { /// Starting epoch, inclusive. Defaults to the chain head, unless `resume` is set and a @@ -727,15 +732,10 @@ impl RpcMethod<1> for IndexBackfill { fn index_backfill_range_spec( params: &IndexBackfillParams, ) -> Result { - use crate::daemon::db_util::RangeSpec; - match (params.to, params.n_tipsets) { - (Some(x), None) => Ok(RangeSpec::To(x)), - (None, Some(x)) => Ok(RangeSpec::NumTipsets(x as usize)), - (None, None) => Err(anyhow::anyhow!("You must provide either 'to' or 'n_tipsets'.").into()), - (Some(_), Some(_)) => { - Err(anyhow::anyhow!("'to' and 'n_tipsets' are mutually exclusive.").into()) - } - } + let n_tipsets = params.n_tipsets.map(|n| n as usize); + Ok(crate::daemon::db_util::RangeSpec::new( + params.to, n_tipsets, + )?) } async fn run_index_backfill_inner( @@ -1491,11 +1491,12 @@ impl ChainGetTipSetFinalityStatus { head: &Tipset, depth: i64, ) -> i64 { - if depth >= 0 { - (head.epoch() - depth).max(0) + let depth = if depth >= 0 { + depth } else { - (head.epoch() - chain_config.policy.chain_finality).max(0) - } + chain_config.policy.chain_finality + }; + (head.epoch() - depth).max(0) } fn get_ec_finality_threshold_depth_with_cache( @@ -1542,13 +1543,8 @@ impl ChainGetTipSetFinalityStatus { chain.push(ts.len() as i64); if let Ok(parent) = chain_index.load_required_tipset(ts.parents()) { // insert 0 for null rounds - if let Ok(n_null_tipsets_to_pad) = usize::try_from(ts.epoch() - parent.epoch() - 1) - && n_null_tipsets_to_pad > 0 - { - let target_len = - (chain.len().saturating_add(n_null_tipsets_to_pad)).min(chain_len); - chain.resize(target_len, 0); - } + let pad = usize::try_from(ts.epoch() - parent.epoch() - 1).unwrap_or_default(); + chain.resize(chain.len().saturating_add(pad).min(chain_len), 0); ts = parent; } else { break; @@ -2087,6 +2083,7 @@ fn quickcheck(val: PathChange) { #[cfg(test)] mod tests { use super::*; + use crate::daemon::db_util::RangeSpec; use crate::{ blocks::{Chain4U, RawBlockHeader, chain4u}, db::{ @@ -2096,8 +2093,28 @@ mod tests { networks::{self, ChainConfig}, }; use PathChange::{Apply, Revert}; + use rstest::rstest; use std::sync::Arc; + #[rstest] + #[case(Some(0), None, Some(RangeSpec::To(0)))] + #[case(Some(-1), None, None)] + #[case(None, Some(10), Some(RangeSpec::NumTipsets(10)))] + #[case(None, None, None)] + #[case(Some(10), Some(10), None)] + fn index_backfill_range_spec_validates_params( + #[case] to: Option, + #[case] n_tipsets: Option, + #[case] expected: Option, + ) { + let params = IndexBackfillParams { + to, + n_tipsets, + ..Default::default() + }; + assert_eq!(index_backfill_range_spec(¶ms).ok(), expected); + } + #[test] fn revert_to_ancestor_linear() { let cs = ChainStore::calibnet(); diff --git a/src/rpc/methods/eth/filter/mod.rs b/src/rpc/methods/eth/filter/mod.rs index f780e99bca5d..e69abe326794 100644 --- a/src/rpc/methods/eth/filter/mod.rs +++ b/src/rpc/methods/eth/filter/mod.rs @@ -681,6 +681,9 @@ fn parse_block_range( max_height >= 0 || max_height == -1, "max_height requested is less than 0" ); + // Unlike `max_height`, `-1` is not a sentinel here: "latest" resolves to `heaviest` above, + // which leaves the `min_height == -1` branch below inert, as it is in Lotus. + ensure!(min_height >= 0, "min_height requested is less than 0"); if min_height == -1 && max_height > 0 { ensure!( @@ -709,11 +712,9 @@ fn parse_block_range( } pub fn hex_str_to_epoch(hex_str: &str) -> Result { - let hex_substring = hex_str - .strip_prefix("0x") - .ok_or_else(|| anyhow!("Not a hex"))?; - i64::from_str_radix(hex_substring, 16) - .map_err(|e| anyhow!("Failed to convert hex to epoch: {}", e)) + // Unsigned: `-1` is the internal "latest" sentinel, never a caller's block number. + let epoch: u64 = crate::utils::encoding::hex::parse_prefixed_int(hex_str)?; + ChainEpoch::try_from(epoch).with_context(|| format!("epoch {epoch} is out of range")) } fn parse_eth_topics( @@ -898,6 +899,7 @@ mod tests { use base64::{Engine, prelude::BASE64_STANDARD}; use fvm_ipld_encoding::DAG_CBOR; use fvm_shared4::event::Flags; + use rstest::rstest; use std::str::FromStr; #[test] @@ -1246,6 +1248,38 @@ mod tests { let hex_str = "0xG"; let result = hex_str_to_epoch(hex_str); assert!(result.is_err()); + // Above `i64::MAX`, so not representable as an epoch. + assert!(hex_str_to_epoch("0xffffffffffffffff").is_err()); + } + + #[rstest] + #[case(-5)] + // `-1` is `to_block`'s "latest" sentinel, not `from_block`'s, so it is not accepted here. + #[case(-1)] + fn test_parse_block_range_rejects_negative_from_block(#[case] from_block: ChainEpoch) { + assert!( + parse_block_range( + 500, + Some(BlockNumberOrHash::from_block_number(from_block)), + Some(BlockNumberOrHash::from_str("0x1").unwrap()), + 100, + ) + .is_err() + ); + } + + /// `-1` is `to_block`'s "latest" sentinel; anything below it is not a height. + #[test] + fn test_parse_block_range_rejects_negative_to_block() { + assert!( + parse_block_range( + 500, + Some(BlockNumberOrHash::from_str("0x1").unwrap()), + Some(BlockNumberOrHash::from_block_number(-2)), + 100, + ) + .is_err() + ); } #[tokio::test] diff --git a/src/rpc/methods/f3/types.rs b/src/rpc/methods/f3/types.rs index ee5d23386e8f..ec86ce42053d 100644 --- a/src/rpc/methods/f3/types.rs +++ b/src/rpc/methods/f3/types.rs @@ -651,14 +651,20 @@ impl F3ParticipationLease { &self.issuer == issuer, "the ticket was not issued by the current node" ); - anyhow::ensure!( - current_instance <= self.from_instance + self.validity_term, - "the ticket has been expired" - ); anyhow::ensure!( self.validity_term <= MAX_LEASE_INSTANCES, "validity_term is too large" ); + // The ticket carries no signature. Lotus wraps here instead: + // + let to_instance = self + .from_instance + .checked_add(self.validity_term) + .context("the ticket instance range is out of bounds")?; + anyhow::ensure!( + current_instance <= to_instance, + "the ticket has been expired" + ); Ok(()) } } @@ -771,6 +777,7 @@ mod tests { use super::*; use crate::utils::encoding::hex; use base64::prelude::*; + use rstest::rstest; #[test] fn decode_f3_participation_lease_ticket_from_lotus() { @@ -808,6 +815,29 @@ mod tests { assert_eq!(lease, decoded); } + #[rstest] + // Overflowing expiry instance. + #[case(u64::MAX, 1, false)] + // Beyond `MAX_LEASE_INSTANCES`, which is 5. + #[case(0, 6, false)] + #[case(0, 5, true)] + fn f3_participation_lease_validate_bounds_instances( + #[case] from_instance: u64, + #[case] validity_term: u64, + #[case] accepted: bool, + ) { + let network = NetworkChain::Calibnet; + let issuer = PeerId::random(); + let lease = F3ParticipationLease { + network: network.clone(), + issuer, + miner_id: 1000, + from_instance, + validity_term, + }; + assert_eq!(lease.validate(&network, &issuer, 1).is_ok(), accepted); + } + #[test] fn f3_lease_manager_tests() { let network = NetworkChain::Calibnet; diff --git a/src/rpc/methods/state.rs b/src/rpc/methods/state.rs index 434d21c76d82..4bd85dae51fc 100644 --- a/src/rpc/methods/state.rs +++ b/src/rpc/methods/state.rs @@ -1068,6 +1068,17 @@ impl RpcMethod<2> for StateMinerAvailableBalance { } } +/// Must be positive: `qa_power_for_weight` divides by it. +fn sector_duration_from_expiration( + expiration: ChainEpoch, + epoch: ChainEpoch, +) -> anyhow::Result { + expiration + .checked_sub(epoch) + .filter(|duration| *duration > 0) + .with_context(|| format!("sector expiration {expiration} must be after epoch {epoch}")) +} + pub enum StateMinerInitialPledgeCollateral {} impl RpcMethod<3> for StateMinerInitialPledgeCollateral { @@ -1101,7 +1112,7 @@ impl RpcMethod<3> for StateMinerInitialPledgeCollateral { ts.epoch(), pci.expiration, )?; - let duration = pci.expiration - ts.epoch(); + let duration = sector_duration_from_expiration(pci.expiration, ts.epoch())?; let sector_weight = qa_power_for_weight(SectorSize::from(sector_size).into(), duration, &w, &vw); @@ -1161,10 +1172,10 @@ impl RpcMethod<3> for StateMinerPreCommitDepositForPower { ts.epoch(), pci.expiration, )?; - let duration = pci.expiration - ts.epoch(); let sector_size = SectorSize::from(sector_size).into(); let sector_weight = if ctx.state_manager.get_network_version(ts.epoch()) < NetworkVersion::V16 { + let duration = sector_duration_from_expiration(pci.expiration, ts.epoch())?; qa_power_for_weight(sector_size, duration, &w, &vw) } else { qa_power_max(sector_size) @@ -2207,6 +2218,22 @@ impl RpcMethod<3> for StateDealProviderCollateralBounds { } } +/// How long to wait for `epoch`'s beacon entry, with a 1s clock drift buffer. +fn beacon_entry_wait( + genesis_timestamp: i64, + block_delay: i64, + epoch: ChainEpoch, + now_timestamp: i64, +) -> anyhow::Result { + let epoch_timestamp = epoch + .checked_mul(block_delay) + .and_then(|ts| ts.checked_add(genesis_timestamp)) + .and_then(|ts| ts.checked_add(1)) + .with_context(|| format!("epoch {epoch} has no representable timestamp"))?; + let seconds = epoch_timestamp.saturating_sub(now_timestamp).max(0); + Ok(Duration::from_secs(seconds as u64)) +} + pub enum StateGetBeaconEntry {} impl RpcMethod<1> for StateGetBeaconEntry { @@ -2224,23 +2251,19 @@ impl RpcMethod<1> for StateGetBeaconEntry { (epoch,): Self::Params, _: &http::Extensions, ) -> Result { - { - let genesis_timestamp = ctx.chain_store().genesis_block_header().timestamp as i64; - let block_delay = i64::from(ctx.chain_config().block_delay_secs); - // Give it a 1s clock drift buffer - let epoch_timestamp = genesis_timestamp + block_delay * epoch + 1; - let now_timestamp = chrono::Utc::now().timestamp(); - match epoch_timestamp.saturating_sub(now_timestamp) { - diff if diff > 0 => { - tokio::time::sleep(Duration::from_secs(diff as u64)).await; - } - _ => {} - }; - } + let genesis_timestamp = i64::try_from(ctx.chain_store().genesis_block_header().timestamp) + .context("genesis timestamp is out of range")?; + tokio::time::sleep(beacon_entry_wait( + genesis_timestamp, + i64::from(ctx.chain_config().block_delay_secs), + epoch, + chrono::Utc::now().timestamp(), + )?) + .await; let (_, beacon) = ctx.beacon().beacon_for_epoch(epoch)?; let network_version = ctx.state_manager.get_network_version(epoch); - let round = beacon.max_beacon_round_for_epoch(network_version, epoch); + let round = beacon.max_beacon_round_for_epoch(network_version, epoch)?; let entry = beacon.entry(round).await?; Ok(entry) } @@ -3527,3 +3550,60 @@ impl RpcMethod<0> for StateActorInfo { Ok(result) } } + +#[cfg(test)] +mod tests { + use super::*; + use quickcheck_macros::quickcheck; + use rstest::rstest; + + const GENESIS: i64 = 1598306400; + const BLOCK_DELAY: i64 = crate::shim::clock::EPOCH_DURATION_SECONDS; + /// Wall clock at epoch 100. + const NOW: i64 = GENESIS + 100 * BLOCK_DELAY; + + #[rstest] + #[case(99, Duration::ZERO)] + // The 1s clock drift buffer puts the current epoch 1s in the future. + #[case(100, Duration::from_secs(1))] + #[case(102, Duration::from_secs(61))] + fn beacon_entry_wait_until_epoch(#[case] epoch: ChainEpoch, #[case] expected: Duration) { + assert_eq!( + beacon_entry_wait(GENESIS, BLOCK_DELAY, epoch, NOW).unwrap(), + expected + ); + } + + #[rstest] + #[case(i64::MIN)] + #[case(i64::MAX)] + fn beacon_entry_wait_rejects_unrepresentable_epochs(#[case] epoch: ChainEpoch) { + assert!(beacon_entry_wait(GENESIS, BLOCK_DELAY, epoch, NOW).is_err()); + } + + #[quickcheck] + fn beacon_entry_wait_no_panic(epoch: ChainEpoch, now_timestamp: i64) { + let _ = beacon_entry_wait(GENESIS, BLOCK_DELAY, epoch, now_timestamp); + } + + #[rstest] + #[case(1_000, Some(600))] + #[case(401, Some(1))] + #[case(400, None)] + #[case(399, None)] + #[case(i64::MIN, None)] + fn sector_duration_from_expiration_requires_positive( + #[case] expiration: ChainEpoch, + #[case] expected: Option, + ) { + assert_eq!( + sector_duration_from_expiration(expiration, 400).ok(), + expected + ); + } + + #[quickcheck] + fn sector_duration_from_expiration_no_panic(expiration: ChainEpoch, epoch: ChainEpoch) { + let _ = sector_duration_from_expiration(expiration, epoch); + } +} diff --git a/src/state_manager/chain_rand.rs b/src/state_manager/chain_rand.rs index 277e0f04b770..a7c6d1e70dda 100644 --- a/src/state_manager/chain_rand.rs +++ b/src/state_manager/chain_rand.rs @@ -126,7 +126,7 @@ impl ChainRand { let mut rand_ts: Tipset = self.get_beacon_randomness_tipset_blocking(epoch, false)?; let (_, beacon) = self.beacon.beacon_for_epoch(epoch)?; let round = - beacon.max_beacon_round_for_epoch(self.chain_config.network_version(epoch), epoch); + beacon.max_beacon_round_for_epoch(self.chain_config.network_version(epoch), epoch)?; for _ in 0..20 { let cbe = &rand_ts.block_headers().first().beacon_entries; diff --git a/src/state_manager/message_search.rs b/src/state_manager/message_search.rs index d55151eb557d..70f90422b39f 100644 --- a/src/state_manager/message_search.rs +++ b/src/state_manager/message_search.rs @@ -454,10 +454,41 @@ mod tests { use crate::utils::db::CborStoreExt as _; use fil_actors_shared::fvm_ipld_amt::Amtv0; use fvm_ipld_blockstore::Blockstore; + use quickcheck_macros::quickcheck; use rstest::rstest; const SENDER: Address = Address::new_id(100); + #[rstest] + #[case(1000, Some(0), None)] + #[case(1000, Some(5), Some(996))] + #[case(1000, Some(2000), Some(0))] + #[case(1000, Some(i64::MAX), Some(0))] + #[case(1000, Some(-1), Some(0))] + #[case(1000, None, Some(0))] + fn max_lookback_epoch_inclusive_examples( + #[case] current_epoch: ChainEpoch, + #[case] look_back_limit: Option, + #[case] expected: Option, + ) { + assert_eq!( + StateManager::max_lookback_epoch_inclusive(current_epoch, look_back_limit), + expected + ); + } + + #[quickcheck] + fn max_lookback_epoch_inclusive_no_panic( + current_epoch: ChainEpoch, + look_back_limit: Option, + ) -> bool { + let current_epoch = current_epoch.max(0); + match StateManager::max_lookback_epoch_inclusive(current_epoch, look_back_limit) { + Some(min_epoch) => min_epoch >= 0 && min_epoch <= current_epoch.max(0), + None => look_back_limit == Some(0), + } + } + fn state_root_with_sender_nonce(db: &Arc, sequence: u64) -> Cid { let mut state_tree = StateTree::new(db, StateTreeVersion::V5).unwrap(); state_tree diff --git a/src/tool/subcommands/index_cmd.rs b/src/tool/subcommands/index_cmd.rs index 8ba0ef5d2f4a..670e9d0d2473 100644 --- a/src/tool/subcommands/index_cmd.rs +++ b/src/tool/subcommands/index_cmd.rs @@ -3,7 +3,6 @@ use std::{path::PathBuf, sync::Arc}; -use anyhow::bail; use clap::Subcommand; use crate::chain::ChainStore; @@ -53,14 +52,7 @@ impl IndexCommands { to, n_tipsets, } => { - let spec = match (to, n_tipsets) { - (Some(x), None) => RangeSpec::To(*x), - (None, Some(x)) => RangeSpec::NumTipsets(*x), - (None, None) => { - bail!("You must provide either '--to' or '--n-tipsets'."); - } - _ => unreachable!(), // Clap ensures this case is handled - }; + let spec = RangeSpec::new(*to, *n_tipsets)?; let (_, config) = read_config(config.as_ref(), chain.clone())?; diff --git a/src/utils/encoding/hex.rs b/src/utils/encoding/hex.rs index 79757be81fb4..a80e83746f52 100644 --- a/src/utils/encoding/hex.rs +++ b/src/utils/encoding/hex.rs @@ -6,6 +6,8 @@ //! call sites keep working. See benchmark results in //! . +use anyhow::Context as _; + #[derive(Debug, Clone, Copy, PartialEq, thiserror::Error)] #[error(transparent)] pub struct DecodeError(#[from] faster_hex_private::Error); @@ -27,6 +29,24 @@ pub fn encode_prefixed(data: impl AsRef<[u8]>) -> String { unsafe { String::from_utf8_unchecked(buf) } } +/// Parses a `0x`-prefixed hex integer, e.g. `0x1a`. +/// +/// A sign is rejected for every `T`, matching Go's `strconv.ParseUint`. +pub fn parse_prefixed_int(input: &str) -> anyhow::Result +where + T: num_traits::Num, + ::FromStrRadixErr: std::fmt::Display, +{ + let digits = input + .strip_prefix("0x") + .with_context(|| format!("not a 0x-prefixed hex integer: {input}"))?; + anyhow::ensure!( + !digits.starts_with(['+', '-']), + "signed hex integer: {input}" + ); + T::from_str_radix(digits, 16).map_err(|e| anyhow::anyhow!("invalid hex integer {input}: {e}")) +} + /// Decodes hex digits (upper, lower or mixed case, no `0x` prefix) into bytes. pub fn decode(input: impl AsRef<[u8]>) -> Result, DecodeError> { let input = input.as_ref(); @@ -63,6 +83,7 @@ pub mod serde { mod tests { use super::*; use quickcheck_macros::quickcheck; + use rstest::rstest; #[quickcheck] fn encode_matches_hex_crate(data: Vec) -> bool { @@ -136,4 +157,49 @@ mod tests { assert!(decode(invalid).is_err(), "{invalid:?} should be rejected"); } } + + #[rstest] + #[case("0x0", 0)] + #[case("0x1a", 26)] + #[case("0x1A", 26)] + fn parse_prefixed_int_accepts(#[case] input: &str, #[case] expected: u64) { + assert_eq!(parse_prefixed_int::(input).unwrap(), expected); + } + + #[rstest] + #[case("")] + #[case("0")] + #[case("1a")] + #[case("0x")] + #[case("0X1a")] + #[case("0xg")] + #[case(" 0x1")] + #[case("0x1 ")] + // Multi-byte UTF-8 at the prefix boundary. + #[case("0é")] + #[case("0x\u{e9}")] + #[case("0x-1")] + #[case("0x+1")] + #[case("0x10000000000000000")] + fn parse_prefixed_int_rejects(#[case] input: &str) { + assert!(parse_prefixed_int::(input).is_err()); + assert!(parse_prefixed_int::(input).is_err()); + } + + #[test] + fn parse_prefixed_int_is_bounded_by_target_type() { + assert_eq!( + parse_prefixed_int::("0x8000000000000000").unwrap(), + 1 << 63 + ); + assert!(parse_prefixed_int::("0x8000000000000000").is_err()); + } + + #[quickcheck] + fn parse_prefixed_int_no_panic(input: String) { + for candidate in [input.clone(), format!("0x{input}")] { + let _ = parse_prefixed_int::(&candidate); + let _ = parse_prefixed_int::(&candidate); + } + } } diff --git a/src/wallet/subcommands/wallet_cmd.rs b/src/wallet/subcommands/wallet_cmd.rs index af4db89cc626..58631eafedff 100644 --- a/src/wallet/subcommands/wallet_cmd.rs +++ b/src/wallet/subcommands/wallet_cmd.rs @@ -554,11 +554,8 @@ impl WalletCommands { message.sequence = MpoolGetNonce::call(&backend.remote, (from,)).await?; let key = crate::key_management::try_find_key(&from, keystore)?; - let eth_chain_id = u64::from_str_radix( - EthChainId::call(&backend.remote, ()) - .await? - .trim_start_matches("0x"), - 16, + let eth_chain_id: u64 = crate::utils::encoding::hex::parse_prefixed_int( + &EthChainId::call(&backend.remote, ()).await?, )?; let smsg = crate::key_management::sign_message(&key, &message, eth_chain_id)?; MpoolPush::call(&backend.remote, (smsg.clone(),)).await?;