From d91d7914fcb0db855d2ab8a1d223de21b5650b02 Mon Sep 17 00:00:00 2001 From: "K. Isom" Date: Wed, 21 Oct 2026 22:44:40 -0700 Subject: [PATCH] fix(hosts): pass the plausible wall range to the store Both hosts open LogStore with plausible_wall_ms(PLAUSIBLE_WALL_MS), so a forged out-of-range peer estimate is dropped before the store picks the latest and can't hide an honest one (decision J, review B3). Un-ignores latest_estimate_wins_over_smallest_age now that the store returns the latest estimate, and adds forged_estimate_does_not_hide_an_honest_one. Co-Authored-By: Claude Opus 5.5 --- tines-ros2/src/node.rs | 5 ++++- tines/src/main.rs | 7 +++++-- tines/src/trust.rs | 8 +++++++- tines/tests/ipc.rs | 26 ++++++++++++++++++++++++-- 4 files changed, 40 insertions(+), 6 deletions(-) diff --git a/tines-ros2/src/node.rs b/tines-ros2/src/node.rs index 3d9b8dc1..58c7af68 100644 --- a/tines-ros2/src/node.rs +++ b/tines-ros2/src/node.rs @@ -172,7 +172,10 @@ impl TinesNode { let executor = context.create_basic_executor(); let node = executor.create_node("tines")?; - let store = LogStore::open(log_path, 0)?; + // Peer estimates outside the plausible range don't count (decision J). + let store = LogStore::builder(log_path) + .plausible_wall_ms(tines::trust::PLAUSIBLE_WALL_MS) + .open(0)?; let transport = BtTransport::new(me, peer, radio); let tines = TinesNodeCore::new(me, store.first_seq(), store, transport); diff --git a/tines/src/main.rs b/tines/src/main.rs index 766ac98c..c17e5332 100644 --- a/tines/src/main.rs +++ b/tines/src/main.rs @@ -124,9 +124,12 @@ fn main() -> Result<(), Box> { // LogStore is the Store (S); BtTransport wraps it as the Transport (T). // unknown_downtime_age_ms (the broadcast floor while the wall clock is // unknown) is optional; leave the LogStore default when unset. + // Peer estimates outside the plausible range don't count (decision J). + let builder = LogStore::builder(Path::new(&config.log_path)) + .plausible_wall_ms(tines::trust::PLAUSIBLE_WALL_MS); let builder = match config.unknown_downtime_age_ms { - Some(ms) => LogStore::builder(Path::new(&config.log_path)).set_unknown_downtime_age_ms(ms), - None => LogStore::builder(Path::new(&config.log_path)), + Some(ms) => builder.set_unknown_downtime_age_ms(ms), + None => builder, }; let mut clock = HostClock::start(); let store = builder diff --git a/tines/src/trust.rs b/tines/src/trust.rs index 39486f57..c8f40b34 100644 --- a/tines/src/trust.rs +++ b/tines/src/trust.rs @@ -48,6 +48,12 @@ pub const WALL_FLOOR_MS: u64 = 1_767_225_600_000; /// still far below where `u64` milliseconds overflow. pub const WALL_CEILING_MS: u64 = WALL_FLOOR_MS + 36_525 * 24 * 60 * 60 * 1000; +/// [`WALL_FLOOR_MS`]`..=`[`WALL_CEILING_MS`] as a range, for +/// `LogStoreBuilder::plausible_wall_ms`. The store drops peer estimates outside +/// it before choosing the latest, so one forged estimate can't hide the honest +/// ones (decision J). +pub const PLAUSIBLE_WALL_MS: core::ops::RangeInclusive = WALL_FLOOR_MS..=WALL_CEILING_MS; + /// The default peer-time window: how long a host keeps collecting peer /// estimates after the first plausible one before adopting the latest /// (decision J, about 10 s). @@ -56,7 +62,7 @@ pub const DEFAULT_PEER_TIME_WINDOW_MS: u64 = 10_000; /// Whether `wall_ms` (ms since the Unix epoch) lies within /// [`WALL_FLOOR_MS`]`..=`[`WALL_CEILING_MS`], so a host may adopt it. pub fn is_plausible_wall_ms(wall_ms: u64) -> bool { - (WALL_FLOOR_MS..=WALL_CEILING_MS).contains(&wall_ms) + PLAUSIBLE_WALL_MS.contains(&wall_ms) } /// The system clock in ms since the Unix epoch, if it is plausible. diff --git a/tines/tests/ipc.rs b/tines/tests/ipc.rs index f54dce91..f86be07b 100644 --- a/tines/tests/ipc.rs +++ b/tines/tests/ipc.rs @@ -478,7 +478,10 @@ fn build_pair(dir: &Path) -> Pair { let world = Arc::new(Mutex::new(SharedInMemoryWorld::new())); let (a_id, b_id) = ([1; 16], [2; 16]); let a_store = LogStore::open(&dir.join("a.store.log"), 0).expect("a store open"); - let b_store = LogStore::open(&dir.join("b.store.log"), 0).expect("b store open"); + let b_store = LogStore::builder(&dir.join("b.store.log")) + .plausible_wall_ms(tines::trust::PLAUSIBLE_WALL_MS) + .open(0) + .expect("b store open"); let ta = SharedInMemory { me: a_id, their: b_id, @@ -816,7 +819,6 @@ fn window_adopts_a_later_larger_estimate() { /// `W + 6000`). Smallest-age selection would adopt `W`; the host adopts the /// latest estimate. #[test] -#[ignore = "needs decision J store change"] fn latest_estimate_wins_over_smallest_age() { let dir = unique_dir("latest"); let (mut node_a, node_b, world) = build_pair(&dir); @@ -868,6 +870,26 @@ fn forged_max_observed_at_is_not_adopted() { assert_not_adopted("forged", u64::MAX); } +/// Decision J with B3's bounds: a forged `u64::MAX` alongside an honest +/// estimate doesn't hide it. The store drops the forged one before choosing +/// the latest, so B adopts the honest time. +#[test] +fn forged_estimate_does_not_hide_an_honest_one() { + let dir = unique_dir("forged-honest"); + let (mut node_a, node_b, world) = build_pair(&dir); + let socket_b = serve_b(node_b, HostClock::start(), &dir); + + inject(&world, &crafted(1, u64::MAX, 0)); + inject(&world, &crafted(2, W, 0)); + + let mut t = 0; + let at = b_adopted_stamp(&socket_b, &mut node_a, &mut t); + assert!( + (W..W + 60_000).contains(&at), + "B adopted the honest estimate: stamp {at}, expected ~{W}" + ); +} + /// B3: an estimate below the floor (here 2001) is not adopted. #[test] fn below_floor_observed_at_is_not_adopted() {