From 8a2e6caa7d50ed4111e514ca084790a5ae2dd354 Mon Sep 17 00:00:00 2001 From: Prajwol Gyawali Date: Sat, 25 Jul 2026 09:25:58 +0545 Subject: [PATCH 1/8] refactor!: move spec versioning primitives from common to params MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fork awareness is meant to live at the spec level: the version table in strata-asm-spec and the schedule the worker and guest feed it. Subprotocols stay fork-unaware — upgrades ship as whole versioned implementations — so nothing below params ever needs SpecId, and keeping it in common wrongly advertised it as subprotocol vocabulary. Inside params, identity and schedule split: spec_id.rs keeps the SpecId enum, runtime.rs owns SpecActivation and the STF/runtime params. SpecActivation was common's last serde user and params' only common import, so common drops serde and params drops strata-asm-common. --- Cargo.lock | 3 - crates/common/Cargo.toml | 2 - crates/common/src/lib.rs | 2 - crates/common/src/spec_id.rs | 212 ------------------------- crates/params/Cargo.toml | 1 - crates/params/src/lib.rs | 6 +- crates/params/src/runtime.rs | 122 +++++++++++++- crates/params/src/spec_id.rs | 97 +++++++++++ guest-builder/sp1/guest-asm/Cargo.lock | 1 - 9 files changed, 221 insertions(+), 225 deletions(-) delete mode 100644 crates/common/src/spec_id.rs create mode 100644 crates/params/src/spec_id.rs diff --git a/Cargo.lock b/Cargo.lock index 16096937..2522e3ea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7627,8 +7627,6 @@ version = "0.1.0" dependencies = [ "bitcoin", "borsh", - "serde", - "serde_json", "ssz", "ssz_derive", "ssz_types", @@ -7734,7 +7732,6 @@ dependencies = [ "serde_json", "ssz", "ssz_derive", - "strata-asm-common", "strata-asm-proto-bridge-types", "strata-btc-types", "strata-btc-verification", diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml index 648875a8..9c26741d 100644 --- a/crates/common/Cargo.toml +++ b/crates/common/Cargo.toml @@ -18,7 +18,6 @@ strata-msg-fmt.workspace = true bitcoin.workspace = true borsh.workspace = true -serde.workspace = true ssz.workspace = true ssz_derive.workspace = true ssz_types.workspace = true @@ -27,6 +26,5 @@ tracing.workspace = true zkaleido-logging.workspace = true [dev-dependencies] -serde_json.workspace = true strata-identifiers.workspace = true strata-test-utils-arb.workspace = true diff --git a/crates/common/src/lib.rs b/crates/common/src/lib.rs index ac8364ab..611f93a8 100644 --- a/crates/common/src/lib.rs +++ b/crates/common/src/lib.rs @@ -9,7 +9,6 @@ mod msg; mod section; pub mod sorted_vec; mod spec; -mod spec_id; mod subprotocol; mod tx; @@ -20,7 +19,6 @@ pub use manifest::*; pub use msg::*; pub use section::*; pub use spec::*; -pub use spec_id::*; // Re-export the anchor state types so downstream crates keep a single import path. pub use strata_asm_state::*; pub use subprotocol::*; diff --git a/crates/common/src/spec_id.rs b/crates/common/src/spec_id.rs deleted file mode 100644 index c20d974d..00000000 --- a/crates/common/src/spec_id.rs +++ /dev/null @@ -1,212 +0,0 @@ -//! Spec-versioned upgradeability primitives. -//! -//! The ASM upgrades EVM-style: STF logic is gated on spec versions with L1 -//! activation heights, so a single binary can execute both sides of an upgrade -//! boundary. The activation schedule is *not* part of committed state — it is -//! baked into each proving artifact (guest ELF / native host) and supplied to -//! the worker via params, with the invariant that every artifact agrees on the -//! gate's outcome at every height it executes (see `AsmStfParams`). - -use serde::{Deserialize, Serialize}; -use strata_identifiers::L1Height; - -/// Identifies a spec version. -/// -/// One variant per protocol upgrade, in activation order. The numeric -/// discriminant is the stable identity: it keys persisted spec-activation -/// records (stored as the raw discriminant byte) and is the raw id carried in -/// ASM VK upgrade actions. The variant name is the human-readable form: it is -/// this type's serde representation (snake_case) and is mirrored by the -/// [`SpecActivation`] params field. -/// -/// The id crosses two boundaries with opposite tolerances: -/// -/// - Parse-time: ASM VK upgrade actions carry the raw id, not this enum, so an artifact predating a -/// spec version can still parse and enact the upgrade that activates it — the wire format never -/// requires knowing the version. -/// - Act-time: a consumer that must *apply* the version's rules (the worker) maps the id via -/// [`TryFrom`]. An id it does not know is not skipped: it means the worker is running old -/// software past an upgrade it cannot execute, so it MUST halt rather than silently limp along on -/// stale rules. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] -#[serde(rename_all = "snake_case")] -#[repr(u8)] -pub enum SpecId { - /// First spec revision after genesis. Genesis rules are simply "no spec - /// version active". - V1 = 0, -} - -impl From for u8 { - fn from(spec: SpecId) -> Self { - spec as u8 - } -} - -impl From for u16 { - fn from(spec: SpecId) -> Self { - spec as u16 - } -} - -impl TryFrom for SpecId { - type Error = u8; - - fn try_from(value: u8) -> Result { - match value { - 0 => Ok(SpecId::V1), - invalid => Err(invalid), - } - } -} - -impl TryFrom for SpecId { - type Error = u16; - - fn try_from(value: u16) -> Result { - u8::try_from(value) - .ok() - .and_then(|v| SpecId::try_from(v).ok()) - .ok_or(value) - } -} - -/// Activation heights for every spec version. -/// -/// A version with activation height `Some(n)` is active at L1 height `h` iff -/// `h >= n` — so `Some(0)` means active since genesis. `None` means disabled. -/// Proving artifacts bake one of the two extremes (`Some(0)` or `None` — an -/// artifact only ever executes one side of an upgrade boundary), while the -/// worker tracks the real activation height discovered from the ASM VK -/// upgrade log. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct SpecActivation { - /// Activation height of [`SpecId::V1`], or `None` if disabled. - pub v1: Option, -} - -impl SpecActivation { - /// Schedule with every spec version disabled (no activation height). - pub const fn all_disabled() -> Self { - Self { v1: None } - } - - /// Returns the activation height of `spec`, or `None` if disabled. - pub fn activation_height_of(&self, spec: SpecId) -> Option { - match spec { - SpecId::V1 => self.v1, - } - } - - /// Returns whether `spec` is active at L1 `height`. - pub fn is_active(&self, spec: SpecId, height: L1Height) -> bool { - self.activation_height_of(spec) - .is_some_and(|activation| height >= activation) - } - - /// Sets the activation height of `spec`. - pub fn set_activation(&mut self, spec: SpecId, height: L1Height) { - match spec { - SpecId::V1 => self.v1 = Some(height), - } - } -} - -impl Default for SpecActivation { - fn default() -> Self { - Self::all_disabled() - } -} - -/// Protocol-rule parameters consumed by the STF, as opposed to the genesis -/// params that only seed the initial state. -/// -/// Every executor of the STF carries its own copy: guest programs hardcode it -/// (so the proof's verifying key commits to it), native proving hosts bake it -/// into their closure, and the worker derives an effective copy from params -/// plus discovered spec activations. -/// -/// `Default` inherits [`SpecActivation`]'s default: everything disabled. -#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] -pub struct AsmStfParams { - /// Spec activation schedule. - pub spec_activation: SpecActivation, -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn is_active_boundaries() { - let activation = SpecActivation { v1: Some(100) }; - assert!(!activation.is_active(SpecId::V1, 99)); - assert!(activation.is_active(SpecId::V1, 100)); - assert!(activation.is_active(SpecId::V1, 101)); - } - - #[test] - fn zero_means_always_active() { - let activation = SpecActivation { v1: Some(0) }; - assert!(activation.is_active(SpecId::V1, 0)); - assert!(activation.is_active(SpecId::V1, L1Height::MAX)); - } - - #[test] - fn none_means_never_active() { - let activation = SpecActivation::all_disabled(); - assert_eq!(activation.activation_height_of(SpecId::V1), None); - assert!(!activation.is_active(SpecId::V1, 0)); - assert!(!activation.is_active(SpecId::V1, L1Height::MAX)); - } - - #[test] - fn set_activation_overrides() { - let mut activation = SpecActivation::all_disabled(); - activation.set_activation(SpecId::V1, 42); - assert_eq!(activation.activation_height_of(SpecId::V1), Some(42)); - assert!(activation.is_active(SpecId::V1, 42)); - assert!(!activation.is_active(SpecId::V1, 41)); - } - - #[test] - fn serde_roundtrip() { - let params = AsmStfParams { - spec_activation: SpecActivation { v1: Some(7) }, - }; - let json = serde_json::to_string(¶ms).unwrap(); - assert_eq!(json, r#"{"spec_activation":{"v1":7}}"#); - let back: AsmStfParams = serde_json::from_str(&json).unwrap(); - assert_eq!(back, params); - - let disabled = AsmStfParams { - spec_activation: SpecActivation::all_disabled(), - }; - let json = serde_json::to_string(&disabled).unwrap(); - assert_eq!(json, r#"{"spec_activation":{"v1":null}}"#); - let back: AsmStfParams = serde_json::from_str(&json).unwrap(); - assert_eq!(back, disabled); - } - - /// Serde is the human-readable form (the variant name). The stable numeric - /// identity used for persistence and the wire is exercised by - /// [`spec_id_u16_roundtrip`] instead. - #[test] - fn spec_id_serde_is_the_variant_name() { - assert_eq!(serde_json::to_string(&SpecId::V1).unwrap(), r#""v1""#); - assert_eq!( - serde_json::from_str::(r#""v1""#).unwrap(), - SpecId::V1 - ); - assert!(serde_json::from_str::(r#""nope""#).is_err()); - } - - /// Raw spec ids on the wire round-trip through the enum; unknown ids - /// surface as errors instead of misparsing. - #[test] - fn spec_id_u16_roundtrip() { - assert_eq!(u16::from(SpecId::V1), 0); - assert_eq!(SpecId::try_from(0u16).unwrap(), SpecId::V1); - assert_eq!(SpecId::try_from(0xFFFFu16), Err(0xFFFF)); - } -} diff --git a/crates/params/Cargo.toml b/crates/params/Cargo.toml index 78d4ba07..2b12972e 100644 --- a/crates/params/Cargo.toml +++ b/crates/params/Cargo.toml @@ -7,7 +7,6 @@ edition = "2024" workspace = true [dependencies] -strata-asm-common.workspace = true strata-asm-proto-bridge-types.workspace = true strata-btc-types.workspace = true strata-btc-verification.workspace = true diff --git a/crates/params/src/lib.rs b/crates/params/src/lib.rs index 2d2246bf..24e2d3d9 100644 --- a/crates/params/src/lib.rs +++ b/crates/params/src/lib.rs @@ -7,15 +7,15 @@ mod genesis; mod runtime; +mod spec_id; mod subprotocols; #[cfg(feature = "arbitrary")] use arbitrary::{Arbitrary, Unstructured}; pub use genesis::AsmGenesisParams; -pub use runtime::AsmRuntimeParams; +pub use runtime::{AsmRuntimeParams, AsmStfParams, SpecActivation}; use serde::{Deserialize, Serialize}; -#[cfg(feature = "arbitrary")] -use strata_asm_common::SpecActivation; +pub use spec_id::SpecId; pub use subprotocols::{ AdminTxType, AdministrationInitConfig, BridgeInitConfig, CheckpointInitConfig, ConfirmationDepths, Role, SubprotocolInstance, UpdateTxType, diff --git a/crates/params/src/runtime.rs b/crates/params/src/runtime.rs index 57a9161a..c710fcac 100644 --- a/crates/params/src/runtime.rs +++ b/crates/params/src/runtime.rs @@ -1,7 +1,76 @@ //! Parameters of the per-block state transition function. +//! +//! The activation schedule is *not* part of committed state — it is baked into +//! each proving artifact (guest ELF / native host) and supplied to the worker +//! via params, with the invariant that every artifact agrees on every gate's +//! outcome at every height it executes (see [`AsmStfParams`]). use serde::{Deserialize, Serialize}; -use strata_asm_common::{AsmStfParams, SpecActivation}; +use strata_identifiers::L1Height; + +use crate::spec_id::SpecId; + +/// Activation heights for every spec version. +/// +/// A version with activation height `Some(n)` is active at L1 height `h` iff +/// `h >= n` — so `Some(0)` means active since genesis. `None` means disabled. +/// Proving artifacts bake one of the two extremes (`Some(0)` or `None` — an +/// artifact only ever executes one side of an upgrade boundary), while the +/// worker tracks the real activation height discovered from the ASM VK +/// upgrade log. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct SpecActivation { + /// Activation height of [`SpecId::V1`], or `None` if disabled. + pub v1: Option, +} + +impl SpecActivation { + /// Schedule with every spec version disabled (no activation height). + pub const fn all_disabled() -> Self { + Self { v1: None } + } + + /// Returns the activation height of `spec`, or `None` if disabled. + pub fn activation_height_of(&self, spec: SpecId) -> Option { + match spec { + SpecId::V1 => self.v1, + } + } + + /// Returns whether `spec` is active at L1 `height`. + pub fn is_active(&self, spec: SpecId, height: L1Height) -> bool { + self.activation_height_of(spec) + .is_some_and(|activation| height >= activation) + } + + /// Sets the activation height of `spec`. + pub fn set_activation(&mut self, spec: SpecId, height: L1Height) { + match spec { + SpecId::V1 => self.v1 = Some(height), + } + } +} + +impl Default for SpecActivation { + fn default() -> Self { + Self::all_disabled() + } +} + +/// Protocol-rule parameters consumed by the STF, as opposed to the genesis +/// params that only seed the initial state. +/// +/// Every executor of the STF carries its own copy: guest programs hardcode it +/// (so the proof's verifying key commits to it), native proving hosts bake it +/// into their closure, and the worker derives an effective copy from params +/// plus discovered spec activations. +/// +/// `Default` inherits [`SpecActivation`]'s default: everything disabled. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct AsmStfParams { + /// Spec activation schedule. + pub spec_activation: SpecActivation, +} /// Runtime parameters of the state transition function. /// @@ -28,6 +97,57 @@ impl AsmRuntimeParams { mod tests { use super::*; + #[test] + fn is_active_boundaries() { + let activation = SpecActivation { v1: Some(100) }; + assert!(!activation.is_active(SpecId::V1, 99)); + assert!(activation.is_active(SpecId::V1, 100)); + assert!(activation.is_active(SpecId::V1, 101)); + } + + #[test] + fn zero_means_always_active() { + let activation = SpecActivation { v1: Some(0) }; + assert!(activation.is_active(SpecId::V1, 0)); + assert!(activation.is_active(SpecId::V1, L1Height::MAX)); + } + + #[test] + fn none_means_never_active() { + let activation = SpecActivation::all_disabled(); + assert_eq!(activation.activation_height_of(SpecId::V1), None); + assert!(!activation.is_active(SpecId::V1, 0)); + assert!(!activation.is_active(SpecId::V1, L1Height::MAX)); + } + + #[test] + fn set_activation_overrides() { + let mut activation = SpecActivation::all_disabled(); + activation.set_activation(SpecId::V1, 42); + assert_eq!(activation.activation_height_of(SpecId::V1), Some(42)); + assert!(activation.is_active(SpecId::V1, 42)); + assert!(!activation.is_active(SpecId::V1, 41)); + } + + #[test] + fn stf_params_serde_roundtrip() { + let params = AsmStfParams { + spec_activation: SpecActivation { v1: Some(7) }, + }; + let json = serde_json::to_string(¶ms).unwrap(); + assert_eq!(json, r#"{"spec_activation":{"v1":7}}"#); + let back: AsmStfParams = serde_json::from_str(&json).unwrap(); + assert_eq!(back, params); + + let disabled = AsmStfParams { + spec_activation: SpecActivation::all_disabled(), + }; + let json = serde_json::to_string(&disabled).unwrap(); + assert_eq!(json, r#"{"spec_activation":{"v1":null}}"#); + let back: AsmStfParams = serde_json::from_str(&json).unwrap(); + assert_eq!(back, disabled); + } + #[test] fn test_runtime_params_deserialize() { let params: AsmRuntimeParams = diff --git a/crates/params/src/spec_id.rs b/crates/params/src/spec_id.rs new file mode 100644 index 00000000..fd85395a --- /dev/null +++ b/crates/params/src/spec_id.rs @@ -0,0 +1,97 @@ +//! Spec version identity. +//! +//! The ASM upgrades EVM-style: STF logic is gated on spec versions with L1 +//! activation heights, so a single binary can execute both sides of an upgrade +//! boundary. This module names the versions; the activation schedule that +//! gates them lives with the runtime params as +//! [`SpecActivation`](crate::SpecActivation). + +use serde::{Deserialize, Serialize}; + +/// Identifies a spec version. +/// +/// One variant per protocol upgrade, in activation order. The numeric +/// discriminant is the stable identity: it keys persisted spec-activation +/// records (stored as the raw discriminant byte) and is the raw id carried in +/// ASM VK upgrade actions. The variant name is the human-readable form: it is +/// this type's serde representation (snake_case) and is mirrored by the +/// [`SpecActivation`](crate::SpecActivation) params field. +/// +/// The id crosses two boundaries with opposite tolerances: +/// +/// - Parse-time: ASM VK upgrade actions carry the raw id, not this enum, so an artifact predating a +/// spec version can still parse and enact the upgrade that activates it — the wire format never +/// requires knowing the version. +/// - Act-time: a consumer that must *apply* the version's rules (the worker) maps the id via +/// [`TryFrom`]. An id it does not know is not skipped: it means the worker is running old +/// software past an upgrade it cannot execute, so it MUST halt rather than silently limp along on +/// stale rules. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +#[repr(u8)] +pub enum SpecId { + /// First spec revision after genesis. Genesis rules are simply "no spec + /// version active". + V1 = 0, +} + +impl From for u8 { + fn from(spec: SpecId) -> Self { + spec as u8 + } +} + +impl From for u16 { + fn from(spec: SpecId) -> Self { + spec as u16 + } +} + +impl TryFrom for SpecId { + type Error = u8; + + fn try_from(value: u8) -> Result { + match value { + 0 => Ok(SpecId::V1), + invalid => Err(invalid), + } + } +} + +impl TryFrom for SpecId { + type Error = u16; + + fn try_from(value: u16) -> Result { + u8::try_from(value) + .ok() + .and_then(|v| SpecId::try_from(v).ok()) + .ok_or(value) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Serde is the human-readable form (the variant name). The stable numeric + /// identity used for persistence and the wire is exercised by + /// [`spec_id_u16_roundtrip`] instead. + #[test] + fn spec_id_serde_is_the_variant_name() { + assert_eq!(serde_json::to_string(&SpecId::V1).unwrap(), r#""v1""#); + assert_eq!( + serde_json::from_str::(r#""v1""#).unwrap(), + SpecId::V1 + ); + assert!(serde_json::from_str::(r#""nope""#).is_err()); + } + + /// Raw spec ids on the wire round-trip through the enum; unknown ids + /// surface as errors instead of misparsing. + #[test] + fn spec_id_u16_roundtrip() { + assert_eq!(u16::from(SpecId::V1), 0); + assert_eq!(SpecId::try_from(0u16).unwrap(), SpecId::V1); + assert_eq!(SpecId::try_from(0xFFFFu16), Err(0xFFFF)); + } +} diff --git a/guest-builder/sp1/guest-asm/Cargo.lock b/guest-builder/sp1/guest-asm/Cargo.lock index 9f5e2438..e64476b8 100644 --- a/guest-builder/sp1/guest-asm/Cargo.lock +++ b/guest-builder/sp1/guest-asm/Cargo.lock @@ -1761,7 +1761,6 @@ version = "0.1.0" dependencies = [ "bitcoin", "borsh", - "serde", "ssz", "ssz_derive", "ssz_types", From e42662c9f83a7ab6ee2c128fb09244068a324cb8 Mon Sep 17 00:00:00 2001 From: Prajwol Gyawali Date: Fri, 24 Jul 2026 22:35:31 +0545 Subject: [PATCH 2/8] feat(params)!: make V0 the genesis spec version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SpecId previously started at V1 with genesis rules modeled as "no spec version active". Give the genesis rules their own version instead: V0 is always active since genesis and V1 becomes the first on-chain upgrade. This lets every ASM VK update name the version its artifact implements — including updates that will ship while still on the genesis spec — and gives gates a uniform is_active check. The schedule (SpecSchedule) encodes those rules structurally instead of one Option field per version: V0 is implicit and later versions only ever activate in succession, so a gapped or v0-disabled schedule is unrepresentable and a new SpecId variant is a one-line change — there is no per-version code to keep in sync. The serialized form keeps the per-version map ({"v0": 0, "v1": null}) and re-validates the invariants on load, so a hand-edited schedule fails fast. The raw identity narrows to u16 only: the discriminant is repr(u16), the u8 conversions are gone, and the primitive conversions are derived (num_enum, erroring with the raw id) so they cannot go stale when a variant is added. --- Cargo.lock | 28 +- Cargo.toml | 1 + crates/params/Cargo.toml | 2 + crates/params/src/lib.rs | 9 +- crates/params/src/runtime.rs | 300 ++++++++++++++---- crates/params/src/spec_id.rs | 113 ++++--- functional-tests/factory/common/asm_params.py | 13 +- guest-builder/sp1/guest-asm/Cargo.lock | 24 ++ 8 files changed, 361 insertions(+), 129 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2522e3ea..0fa5902f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4537,7 +4537,17 @@ version = "0.5.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1f646caf906c20226733ed5b1374287eb97e3c2a5c227ce668c1f2ce20ae57c9" dependencies = [ - "num_enum_derive", + "num_enum_derive 0.5.11", +] + +[[package]] +name = "num_enum" +version = "0.7.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d0bca838442ec211fa11de3a8b0e0e8f3a4522575b5c4c06ed722e005036f26" +dependencies = [ + "num_enum_derive 0.7.6", + "rustversion", ] [[package]] @@ -4552,6 +4562,18 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "num_enum_derive" +version = "0.7.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "680998035259dcfcafe653688bf2aa6d3e2dc05e98be6ab46afb089dc84f1df8" +dependencies = [ + "proc-macro-crate 3.5.0", + "proc-macro2", + "quote", + "syn 2.0.118", +] + [[package]] name = "number_prefix" version = "0.4.0" @@ -5770,7 +5792,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "efd079cd303257a4cb4e5aadfa79a7fe23f3c8301aa4740ccc3a99673485a352" dependencies = [ "downcast-rs", - "num_enum", + "num_enum 0.5.11", "paste", ] @@ -7727,6 +7749,7 @@ version = "0.1.0" dependencies = [ "arbitrary", "bitcoin", + "num_enum 0.7.6", "proptest", "serde", "serde_json", @@ -7739,6 +7762,7 @@ dependencies = [ "strata-identifiers", "strata-l1-txfmt", "strata-predicate", + "thiserror 2.0.18", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 0f162c3f..bf0b2d2c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -172,6 +172,7 @@ hex-literal = "1.1" jsonrpsee = "0.26.0" k256 = "0.13.4" musig2 = { version = "0.1.0", features = ["serde"] } +num_enum = "0.7" proptest = "1.9.0" rand = "0.8.5" rand_chacha = { version = "0.9.0", default-features = false } diff --git a/crates/params/Cargo.toml b/crates/params/Cargo.toml index 2b12972e..48fe6d6a 100644 --- a/crates/params/Cargo.toml +++ b/crates/params/Cargo.toml @@ -17,9 +17,11 @@ strata-predicate = { workspace = true, features = ["serde"] } arbitrary = { workspace = true, optional = true } bitcoin = { workspace = true, optional = true } +num_enum.workspace = true serde.workspace = true ssz.workspace = true ssz_derive.workspace = true +thiserror.workspace = true [dev-dependencies] proptest.workspace = true diff --git a/crates/params/src/lib.rs b/crates/params/src/lib.rs index 24e2d3d9..b2389056 100644 --- a/crates/params/src/lib.rs +++ b/crates/params/src/lib.rs @@ -13,7 +13,7 @@ mod subprotocols; #[cfg(feature = "arbitrary")] use arbitrary::{Arbitrary, Unstructured}; pub use genesis::AsmGenesisParams; -pub use runtime::{AsmRuntimeParams, AsmStfParams, SpecActivation}; +pub use runtime::{AsmRuntimeParams, AsmStfParams, SpecSchedule, SpecScheduleError}; use serde::{Deserialize, Serialize}; pub use spec_id::SpecId; pub use subprotocols::{ @@ -44,7 +44,7 @@ impl<'a> Arbitrary<'a> for AsmParams { Ok(Self { genesis: AsmGenesisParams::arbitrary(u)?, runtime: AsmRuntimeParams { - spec_activation: SpecActivation::all_disabled(), + spec_schedule: SpecSchedule::genesis(), }, }) } @@ -135,14 +135,15 @@ mod tests { } ], "spec_activation": { - "v1": 0 + "v0": 0, + "v1": null } } "#; let params: AsmParams = serde_json::from_str(raw_json).expect("deserialization from raw JSON should succeed"); - assert_eq!(params.runtime.spec_activation.v1, Some(0)); + assert_eq!(params.runtime.spec_schedule, SpecSchedule::genesis()); } #[cfg(feature = "arbitrary")] diff --git a/crates/params/src/runtime.rs b/crates/params/src/runtime.rs index c710fcac..74d79aa1 100644 --- a/crates/params/src/runtime.rs +++ b/crates/params/src/runtime.rs @@ -5,35 +5,83 @@ //! via params, with the invariant that every artifact agrees on every gate's //! outcome at every height it executes (see [`AsmStfParams`]). +use std::collections::BTreeMap; + use serde::{Deserialize, Serialize}; use strata_identifiers::L1Height; +use thiserror::Error; use crate::spec_id::SpecId; -/// Activation heights for every spec version. +/// The spec activation schedule: which versions are scheduled and from which +/// L1 height each one applies. +/// +/// [`SpecId::V0`] is the genesis version, always active from height 0; the +/// schedule only tracks the upgrades after it, as the activation heights of a +/// contiguous run of successors (`upgrades[i]` belongs to the version with +/// discriminant `i + 1`). Versions activate strictly in succession, so a +/// gapped schedule ("v2 scheduled, v1 disabled") is unrepresentable, and a +/// new [`SpecId`] variant needs no change here — every method derives its +/// answer from the discriminant. /// -/// A version with activation height `Some(n)` is active at L1 height `h` iff -/// `h >= n` — so `Some(0)` means active since genesis. `None` means disabled. -/// Proving artifacts bake one of the two extremes (`Some(0)` or `None` — an -/// artifact only ever executes one side of an upgrade boundary), while the -/// worker tracks the real activation height discovered from the ASM VK -/// upgrade log. +/// A version with activation height `n` is active at L1 height `h` iff +/// `h >= n`; versions past the scheduled run are disabled. Proving artifacts +/// bake one of the two extremes (`0` or unscheduled — an artifact only ever +/// executes one side of an upgrade boundary), while the worker tracks the +/// real activation heights discovered from the ASM VK upgrade log. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct SpecActivation { - /// Activation height of [`SpecId::V1`], or `None` if disabled. - pub v1: Option, +#[serde(try_from = "SpecScheduleRepr", into = "SpecScheduleRepr")] +pub struct SpecSchedule { + /// Activation height of each scheduled post-genesis version, indexed by + /// predecessor count: `upgrades[i]` activates discriminant `i + 1`. + upgrades: Vec, +} + +/// A schedule update that would violate [`SpecSchedule`]'s invariants. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Error)] +pub enum SpecScheduleError { + /// [`SpecId::V0`] is the genesis version: it is always active from height + /// 0 and can be neither rescheduled nor disabled. + #[error( + "v0 is the genesis version, always active from height 0; it cannot be rescheduled or disabled" + )] + GenesisFixed, + + /// Scheduling `spec` while its predecessor is unscheduled would leave a + /// gap in the activation sequence. + #[error( + "cannot schedule {spec:?} while its predecessor is unscheduled (latest scheduled: {latest:?})" + )] + Gap { + /// The version whose scheduling was rejected. + spec: SpecId, + /// The newest scheduled version at the time of the attempt. + latest: SpecId, + }, } -impl SpecActivation { - /// Schedule with every spec version disabled (no activation height). - pub const fn all_disabled() -> Self { - Self { v1: None } +impl SpecSchedule { + /// The genesis schedule: [`SpecId::V0`] active since genesis, every later + /// version unscheduled until an ASM VK upgrade activates it. + pub const fn genesis() -> Self { + Self { + upgrades: Vec::new(), + } + } + + /// Returns the newest scheduled version (regardless of whether its + /// activation height has been reached). Its successor is the version the + /// next ASM VK upgrade activates. + pub fn latest_scheduled(&self) -> SpecId { + SpecId::try_from(self.upgrades.len() as u16) + .expect("SpecSchedule invariant: every scheduled version has a SpecId variant") } - /// Returns the activation height of `spec`, or `None` if disabled. + /// Returns the activation height of `spec`, or `None` if unscheduled. pub fn activation_height_of(&self, spec: SpecId) -> Option { - match spec { - SpecId::V1 => self.v1, + match u16::from(spec) { + 0 => Some(0), + d => self.upgrades.get(usize::from(d) - 1).copied(), } } @@ -43,17 +91,96 @@ impl SpecActivation { .is_some_and(|activation| height >= activation) } - /// Sets the activation height of `spec`. - pub fn set_activation(&mut self, spec: SpecId, height: L1Height) { - match spec { - SpecId::V1 => self.v1 = Some(height), + /// Schedules the successor of the newest scheduled version at `height` + /// and returns which version that is. + /// + /// This is the discovery-side entry point: an enacted ASM VK upgrade does + /// not name the version it activates (the wire only carries the new VK), + /// so the activating version is *defined* as the successor. Errs with the + /// successor's raw id when this binary has no [`SpecId`] variant for it — + /// the caller is running old software past an upgrade it cannot execute. + pub fn schedule_successor(&mut self, height: L1Height) -> Result { + let successor = SpecId::try_from(self.upgrades.len() as u16 + 1)?; + self.upgrades.push(height); + Ok(successor) + } + + /// Schedules `spec` at `height`, overwriting its height if it is already + /// scheduled. + /// + /// This is the replay-side entry point, for re-applying a persisted + /// activation record (which *does* name its version) on top of a base + /// schedule. Unlike [`Self::schedule_successor`] it accepts already- + /// scheduled versions — the discovered height overrides the base — but + /// still rejects anything that would break the invariants: rescheduling + /// [`SpecId::V0`] or skipping past an unscheduled predecessor. + pub fn schedule(&mut self, spec: SpecId, height: L1Height) -> Result<(), SpecScheduleError> { + let idx = match usize::from(u16::from(spec)).checked_sub(1) { + None => return Err(SpecScheduleError::GenesisFixed), + Some(idx) => idx, + }; + if idx > self.upgrades.len() { + return Err(SpecScheduleError::Gap { + spec, + latest: self.latest_scheduled(), + }); + } + match self.upgrades.get_mut(idx) { + Some(slot) => *slot = height, + None => self.upgrades.push(height), } + Ok(()) } } -impl Default for SpecActivation { +impl Default for SpecSchedule { fn default() -> Self { - Self::all_disabled() + Self::genesis() + } +} + +/// Serialized form of [`SpecSchedule`]: one entry per known version, `null` +/// when unscheduled (e.g. `{"v0": 0, "v1": null}`). Kept for params-file +/// compatibility with the former per-version struct; conversion back +/// re-validates the invariants, so a hand-edited gapped or v0-disabled +/// schedule is rejected at load. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(transparent)] +struct SpecScheduleRepr(BTreeMap>); + +/// Every known version, in discriminant order. +fn known_versions() -> impl Iterator { + (0u16..).map_while(|d| SpecId::try_from(d).ok()) +} + +impl From for SpecScheduleRepr { + fn from(schedule: SpecSchedule) -> Self { + Self( + known_versions() + .map(|spec| (spec, schedule.activation_height_of(spec))) + .collect(), + ) + } +} + +impl TryFrom for SpecSchedule { + type Error = SpecScheduleError; + + fn try_from(repr: SpecScheduleRepr) -> Result { + let height_of = |spec| repr.0.get(&spec).copied().flatten(); + if height_of(SpecId::V0) != Some(0) { + return Err(SpecScheduleError::GenesisFixed); + } + let mut schedule = SpecSchedule::genesis(); + for spec in known_versions().skip(1) { + match height_of(spec) { + // An unscheduled version ends the run; `schedule` rejects any + // scheduled one after it as a gap. + None => continue, + Some(height) => schedule.schedule(spec, height)?, + } + } + Ok(schedule) } } @@ -65,30 +192,35 @@ impl Default for SpecActivation { /// into their closure, and the worker derives an effective copy from params /// plus discovered spec activations. /// -/// `Default` inherits [`SpecActivation`]'s default: everything disabled. +/// `Default` inherits [`SpecSchedule`]'s default: the genesis schedule. #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct AsmStfParams { - /// Spec activation schedule. - pub spec_activation: SpecActivation, + /// Spec activation schedule. The serde key keeps the name the params + /// format was born with. + #[serde(rename = "spec_activation")] + pub spec_schedule: SpecSchedule, } /// Runtime parameters of the state transition function. /// -/// `spec_activation` is the base activation schedule the worker starts from — +/// `spec_schedule` is the base activation schedule the worker starts from — /// the part that, on the proving side, is baked into guest programs as /// [`AsmStfParams`]. The worker overlays it with activations discovered from -/// enacted ASM VK upgrades, each of which names the spec version it activates. +/// enacted ASM VK upgrades, each of which activates the successor of the +/// newest scheduled version. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct AsmRuntimeParams { - /// Base spec activation schedule. - pub spec_activation: SpecActivation, + /// Base spec activation schedule. The serde key keeps the name the params + /// format was born with. + #[serde(rename = "spec_activation")] + pub spec_schedule: SpecSchedule, } impl AsmRuntimeParams { /// The STF-facing view of these params, before any dynamic activations. pub fn stf_params(&self) -> AsmStfParams { AsmStfParams { - spec_activation: self.spec_activation.clone(), + spec_schedule: self.spec_schedule.clone(), } } } @@ -97,61 +229,107 @@ impl AsmRuntimeParams { mod tests { use super::*; + /// A schedule with `SpecId::V1` activating at `height`. + fn v1_at(height: L1Height) -> SpecSchedule { + let mut schedule = SpecSchedule::genesis(); + assert_eq!(schedule.schedule_successor(height), Ok(SpecId::V1)); + schedule + } + #[test] fn is_active_boundaries() { - let activation = SpecActivation { v1: Some(100) }; - assert!(!activation.is_active(SpecId::V1, 99)); - assert!(activation.is_active(SpecId::V1, 100)); - assert!(activation.is_active(SpecId::V1, 101)); + let schedule = v1_at(100); + assert!(!schedule.is_active(SpecId::V1, 99)); + assert!(schedule.is_active(SpecId::V1, 100)); + assert!(schedule.is_active(SpecId::V1, 101)); + } + + #[test] + fn v0_is_always_active() { + let schedule = SpecSchedule::genesis(); + assert_eq!(schedule.activation_height_of(SpecId::V0), Some(0)); + assert!(schedule.is_active(SpecId::V0, 0)); + assert!(schedule.is_active(SpecId::V0, L1Height::MAX)); } #[test] - fn zero_means_always_active() { - let activation = SpecActivation { v1: Some(0) }; - assert!(activation.is_active(SpecId::V1, 0)); - assert!(activation.is_active(SpecId::V1, L1Height::MAX)); + fn unscheduled_means_never_active() { + let schedule = SpecSchedule::genesis(); + assert_eq!(schedule.activation_height_of(SpecId::V1), None); + assert!(!schedule.is_active(SpecId::V1, 0)); + assert!(!schedule.is_active(SpecId::V1, L1Height::MAX)); } #[test] - fn none_means_never_active() { - let activation = SpecActivation::all_disabled(); - assert_eq!(activation.activation_height_of(SpecId::V1), None); - assert!(!activation.is_active(SpecId::V1, 0)); - assert!(!activation.is_active(SpecId::V1, L1Height::MAX)); + fn latest_scheduled_is_the_newest_scheduled_version() { + assert_eq!(SpecSchedule::genesis().latest_scheduled(), SpecId::V0); + // A scheduled-but-unreached height still counts: the successor is + // relative to what the schedule knows, not to what is active yet. + assert_eq!(v1_at(L1Height::MAX).latest_scheduled(), SpecId::V1); } #[test] - fn set_activation_overrides() { - let mut activation = SpecActivation::all_disabled(); - activation.set_activation(SpecId::V1, 42); - assert_eq!(activation.activation_height_of(SpecId::V1), Some(42)); - assert!(activation.is_active(SpecId::V1, 42)); - assert!(!activation.is_active(SpecId::V1, 41)); + fn schedule_successor_chains_and_errs_past_known_versions() { + let mut schedule = SpecSchedule::genesis(); + assert_eq!(schedule.schedule_successor(42), Ok(SpecId::V1)); + assert_eq!(schedule.activation_height_of(SpecId::V1), Some(42)); + // Every known version is scheduled, so the next successor's raw id + // has no variant. + assert_eq!(schedule.schedule_successor(43), Err(2)); + assert_eq!(schedule, v1_at(42), "failed call must not mutate"); } #[test] - fn stf_params_serde_roundtrip() { + fn schedule_overwrites_but_pins_genesis() { + let mut schedule = v1_at(42); + schedule.schedule(SpecId::V1, 100).unwrap(); + assert_eq!(schedule.activation_height_of(SpecId::V1), Some(100)); + assert_eq!( + schedule.schedule(SpecId::V0, 7), + Err(SpecScheduleError::GenesisFixed) + ); + } + + #[test] + fn serde_keeps_the_per_version_map_format() { let params = AsmStfParams { - spec_activation: SpecActivation { v1: Some(7) }, + spec_schedule: v1_at(7), }; let json = serde_json::to_string(¶ms).unwrap(); - assert_eq!(json, r#"{"spec_activation":{"v1":7}}"#); + assert_eq!(json, r#"{"spec_activation":{"v0":0,"v1":7}}"#); let back: AsmStfParams = serde_json::from_str(&json).unwrap(); assert_eq!(back, params); - let disabled = AsmStfParams { - spec_activation: SpecActivation::all_disabled(), - }; - let json = serde_json::to_string(&disabled).unwrap(); - assert_eq!(json, r#"{"spec_activation":{"v1":null}}"#); + let genesis = AsmStfParams::default(); + let json = serde_json::to_string(&genesis).unwrap(); + assert_eq!(json, r#"{"spec_activation":{"v0":0,"v1":null}}"#); let back: AsmStfParams = serde_json::from_str(&json).unwrap(); - assert_eq!(back, disabled); + assert_eq!(back, genesis); + } + + #[test] + fn deserialize_rejects_invalid_schedules() { + // V0 disabled, missing, or moved off genesis. + for json in [ + r#"{"v0":null,"v1":null}"#, + r#"{"v1":7}"#, + r#"{"v0":5,"v1":null}"#, + ] { + assert!( + serde_json::from_str::(json).is_err(), + "{json}" + ); + } + // A version this binary has no variant for. + assert!(serde_json::from_str::(r#"{"v0":0,"v7":9}"#).is_err()); } #[test] fn test_runtime_params_deserialize() { let params: AsmRuntimeParams = - serde_json::from_str(r#"{"spec_activation":{"v1":5}}"#).unwrap(); - assert_eq!(params.stf_params().spec_activation.v1, Some(5)); + serde_json::from_str(r#"{"spec_activation":{"v0":0,"v1":5}}"#).unwrap(); + let schedule = params.stf_params().spec_schedule; + assert_eq!(schedule.activation_height_of(SpecId::V0), Some(0)); + assert_eq!(schedule.activation_height_of(SpecId::V1), Some(5)); } } diff --git a/crates/params/src/spec_id.rs b/crates/params/src/spec_id.rs index fd85395a..5398ded2 100644 --- a/crates/params/src/spec_id.rs +++ b/crates/params/src/spec_id.rs @@ -4,69 +4,63 @@ //! activation heights, so a single binary can execute both sides of an upgrade //! boundary. This module names the versions; the activation schedule that //! gates them lives with the runtime params as -//! [`SpecActivation`](crate::SpecActivation). +//! [`SpecSchedule`](crate::SpecSchedule). +use core::convert::identity; + +use num_enum::{IntoPrimitive, TryFromPrimitive}; use serde::{Deserialize, Serialize}; /// Identifies a spec version. /// -/// One variant per protocol upgrade, in activation order. The numeric -/// discriminant is the stable identity: it keys persisted spec-activation -/// records (stored as the raw discriminant byte) and is the raw id carried in -/// ASM VK upgrade actions. The variant name is the human-readable form: it is -/// this type's serde representation (snake_case) and is mirrored by the -/// [`SpecActivation`](crate::SpecActivation) params field. +/// One variant per protocol revision, in activation order, starting with the +/// genesis rules as [`SpecId::V0`]. The numeric discriminant is the stable +/// identity: it keys persisted spec-activation records and orders versions, +/// making "the successor of a version" well-defined. Discriminants MUST stay +/// contiguous from 0 — [`SpecSchedule`](crate::SpecSchedule) indexes its +/// activation heights by discriminant, so adding a variant is only this one +/// line, but a gap would desynchronize the schedule. The variant name is the +/// human-readable form: it is this type's serde representation (snake_case) +/// and keys the schedule's serialized form. /// -/// The id crosses two boundaries with opposite tolerances: +/// Nothing on the wire carries the id: an ASM VK upgrade action knows only +/// the new verifying key, so an artifact predating a spec version can still +/// parse and enact the upgrade that activates it. The consumer that must +/// *apply* the version's rules (the worker) instead derives each upgrade's +/// activating version via +/// [`SpecSchedule::schedule_successor`](crate::SpecSchedule::schedule_successor). +/// A successor it cannot map is not skipped: it means the worker is running +/// old software past an upgrade it cannot execute, so it MUST halt rather +/// than silently limp along on stale rules. /// -/// - Parse-time: ASM VK upgrade actions carry the raw id, not this enum, so an artifact predating a -/// spec version can still parse and enact the upgrade that activates it — the wire format never -/// requires knowing the version. -/// - Act-time: a consumer that must *apply* the version's rules (the worker) maps the id via -/// [`TryFrom`]. An id it does not know is not skipped: it means the worker is running old -/// software past an upgrade it cannot execute, so it MUST halt rather than silently limp along on -/// stale rules. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +/// The primitive conversions are derived so they cannot go stale when a +/// variant is added; [`TryFrom`] errs with the raw id it has no variant +/// for. +#[derive( + Debug, + Clone, + Copy, + PartialEq, + Eq, + PartialOrd, + Ord, + Hash, + Serialize, + Deserialize, + IntoPrimitive, + TryFromPrimitive, +)] #[serde(rename_all = "snake_case")] -#[repr(u8)] +#[num_enum(error_type(name = u16, constructor = identity))] +#[repr(u16)] pub enum SpecId { - /// First spec revision after genesis. Genesis rules are simply "no spec - /// version active". - V1 = 0, -} - -impl From for u8 { - fn from(spec: SpecId) -> Self { - spec as u8 - } -} - -impl From for u16 { - fn from(spec: SpecId) -> Self { - spec as u16 - } -} - -impl TryFrom for SpecId { - type Error = u8; + /// Genesis spec version: the rules in force from the genesis anchor + /// onward, active since genesis in every schedule. + V0 = 0, - fn try_from(value: u8) -> Result { - match value { - 0 => Ok(SpecId::V1), - invalid => Err(invalid), - } - } -} - -impl TryFrom for SpecId { - type Error = u16; - - fn try_from(value: u16) -> Result { - u8::try_from(value) - .ok() - .and_then(|v| SpecId::try_from(v).ok()) - .ok_or(value) - } + /// First protocol upgrade; placeholder name until that upgrade is + /// defined. + V1 = 1, } #[cfg(test)] @@ -78,7 +72,7 @@ mod tests { /// [`spec_id_u16_roundtrip`] instead. #[test] fn spec_id_serde_is_the_variant_name() { - assert_eq!(serde_json::to_string(&SpecId::V1).unwrap(), r#""v1""#); + assert_eq!(serde_json::to_string(&SpecId::V0).unwrap(), r#""v0""#); assert_eq!( serde_json::from_str::(r#""v1""#).unwrap(), SpecId::V1 @@ -87,11 +81,16 @@ mod tests { } /// Raw spec ids on the wire round-trip through the enum; unknown ids - /// surface as errors instead of misparsing. + /// surface as errors instead of misparsing. Pinning the *first* unknown + /// discriminant also guards contiguity: when a new variant lands, this + /// assertion fails and must be bumped alongside it. #[test] fn spec_id_u16_roundtrip() { - assert_eq!(u16::from(SpecId::V1), 0); - assert_eq!(SpecId::try_from(0u16).unwrap(), SpecId::V1); + assert_eq!(u16::from(SpecId::V0), 0); + assert_eq!(u16::from(SpecId::V1), 1); + assert_eq!(SpecId::try_from(0u16).unwrap(), SpecId::V0); + assert_eq!(SpecId::try_from(1u16).unwrap(), SpecId::V1); + assert_eq!(SpecId::try_from(2u16), Err(2)); assert_eq!(SpecId::try_from(0xFFFFu16), Err(0xFFFF)); } } diff --git a/functional-tests/factory/common/asm_params.py b/functional-tests/factory/common/asm_params.py index f042454a..746b6086 100644 --- a/functional-tests/factory/common/asm_params.py +++ b/functional-tests/factory/common/asm_params.py @@ -84,17 +84,20 @@ class AsmParams: magic: str anchor: L1Anchor subprotocols: list[dict[str, Any]] - # STF config: base spec activation schedule. `None` disables the spec - # version. Dynamic activations come from enacted ASM VK upgrades, which - # name the spec version they activate. - v1_height: int | None = 0 + # STF config: base spec schedule. V0 is the genesis spec, always active + # since genesis — the loader rejects any other v0 value. `None` leaves a + # later version unscheduled; versions only activate in succession (a + # gapped schedule is rejected). Dynamic activations of later versions + # come from enacted ASM VK upgrades. + v0_height: int = 0 + v1_height: int | None = None def to_dict(self) -> dict[str, Any]: return { "magic": self.magic, "anchor": asdict(self.anchor), "subprotocols": self.subprotocols, - "spec_activation": {"v1": self.v1_height}, + "spec_activation": {"v0": self.v0_height, "v1": self.v1_height}, } diff --git a/guest-builder/sp1/guest-asm/Cargo.lock b/guest-builder/sp1/guest-asm/Cargo.lock index e64476b8..79cb0356 100644 --- a/guest-builder/sp1/guest-asm/Cargo.lock +++ b/guest-builder/sp1/guest-asm/Cargo.lock @@ -993,6 +993,28 @@ dependencies = [ "libm", ] +[[package]] +name = "num_enum" +version = "0.7.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d0bca838442ec211fa11de3a8b0e0e8f3a4522575b5c4c06ed722e005036f26" +dependencies = [ + "num_enum_derive", + "rustversion", +] + +[[package]] +name = "num_enum_derive" +version = "0.7.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "680998035259dcfcafe653688bf2aa6d3e2dc05e98be6ab46afb089dc84f1df8" +dependencies = [ + "proc-macro-crate", + "proc-macro2", + "quote", + "syn 2.0.117", +] + [[package]] name = "once_cell" version = "1.21.4" @@ -1816,6 +1838,7 @@ version = "0.1.0" dependencies = [ "arbitrary", "bitcoin", + "num_enum", "serde", "ssz", "ssz_derive", @@ -1827,6 +1850,7 @@ dependencies = [ "strata-identifiers", "strata-l1-txfmt", "strata-predicate", + "thiserror", ] [[package]] From 72d2a42eedd1b108b3c240dcb34c50eb97dd08a4 Mon Sep 17 00:00:00 2001 From: Prajwol Gyawali Date: Fri, 24 Jul 2026 22:41:35 +0545 Subject: [PATCH 3/8] feat(storage): add sled-backed spec-activation store Discovered spec activations must survive worker restarts: the enacted update is dropped from the admin queue at enactment, so without a durable record a restarted worker could not reconstruct which versions the chain has activated (or the VK each boundary switched to). Keyed by (enacting_height, version) with a big-endian height prefix so put is an idempotent overwrite under crash-replay and prune_after (the reorg path) is a range scan. The predicate is the value, borsh-encoded. The version is stored and returned as its raw u16 id: mapping it to a known spec version is act-time worker logic, so the store stays opaque to it (the worker owns the typed record). --- Cargo.lock | 2 + crates/storage/Cargo.toml | 1 + crates/storage/src/lib.rs | 8 +- crates/storage/src/sled/mod.rs | 3 +- crates/storage/src/sled/spec_activation.rs | 178 +++++++++++++++++++++ crates/storage/src/spec_activation.rs | 48 ++++++ 6 files changed, 238 insertions(+), 2 deletions(-) create mode 100644 crates/storage/src/sled/spec_activation.rs create mode 100644 crates/storage/src/spec_activation.rs diff --git a/Cargo.lock b/Cargo.lock index 0fa5902f..00ffd200 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -804,6 +804,7 @@ dependencies = [ "strata-identifiers", "strata-merkle", "strata-merkle-node-store", + "strata-predicate", "tempfile", ] @@ -8490,6 +8491,7 @@ version = "0.1.0" source = "git+https://github.com/alpenlabs/strata-common?tag=v0.3.0-rc.2#c3698c0f26d8d3d16e56a478eb375e077d813775" dependencies = [ "arbitrary", + "borsh", "hex", "k256", "serde", diff --git a/crates/storage/Cargo.toml b/crates/storage/Cargo.toml index d129f2bc..de2e4b63 100644 --- a/crates/storage/Cargo.toml +++ b/crates/storage/Cargo.toml @@ -11,6 +11,7 @@ strata-asm-common.workspace = true strata-identifiers.workspace = true strata-merkle = { workspace = true, features = ["ssz"] } strata-merkle-node-store.workspace = true +strata-predicate = { workspace = true, features = ["borsh"] } anyhow.workspace = true borsh.workspace = true diff --git a/crates/storage/src/lib.rs b/crates/storage/src/lib.rs index 24392992..94280a7b 100644 --- a/crates/storage/src/lib.rs +++ b/crates/storage/src/lib.rs @@ -10,6 +10,8 @@ //! - [`AsmAuxDataDb`] / [`SledAsmAuxDataDb`] — auxiliary data, keyed by block commitment //! - [`AsmManifestDb`] / [`SledAsmManifestDb`] — full manifests, keyed by block commitment //! - [`AsmManifestMmrDb`] / [`SledAsmManifestMmrDb`] — manifest hash MMR, keyed by L1 height +//! - [`AsmSpecActivationDb`] / [`SledSpecActivationDb`] — discovered spec activations, keyed by +//! `(enacting height, version)` //! //! The commitment-keyed stores ([`AsmStateDb`], [`AsmAuxDataDb`], //! [`AsmManifestDb`]) key each entry by its [`L1BlockCommitment`] — height plus @@ -30,10 +32,14 @@ mod aux; mod manifest; mod manifest_mmr; mod sled; +mod spec_activation; mod state; pub use aux::AsmAuxDataDb; pub use manifest::AsmManifestDb; pub use manifest_mmr::AsmManifestMmrDb; -pub use sled::{SledAsmAuxDataDb, SledAsmManifestDb, SledAsmManifestMmrDb, SledAsmStateDb}; +pub use sled::{ + SledAsmAuxDataDb, SledAsmManifestDb, SledAsmManifestMmrDb, SledAsmStateDb, SledSpecActivationDb, +}; +pub use spec_activation::{AsmSpecActivationDb, RawSpecActivation}; pub use state::AsmStateDb; diff --git a/crates/storage/src/sled/mod.rs b/crates/storage/src/sled/mod.rs index 6f658c27..c13284ee 100644 --- a/crates/storage/src/sled/mod.rs +++ b/crates/storage/src/sled/mod.rs @@ -12,11 +12,12 @@ use strata_identifiers::{Buf32, L1BlockCommitment, L1BlockId}; mod aux; mod manifest; mod manifest_mmr; +mod spec_activation; mod state; pub use self::{ aux::SledAsmAuxDataDb, manifest::SledAsmManifestDb, manifest_mmr::SledAsmManifestMmrDb, - state::SledAsmStateDb, + spec_activation::SledSpecActivationDb, state::SledAsmStateDb, }; // ── Key encoding ────────────────────────────────────────────────────── diff --git a/crates/storage/src/sled/spec_activation.rs b/crates/storage/src/sled/spec_activation.rs new file mode 100644 index 00000000..7337fa7c --- /dev/null +++ b/crates/storage/src/sled/spec_activation.rs @@ -0,0 +1,178 @@ +//! [`AsmSpecActivationDb`] implementation backed by sled. + +use anyhow::{Context, Result}; +use strata_identifiers::L1Height; +use strata_predicate::PredicateKey; + +use crate::spec_activation::{AsmSpecActivationDb, RawSpecActivation}; + +/// Size of an encoded activation key: 4-byte BE enacting height + 2-byte BE +/// spec version. +const ENCODED_KEY_SIZE: usize = 4 + 2; + +/// Sled-backed [`AsmSpecActivationDb`] keyed by `(enacting_height, version)`, +/// with the enacted predicate as the value. +/// +/// The composite key allows several spec activations at one enacting height; +/// the big-endian height prefix keeps sled's lexicographic ordering aligned +/// with height ordering so `prune_after` can range-scan. +#[derive(Debug, Clone)] +pub struct SledSpecActivationDb { + activations: sled::Tree, +} + +impl SledSpecActivationDb { + /// Opens or creates the spec-activation tree in the given sled instance. + pub fn open(db: &sled::Db) -> Result { + Ok(Self { + activations: db.open_tree("asm_spec_activations")?, + }) + } + + /// Synchronous variant of [`AsmSpecActivationDb::put`]. The ASM worker + /// runs on a sync thread (via `ServiceBuilder::launch_sync`), where + /// awaiting is not possible; calling this directly avoids that. + pub fn put( + &self, + enacting_height: L1Height, + version: u16, + new_predicate: &PredicateKey, + ) -> Result<()> { + let key = encode_key(enacting_height, version); + let value = borsh::to_vec(new_predicate)?; + self.activations.insert(key, value)?; + Ok(()) + } + + /// Synchronous variant of [`AsmSpecActivationDb::list`]. See [`Self::put`]. + pub fn list(&self) -> Result> { + self.activations + .iter() + .map(|entry| { + let (key, value) = entry?; + decode_entry(&key, &value) + }) + .collect() + } + + /// Synchronous variant of [`AsmSpecActivationDb::prune_after`]. See + /// [`Self::put`]. + pub fn prune_after(&self, after_height: L1Height) -> Result<()> { + let Some(first_removed) = after_height.checked_add(1) else { + return Ok(()); + }; + let lower: &[u8] = &first_removed.to_be_bytes(); + for entry in self.activations.range(lower..) { + let (key, _) = entry?; + self.activations.remove(&key)?; + } + Ok(()) + } +} + +impl AsmSpecActivationDb for SledSpecActivationDb { + type Error = anyhow::Error; + + async fn put( + &self, + enacting_height: L1Height, + version: u16, + new_predicate: &PredicateKey, + ) -> Result<()> { + self.put(enacting_height, version, new_predicate) + } + + async fn list(&self) -> Result> { + self.list() + } + + async fn prune_after(&self, after_height: L1Height) -> Result<()> { + self.prune_after(after_height) + } +} + +/// Encodes an activation key as `[enacting_height_be(4)][version_be(2)]`. +fn encode_key(enacting_height: L1Height, version: u16) -> [u8; ENCODED_KEY_SIZE] { + let mut buf = [0u8; ENCODED_KEY_SIZE]; + buf[0..4].copy_from_slice(&enacting_height.to_be_bytes()); + buf[4..6].copy_from_slice(&version.to_be_bytes()); + buf +} + +/// Decodes a tree entry back into a [`RawSpecActivation`]. +fn decode_entry(key: &[u8], value: &[u8]) -> Result { + let enacting_height = L1Height::from_be_bytes( + key[0..4] + .try_into() + .context("spec activation key shorter than 4 bytes")?, + ); + let version = u16::from_be_bytes( + key[4..6] + .try_into() + .context("spec activation key shorter than 6 bytes")?, + ); + let new_predicate = borsh::from_slice::(value) + .context("malformed predicate in spec activation store")?; + Ok((enacting_height, version, new_predicate)) +} + +#[cfg(test)] +mod tests { + use strata_predicate::PredicateTypeId; + + use super::*; + use crate::sled::test_util::test_db; + + const VERSION: u16 = 1; + + /// A per-height predicate, so roundtrip failures can't hide behind a + /// shared constant. + fn predicate(enacting_height: L1Height) -> PredicateKey { + PredicateKey::new(PredicateTypeId::AlwaysAccept, vec![enacting_height as u8]) + } + + fn activation(enacting_height: L1Height) -> RawSpecActivation { + (enacting_height, VERSION, predicate(enacting_height)) + } + + fn put(store: &SledSpecActivationDb, enacting_height: L1Height) { + store + .put(enacting_height, VERSION, &predicate(enacting_height)) + .unwrap(); + } + + #[test] + fn put_list_roundtrip() { + let (db, _dir) = test_db(); + let store = SledSpecActivationDb::open(&db).unwrap(); + + put(&store, 7); + put(&store, 3); + + // Ascending by enacting height regardless of insertion order. + assert_eq!(store.list().unwrap(), vec![activation(3), activation(7)]); + } + + #[test] + fn put_is_idempotent() { + let (db, _dir) = test_db(); + let store = SledSpecActivationDb::open(&db).unwrap(); + + put(&store, 5); + put(&store, 5); + assert_eq!(store.list().unwrap(), vec![activation(5)]); + } + + #[test] + fn prune_after_drops_only_higher_entries() { + let (db, _dir) = test_db(); + let store = SledSpecActivationDb::open(&db).unwrap(); + + put(&store, 3); + put(&store, 5); + put(&store, 6); + + store.prune_after(5).unwrap(); + assert_eq!(store.list().unwrap(), vec![activation(3), activation(5)]); + } +} diff --git a/crates/storage/src/spec_activation.rs b/crates/storage/src/spec_activation.rs new file mode 100644 index 00000000..4b6ce38d --- /dev/null +++ b/crates/storage/src/spec_activation.rs @@ -0,0 +1,48 @@ +//! Storage trait for discovered spec activations. +//! +//! Each row says "the block at `enacting_height` enacted an ASM VK upgrade +//! that activates spec version `version`", carrying the predicate the upgrade +//! switched the ASM STF to. The version travels as its raw u16 id: mapping it +//! to a known spec version is act-time worker logic, so the store passes it +//! through opaquely (the worker's `SpecActivationRecord` is the typed form). +//! The worker persists a row *before* committing the enacting block's anchor +//! state, so an activation can never lag a committed anchor, and prunes rows +//! above the fork point when a reorg abandons the enacting block. + +use std::fmt::Debug; + +use strata_identifiers::L1Height; +use strata_predicate::PredicateKey; + +/// A stored spec activation row: the enacting height, the raw spec version +/// id, and the predicate the upgrade enacted. +pub type RawSpecActivation = (L1Height, u16, PredicateKey); + +/// Persistence interface for spec-activation rows. +/// +/// Async methods with an associated error type. +pub trait AsmSpecActivationDb { + /// The error type returned by database operations. + type Error: Debug; + + /// Stores a spec activation, keyed by `(enacting_height, version)`. + /// + /// Idempotent: replaying the enacting block rewrites the same row. + fn put( + &self, + enacting_height: L1Height, + version: u16, + new_predicate: &PredicateKey, + ) -> impl Future> + Send; + + /// Returns every stored activation, ascending by enacting height. + fn list(&self) -> impl Future, Self::Error>> + Send; + + /// Removes all activations whose enacting height is strictly above + /// `after_height` (which is kept). Used on reorgs to drop activations + /// enacted on the abandoned branch. + fn prune_after( + &self, + after_height: L1Height, + ) -> impl Future> + Send; +} From f315dffa15ff41e8c299b6e4e751847a42aefe8e Mon Sep 17 00:00:00 2001 From: Prajwol Gyawali Date: Fri, 24 Jul 2026 22:43:58 +0545 Subject: [PATCH 4/8] feat(worker): add spec-activation persistence as a context concern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New SpecActivationStore concern on the worker context, wired to the sled store in the runner and the test context. The SpecActivationRecord it persists lives here rather than in strata-asm-common: a discovered activation is worker bookkeeping, not protocol surface (keeping it out of common also keeps strata-predicate out of the guest's common deps). The store speaks raw parts — the context impls map the raw version id back through SpecId on load, which is where act-time mapping belongs. Record-before-anchor-commit and prune-above-base are contracts the discovery logic (next commit) relies on for crash and reorg safety; they are documented on the trait so every backend upholds them. --- Cargo.lock | 2 + bin/asm-runner/src/bootstrap.rs | 2 + bin/asm-runner/src/storage.rs | 6 +- bin/asm-runner/src/worker_context.rs | 58 +++++++++++-- crates/worker/Cargo.toml | 2 + crates/worker/src/lib.rs | 5 +- crates/worker/src/test_utils.rs | 51 ++++++++++- crates/worker/src/traits.rs | 124 ++++++++++++++++++++++++--- 8 files changed, 229 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 00ffd200..b6e91ecb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8232,12 +8232,14 @@ dependencies = [ "serde", "sled", "strata-asm-common", + "strata-asm-params", "strata-asm-stf", "strata-btc-types", "strata-btc-verification", "strata-identifiers", "strata-l1-txfmt", "strata-merkle", + "strata-predicate", "strata-service", "strata-tasks", "strata-test-utils-btcio", diff --git a/bin/asm-runner/src/bootstrap.rs b/bin/asm-runner/src/bootstrap.rs index 484a2b13..72b62b80 100644 --- a/bin/asm-runner/src/bootstrap.rs +++ b/bin/asm-runner/src/bootstrap.rs @@ -33,6 +33,7 @@ pub(crate) async fn bootstrap( aux_db, manifest_db, mmr_db, + spec_activation_db, } = create_asm_storage(&config.database.asm_path)?; let MohoStorage { state_db: moho_state_db, @@ -67,6 +68,7 @@ pub(crate) async fn bootstrap( aux_db.clone(), manifest_db.clone(), mmr_db.clone(), + spec_activation_db, ); // 5. Launch ASM worker. diff --git a/bin/asm-runner/src/storage.rs b/bin/asm-runner/src/storage.rs index 4fba0f77..15e44454 100644 --- a/bin/asm-runner/src/storage.rs +++ b/bin/asm-runner/src/storage.rs @@ -9,7 +9,9 @@ use std::{path::Path, sync::Arc}; use anyhow::Result; -use asm_storage::{SledAsmAuxDataDb, SledAsmManifestDb, SledAsmManifestMmrDb, SledAsmStateDb}; +use asm_storage::{ + SledAsmAuxDataDb, SledAsmManifestDb, SledAsmManifestMmrDb, SledAsmStateDb, SledSpecActivationDb, +}; use strata_asm_moho_storage::{SledExportEntriesDb, SledMohoStateDb}; use strata_asm_prover_storage::SledProofDb; @@ -19,6 +21,7 @@ pub(crate) struct AsmStorage { pub aux_db: Arc, pub manifest_db: Arc, pub mmr_db: Arc, + pub spec_activation_db: Arc, } /// Moho storage backends, both opened on the Moho sled database. @@ -35,6 +38,7 @@ pub(crate) fn create_asm_storage(path: &Path) -> Result { aux_db: Arc::new(SledAsmAuxDataDb::open(&db)?), manifest_db: Arc::new(SledAsmManifestDb::open(&db)?), mmr_db: Arc::new(SledAsmManifestMmrDb::open(&db)?), + spec_activation_db: Arc::new(SledSpecActivationDb::open(&db)?), }) } diff --git a/bin/asm-runner/src/worker_context.rs b/bin/asm-runner/src/worker_context.rs index 640411e2..29eb55c4 100644 --- a/bin/asm-runner/src/worker_context.rs +++ b/bin/asm-runner/src/worker_context.rs @@ -1,21 +1,25 @@ //! Worker-context trait implementations for the ASM runner. //! -//! Implements the four [`WorkerContext`](strata_asm_worker::WorkerContext) +//! Implements the five [`WorkerContext`](strata_asm_worker::WorkerContext) //! concern traits ([`L1DataProvider`], [`AnchorStateStore`], -//! [`ManifestMmrStore`], [`AuxDataStore`]) for [`AsmWorkerContext`]. +//! [`ManifestMmrStore`], [`AuxDataStore`], [`SpecActivationStore`]) for +//! [`AsmWorkerContext`]. use std::sync::Arc; -use anyhow::Context; -use asm_storage::{SledAsmAuxDataDb, SledAsmManifestDb, SledAsmManifestMmrDb, SledAsmStateDb}; +use anyhow::{Context, anyhow}; +use asm_storage::{ + SledAsmAuxDataDb, SledAsmManifestDb, SledAsmManifestMmrDb, SledAsmStateDb, SledSpecActivationDb, +}; use bitcoin::{Block, BlockHash, Network, block::Header}; use bitcoind_async_client::{Client, traits::Reader}; use strata_asm_common::{AnchorState, AsmManifest, AsmManifestHash, AuxData}; use strata_asm_worker::{ - AnchorStateStore, AuxDataStore, L1DataProvider, ManifestMmrStore, WorkerError, WorkerResult, + AnchorStateStore, AuxDataStore, L1DataProvider, ManifestMmrStore, SpecActivationRecord, + SpecActivationStore, WorkerError, WorkerResult, }; use strata_btc_types::{BitcoinTxid, L1BlockIdBitcoinExt, RawBitcoinTx}; -use strata_identifiers::{L1BlockCommitment, L1BlockId}; +use strata_identifiers::{L1BlockCommitment, L1BlockId, L1Height}; use strata_merkle::MerkleProofB32; use tokio::runtime::Handle; @@ -37,9 +41,14 @@ pub(crate) struct AsmWorkerContext { aux_db: Arc, manifest_db: Arc, mmr_db: Arc, + spec_activation_db: Arc, } impl AsmWorkerContext { + #[expect( + clippy::too_many_arguments, + reason = "one argument per storage concern" + )] pub(crate) fn new( runtime_handle: Handle, bitcoin_client: Arc, @@ -48,6 +57,7 @@ impl AsmWorkerContext { aux_db: Arc, manifest_db: Arc, mmr_db: Arc, + spec_activation_db: Arc, ) -> Self { Self { runtime_handle, @@ -58,6 +68,7 @@ impl AsmWorkerContext { aux_db, manifest_db, mmr_db, + spec_activation_db, } } } @@ -219,6 +230,41 @@ impl ManifestMmrStore for AsmWorkerContext { } } +impl SpecActivationStore for AsmWorkerContext { + fn record_spec_activation(&self, activation: SpecActivationRecord) -> WorkerResult<()> { + self.spec_activation_db + .put( + activation.enacting_height, + activation.version.into(), + &activation.new_predicate, + ) + .map_err(WorkerError::DbError) + } + + fn list_spec_activations(&self) -> WorkerResult> { + self.spec_activation_db + .list() + .map_err(WorkerError::DbError)? + .into_iter() + .map(|(enacting_height, version, new_predicate)| { + SpecActivationRecord::from_raw(enacting_height, version, new_predicate).map_err( + |id| { + WorkerError::DbError(anyhow!( + "unknown spec version {id} in spec activation store" + )) + }, + ) + }) + .collect() + } + + fn prune_spec_activations_after(&self, after_height: L1Height) -> WorkerResult<()> { + self.spec_activation_db + .prune_after(after_height) + .map_err(WorkerError::DbError) + } +} + impl AuxDataStore for AsmWorkerContext { fn store_aux_data(&self, blockid: &L1BlockCommitment, data: &AuxData) -> WorkerResult<()> { self.aux_db.put(blockid, data).map_err(WorkerError::DbError) diff --git a/crates/worker/Cargo.toml b/crates/worker/Cargo.toml index b5e57c9c..bfc5659d 100644 --- a/crates/worker/Cargo.toml +++ b/crates/worker/Cargo.toml @@ -8,11 +8,13 @@ workspace = true [dependencies] strata-asm-common.workspace = true +strata-asm-params.workspace = true strata-asm-stf.workspace = true strata-btc-types.workspace = true strata-btc-verification.workspace = true strata-identifiers.workspace = true strata-merkle = { workspace = true, features = ["ssz"] } +strata-predicate.workspace = true strata-service.workspace = true strata-tasks.workspace = true diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index f4eacf63..354795cd 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -26,4 +26,7 @@ pub use service::{AsmWorkerService, AsmWorkerStatus}; pub use state::AsmWorkerServiceState; pub use subscription::{Subscribers, Subscription}; pub use sync::{SyncError, SyncPlan, plan_sync}; -pub use traits::{AnchorStateStore, AuxDataStore, L1DataProvider, ManifestMmrStore, WorkerContext}; +pub use traits::{ + AnchorStateStore, AuxDataStore, L1DataProvider, ManifestMmrStore, SpecActivationRecord, + SpecActivationStore, WorkerContext, +}; diff --git a/crates/worker/src/test_utils.rs b/crates/worker/src/test_utils.rs index 003d48d2..045a524e 100644 --- a/crates/worker/src/test_utils.rs +++ b/crates/worker/src/test_utils.rs @@ -11,19 +11,23 @@ use std::sync::Arc; -use asm_storage::{SledAsmAuxDataDb, SledAsmManifestDb, SledAsmManifestMmrDb, SledAsmStateDb}; +use anyhow::anyhow; +use asm_storage::{ + SledAsmAuxDataDb, SledAsmManifestDb, SledAsmManifestMmrDb, SledAsmStateDb, SledSpecActivationDb, +}; use bitcoin::{Block, BlockHash, Network, Txid, block::Header, params::Params}; use bitcoind_async_client::{Client, traits::Reader}; use strata_asm_common::{AnchorState, AsmManifest, AsmManifestHash}; use strata_btc_types::{BitcoinTxid, BlockHashExt, L1BlockIdBitcoinExt, RawBitcoinTx}; use strata_btc_verification::{L1Anchor, get_relative_difficulty_adjustment_height}; -use strata_identifiers::{L1BlockCommitment, L1BlockId}; +use strata_identifiers::{L1BlockCommitment, L1BlockId, L1Height}; use strata_merkle::MerkleProofB32; use tempfile::TempDir; use tokio::{runtime::Handle, task::block_in_place}; use crate::{ - AnchorStateStore, AuxDataStore, L1DataProvider, ManifestMmrStore, WorkerError, WorkerResult, + AnchorStateStore, AuxDataStore, L1DataProvider, ManifestMmrStore, SpecActivationRecord, + SpecActivationStore, WorkerError, WorkerResult, }; /// Sled-backed state stores for the test worker context. @@ -39,6 +43,7 @@ pub struct AsmWorkerState { aux_db: SledAsmAuxDataDb, manifest_db: SledAsmManifestDb, mmr_db: SledAsmManifestMmrDb, + spec_activation_db: SledSpecActivationDb, /// Temp dir the sled database lives in; deleted when this is dropped. _tempdir: TempDir, } @@ -75,6 +80,7 @@ impl TestAsmWorkerContext { let aux_db = SledAsmAuxDataDb::open(&db).expect("open aux db"); let manifest_db = SledAsmManifestDb::open(&db).expect("open manifest db"); let mmr_db = SledAsmManifestMmrDb::open(&db).expect("open manifest mmr db"); + let spec_activation_db = SledSpecActivationDb::open(&db).expect("open spec activation db"); Self { client: Arc::new(client), @@ -84,6 +90,7 @@ impl TestAsmWorkerContext { aux_db, manifest_db, mmr_db, + spec_activation_db, _tempdir: tempdir, }), } @@ -254,6 +261,44 @@ impl ManifestMmrStore for TestAsmWorkerContext { } } +impl SpecActivationStore for TestAsmWorkerContext { + fn record_spec_activation(&self, activation: SpecActivationRecord) -> WorkerResult<()> { + self.state + .spec_activation_db + .put( + activation.enacting_height, + activation.version.into(), + &activation.new_predicate, + ) + .map_err(WorkerError::DbError) + } + + fn list_spec_activations(&self) -> WorkerResult> { + self.state + .spec_activation_db + .list() + .map_err(WorkerError::DbError)? + .into_iter() + .map(|(enacting_height, version, new_predicate)| { + SpecActivationRecord::from_raw(enacting_height, version, new_predicate).map_err( + |id| { + WorkerError::DbError(anyhow!( + "unknown spec version {id} in spec activation store" + )) + }, + ) + }) + .collect() + } + + fn prune_spec_activations_after(&self, after_height: L1Height) -> WorkerResult<()> { + self.state + .spec_activation_db + .prune_after(after_height) + .map_err(WorkerError::DbError) + } +} + impl AuxDataStore for TestAsmWorkerContext { fn store_aux_data( &self, diff --git a/crates/worker/src/traits.rs b/crates/worker/src/traits.rs index e579bd5f..fb3930f0 100644 --- a/crates/worker/src/traits.rs +++ b/crates/worker/src/traits.rs @@ -1,15 +1,16 @@ //! Traits for the chain worker to interface with the underlying system. //! -//! The worker's dependencies split into four concerns, each backed by a +//! The worker's dependencies split into five concerns, each backed by a //! distinct subsystem in production: //! //! - [`L1DataProvider`] — reads L1 data from the Bitcoin node (blocks, txs, network). //! - [`AnchorStateStore`] — persists and loads the [`AnchorState`]. //! - [`ManifestMmrStore`] — manifest persistence and the manifest-hash MMR. //! - [`AuxDataStore`] — per-block [`AuxData`] for prover consumption. +//! - [`SpecActivationStore`] — spec activations discovered from ASM VK upgrade logs. //! -//! [`WorkerContext`] is the umbrella that combines all four. It has a blanket -//! impl, so an implementor just implements the four concern traits and gets +//! [`WorkerContext`] is the umbrella that combines all five. It has a blanket +//! impl, so an implementor just implements the five concern traits and gets //! `WorkerContext` for free; consumers that only need one concern can depend on //! the narrower trait instead of the whole context. @@ -17,12 +18,62 @@ use bitcoin::{Block, Network, block::Header}; use strata_asm_common::{ AnchorState, AsmManifest, AsmManifestHash, AuxData, MMR_SENTINEL_DUMMY_LEAF, }; +use strata_asm_params::SpecId; use strata_btc_types::{BitcoinTxid, RawBitcoinTx}; -use strata_identifiers::{L1BlockCommitment, L1BlockId}; +use strata_identifiers::{L1BlockCommitment, L1BlockId, L1Height}; use strata_merkle::MerkleProofB32; +use strata_predicate::PredicateKey; use crate::WorkerResult; +/// A discovered spec activation. +/// +/// Records that the block at `enacting_height` enacted an ASM VK upgrade +/// whose new artifact implements `version`, activating it from the next block +/// onward and switching the ASM STF predicate to `new_predicate`. +/// +/// Worker bookkeeping, not protocol surface: the chain only carries the raw +/// version id in the enacted update's log, and the store persists it that +/// way; this is the act-time form, with the id mapped through [`SpecId`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SpecActivationRecord { + /// Height of the L1 block whose ASM VK upgrade enactment triggered the + /// activation. + pub enacting_height: L1Height, + + /// The activated spec version. + pub version: SpecId, + + /// The ASM STF predicate the upgrade enacted. Enactment removes the + /// update from the admin queue and only surfaces it in the emitted log, + /// so this record is the worker's durable copy of the VK the boundary + /// switched to. + pub new_predicate: PredicateKey, +} + +impl SpecActivationRecord { + /// Reassembles a record from its raw stored parts, mapping the raw + /// version id through [`SpecId`]. Errs with the raw id when this binary + /// has no variant for it. + pub fn from_raw( + enacting_height: L1Height, + version: u16, + new_predicate: PredicateKey, + ) -> Result { + Ok(Self { + enacting_height, + version: SpecId::try_from(version)?, + new_predicate, + }) + } + + /// Height from which the version's rules apply: the block after the + /// enacting one. + pub fn activation_height(&self) -> L1Height { + self.enacting_height.saturating_add(1) + } +} + /// Reads L1 data from the backing Bitcoin source. pub trait L1DataProvider { /// Fetches a Bitcoin [`Block`] at a given height. @@ -187,18 +238,71 @@ pub trait AuxDataStore { fn get_aux_data(&self, blockid: &L1BlockCommitment) -> WorkerResult; } +/// Persists spec activations discovered from ASM VK upgrade logs. +pub trait SpecActivationStore { + /// Records a discovered spec activation. + /// + /// Called *before* the enacting block's anchor state is committed, so an + /// activation can never lag a committed anchor. Idempotent: crash-replay + /// of the enacting block rewrites the same record. + fn record_spec_activation(&self, activation: SpecActivationRecord) -> WorkerResult<()>; + + /// Returns every recorded activation, ascending by enacting height. + fn list_spec_activations(&self) -> WorkerResult>; + + /// Removes activations whose enacting height is strictly above + /// `after_height` (which is kept). Called on reorgs so activations enacted + /// on the abandoned branch are dropped; re-processing the new branch + /// re-discovers any that survive. + fn prune_spec_activations_after(&self, after_height: L1Height) -> WorkerResult<()>; +} + /// Context trait for a worker to interact with the database and Bitcoin Client. /// -/// Umbrella over the four concern traits ([`L1DataProvider`], -/// [`AnchorStateStore`], [`ManifestMmrStore`], [`AuxDataStore`]). The blanket -/// impl means any type that implements all four automatically implements -/// `WorkerContext`, so implementors never name it directly. +/// Umbrella over the five concern traits ([`L1DataProvider`], +/// [`AnchorStateStore`], [`ManifestMmrStore`], [`AuxDataStore`], +/// [`SpecActivationStore`]). The blanket impl means any type that implements +/// all five automatically implements `WorkerContext`, so implementors never +/// name it directly. pub trait WorkerContext: - L1DataProvider + AnchorStateStore + ManifestMmrStore + AuxDataStore + L1DataProvider + AnchorStateStore + ManifestMmrStore + AuxDataStore + SpecActivationStore { } impl WorkerContext for T where - T: L1DataProvider + AnchorStateStore + ManifestMmrStore + AuxDataStore + T: L1DataProvider + AnchorStateStore + ManifestMmrStore + AuxDataStore + SpecActivationStore { } + +#[cfg(test)] +mod tests { + use strata_predicate::PredicateTypeId; + + use super::*; + + fn predicate() -> PredicateKey { + PredicateKey::new(PredicateTypeId::AlwaysAccept, vec![]) + } + + #[test] + fn activation_is_block_after_enactment() { + let record = SpecActivationRecord { + enacting_height: 41, + version: SpecId::V1, + new_predicate: predicate(), + }; + assert_eq!(record.activation_height(), 42); + } + + /// Raw stored parts round-trip through the typed record; an id this + /// binary has no variant for surfaces as an error instead of misparsing. + #[test] + fn from_raw_maps_known_versions_only() { + let record = SpecActivationRecord::from_raw(41, SpecId::V1.into(), predicate()).unwrap(); + assert_eq!(record.version, SpecId::V1); + assert_eq!( + SpecActivationRecord::from_raw(41, 0xBEEF, predicate()), + Err(0xBEEF) + ); + } +} From 0b36872a055d8ef7e91e548227f2053a1b9201e7 Mon Sep 17 00:00:00 2001 From: Prajwol Gyawali Date: Fri, 24 Jul 2026 22:51:50 +0545 Subject: [PATCH 5/8] feat(worker): discover spec activations from ASM VK upgrade logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The worker maintains an effective spec schedule: the base from runtime params (a new required builder input) overlaid with every activation discovered from enacted AsmStfUpdate logs. The log carries no version — future versions must be enactable by artifacts that predate them, so the wire cannot require knowing them. Instead each enacted upgrade activates the successor of the newest scheduled version (SpecSchedule::schedule_successor). A successor this binary has no variant for halts the worker without committing the block: it is running old software past an upgrade it cannot execute, and limping along on stale rules would silently diverge from the chain. Nothing is persisted for the rejected block, so a restart with a newer image simply retries it. Replaying persisted activations onto the base schedule is validated the same way: a record the schedule cannot fit (e.g. params downgraded below activations already committed) fails startup instead of silently producing a gapped schedule. Crash/reorg safety is carried entirely by ordering: the activation record is written before the enacting block's anchor commit (a committed anchor can never lack its activation), and every sync rebase prunes activations above the base before re-processing, so a reorged-out enactment cannot leak into the new branch — its blocks re-discover whatever survives. The activation height is derived, not stored: always the block after the enacting one. Nothing consumes the schedule yet — threading it into the STF hooks lands with the AsmSpec pipeline follow-up. --- Cargo.lock | 1 + bin/asm-runner/src/bootstrap.rs | 1 + crates/worker/Cargo.toml | 1 + crates/worker/src/builder.rs | 17 +- crates/worker/src/errors.rs | 26 +- crates/worker/src/service.rs | 192 ++++++++++++-- crates/worker/src/state.rs | 330 ++++++++++++++++++++++++- crates/worker/src/test_utils.rs | 12 +- guest-builder/sp1/guest-asm/Cargo.lock | 1 - tests/harness/test_harness.rs | 6 +- 10 files changed, 559 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b6e91ecb..3591cda0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8232,6 +8232,7 @@ dependencies = [ "serde", "sled", "strata-asm-common", + "strata-asm-logs", "strata-asm-params", "strata-asm-stf", "strata-btc-types", diff --git a/bin/asm-runner/src/bootstrap.rs b/bin/asm-runner/src/bootstrap.rs index 72b62b80..fae0ccb1 100644 --- a/bin/asm-runner/src/bootstrap.rs +++ b/bin/asm-runner/src/bootstrap.rs @@ -82,6 +82,7 @@ pub(crate) async fn bootstrap( .with_context(worker_context) .with_asm_spec(StrataAsmSpec) .with_params(params.genesis.clone()) + .with_spec_schedule(params.runtime.spec_schedule.clone()) .launch(&executor) })?; diff --git a/crates/worker/Cargo.toml b/crates/worker/Cargo.toml index bfc5659d..2baa1428 100644 --- a/crates/worker/Cargo.toml +++ b/crates/worker/Cargo.toml @@ -8,6 +8,7 @@ workspace = true [dependencies] strata-asm-common.workspace = true +strata-asm-logs.workspace = true strata-asm-params.workspace = true strata-asm-stf.workspace = true strata-btc-types.workspace = true diff --git a/crates/worker/src/builder.rs b/crates/worker/src/builder.rs index 53d5224a..735eab9f 100644 --- a/crates/worker/src/builder.rs +++ b/crates/worker/src/builder.rs @@ -1,4 +1,5 @@ use strata_asm_common::AsmSpec; +use strata_asm_params::SpecSchedule; use strata_service::ServiceBuilder; use strata_tasks::TaskExecutor; @@ -26,6 +27,7 @@ pub struct AsmWorkerBuilder { context: Option, params: Option, spec: Option, + spec_schedule: Option, } impl AsmWorkerBuilder { @@ -35,6 +37,7 @@ impl AsmWorkerBuilder { context: None, params: None, spec: None, + spec_schedule: None, } } @@ -59,6 +62,14 @@ impl AsmWorkerBuilder { self } + /// Set the base spec schedule the worker starts from (the runtime + /// params' schedule). Spec activations discovered from the ASM VK + /// upgrade log are overlaid on top of it to form the effective schedule. + pub fn with_spec_schedule(mut self, spec_schedule: SpecSchedule) -> Self { + self.spec_schedule = Some(spec_schedule); + self + } + /// Launch the ASM worker service and return a handle to it. /// /// This method validates all required dependencies, creates the service state, @@ -77,6 +88,9 @@ impl AsmWorkerBuilder { .params .ok_or(WorkerError::MissingDependency("params"))?; let spec = self.spec.ok_or(WorkerError::MissingDependency("spec"))?; + let spec_schedule = self + .spec_schedule + .ok_or(WorkerError::MissingDependency("spec_schedule"))?; // Shared between the service state (which emits) and the handle (which // hands out subscriptions), so a `subscribe_blocks()` on the handle @@ -84,7 +98,8 @@ impl AsmWorkerBuilder { let subscribers = Subscribers::default(); // Create the service state. - let service_state = AsmWorkerServiceState::new(context, spec, params, subscribers.clone())?; + let service_state = + AsmWorkerServiceState::new(context, spec, params, spec_schedule, subscribers.clone())?; // Create the service builder and get command handle. let mut service_builder = diff --git a/crates/worker/src/errors.rs b/crates/worker/src/errors.rs index 32863736..e97a8890 100644 --- a/crates/worker/src/errors.rs +++ b/crates/worker/src/errors.rs @@ -1,6 +1,7 @@ use bitcoin::Network; +use strata_asm_params::SpecScheduleError; use strata_btc_types::BitcoinTxid; -use strata_identifiers::{L1BlockCommitment, L1BlockId}; +use strata_identifiers::{L1BlockCommitment, L1BlockId, L1Height}; use strata_service::ServiceError; use thiserror::Error; @@ -67,6 +68,29 @@ pub enum WorkerError { #[error("missing aux data for the block {0:?}")] MissingAuxData(L1BlockCommitment), + /// An enacted ASM VK upgrade activates a spec version this binary has no + /// [`SpecId`](strata_asm_params::SpecId) variant for — the successor of + /// the newest scheduled version falls past the known set. The worker is + /// running old software past an upgrade it cannot execute, so it refuses + /// to commit the enacting block rather than silently limp along on stale + /// rules. + #[error( + "cannot process L1 block at height {block_height}: ASM VK upgrade activates unsupported spec version {version}; worker remains stuck at height {stuck_height}; load an image that supports the version" + )] + UnsupportedSpecActivation { + version: u16, + block_height: L1Height, + stuck_height: L1Height, + }, + + /// A spec activation record does not fit the schedule it is applied to — + /// the store and the configured base schedule disagree (e.g. the params' + /// schedule was downgraded below activations already persisted). Surfaced + /// when replaying persisted activations rather than silently producing a + /// gapped schedule. + #[error("persisted spec activation does not fit the configured schedule: {0}")] + InconsistentSpecSchedule(#[from] SpecScheduleError), + /// A Bitcoin RPC call failed after exhausting its retry budget. /// /// Carries the underlying error as a `#[source]` so `Error::source()` chains diff --git a/crates/worker/src/service.rs b/crates/worker/src/service.rs index f2f92407..8ec8a334 100644 --- a/crates/worker/src/service.rs +++ b/crates/worker/src/service.rs @@ -178,6 +178,13 @@ where state.update_anchor_state(base_state, base_block); + // Spec activations enacted above the base belong to the branch being + // rewritten (or to blocks about to be deterministically re-applied); + // drop them before re-processing so the effective schedule cannot leak + // a rolled-back activation into the first re-processed block. A linear + // extension has nothing above the base, making this a no-op. + state.rollback_spec_activations(base_block.height())?; + // Phase 2: process the pending blocks oldest first. Collect them in applied // order so the caller can drive per-block follow-up work (e.g. proof // requests) over exactly the blocks the worker processed for this submit. @@ -239,19 +246,24 @@ fn plan_block_processing( }) } -/// Runs the STF for `block_id`, then persists the results in a deliberate -/// order — the manifest (into the height-indexed MMR) and the prover aux data -/// first, the anchor state last — before advancing the in-memory anchor. +/// Runs the STF for `block_id`, discovers any spec activations from its logs, +/// then persists the results in a deliberate order — the manifest (into the +/// height-indexed MMR) and the prover aux data first, the anchor state last — +/// before advancing the in-memory anchor. /// -/// The order is the crash-safety contract. The anchor state is this block's -/// commit point: [`plan_block_processing`] treats a block as processed only -/// once its anchor state is stored, so it is written after everything derived -/// from the block. If an error aborts after the manifest or aux data write but -/// before the anchor state, the block stays uncommitted and the next sync -/// re-runs its STF. That re-run is safe: every write on this path is an -/// idempotent, block-keyed overwrite (the MMR leaf is replaced by height, aux -/// data and anchor state are keyed by block id, and the STF is deterministic, -/// so it reproduces identical values. +/// Spec activation discovery happens before any per-block persistence. If a +/// block's ASM VK upgrade does not map to a valid new activation — the +/// successor of the newest scheduled version is unknown to this binary — the +/// worker returns a specific error and leaves the block entirely uncommitted: +/// no manifest leaf, aux data, or anchor state is written. Otherwise, the anchor +/// state remains the block's commit point: [`plan_block_processing`] treats a +/// block as processed only once its anchor state is stored, so it is written +/// after everything derived from the block. If an error aborts after the +/// manifest or aux data write but before the anchor state, the block stays +/// uncommitted and the next sync re-runs its STF. That re-run is safe: every +/// write on this path is an idempotent, block-keyed overwrite (the MMR leaf is +/// replaced by height, aux data and anchor state are keyed by block id, and +/// the STF is deterministic, so it reproduces identical values). fn apply_block( state: &mut AsmWorkerServiceState, block_id: &L1BlockCommitment, @@ -266,6 +278,14 @@ where let block = state.context.get_l1_block(block_id.blkid())?; let (asm_stf_out, aux_data) = state.transition(&block)?; + // Spec discovery before any per-block persistence: if this block enacted + // an ASM VK upgrade the worker cannot accept (the activating version is + // unknown to this binary), the block is not committed at all. For valid + // upgrades, persist the activation now so a committed anchor can never + // lack the activation it enacted. + let activations = state.discover_spec_activations(block_id, asm_stf_out.manifest.logs())?; + state.apply_spec_activations(activations)?; + // Persist the manifest and record its hash in the height-indexed MMR. state .context @@ -301,9 +321,16 @@ mod tests { use std::thread; use bitcoind_async_client::traits::Reader; - use strata_asm_common::AuxRequestCollector; + use strata_asm_common::{ + AsmLogEntry, AuxRequestCollector, HeaderVerificationState, MsgRelayer, NullMsg, + SectionState, SectionStateExt, Stage, Subprotocol, SubprotocolId, TxInputRef, + VerifiedAuxData, + }; + use strata_asm_logs::AsmStfUpdate; + use strata_asm_params::{SpecId, SpecSchedule}; use strata_btc_types::L1BlockIdBitcoinExt; use strata_identifiers::{Buf32, L1BlockId}; + use strata_predicate::PredicateKey; use strata_service::CommandCompletionSender; use tokio::{sync::oneshot, task::block_in_place}; @@ -527,9 +554,14 @@ mod tests { // ...so a restart over the same store resumes at the tip. let context = fx.state.context.clone(); let params = fixtures::genesis_params(&fx.client, 101).await; - let reloaded = - AsmWorkerServiceState::new(context, TestAsmSpec, params, Subscribers::default()) - .unwrap(); + let reloaded = AsmWorkerServiceState::new( + context, + TestAsmSpec, + params, + SpecSchedule::genesis(), + Subscribers::default(), + ) + .unwrap(); assert_eq!( reloaded.blkid, tip, "restart resumes from the tip, not the stale notification", @@ -587,6 +619,134 @@ mod tests { ); } + const EMIT_UPDATE_SUBPROTO_ID: SubprotocolId = 253; + + /// Emits an [`AsmStfUpdate`] on every processed block. Whether the + /// activation it triggers is supported depends on the schedule the worker + /// starts from. + #[derive(Debug)] + struct UnsupportedSpecLogSubproto; + + impl Subprotocol for UnsupportedSpecLogSubproto { + const ID: SubprotocolId = EMIT_UPDATE_SUBPROTO_ID; + const STATE_VERSION: u8 = 1; + + type InitConfig = (); + type State = u8; + type Msg = NullMsg; + + fn init(_config: &Self::InitConfig) -> Self::State { + 0 + } + + fn process_txs( + _state: &mut Self::State, + _txs: &[TxInputRef<'_>], + _header_vs: &HeaderVerificationState, + _verified_aux_data: &VerifiedAuxData, + relayer: &mut impl MsgRelayer, + ) { + let log = AsmLogEntry::from_log(&AsmStfUpdate::new(PredicateKey::always_accept())) + .expect("AsmStfUpdate encoding is infallible"); + relayer.emit_log(log); + } + + fn process_msgs(_state: &mut Self::State, _msgs: &[Self::Msg], _l1ref: &L1BlockCommitment) { + } + } + + /// [`TestAsmSpec`] plus the one subprotocol emitting the unsupported + /// update. + #[derive(Debug)] + struct UnsupportedSpecLogSpec; + + impl AsmSpec for UnsupportedSpecLogSpec { + type Params = fixtures::TestAsmParams; + + fn call_subprotocols(&self, stage: &mut impl Stage) { + stage.invoke_subprotocol::(); + } + + fn construct_genesis_state(&self, params: &Self::Params) -> AnchorState { + let mut state = TestAsmSpec.construct_genesis_state(params); + state.sections = vec![ + SectionState::from_state::(&0) + .expect("test section fits"), + ] + .try_into() + .expect("single test section fits"); + state + } + + fn genesis_l1_height(&self, params: &Self::Params) -> u64 { + TestAsmSpec.genesis_l1_height(params) + } + } + + /// A block that enacts an ASM VK update whose activating version is + /// unknown to this binary is not committed. Retrying after a restart will + /// target the same block again, so no unsupported marker has to be + /// persisted. + #[tokio::test(flavor = "multi_thread")] + async fn sync_rejects_block_with_unsupported_spec_update() { + let fx = fixtures::setup_context(101).await; + let params = fixtures::genesis_params(&fx.client, 101).await; + // Every known version is already scheduled, so the enacted upgrade's + // successor (2) falls past the known set. + let mut state = AsmWorkerServiceState::<_, UnsupportedSpecLogSpec>::new( + fx.context.clone(), + UnsupportedSpecLogSpec, + params, + { + let mut schedule = SpecSchedule::genesis(); + schedule.schedule(SpecId::V1, 0).expect("schedule V1"); + schedule + }, + Subscribers::default(), + ) + .expect("create service state"); + let stuck = state.blkid; + let target = fixtures::mine(&fx._node, &fx.client, 1).await[0]; // 102 + + let err = sync_to_block(&mut state, target.blkid()) + .expect_err("sync should reject the unsupported spec update block"); + + assert!( + matches!( + &err, + WorkerError::UnsupportedSpecActivation { + version: 2, + block_height: 102, + stuck_height: 101, + } + ), + "expected unsupported spec activation error, got {err:?}", + ); + assert!( + err.to_string() + .contains("worker remains stuck at height 101"), + "error should name the stuck height: {err}", + ); + assert!( + err.to_string() + .contains("load an image that supports the version"), + "error should tell operators how to proceed: {err}", + ); + assert_eq!(state.blkid, stuck, "in-memory anchor stays stuck"); + assert!( + matches!( + state.context.get_anchor_state(&target), + Err(WorkerError::MissingAsmState(_)) + ), + "rejected block must not get an anchor state", + ); + assert_eq!( + state.context.mmr_leaf_count(), + 102, + "no manifest leaf is written for the rejected block", + ); + } + /// End-to-end at the resolver boundary: drive the real STF over a chain, /// then reorg to a shorter branch and probe what the post-reorg context can /// serve to a prover. diff --git a/crates/worker/src/state.rs b/crates/worker/src/state.rs index 38bfd717..867f0307 100644 --- a/crates/worker/src/state.rs +++ b/crates/worker/src/state.rs @@ -1,17 +1,19 @@ use bitcoin::{Block, CompactTarget, params::Params}; -use strata_asm_common::{AnchorState, AsmSpec, AuxData, HeaderVerificationState}; +use strata_asm_common::{AnchorState, AsmLogEntry, AsmSpec, AuxData, HeaderVerificationState}; +use strata_asm_logs::AsmStfUpdate; +use strata_asm_params::SpecSchedule; use strata_asm_stf::AsmStfOutput; use strata_btc_types::BlockHashExt; use strata_btc_verification::{ TxidInclusionProof, compute_block_hash, get_relative_difficulty_adjustment_height, }; -use strata_identifiers::L1BlockCommitment; +use strata_identifiers::{L1BlockCommitment, L1Height}; use strata_service::ServiceState; use tracing::field::Empty; use crate::{ - AnchorMismatch, L1DataProvider, Subscribers, WorkerContext, WorkerError, WorkerResult, - aux_resolver::AuxDataResolver, constants, + AnchorMismatch, L1DataProvider, SpecActivationRecord, Subscribers, WorkerContext, WorkerError, + WorkerResult, aux_resolver::AuxDataResolver, constants, }; /// Service state for the ASM worker. @@ -42,6 +44,14 @@ pub struct AsmWorkerServiceState { /// the service fans the new commitment out to these; see /// [`crate::AsmWorkerHandle::subscribe_blocks`]. pub(crate) subscribers: Subscribers, + + /// Base spec schedule, as configured via params. + pub(crate) base_spec_schedule: SpecSchedule, + + /// Effective schedule: `base_spec_schedule` overlaid with every + /// discovered activation. This is what discovery validates against, and + /// what params threading into the STF will consume once it lands. + pub(crate) spec_schedule: SpecSchedule, } impl AsmWorkerServiceState @@ -52,16 +62,31 @@ where { /// Creates a new service state, loading the latest anchor or creating genesis. /// + /// `spec_schedule` is the configured base schedule; activations + /// discovered before a restart are overlaid to resume the effective + /// schedule. + /// /// Construction goes through [`crate::AsmWorkerBuilder`], which owns the /// shared [`Subscribers`] registry — hence `pub(crate)`. pub(crate) fn new( context: W, spec: S, params: S::Params, + spec_schedule: SpecSchedule, subscribers: Subscribers, ) -> WorkerResult { let genesis_height = spec.genesis_l1_height(¶ms); + let base_spec_schedule = spec_schedule; + let activations = context.list_spec_activations()?; + let spec_schedule = effective_schedule(&base_spec_schedule, &activations)?; + if !activations.is_empty() { + tracing::info!( + ?activations, + "resuming with persisted spec activations applied" + ); + } + // Align the manifest MMR with L1 heights before processing any block: // it is height-indexed, prefilled with sentinels for heights // `0..=genesis_height` so the manifest for height `h` lands at index @@ -98,6 +123,8 @@ where blkid, genesis_height, subscribers, + base_spec_schedule, + spec_schedule, }) } @@ -159,6 +186,115 @@ where self.anchor = anchor; self.blkid = blkid; } + + /// Scans a processed block's logs for enacted ASM VK upgrades and derives + /// the spec activation each one triggers, without touching any state; + /// [`Self::apply_spec_activations`] enacts them. + /// + /// The upgrade log does not name a version — the artifact enacting an + /// upgrade may predate the version it activates, so the wire cannot + /// require knowing it. Instead each upgrade activates the successor of + /// the newest version the effective schedule knows + /// ([`SpecSchedule::schedule_successor`]), chaining through upgrades + /// earlier in the same block. + /// + /// An error means the block must not be committed: the successor id has + /// no [`SpecId`](strata_asm_params::SpecId) variant + /// ([`WorkerError::UnsupportedSpecActivation`]), so the worker is running + /// old software past an upgrade it cannot execute and must halt until + /// restarted with an image that supports the version. + /// + /// Collects into a `Vec` deliberately: every update must validate before + /// [`Self::apply_spec_activations`] persists anything, so a flawed update + /// cannot leave a prefix of the block's activations on disk. + pub(crate) fn discover_spec_activations( + &self, + block_id: &L1BlockCommitment, + logs: &[AsmLogEntry], + ) -> WorkerResult> { + let enacting_height = block_id.height(); + let stuck_height = enacting_height.saturating_sub(1); + let activation_height = enacting_height.saturating_add(1); + // Scheduled on a scratch copy so an upgrade earlier in this block + // moves the successor forward for the ones after it, without touching + // the schedule before the whole block validates. + let mut schedule = self.spec_schedule.clone(); + logs.iter() + .filter_map(|l| l.try_into_log::().ok()) + .map(|update| { + let version = + schedule + .schedule_successor(activation_height) + .map_err(|version| WorkerError::UnsupportedSpecActivation { + version, + block_height: enacting_height, + stuck_height, + })?; + Ok(SpecActivationRecord { + enacting_height, + version, + new_predicate: update.into_new_predicate(), + }) + }) + .collect() + } + + /// Persists each discovered activation and applies it to the in-memory + /// effective schedule. + /// + /// MUST run before the enacting block's anchor state is committed: + /// persisting the activation first guarantees a committed anchor never + /// lacks the activation it enacted (crash-replay simply rewrites the same + /// record). + pub(crate) fn apply_spec_activations( + &mut self, + activations: Vec, + ) -> WorkerResult<()> { + for activation in activations { + let version = activation.version; + let enacting_height = activation.enacting_height; + let activation_height = activation.activation_height(); + self.context.record_spec_activation(activation)?; + self.spec_schedule.schedule(version, activation_height)?; + tracing::info!( + ?version, + activation_height, + enacting_height, + "spec version activated by ASM VK upgrade" + ); + } + Ok(()) + } + + /// Drops spec activations enacted strictly above `base_height` and + /// recomputes the effective schedule. + /// + /// Called when a sync rebases onto an ancestor (reorg): activations + /// enacted on the abandoned branch must not leak into re-processing; + /// any still on the new branch are re-discovered as its blocks re-apply. + pub(crate) fn rollback_spec_activations(&mut self, base_height: L1Height) -> WorkerResult<()> { + self.context.prune_spec_activations_after(base_height)?; + let activations = self.context.list_spec_activations()?; + self.spec_schedule = effective_schedule(&self.base_spec_schedule, &activations)?; + Ok(()) + } +} + +/// Overlays `activations` onto the `base` schedule. +/// +/// Errs when a record does not fit the base schedule +/// ([`WorkerError::InconsistentSpecSchedule`]) — the store and the configured +/// params disagree, so the worker must not start on a schedule missing +/// activations it already committed blocks under. +fn effective_schedule( + base: &SpecSchedule, + activations: &[SpecActivationRecord], +) -> WorkerResult { + let mut schedule = base.clone(); + for activation in activations { + schedule.schedule(activation.version, activation.activation_height())?; + } + Ok(schedule) } impl ServiceState for AsmWorkerServiceState @@ -334,6 +470,7 @@ mod tests { seed.state.context.clone(), TestAsmSpec, params, + SpecSchedule::genesis(), Subscribers::default(), ) .unwrap(); @@ -460,7 +597,14 @@ mod tests { let context = fx.state.context.clone(); let params = fixtures::genesis_params(&fx.client, 101).await; - AsmWorkerServiceState::new(context, TestAsmSpec, params, Subscribers::default()).unwrap(); + AsmWorkerServiceState::new( + context, + TestAsmSpec, + params, + SpecSchedule::genesis(), + Subscribers::default(), + ) + .unwrap(); assert_eq!( fx.state.context.mmr_leaf_count(), @@ -468,4 +612,180 @@ mod tests { "prefill is idempotent across restart", ); } + + mod spec_activation { + use strata_asm_logs::AsmStfUpdate; + use strata_asm_params::SpecId; + use strata_identifiers::Buf32; + use strata_predicate::{PredicateKey, PredicateTypeId}; + + use super::*; + use crate::SpecActivationStore; + + /// Stands in for the upgraded artifact's VK, persisted with the + /// activation discovery records. + fn upgrade_predicate() -> PredicateKey { + PredicateKey::new(PredicateTypeId::Bip340Schnorr, vec![0x33; 32]) + } + + fn upgrade_log() -> AsmLogEntry { + AsmLogEntry::from_log(&AsmStfUpdate::new(upgrade_predicate())) + .expect("AsmStfUpdate encoding is infallible") + } + + fn block_at(height: L1Height) -> L1BlockCommitment { + L1BlockCommitment::new(height, Buf32::new([0xEE; 32]).into()) + } + + /// An upgrade log yields the activation of the successor of the + /// newest scheduled version; applying it activates that version at + /// H+1, in memory and on disk. Discovery alone touches nothing. + #[tokio::test(flavor = "multi_thread")] + async fn discover_then_apply_activates_successor_version() { + let mut fx = fixtures::setup_state(101).await; + + let discovered = fx + .state + .discover_spec_activations(&block_at(150), &[upgrade_log()]) + .unwrap(); + + let expected = vec![SpecActivationRecord { + enacting_height: 150, + version: SpecId::V1, + new_predicate: upgrade_predicate(), + }]; + assert_eq!(discovered, expected); + assert_eq!(fx.state.spec_schedule, SpecSchedule::genesis()); + assert!(fx.state.context.list_spec_activations().unwrap().is_empty()); + + fx.state.apply_spec_activations(discovered).unwrap(); + + assert_eq!( + fx.state.spec_schedule.activation_height_of(SpecId::V1), + Some(151) + ); + assert_eq!(fx.state.context.list_spec_activations().unwrap(), expected); + } + + /// An upgrade whose successor this binary has no variant for prevents + /// the enacting block from being committed at all. + #[tokio::test(flavor = "multi_thread")] + async fn discover_rejects_unknown_successor() { + let mut fx = fixtures::setup_state(101).await; + // Every known version is already scheduled, so the next upgrade's + // successor (2) falls past the known set. + fx.state + .spec_schedule + .schedule(SpecId::V1, 10) + .expect("schedule V1"); + + let err = fx + .state + .discover_spec_activations(&block_at(150), &[upgrade_log()]) + .unwrap_err(); + + assert!( + matches!( + err, + WorkerError::UnsupportedSpecActivation { + version: 2, + block_height: 150, + stuck_height: 149, + } + ), + "expected unsupported spec activation error, got {err:?}", + ); + assert!(fx.state.context.list_spec_activations().unwrap().is_empty()); + } + + /// A flawed update anywhere in the block's logs fails discovery as a + /// whole: the valid earlier update must not slip through, so nothing + /// is ever persisted for a block the worker refuses to commit. This + /// also exercises chaining within a block: the second upgrade's + /// successor is 2 only because the first one moved it past V1. + #[tokio::test(flavor = "multi_thread")] + async fn discover_rejects_block_with_valid_and_flawed_updates() { + let fx = fixtures::setup_state(101).await; + let logs = [upgrade_log(), upgrade_log()]; + + let err = fx + .state + .discover_spec_activations(&block_at(150), &logs) + .unwrap_err(); + + assert!( + matches!( + err, + WorkerError::UnsupportedSpecActivation { version: 2, .. } + ), + "expected unsupported spec activation error, got {err:?}", + ); + assert_eq!(fx.state.spec_schedule, SpecSchedule::genesis()); + assert!(fx.state.context.list_spec_activations().unwrap().is_empty()); + } + + /// A restart resumes the effective schedule from persisted + /// activations. + #[tokio::test(flavor = "multi_thread")] + async fn new_resumes_persisted_activations() { + let fx = fixtures::setup_state(101).await; + let context = fx.state.context.clone(); // shares the sled store + context + .record_spec_activation(SpecActivationRecord { + enacting_height: 120, + version: SpecId::V1, + new_predicate: upgrade_predicate(), + }) + .unwrap(); + + let params = fixtures::genesis_params(&fx.client, 101).await; + let reloaded = AsmWorkerServiceState::new( + context, + TestAsmSpec, + params, + SpecSchedule::genesis(), + Subscribers::default(), + ) + .unwrap(); + + assert_eq!( + reloaded.spec_schedule.activation_height_of(SpecId::V1), + Some(121), + "restart must resume the discovered activation", + ); + } + + /// A reorg rollback prunes activations above the base and recomputes + /// the effective schedule from what survives. + #[tokio::test(flavor = "multi_thread")] + async fn rollback_prunes_and_recomputes() { + let mut fx = fixtures::setup_state(101).await; + let logs = [upgrade_log()]; + + let activations = fx + .state + .discover_spec_activations(&block_at(150), &logs) + .unwrap(); + fx.state.apply_spec_activations(activations).unwrap(); + assert!(fx.state.spec_schedule.is_active(SpecId::V1, 151)); + + // Reorg to a base below the enacting block: back to the base schedule. + fx.state.rollback_spec_activations(140).unwrap(); + assert_eq!( + fx.state.spec_schedule, + SpecSchedule::genesis(), + "activation enacted above the base must be dropped", + ); + assert!(fx.state.context.list_spec_activations().unwrap().is_empty()); + + // A rollback at or above the enacting height keeps the activation. + let activations = fx + .state + .discover_spec_activations(&block_at(150), &logs) + .unwrap(); + fx.state.apply_spec_activations(activations).unwrap(); + fx.state.rollback_spec_activations(150).unwrap(); + assert!(fx.state.spec_schedule.is_active(SpecId::V1, 151)); + } + } } diff --git a/crates/worker/src/test_utils.rs b/crates/worker/src/test_utils.rs index 045a524e..94bfc57e 100644 --- a/crates/worker/src/test_utils.rs +++ b/crates/worker/src/test_utils.rs @@ -371,6 +371,7 @@ pub(crate) mod fixtures { AnchorState, AsmHistoryAccumulatorState, AsmSpec, ChainViewState, HeaderVerificationState, Stage, }; + use strata_asm_params::SpecSchedule; use strata_btc_types::BlockHashExt; use strata_btc_verification::L1Anchor; use strata_identifiers::L1BlockCommitment; @@ -443,9 +444,14 @@ pub(crate) mod fixtures { let params = genesis_params(&client, genesis_height).await; let context = TestAsmWorkerContext::new((*client).clone()); - let state = - AsmWorkerServiceState::new(context, TestAsmSpec, params, Subscribers::default()) - .expect("create service state"); + let state = AsmWorkerServiceState::new( + context, + TestAsmSpec, + params, + SpecSchedule::genesis(), + Subscribers::default(), + ) + .expect("create service state"); StateFixture { node, diff --git a/guest-builder/sp1/guest-asm/Cargo.lock b/guest-builder/sp1/guest-asm/Cargo.lock index 79cb0356..5fdccb31 100644 --- a/guest-builder/sp1/guest-asm/Cargo.lock +++ b/guest-builder/sp1/guest-asm/Cargo.lock @@ -1842,7 +1842,6 @@ dependencies = [ "serde", "ssz", "ssz_derive", - "strata-asm-common", "strata-asm-proto-bridge-types", "strata-btc-types", "strata-btc-verification", diff --git a/tests/harness/test_harness.rs b/tests/harness/test_harness.rs index 44276210..da07185d 100644 --- a/tests/harness/test_harness.rs +++ b/tests/harness/test_harness.rs @@ -54,7 +54,7 @@ use rand::RngCore; use strata_asm_common::{AnchorState, AsmLogEntry}; use strata_asm_manifest_types::AsmLog; use strata_asm_moho_worker::{MohoStateStore, MohoWorkerBuilder, MohoWorkerHandle}; -use strata_asm_params::{AdministrationInitConfig, AsmParams, SubprotocolInstance}; +use strata_asm_params::{AdministrationInitConfig, AsmParams, SpecSchedule, SubprotocolInstance}; use strata_asm_spec::StrataAsmSpec; use strata_asm_worker::{ test_utils::{get_l1_anchor, TestAsmWorkerContext}, @@ -834,6 +834,9 @@ impl AsmTestHarnessBuilder { SubprotocolInstance::Checkpoint(cfg) => *cfg = checkpoint_config.clone(), } } + // Production base schedule: the genesis spec version active since + // genesis, later versions activated only by enacted ASM VK upgrades. + asm_params.runtime.spec_schedule = SpecSchedule::genesis(); let asm_params = Arc::new(asm_params); // 5. Create worker context. The worker prefills the height-indexed MMR @@ -854,6 +857,7 @@ impl AsmTestHarnessBuilder { .with_context(context.clone()) .with_asm_spec(StrataAsmSpec) .with_params(asm_params.genesis.clone()) + .with_spec_schedule(asm_params.runtime.spec_schedule.clone()) .launch(&executor)?; // 8. Launch the Moho worker, driven by the ASM worker's per-block commit From ad043e8ed84fd2038714afa2b61a63b48d5f2e29 Mon Sep 17 00:00:00 2001 From: Prajwol Gyawali Date: Fri, 24 Jul 2026 23:00:12 +0545 Subject: [PATCH 6/8] test: cover the spec upgrade lifecycle end to end Worker-level integration coverage for the full on-chain choreography: the admin enacts an ASM VK upgrade, the worker derives V1 as the successor of genesis-active V0 and records the activation at H+1 carrying the enacted VK, and the enacting block's manifest holds the log it was discovered from. The reorg path abandons the submission block and asserts the activation rolls back, then re-enacts one height later once the resurrected admin txs re-mine. The unstake-style feature gating that motivated the old fork-based variant of this test is deliberately absent: nothing consumes the schedule yet. --- tests/Cargo.toml | 4 + tests/asm/spec_activation.rs | 149 +++++++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+) create mode 100644 tests/asm/spec_activation.rs diff --git a/tests/Cargo.toml b/tests/Cargo.toml index 57b23564..fa895dcb 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -99,3 +99,7 @@ path = "asm/admin_to_stf.rs" [[test]] name = "asm_admin_to_ee_stf" path = "asm/admin_to_ee_stf.rs" + +[[test]] +name = "asm_spec_activation" +path = "asm/spec_activation.rs" diff --git a/tests/asm/spec_activation.rs b/tests/asm/spec_activation.rs new file mode 100644 index 00000000..75a9cf87 --- /dev/null +++ b/tests/asm/spec_activation.rs @@ -0,0 +1,149 @@ +//! End-to-end spec activation flow at the worker level. +//! +//! Drives the exact choreography an ASM upgrade performs on-chain: the admin +//! enacts an ASM VK upgrade, and the worker discovers the activation from the +//! enacted update's log — deriving the activating version as the successor of +//! the newest scheduled one — recording it for the block after the enacting +//! one. Also exercises the reorg path: abandoning the enacting block rolls +//! the activation back until the new branch re-enacts it. + +#![allow( + unused_crate_dependencies, + reason = "test dependencies shared across test suite" +)] + +use bitcoind_async_client::traits::Reader; +use harness::{ + admin::{asm_stf_vk_update, submit_and_activate, DEFAULT_CONFIRMATION_DEPTH}, + test_harness::{AsmTestHarnessBuilder, Setup}, +}; +use integration_tests::harness; +use strata_asm_logs::AsmStfUpdate; +use strata_asm_params::SpecId; +use strata_asm_worker::SpecActivationStore; +use strata_predicate::{PredicateKey, PredicateTypeId}; + +/// Stands in for the upgraded proving artifact's VK. At worker level nothing +/// verifies proofs, so any distinct predicate will do. +fn post_upgrade_predicate() -> PredicateKey { + PredicateKey::new(PredicateTypeId::Bip340Schnorr, vec![0x77; 32]) +} + +/// The full upgrade lifecycle: the admin enacts an ASM VK upgrade, and the +/// worker records the activation of V1 — the successor of genesis-active V0 — +/// for the block after the enacting one, carrying the enacted VK. +#[tokio::test(flavor = "multi_thread")] +async fn test_spec_upgrade_activation_lifecycle() { + let Setup { + harness, + admin: mut admin_ctx, + .. + } = AsmTestHarnessBuilder::default().build().await; + + assert!( + harness.context.list_spec_activations().unwrap().is_empty(), + "nothing must activate before the upgrade", + ); + + // Enact the ASM VK upgrade switching to the new artifact's key. + submit_and_activate( + &harness, + &mut admin_ctx, + asm_stf_vk_update(post_upgrade_predicate()), + ) + .await; + + let activations = harness.context.list_spec_activations().unwrap(); + assert_eq!(activations.len(), 1, "exactly one activation expected"); + assert_eq!(activations[0].version, SpecId::V1); + assert_eq!( + activations[0].activation_height(), + activations[0].enacting_height + 1, + "the version's rules apply from the block after the enacting one", + ); + assert_eq!( + activations[0].new_predicate, + post_upgrade_predicate(), + "the record must carry the VK the upgrade enacted", + ); + + // The enacting block's manifest carries the AsmStfUpdate log the worker + // discovered the activation from. + let enacting_hash = harness + .client + .get_block_hash(activations[0].enacting_height as u64) + .await + .unwrap(); + let enacting_block = harness.commitment_of(enacting_hash).await.unwrap(); + let update = harness + .get_logs_at(&enacting_block) + .iter() + .find_map(|log| log.try_into_log::().ok()) + .expect("the enacting block's manifest must carry the AsmStfUpdate log"); + assert_eq!(update.new_predicate(), &post_upgrade_predicate()); +} + +/// Reorging out the upgrade rolls the activation back, and the new branch +/// re-enacts it once the resurrected update reaches its activation height +/// again. +/// +/// The reorg invalidates the *submission* block, so the admin commit/reveal +/// txs are evicted to the mempool. The replacement block is mined empty; the +/// next mempool-including block re-mines the txs one height later, so the +/// update re-queues and re-enacts with everything shifted by one block. +#[tokio::test(flavor = "multi_thread")] +async fn test_reorg_rolls_back_and_rediscovers_activation() { + let Setup { + harness, + admin: mut admin_ctx, + .. + } = AsmTestHarnessBuilder::default().build().await; + + submit_and_activate( + &harness, + &mut admin_ctx, + asm_stf_vk_update(post_upgrade_predicate()), + ) + .await; + + let activations = harness.context.list_spec_activations().unwrap(); + assert_eq!(activations.len(), 1, "activation recorded at enactment"); + let enacting_height = activations[0].enacting_height; + let submission_height = enacting_height as u64 - DEFAULT_CONFIRMATION_DEPTH as u64; + + // Reorg out the submission block and everything above it, replacing it + // with one empty block so the tip sits back at the submission height. + let submission_hash = harness + .client + .get_block_hash(submission_height) + .await + .unwrap(); + harness.reorg(submission_hash, 1).await.unwrap(); + + assert!( + harness.context.list_spec_activations().unwrap().is_empty(), + "activation enacted on the abandoned branch must be rolled back", + ); + + // Mine through re-submission (the evicted txs return from the mempool) + // and the confirmation depth: the update re-enacts on the new branch, one + // height above the original enactment. + harness + .mine_blocks(1 + DEFAULT_CONFIRMATION_DEPTH as usize) + .await + .unwrap(); + + let activations = harness.context.list_spec_activations().unwrap(); + assert_eq!( + activations.len(), + 1, + "the re-mined update must re-enact on the new branch", + ); + assert_eq!(activations[0].version, SpecId::V1); + assert_eq!( + activations[0].enacting_height, + enacting_height + 1, + "re-submission lands one block later, shifting enactment by one", + ); + assert_eq!(activations[0].new_predicate, post_upgrade_predicate()); +} From 632abc1e4256c99aceccd760b656725c3a3076d2 Mon Sep 17 00:00:00 2001 From: Prajwol Gyawali Date: Sun, 26 Jul 2026 10:53:29 +0545 Subject: [PATCH 7/8] feat(params)!: enforce nondecreasing spec activations and drop null schedule entries Both mutation paths now reject an activation height that would order a version against its neighbor the wrong way, so an inverted schedule ("v2 active while v1 is not") is unrepresentable; equal heights stay legal because several upgrades enacted in one block activate at the same height. The worker maps an unknown successor to UnsupportedSpecActivation as before and an out-of-order activation to InconsistentSpecSchedule. The serde form now lists exactly the scheduled versions: an explicit "v1": null was indistinguishable from omitting the key, so absence becomes the only spelling of "unscheduled" and null is rejected at decode. --- crates/params/src/lib.rs | 3 +- crates/params/src/runtime.rs | 112 ++++++++++++++++++++++++++++------- crates/worker/src/errors.rs | 11 ++-- crates/worker/src/state.rs | 30 ++++++---- 4 files changed, 116 insertions(+), 40 deletions(-) diff --git a/crates/params/src/lib.rs b/crates/params/src/lib.rs index b2389056..29cb06b4 100644 --- a/crates/params/src/lib.rs +++ b/crates/params/src/lib.rs @@ -135,8 +135,7 @@ mod tests { } ], "spec_activation": { - "v0": 0, - "v1": null + "v0": 0 } } "#; diff --git a/crates/params/src/runtime.rs b/crates/params/src/runtime.rs index 74d79aa1..e9586a34 100644 --- a/crates/params/src/runtime.rs +++ b/crates/params/src/runtime.rs @@ -19,8 +19,9 @@ use crate::spec_id::SpecId; /// [`SpecId::V0`] is the genesis version, always active from height 0; the /// schedule only tracks the upgrades after it, as the activation heights of a /// contiguous run of successors (`upgrades[i]` belongs to the version with -/// discriminant `i + 1`). Versions activate strictly in succession, so a -/// gapped schedule ("v2 scheduled, v1 disabled") is unrepresentable, and a +/// discriminant `i + 1`). Versions activate strictly in succession at +/// nondecreasing heights, so a gapped schedule ("v2 scheduled, v1 disabled") +/// or an inverted one ("v2 active while v1 is not") is unrepresentable, and a /// new [`SpecId`] variant needs no change here — every method derives its /// answer from the discriminant. /// @@ -58,6 +59,24 @@ pub enum SpecScheduleError { /// The newest scheduled version at the time of the attempt. latest: SpecId, }, + + /// Scheduling at `height` would order the version's activation the wrong + /// way around an adjacent version's; activation heights must be + /// nondecreasing in version order. + #[error( + "activation heights must be nondecreasing: {height} is out of order with the adjacent activation at {adjacent}" + )] + OutOfOrder { + /// The rejected activation height. + height: L1Height, + /// The adjacent version's activation height it conflicts with. + adjacent: L1Height, + }, + + /// The version to schedule has no [`SpecId`] variant in this binary — + /// old software has hit an upgrade it cannot execute. + #[error("no spec version with id {0} in this binary")] + UnknownSuccessor(u16), } impl SpecSchedule { @@ -96,11 +115,23 @@ impl SpecSchedule { /// /// This is the discovery-side entry point: an enacted ASM VK upgrade does /// not name the version it activates (the wire only carries the new VK), - /// so the activating version is *defined* as the successor. Errs with the - /// successor's raw id when this binary has no [`SpecId`] variant for it — - /// the caller is running old software past an upgrade it cannot execute. - pub fn schedule_successor(&mut self, height: L1Height) -> Result { - let successor = SpecId::try_from(self.upgrades.len() as u16 + 1)?; + /// so the activating version is *defined* as the successor. Errs when + /// `height` precedes the newest scheduled activation + /// ([`SpecScheduleError::OutOfOrder`]) or when this binary has no + /// [`SpecId`] variant for the successor + /// ([`SpecScheduleError::UnknownSuccessor`] — the caller is running old + /// software past an upgrade it cannot execute). + pub fn schedule_successor(&mut self, height: L1Height) -> Result { + if let Some(&prev) = self.upgrades.last() + && height < prev + { + return Err(SpecScheduleError::OutOfOrder { + height, + adjacent: prev, + }); + } + let successor = SpecId::try_from(self.upgrades.len() as u16 + 1) + .map_err(SpecScheduleError::UnknownSuccessor)?; self.upgrades.push(height); Ok(successor) } @@ -113,7 +144,8 @@ impl SpecSchedule { /// schedule. Unlike [`Self::schedule_successor`] it accepts already- /// scheduled versions — the discovered height overrides the base — but /// still rejects anything that would break the invariants: rescheduling - /// [`SpecId::V0`] or skipping past an unscheduled predecessor. + /// [`SpecId::V0`], skipping past an unscheduled predecessor, or moving + /// an activation out of order with a neighbor's. pub fn schedule(&mut self, spec: SpecId, height: L1Height) -> Result<(), SpecScheduleError> { let idx = match usize::from(u16::from(spec)).checked_sub(1) { None => return Err(SpecScheduleError::GenesisFixed), @@ -125,6 +157,22 @@ impl SpecSchedule { latest: self.latest_scheduled(), }); } + if let Some(&prev) = idx.checked_sub(1).and_then(|i| self.upgrades.get(i)) + && height < prev + { + return Err(SpecScheduleError::OutOfOrder { + height, + adjacent: prev, + }); + } + if let Some(&next) = self.upgrades.get(idx + 1) + && next < height + { + return Err(SpecScheduleError::OutOfOrder { + height, + adjacent: next, + }); + } match self.upgrades.get_mut(idx) { Some(slot) => *slot = height, None => self.upgrades.push(height), @@ -139,14 +187,13 @@ impl Default for SpecSchedule { } } -/// Serialized form of [`SpecSchedule`]: one entry per known version, `null` -/// when unscheduled (e.g. `{"v0": 0, "v1": null}`). Kept for params-file -/// compatibility with the former per-version struct; conversion back -/// re-validates the invariants, so a hand-edited gapped or v0-disabled +/// Serialized form of [`SpecSchedule`]: one entry per *scheduled* version +/// (e.g. `{"v0": 0, "v1": 7}`); an absent version is unscheduled. Conversion +/// back re-validates the invariants, so a gapped, inverted, or v0-disabled /// schedule is rejected at load. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(transparent)] -struct SpecScheduleRepr(BTreeMap>); +struct SpecScheduleRepr(BTreeMap); /// Every known version, in discriminant order. fn known_versions() -> impl Iterator { @@ -157,7 +204,7 @@ impl From for SpecScheduleRepr { fn from(schedule: SpecSchedule) -> Self { Self( known_versions() - .map(|spec| (spec, schedule.activation_height_of(spec))) + .filter_map(|spec| Some((spec, schedule.activation_height_of(spec)?))) .collect(), ) } @@ -167,7 +214,7 @@ impl TryFrom for SpecSchedule { type Error = SpecScheduleError; fn try_from(repr: SpecScheduleRepr) -> Result { - let height_of = |spec| repr.0.get(&spec).copied().flatten(); + let height_of = |spec| repr.0.get(&spec).copied(); if height_of(SpecId::V0) != Some(0) { return Err(SpecScheduleError::GenesisFixed); } @@ -275,10 +322,28 @@ mod tests { assert_eq!(schedule.activation_height_of(SpecId::V1), Some(42)); // Every known version is scheduled, so the next successor's raw id // has no variant. - assert_eq!(schedule.schedule_successor(43), Err(2)); + assert_eq!( + schedule.schedule_successor(43), + Err(SpecScheduleError::UnknownSuccessor(2)) + ); assert_eq!(schedule, v1_at(42), "failed call must not mutate"); } + /// A height behind the newest scheduled activation would activate the + /// successor before its predecessor. + #[test] + fn schedule_successor_rejects_regressing_heights() { + let mut schedule = v1_at(100); + assert_eq!( + schedule.schedule_successor(50), + Err(SpecScheduleError::OutOfOrder { + height: 50, + adjacent: 100 + }) + ); + assert_eq!(schedule, v1_at(100), "failed call must not mutate"); + } + #[test] fn schedule_overwrites_but_pins_genesis() { let mut schedule = v1_at(42); @@ -291,7 +356,7 @@ mod tests { } #[test] - fn serde_keeps_the_per_version_map_format() { + fn serde_is_the_scheduled_version_map_format() { let params = AsmStfParams { spec_schedule: v1_at(7), }; @@ -300,20 +365,25 @@ mod tests { let back: AsmStfParams = serde_json::from_str(&json).unwrap(); assert_eq!(back, params); + // Unscheduled versions are absent, not null. let genesis = AsmStfParams::default(); let json = serde_json::to_string(&genesis).unwrap(); - assert_eq!(json, r#"{"spec_activation":{"v0":0,"v1":null}}"#); + assert_eq!(json, r#"{"spec_activation":{"v0":0}}"#); let back: AsmStfParams = serde_json::from_str(&json).unwrap(); assert_eq!(back, genesis); } #[test] fn deserialize_rejects_invalid_schedules() { - // V0 disabled, missing, or moved off genesis. + // V0 missing entirely, missing while v1 is scheduled, moved off + // genesis, or a null height (absence is the only spelling of + // "unscheduled"). for json in [ - r#"{"v0":null,"v1":null}"#, + r#"{}"#, r#"{"v1":7}"#, - r#"{"v0":5,"v1":null}"#, + r#"{"v0":5}"#, + r#"{"v0":null}"#, + r#"{"v0":0,"v1":null}"#, ] { assert!( serde_json::from_str::(json).is_err(), diff --git a/crates/worker/src/errors.rs b/crates/worker/src/errors.rs index e97a8890..00781f56 100644 --- a/crates/worker/src/errors.rs +++ b/crates/worker/src/errors.rs @@ -83,11 +83,12 @@ pub enum WorkerError { stuck_height: L1Height, }, - /// A spec activation record does not fit the schedule it is applied to — - /// the store and the configured base schedule disagree (e.g. the params' - /// schedule was downgraded below activations already persisted). Surfaced - /// when replaying persisted activations rather than silently producing a - /// gapped schedule. + /// A spec activation does not fit the schedule it is applied to — the + /// store or chain and the configured base schedule disagree (e.g. the + /// params' schedule was downgraded below activations already persisted, + /// or an enacted upgrade activates below a height the schedule already + /// holds). Surfaced when replaying persisted activations or discovering + /// new ones rather than silently producing a gapped or inverted schedule. #[error("persisted spec activation does not fit the configured schedule: {0}")] InconsistentSpecSchedule(#[from] SpecScheduleError), diff --git a/crates/worker/src/state.rs b/crates/worker/src/state.rs index 867f0307..3a357c0e 100644 --- a/crates/worker/src/state.rs +++ b/crates/worker/src/state.rs @@ -1,7 +1,7 @@ use bitcoin::{Block, CompactTarget, params::Params}; use strata_asm_common::{AnchorState, AsmLogEntry, AsmSpec, AuxData, HeaderVerificationState}; use strata_asm_logs::AsmStfUpdate; -use strata_asm_params::SpecSchedule; +use strata_asm_params::{SpecSchedule, SpecScheduleError}; use strata_asm_stf::AsmStfOutput; use strata_btc_types::BlockHashExt; use strata_btc_verification::{ @@ -198,11 +198,13 @@ where /// ([`SpecSchedule::schedule_successor`]), chaining through upgrades /// earlier in the same block. /// - /// An error means the block must not be committed: the successor id has - /// no [`SpecId`](strata_asm_params::SpecId) variant + /// An error means the block must not be committed: either the successor + /// id has no [`SpecId`](strata_asm_params::SpecId) variant /// ([`WorkerError::UnsupportedSpecActivation`]), so the worker is running /// old software past an upgrade it cannot execute and must halt until - /// restarted with an image that supports the version. + /// restarted with an image that supports the version — or the upgrade + /// would activate below a height the configured schedule already put its + /// predecessor at ([`WorkerError::InconsistentSpecSchedule`]). /// /// Collects into a `Vec` deliberately: every update must validate before /// [`Self::apply_spec_activations`] persists anything, so a flawed update @@ -222,14 +224,18 @@ where logs.iter() .filter_map(|l| l.try_into_log::().ok()) .map(|update| { - let version = - schedule - .schedule_successor(activation_height) - .map_err(|version| WorkerError::UnsupportedSpecActivation { - version, - block_height: enacting_height, - stuck_height, - })?; + let version = schedule + .schedule_successor(activation_height) + .map_err(|err| match err { + SpecScheduleError::UnknownSuccessor(version) => { + WorkerError::UnsupportedSpecActivation { + version, + block_height: enacting_height, + stuck_height, + } + } + err => WorkerError::InconsistentSpecSchedule(err), + })?; Ok(SpecActivationRecord { enacting_height, version, From ce8eb2fb50a59f1da4bb3e6ad62f120d43f9586b Mon Sep 17 00:00:00 2001 From: Prajwol Gyawali Date: Thu, 30 Jul 2026 15:58:49 +0545 Subject: [PATCH 8/8] refactor(worker): encapsulate SpecActivationRecord fields Public fields let callers construct or mutate a record without going through from_raw, bypassing the SpecId validation it does. Make the fields private and add a new() constructor plus accessors, so construction always goes through one of the two typed entry points. --- bin/asm-runner/src/worker_context.rs | 6 +-- crates/worker/src/state.rs | 32 ++++++++-------- crates/worker/src/test_utils.rs | 6 +-- crates/worker/src/traits.rs | 57 ++++++++++++++++++---------- tests/asm/spec_activation.rs | 18 ++++----- 5 files changed, 67 insertions(+), 52 deletions(-) diff --git a/bin/asm-runner/src/worker_context.rs b/bin/asm-runner/src/worker_context.rs index 29eb55c4..847f326c 100644 --- a/bin/asm-runner/src/worker_context.rs +++ b/bin/asm-runner/src/worker_context.rs @@ -234,9 +234,9 @@ impl SpecActivationStore for AsmWorkerContext { fn record_spec_activation(&self, activation: SpecActivationRecord) -> WorkerResult<()> { self.spec_activation_db .put( - activation.enacting_height, - activation.version.into(), - &activation.new_predicate, + activation.enacting_height(), + activation.version().into(), + activation.new_predicate(), ) .map_err(WorkerError::DbError) } diff --git a/crates/worker/src/state.rs b/crates/worker/src/state.rs index 3a357c0e..1d5a3726 100644 --- a/crates/worker/src/state.rs +++ b/crates/worker/src/state.rs @@ -236,11 +236,11 @@ where } err => WorkerError::InconsistentSpecSchedule(err), })?; - Ok(SpecActivationRecord { + Ok(SpecActivationRecord::new( enacting_height, version, - new_predicate: update.into_new_predicate(), - }) + update.into_new_predicate(), + )) }) .collect() } @@ -257,8 +257,8 @@ where activations: Vec, ) -> WorkerResult<()> { for activation in activations { - let version = activation.version; - let enacting_height = activation.enacting_height; + let version = activation.version(); + let enacting_height = activation.enacting_height(); let activation_height = activation.activation_height(); self.context.record_spec_activation(activation)?; self.spec_schedule.schedule(version, activation_height)?; @@ -298,7 +298,7 @@ fn effective_schedule( ) -> WorkerResult { let mut schedule = base.clone(); for activation in activations { - schedule.schedule(activation.version, activation.activation_height())?; + schedule.schedule(activation.version(), activation.activation_height())?; } Ok(schedule) } @@ -655,11 +655,11 @@ mod tests { .discover_spec_activations(&block_at(150), &[upgrade_log()]) .unwrap(); - let expected = vec![SpecActivationRecord { - enacting_height: 150, - version: SpecId::V1, - new_predicate: upgrade_predicate(), - }]; + let expected = vec![SpecActivationRecord::new( + 150, + SpecId::V1, + upgrade_predicate(), + )]; assert_eq!(discovered, expected); assert_eq!(fx.state.spec_schedule, SpecSchedule::genesis()); assert!(fx.state.context.list_spec_activations().unwrap().is_empty()); @@ -737,11 +737,11 @@ mod tests { let fx = fixtures::setup_state(101).await; let context = fx.state.context.clone(); // shares the sled store context - .record_spec_activation(SpecActivationRecord { - enacting_height: 120, - version: SpecId::V1, - new_predicate: upgrade_predicate(), - }) + .record_spec_activation(SpecActivationRecord::new( + 120, + SpecId::V1, + upgrade_predicate(), + )) .unwrap(); let params = fixtures::genesis_params(&fx.client, 101).await; diff --git a/crates/worker/src/test_utils.rs b/crates/worker/src/test_utils.rs index 94bfc57e..b7cff9f5 100644 --- a/crates/worker/src/test_utils.rs +++ b/crates/worker/src/test_utils.rs @@ -266,9 +266,9 @@ impl SpecActivationStore for TestAsmWorkerContext { self.state .spec_activation_db .put( - activation.enacting_height, - activation.version.into(), - &activation.new_predicate, + activation.enacting_height(), + activation.version().into(), + activation.new_predicate(), ) .map_err(WorkerError::DbError) } diff --git a/crates/worker/src/traits.rs b/crates/worker/src/traits.rs index fb3930f0..5b0d3815 100644 --- a/crates/worker/src/traits.rs +++ b/crates/worker/src/traits.rs @@ -37,21 +37,21 @@ use crate::WorkerResult; /// way; this is the act-time form, with the id mapped through [`SpecId`]. #[derive(Debug, Clone, PartialEq, Eq)] pub struct SpecActivationRecord { - /// Height of the L1 block whose ASM VK upgrade enactment triggered the - /// activation. - pub enacting_height: L1Height, - - /// The activated spec version. - pub version: SpecId, - - /// The ASM STF predicate the upgrade enacted. Enactment removes the - /// update from the admin queue and only surfaces it in the emitted log, - /// so this record is the worker's durable copy of the VK the boundary - /// switched to. - pub new_predicate: PredicateKey, + enacting_height: L1Height, + version: SpecId, + new_predicate: PredicateKey, } impl SpecActivationRecord { + /// Builds a record from its already-typed parts. + pub fn new(enacting_height: L1Height, version: SpecId, new_predicate: PredicateKey) -> Self { + Self { + enacting_height, + version, + new_predicate, + } + } + /// Reassembles a record from its raw stored parts, mapping the raw /// version id through [`SpecId`]. Errs with the raw id when this binary /// has no variant for it. @@ -60,11 +60,30 @@ impl SpecActivationRecord { version: u16, new_predicate: PredicateKey, ) -> Result { - Ok(Self { + Ok(Self::new( enacting_height, - version: SpecId::try_from(version)?, + SpecId::try_from(version)?, new_predicate, - }) + )) + } + + /// Height of the L1 block whose ASM VK upgrade enactment triggered the + /// activation. + pub fn enacting_height(&self) -> L1Height { + self.enacting_height + } + + /// The activated spec version. + pub fn version(&self) -> SpecId { + self.version + } + + /// The ASM STF predicate the upgrade enacted. Enactment removes the + /// update from the admin queue and only surfaces it in the emitted log, + /// so this record is the worker's durable copy of the VK the boundary + /// switched to. + pub fn new_predicate(&self) -> &PredicateKey { + &self.new_predicate } /// Height from which the version's rules apply: the block after the @@ -286,11 +305,7 @@ mod tests { #[test] fn activation_is_block_after_enactment() { - let record = SpecActivationRecord { - enacting_height: 41, - version: SpecId::V1, - new_predicate: predicate(), - }; + let record = SpecActivationRecord::new(41, SpecId::V1, predicate()); assert_eq!(record.activation_height(), 42); } @@ -299,7 +314,7 @@ mod tests { #[test] fn from_raw_maps_known_versions_only() { let record = SpecActivationRecord::from_raw(41, SpecId::V1.into(), predicate()).unwrap(); - assert_eq!(record.version, SpecId::V1); + assert_eq!(record.version(), SpecId::V1); assert_eq!( SpecActivationRecord::from_raw(41, 0xBEEF, predicate()), Err(0xBEEF) diff --git a/tests/asm/spec_activation.rs b/tests/asm/spec_activation.rs index 75a9cf87..c9995b7a 100644 --- a/tests/asm/spec_activation.rs +++ b/tests/asm/spec_activation.rs @@ -55,15 +55,15 @@ async fn test_spec_upgrade_activation_lifecycle() { let activations = harness.context.list_spec_activations().unwrap(); assert_eq!(activations.len(), 1, "exactly one activation expected"); - assert_eq!(activations[0].version, SpecId::V1); + assert_eq!(activations[0].version(), SpecId::V1); assert_eq!( activations[0].activation_height(), - activations[0].enacting_height + 1, + activations[0].enacting_height() + 1, "the version's rules apply from the block after the enacting one", ); assert_eq!( - activations[0].new_predicate, - post_upgrade_predicate(), + activations[0].new_predicate(), + &post_upgrade_predicate(), "the record must carry the VK the upgrade enacted", ); @@ -71,7 +71,7 @@ async fn test_spec_upgrade_activation_lifecycle() { // discovered the activation from. let enacting_hash = harness .client - .get_block_hash(activations[0].enacting_height as u64) + .get_block_hash(activations[0].enacting_height() as u64) .await .unwrap(); let enacting_block = harness.commitment_of(enacting_hash).await.unwrap(); @@ -108,7 +108,7 @@ async fn test_reorg_rolls_back_and_rediscovers_activation() { let activations = harness.context.list_spec_activations().unwrap(); assert_eq!(activations.len(), 1, "activation recorded at enactment"); - let enacting_height = activations[0].enacting_height; + let enacting_height = activations[0].enacting_height(); let submission_height = enacting_height as u64 - DEFAULT_CONFIRMATION_DEPTH as u64; // Reorg out the submission block and everything above it, replacing it @@ -139,11 +139,11 @@ async fn test_reorg_rolls_back_and_rediscovers_activation() { 1, "the re-mined update must re-enact on the new branch", ); - assert_eq!(activations[0].version, SpecId::V1); + assert_eq!(activations[0].version(), SpecId::V1); assert_eq!( - activations[0].enacting_height, + activations[0].enacting_height(), enacting_height + 1, "re-submission lands one block later, shifting enactment by one", ); - assert_eq!(activations[0].new_predicate, post_upgrade_predicate()); + assert_eq!(activations[0].new_predicate(), &post_upgrade_predicate()); }