From bf9196cc25055dd9baa43cf616a62d34cc214013 Mon Sep 17 00:00:00 2001 From: James Ross Date: Wed, 14 Oct 2026 23:50:22 -0700 Subject: [PATCH 1/3] Comments: NUM_SHARDS is not hashed; GenSet is per-transaction Two source comments, found false by an earlier audit (archived branch core-trace-milestone-1) and still on main: - NUM_SHARDS claimed to be recorded in the commit hash via compute_patch_digest_v2. No digest or state root references it; deltas are merged canonically, so it only affects load balance. - GenSet claimed O(1) checks via a new generation per transaction. The generation is set to 1 and never advanced, each transaction gets fresh sets, and lookups go through a BTreeMap (O(log n)). Co-Authored-By: Claude Opus 5.5 --- crates/warp-core/src/parallel/shard.rs | 11 +++++------ crates/warp-core/src/scheduler.rs | 11 ++++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/warp-core/src/parallel/shard.rs b/crates/warp-core/src/parallel/shard.rs index 60695c63e..8f9888ade 100644 --- a/crates/warp-core/src/parallel/shard.rs +++ b/crates/warp-core/src/parallel/shard.rs @@ -28,13 +28,12 @@ use crate::NodeId; /// Number of virtual shards (power of two for fast modulo). /// -/// # Protocol Constant +/// # Not part of any hash /// -/// This value is recorded in the commit hash domain via `compute_patch_digest_v2`. -/// Changing it would produce different hashes for the same world state. -/// -/// If you ever need to change this, it requires a protocol version bump -/// and explicit migration handling. +/// No digest or state root includes this value. It only decides how execution +/// items are partitioned among workers; worker deltas are merged in canonical +/// order (`parallel::merge`), so the committed result does not depend on it. +/// Changing it changes load balance, not hashes. /// /// # Value Choice /// diff --git a/crates/warp-core/src/scheduler.rs b/crates/warp-core/src/scheduler.rs index 420e15358..6c8d9364d 100644 --- a/crates/warp-core/src/scheduler.rs +++ b/crates/warp-core/src/scheduler.rs @@ -475,14 +475,15 @@ fn bucket16(r: &RewriteThin, pass: usize) -> u16 { } // ============================================================================ -// Generation-stamped conflict set for O(1) independence checks +// Per-transaction conflict sets // ============================================================================ -/// Generation-stamped set for O(1) conflict detection. +/// Set of keys seen by one transaction, for conflict detection. /// -/// This data structure allows O(1) conflict checking without clearing hash tables -/// between transactions by using generation counters. Each transaction gets a new -/// generation, and we track which generation last saw each key. +/// Each transaction gets fresh sets (`ActiveFootprints::new`) and they are +/// dropped with it, so the generation stays at 1: it is never advanced, and +/// nothing is reused across transactions. Lookups go through a `BTreeMap`, so +/// they cost O(log n), not O(1). #[derive(Debug)] pub(crate) struct GenSet { gen: u32, From 026919e75662a2fe53a347c8e4be7941c46ac668 Mon Sep 17 00:00:00 2001 From: James Ross Date: Thu, 15 Oct 2026 00:58:08 -0700 Subject: [PATCH 2/3] Comments: NUM_SHARDS caps workers; correct every GenSet claim - NUM_SHARDS also caps the effective worker count, which decides how many per-worker deltas the parallel APIs return and which plan they select. Only the committed result is independent of it. - ActiveFootprints, reserve() and the scheduler spec still described generation-stamped O(1) checks. They now describe per-transaction BTreeMap-backed sets at O(log n) per lookup. Found by review of #866. Co-Authored-By: Claude Opus 5.5 --- crates/warp-core/src/parallel/shard.rs | 11 +++++++---- crates/warp-core/src/scheduler.rs | 9 +++++---- docs/spec/scheduler-warp-core.md | 4 +++- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/crates/warp-core/src/parallel/shard.rs b/crates/warp-core/src/parallel/shard.rs index 8f9888ade..0edc456f4 100644 --- a/crates/warp-core/src/parallel/shard.rs +++ b/crates/warp-core/src/parallel/shard.rs @@ -30,10 +30,13 @@ use crate::NodeId; /// /// # Not part of any hash /// -/// No digest or state root includes this value. It only decides how execution -/// items are partitioned among workers; worker deltas are merged in canonical -/// order (`parallel::merge`), so the committed result does not depend on it. -/// Changing it changes load balance, not hashes. +/// No digest or state root includes this value, and worker deltas are merged in +/// canonical order (`parallel::merge`), so the committed result does not depend +/// on it. It is still observable: it partitions execution items, and it caps the +/// effective worker count (`parallel::exec`), which decides how many per-worker +/// deltas the parallel execution APIs return (even for empty input) and which +/// execution plan they select. Changing it changes those API-visible shapes and +/// load balance, not hashes. /// /// # Value Choice /// diff --git a/crates/warp-core/src/scheduler.rs b/crates/warp-core/src/scheduler.rs index 6c8d9364d..348323a37 100644 --- a/crates/warp-core/src/scheduler.rs +++ b/crates/warp-core/src/scheduler.rs @@ -20,7 +20,8 @@ use crate::telemetry::TelemetrySink; use crate::tick_delta::OpOrigin; use crate::tx::TxId; -/// Active footprint tracking using generation-stamped sets for O(1) conflict detection. +/// Active footprint tracking for one transaction, using per-transaction +/// [`GenSet`]s (`BTreeMap`-backed, O(log n) per lookup). /// /// All resource keys are warp-scoped: they include both `warp_id` and local identifiers. /// This ensures rewrites in different warps don't cause false conflicts when they @@ -118,11 +119,11 @@ impl RadixScheduler { } /// Attempts to reserve a rewrite by checking full footprint independence - /// using generation-stamped conflict detection. + /// against the transaction's active sets. /// /// Checks all aspects of the footprint: node read/write sets, edge read/write - /// sets, and boundary ports. Uses O(1) `GenSet` lookups for each resource, - /// making this O(m) where m is the size of the current footprint. + /// sets, and boundary ports. Each resource is an O(log n) `GenSet` lookup, + /// so this is O(m log n) for a footprint of size m against n reserved keys. /// /// On success, marks all resources in the active `GenSets` and transitions /// the phase to `Reserved`. diff --git a/docs/spec/scheduler-warp-core.md b/docs/spec/scheduler-warp-core.md index e1d5b208b..d9ec7daf6 100644 --- a/docs/spec/scheduler-warp-core.md +++ b/docs/spec/scheduler-warp-core.md @@ -58,7 +58,9 @@ Pending rewrites drain in lexicographic order derived from `scope_hash`, stable ## Decision 5: Radix reservation is the default implementation -The default scheduler uses generation-stamped active sets for membership checks. Expected reservation cost is proportional to candidate footprint size. +The default scheduler uses per-transaction active sets (`BTreeMap`-backed) for +membership checks. Reservation costs O(log n) per footprint resource, where n +is the number of keys already reserved in the transaction. ## Decision 6: Executable Actions are evaluated only inside Tick construction From 031306f560683ef2844fa9ae6cee3655ffc1dc17 Mon Sep 17 00:00:00 2001 From: James Ross Date: Thu, 15 Oct 2026 01:08:19 -0700 Subject: [PATCH 3/3] Comments: the active-set field claimed O(m) checks Reservation is O(log n) per resource through BTreeMap-backed sets. Found by review of #866. Co-Authored-By: Claude Opus 5.5 --- crates/warp-core/src/scheduler.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/warp-core/src/scheduler.rs b/crates/warp-core/src/scheduler.rs index 348323a37..3f94c8c10 100644 --- a/crates/warp-core/src/scheduler.rs +++ b/crates/warp-core/src/scheduler.rs @@ -63,7 +63,8 @@ impl ActiveFootprints { pub(crate) struct RadixScheduler { /// Pending rewrites per transaction, stored for O(1) enqueue and O(n) drain. pending: BTreeMap>, - /// Active footprints per transaction for O(m) independence checking via `GenSets`. + /// Active footprints per transaction, checked through `GenSet`s at O(log n) per + /// resource, so O(m log n) for a footprint of size m. /// Checks all aspects: nodes (read/write), edges (read/write), and boundary ports. pub(crate) active: BTreeMap, }