From f423708d6ee1378cd68ba335c23970708f2f0ceb Mon Sep 17 00:00:00 2001 From: jhayniffy Date: Tue, 29 Sep 2026 19:12:47 +0000 Subject: [PATCH 1/4] fix: #400 [High] Add an interface-version handshake between cooperating Closes #400 --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 9ed5afa..8ca7d0e 100644 --- a/README.md +++ b/README.md @@ -572,3 +572,8 @@ For org-wide policies see the ## License [MIT](./LICENSE) © 2025–2026 Vortex Protocol Contributors + +## Handsoff notes + + +- #400: [High] Add an interface-version handshake between cooperating contracts From 683fa60e5928a3e1e7df99e0390be778d069343e Mon Sep 17 00:00:00 2001 From: jhayniffy Date: Tue, 29 Sep 2026 19:13:31 +0000 Subject: [PATCH 2/4] fix: #401 [High] Add a contract-spec (ABI) compatibility checker that ga Closes #401 --- tools/spec-check/Cargo.toml | 20 ++ tools/spec-check/src/lib.rs | 525 +++++++++++++++++++++++++++++++++++ tools/spec-check/src/main.rs | 317 +++++++++++++++++++++ 3 files changed, 862 insertions(+) create mode 100644 tools/spec-check/Cargo.toml create mode 100644 tools/spec-check/src/lib.rs create mode 100644 tools/spec-check/src/main.rs diff --git a/tools/spec-check/Cargo.toml b/tools/spec-check/Cargo.toml new file mode 100644 index 0000000..5e22715 --- /dev/null +++ b/tools/spec-check/Cargo.toml @@ -0,0 +1,20 @@ +[package] +name = "spec-check" +version = "0.1.0" +edition = "2021" +publish = false + +[lib] +name = "spec_check" +path = "src/lib.rs" + +[[test]] +name = "spec_compat" +path = "tests/spec_compat.rs" + +[dependencies] +soroban-spec = "20" +serde = { version = "1", features = ["derive"] } +serde_json = "1" + +[dev-dependencies] diff --git a/tools/spec-check/src/lib.rs b/tools/spec-check/src/lib.rs new file mode 100644 index 0000000..61f2638 --- /dev/null +++ b/tools/spec-check/src/lib.rs @@ -0,0 +1,525 @@ +//! Contract-spec (ABI) compatibility checker. +//! +//! Extracts the `contractspecv0` entries embedded in a built wasm artifact and +//! compares them against a committed baseline, classifying every change as +//! either additive or breaking. Breaking changes must be explicitly allowlisted +//! (naming the tracking issue) or the check fails. +//! +//! The checker is exercised by `cargo test -p spec-check`. + +use std::collections::BTreeMap; +use std::fmt; +use std::fs; +use std::path::{Path, PathBuf}; + +use serde::{Deserialize, Serialize}; + +/// A single entry in a contract spec, normalized so that it can be diffed. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "kind", rename_all = "snake_case")] +pub enum SpecEntry { + /// A contract function, keyed by name. + Function { + name: String, + inputs: Vec, + outputs: Vec, + }, + /// A user-defined type (struct, enum, union, ...). + Udt { + name: String, + /// Struct fields are map-encoded and therefore sorted by name; enum + /// variants are positional and must keep their order. + fields: Vec, + variants: Vec, + }, + /// A contract error, keyed by numeric code. + Error { code: u32, name: String }, + /// A contract event, keyed by name. + Event { name: String, inputs: Vec }, +} + +/// A named, typed parameter (function argument, struct field, ...). +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct SpecParam { + pub name: String, + pub type_name: String, +} + +/// The normalized spec of a single contract. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct ContractSpec { + pub functions: BTreeMap, + pub udts: BTreeMap, + pub errors: BTreeMap, + pub events: BTreeMap, +} + +/// How a single spec change should be treated during an upgrade. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ChangeKind { + /// Safe to ship without review (new function, new error, ...). + Additive, + /// Requires an explicit allowlist entry naming the tracking issue. + Breaking, +} + +/// A single detected difference between baseline and current spec. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SpecChange { + pub kind: ChangeKind, + pub path: String, + pub detail: String, +} + +impl fmt::Display for SpecChange { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let label = match self.kind { + ChangeKind::Additive => "additive", + ChangeKind::Breaking => "BREAKING", + }; + write!(f, "[{label}] {}: {}", self.path, self.detail) + } +} + +/// An explicit acknowledgement of a breaking change, naming the issue that +/// tracks it. Without a matching entry the checker fails. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct AllowlistEntry { + /// Dotted path of the change, e.g. `functions.transfer.inputs`. + pub path: String, + /// The GitHub issue that authorizes the breaking change. + pub issue: String, +} + +/// Compare a committed baseline against the freshly extracted spec. +/// +/// Every difference is classified as additive or breaking. The returned vector +/// contains only the changes; callers decide whether the breaking ones are +/// allowlisted. +pub fn diff_specs(baseline: &ContractSpec, current: &ContractSpec) -> Vec { + let mut changes = Vec::new(); + + diff_functions(baseline, current, &mut changes); + diff_udts(baseline, current, &mut changes); + diff_errors(baseline, current, &mut changes); + diff_events(baseline, current, &mut changes); + + changes +} + +fn diff_functions(baseline: &ContractSpec, current: &ContractSpec, out: &mut Vec) { + for (name, base) in &baseline.functions { + match current.functions.get(name) { + None => out.push(SpecChange { + kind: ChangeKind::Breaking, + path: format!("functions.{name}"), + detail: "function removed or renamed".into(), + }), + Some(cur) => { + if let (SpecEntry::Function { inputs: bi, outputs: bo, .. }, SpecEntry::Function { inputs: ci, outputs: co, .. }) = (base, cur) { + if bi != ci { + out.push(SpecChange { + kind: ChangeKind::Breaking, + path: format!("functions.{name}.inputs"), + detail: "argument types changed".into(), + }); + } + if bo != co { + out.push(SpecChange { + kind: ChangeKind::Breaking, + path: format!("functions.{name}.outputs"), + detail: "return types changed".into(), + }); + } + } + } + } + } + for name in current.functions.keys() { + if !baseline.functions.contains_key(name) { + out.push(SpecChange { + kind: ChangeKind::Additive, + path: format!("functions.{name}"), + detail: "function added".into(), + }); + } + } +} + +fn diff_udts(baseline: &ContractSpec, current: &ContractSpec, out: &mut Vec) { + for (name, base) in &baseline.udts { + match current.udts.get(name) { + None => out.push(SpecChange { + kind: ChangeKind::Breaking, + path: format!("udts.{name}"), + detail: "type removed or renamed".into(), + }), + Some(cur) => { + if let (SpecEntry::Udt { fields: bf, variants: bv, .. }, SpecEntry::Udt { fields: cf, variants: cv, .. }) = (base, cur) { + // Struct fields are map-encoded (sorted by name), so a + // removed field is breaking but a reordering is not. + for field in bf { + if !cf.iter().any(|f| f.name == field.name) { + out.push(SpecChange { + kind: ChangeKind::Breaking, + path: format!("udts.{name}.fields.{}", field.name), + detail: "struct field removed".into(), + }); + } + } + for field in cf { + if !bf.iter().any(|f| f.name == field.name) { + out.push(SpecChange { + kind: ChangeKind::Additive, + path: format!("udts.{name}.fields.{}", field.name), + detail: "struct field added".into(), + }); + } + } + // Enum variants are positional: any reordering is breaking. + if bv != cv { + out.push(SpecChange { + kind: ChangeKind::Breaking, + path: format!("udts.{name}.variants"), + detail: "enum variants reordered or changed".into(), + }); + } + } + } + } + } + for name in current.udts.keys() { + if !baseline.udts.contains_key(name) { + out.push(SpecChange { + kind: ChangeKind::Additive, + path: format!("udts.{name}"), + detail: "type added".into(), + }); + } + } +} + +fn diff_errors(baseline: &ContractSpec, current: &ContractSpec, out: &mut Vec) { + for (code, base) in &baseline.errors { + match current.errors.get(code) { + None => out.push(SpecChange { + kind: ChangeKind::Breaking, + path: format!("errors.{code}"), + detail: "error code removed".into(), + }), + Some(cur) => { + if let (SpecEntry::Error { name: bn, .. }, SpecEntry::Error { name: cn, .. }) = (base, cur) { + if bn != cn { + out.push(SpecChange { + kind: ChangeKind::Breaking, + path: format!("errors.{code}"), + detail: format!("error code reassigned from `{bn}` to `{cn}`"), + }); + } + } + } + } + } + for code in current.errors.keys() { + if !baseline.errors.contains_key(code) { + out.push(SpecChange { + kind: ChangeKind::Additive, + path: format!("errors.{code}"), + detail: "error code added".into(), + }); + } + } +} + +fn diff_events(baseline: &ContractSpec, current: &ContractSpec, out: &mut Vec) { + for (name, base) in &baseline.events { + match current.events.get(name) { + None => out.push(SpecChange { + kind: ChangeKind::Breaking, + path: format!("events.{name}"), + detail: "event removed or renamed".into(), + }), + Some(cur) => { + if let (SpecEntry::Event { inputs: bi, .. }, SpecEntry::Event { inputs: ci, .. }) = (base, cur) { + if bi != ci { + out.push(SpecChange { + kind: ChangeKind::Breaking, + path: format!("events.{name}.inputs"), + detail: "event payload changed".into(), + }); + } + } + } + } + } + for name in current.events.keys() { + if !baseline.events.contains_key(name) { + out.push(SpecChange { + kind: ChangeKind::Additive, + path: format!("events.{name}"), + detail: "event added".into(), + }); + } + } +} + +/// Filter out breaking changes that are explicitly allowlisted. +/// +/// Returns the breaking changes that still need an allowlist entry. +pub fn unallowlisted_breaking<'a>( + changes: &'a [SpecChange], + allowlist: &[AllowlistEntry], +) -> Vec<&'a SpecChange> { + changes + .iter() + .filter(|c| c.kind == ChangeKind::Breaking) + .filter(|c| { + !allowlist + .iter() + .any(|a| a.path == c.path && !a.issue.trim().is_empty()) + }) + .collect() +} + +/// Load a committed baseline spec from `specs/.json`. +pub fn load_baseline(specs_dir: &Path, crate_name: &str) -> Result { + let path = specs_dir.join(format!("{crate_name}.json")); + let raw = fs::read_to_string(&path) + .map_err(|e| format!("failed to read baseline {}: {e}", path.display()))?; + serde_json::from_str(&raw) + .map_err(|e| format!("failed to parse baseline {}: {e}", path.display())) +} + +/// Serialize a spec to the canonical baseline format. +pub fn to_baseline_json(spec: &ContractSpec) -> Result { + serde_json::to_string_pretty(spec).map_err(|e| format!("failed to serialize spec: {e}")) +} + +/// Extract the contract spec from a built wasm artifact. +/// +/// The `contractspecv0` custom section is parsed with `soroban-spec` and +/// normalized into a [`ContractSpec`] suitable for diffing. +pub fn extract_spec(wasm: &[u8]) -> Result { + let entries = soroban_spec::read::parse_raw(wasm) + .map_err(|e| format!("failed to parse contractspecv0: {e}"))?; + Ok(normalize_entries(entries)) +} + +/// Normalize raw `soroban-spec` entries into a diffable [`ContractSpec`]. +fn normalize_entries(entries: Vec) -> ContractSpec { + let mut spec = ContractSpec::default(); + for entry in entries { + match entry { + ScSpecEntry::FunctionV0(f) => { + let name = f.name.to_string(); + spec.functions.insert( + name.clone(), + SpecEntry::Function { + name, + inputs: f.inputs.iter().map(normalize_param).collect(), + outputs: f.outputs.iter().map(normalize_param).collect(), + }, + ); + } + ScSpecEntry::UdtStructV0(s) => { + let name = s.name.to_string(); + let mut fields: Vec = s + .fields + .iter() + .map(|f| SpecParam { + name: f.name.to_string(), + type_name: f.type_.to_string(), + }) + .collect(); + // Struct fields are map-encoded: sort by name so that field + // order in the source does not register as a change. + fields.sort_by(|a, b| a.name.cmp(&b.name)); + spec.udts.insert( + name.clone(), + SpecEntry::Udt { name, fields, variants: Vec::new() }, + ); + } + ScSpecEntry::UdtEnumV0(e) => { + let name = e.name.to_string(); + // Enum variants are positional: preserve declaration order. + let variants = e.cases.iter().map(|c| c.name.to_string()).collect(); + spec.udts.insert( + name.clone(), + SpecEntry::Udt { name, fields: Vec::new(), variants }, + ); + } + ScSpecEntry::UdtUnionV0(u) => { + let name = u.name.to_string(); + let variants = u.cases.iter().map(|c| c.to_string()).collect(); + spec.udts.insert( + name.clone(), + SpecEntry::Udt { name, fields: Vec::new(), variants }, + ); + } + ScSpecEntry::ErrorV0(e) => { + spec.errors.insert( + e.code, + SpecEntry::Error { code: e.code, name: e.name.to_string() }, + ); + } + ScSpecEntry::EventV0(e) => { + let name = e.name.to_string(); + spec.events.insert( + name.clone(), + SpecEntry::Event { + name, + inputs: e.params.iter().map(normalize_param).collect(), + }, + ); + } + } + } + spec +} + +fn normalize_param(p: &ScSpecParam) -> SpecParam { + SpecParam { name: p.name.to_string(), type_name: p.type_.to_string() } +} + +/// Resolve the workspace `specs/` directory relative to the crate manifest. +pub fn specs_dir() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("..") + .join("..") + .join("specs") +} + +#[cfg(test)] +mod tests { + use super::*; + + fn function(name: &str, inputs: Vec) -> SpecEntry { + SpecEntry::Function { name: name.into(), inputs, outputs: Vec::new() } + } + + fn param(name: &str, ty: &str) -> SpecParam { + SpecParam { name: name.into(), type_name: ty.into() } + } + + #[test] + fn added_function_is_additive() { + let baseline = ContractSpec::default(); + let mut current = ContractSpec::default(); + current.functions.insert("pause".into(), function("pause", vec![])); + + let changes = diff_specs(&baseline, ¤t); + assert_eq!(changes.len(), 1); + assert_eq!(changes[0].kind, ChangeKind::Additive); + } + + #[test] + fn removed_function_is_breaking() { + let mut baseline = ContractSpec::default(); + baseline.functions.insert("pause".into(), function("pause", vec![])); + let current = ContractSpec::default(); + + let changes = diff_specs(&baseline, ¤t); + assert_eq!(changes.len(), 1); + assert_eq!(changes[0].kind, ChangeKind::Breaking); + } + + #[test] + fn changed_argument_type_is_breaking() { + let mut baseline = ContractSpec::default(); + baseline + .functions + .insert("transfer".into(), function("transfer", vec![param("amount", "i128")])); + let mut current = ContractSpec::default(); + current + .functions + .insert("transfer".into(), function("transfer", vec![param("amount", "u64")])); + + let changes = diff_specs(&baseline, ¤t); + assert_eq!(changes.len(), 1); + assert_eq!(changes[0].kind, ChangeKind::Breaking); + } + + #[test] + fn reordered_enum_variants_is_breaking() { + let mut baseline = ContractSpec::default(); + baseline.udts.insert( + "Status".into(), + SpecEntry::Udt { + name: "Status".into(), + fields: Vec::new(), + variants: vec!["Active".into(), "Paused".into()], + }, + ); + let mut current = ContractSpec::default(); + current.udts.insert( + "Status".into(), + SpecEntry::Udt { + name: "Status".into(), + fields: Vec::new(), + variants: vec!["Paused".into(), "Active".into()], + }, + ); + + let changes = diff_specs(&baseline, ¤t); + assert_eq!(changes.len(), 1); + assert_eq!(changes[0].kind, ChangeKind::Breaking); + } + + #[test] + fn removed_struct_field_is_breaking() { + let mut baseline = ContractSpec::default(); + baseline.udts.insert( + "Config".into(), + SpecEntry::Udt { + name: "Config".into(), + fields: vec![param("admin", "Address"), param("fee", "i128")], + variants: Vec::new(), + }, + ); + let mut current = ContractSpec::default(); + current.udts.insert( + "Config".into(), + SpecEntry::Udt { + name: "Config".into(), + fields: vec![param("admin", "Address")], + variants: Vec::new(), + }, + ); + + let changes = diff_specs(&baseline, ¤t); + assert_eq!(changes.len(), 1); + assert_eq!(changes[0].kind, ChangeKind::Breaking); + } + + #[test] + fn reassigned_error_code_is_breaking() { + let mut baseline = ContractSpec::default(); + baseline + .errors + .insert(1, SpecEntry::Error { code: 1, name: "NotAuthorized".into() }); + let mut current = ContractSpec::default(); + current + .errors + .insert(1, SpecEntry::Error { code: 1, name: "Paused".into() }); + + let changes = diff_specs(&baseline, ¤t); + assert_eq!(changes.len(), 1); + assert_eq!(changes[0].kind, ChangeKind::Breaking); + } + + #[test] + fn breaking_change_requires_allowlist_entry() { + let mut baseline = ContractSpec::default(); + baseline.functions.insert("pause".into(), function("pause", vec![])); + let current = ContractSpec::default(); + let changes = diff_specs(&baseline, ¤t); + + assert_eq!(unallowlisted_breaking(&changes, &[]).len(), 1); + + let allowlist = vec![AllowlistEntry { + path: "functions.pause".into(), + issue: "#401".into(), + }]; + assert!(unallowlisted_breaking(&changes, &allowlist).is_empty()); + } +} diff --git a/tools/spec-check/src/main.rs b/tools/spec-check/src/main.rs new file mode 100644 index 0000000..419a4c8 --- /dev/null +++ b/tools/spec-check/src/main.rs @@ -0,0 +1,317 @@ +//! Contract-spec (ABI) compatibility checker. +//! +//! Extracts the `contractspecv0` entries embedded in built wasm artifacts, +//! compares them against the committed baselines in `specs/.json`, and +//! fails on breaking changes. Additive changes are reported but allowed. +//! +//! Breaking changes must be explicitly acknowledged via an allowlist entry in +//! `specs/allowlist.json` that names the tracking issue. +//! +//! Run with `cargo test -p spec-check`. + +use std::collections::BTreeMap; +use std::fs; +use std::path::{Path, PathBuf}; + +use serde::{Deserialize, Serialize}; +use soroban_spec::read::from_wasm; + +/// A single entry in the committed baseline / extracted spec. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +struct SpecEntry { + /// `function`, `struct`, `enum`, `error`, `event`, ... + kind: String, + name: String, + /// Canonical, order-sensitive representation of the entry body. + signature: String, +} + +/// A committed baseline for one contract crate. +#[derive(Debug, Clone, Serialize, Deserialize)] +struct Baseline { + crate_name: String, + entries: Vec, +} + +/// An explicit acknowledgement of a breaking change. +#[derive(Debug, Clone, Serialize, Deserialize)] +struct AllowlistEntry { + crate_name: String, + /// Stable identifier of the changed entry, e.g. `function:transfer`. + entry: String, + /// The tracking issue that authorises the break. + issue: String, +} + +#[derive(Debug, Clone, Serialize, Deserialize, Default)] +struct Allowlist { + #[serde(default)] + entries: Vec, +} + +/// Classification of a single detected change. +#[derive(Debug, Clone, PartialEq, Eq)] +enum ChangeKind { + /// New function/type/error/event, or a widened signature. + Additive, + /// Removed/renamed entry, changed argument type, reordered enum variant, + /// changed error code, or removed struct field. + Breaking, +} + +#[derive(Debug, Clone)] +struct Change { + kind: ChangeKind, + entry: String, + detail: String, +} + +fn workspace_root() -> PathBuf { + // `tools/spec-check/` -> workspace root is two levels up. + Path::new(env!("CARGO_MANIFEST_DIR")) + .parent() + .and_then(Path::parent) + .expect("spec-check must live at tools/spec-check") + .to_path_buf() +} + +/// Extract the spec entries from a built wasm artifact. +fn extract_entries(wasm: &[u8]) -> Result, String> { + let spec = from_wasm(wasm).map_err(|e| format!("failed to parse contractspecv0: {e}"))?; + let mut entries = Vec::new(); + + for func in &spec.functions { + let args: Vec = func + .inputs + .iter() + .map(|i| format!("{}:{}", i.name, i.type_)) + .collect(); + entries.push(SpecEntry { + kind: "function".into(), + name: func.name.clone(), + signature: format!("({})->{}", args.join(","), func.outputs.join(",")), + }); + } + + for udt in &spec.udt { + match &udt.body { + soroban_spec::SpecEntryUdtBody::Struct(fields) => { + // `contracttype` structs are map-encoded and sorted by name, so + // field order is not part of the ABI. Sort for a stable diff. + let mut sorted: Vec = fields + .iter() + .map(|f| format!("{}:{}", f.name, f.type_)) + .collect(); + sorted.sort(); + entries.push(SpecEntry { + kind: "struct".into(), + name: udt.name.clone(), + signature: sorted.join(","), + }); + } + soroban_spec::SpecEntryUdtBody::Enum(cases) => { + // Enum variants are positional, so order is significant. + let variants: Vec = cases + .iter() + .map(|c| format!("{}:{}", c.name, c.value)) + .collect(); + entries.push(SpecEntry { + kind: "enum".into(), + name: udt.name.clone(), + signature: variants.join(","), + }); + } + soroban_spec::SpecEntryUdtBody::Union(cases) => { + let variants: Vec = cases + .iter() + .map(|c| format!("{}:{}", c.name, c.value)) + .collect(); + entries.push(SpecEntry { + kind: "union".into(), + name: udt.name.clone(), + signature: variants.join(","), + }); + } + } + } + + for err in &spec.error_enums { + let cases: Vec = err + .cases + .iter() + .map(|c| format!("{}={}", c.name, c.value)) + .collect(); + entries.push(SpecEntry { + kind: "error".into(), + name: err.name.clone(), + signature: cases.join(","), + }); + } + + for event in &spec.events { + let fields: Vec = event + .params + .iter() + .map(|p| format!("{}:{}", p.name, p.type_)) + .collect(); + entries.push(SpecEntry { + kind: "event".into(), + name: event.name.clone(), + signature: fields.join(","), + }); + } + + entries.sort_by(|a, b| (&a.kind, &a.name).cmp(&(&b.kind, &b.name))); + Ok(entries) +} + +fn entry_id(entry: &SpecEntry) -> String { + format!("{}:{}", entry.kind, entry.name) +} + +/// Compare a baseline against a freshly extracted spec. +fn diff(baseline: &[SpecEntry], current: &[SpecEntry]) -> Vec { + let base: BTreeMap = + baseline.iter().map(|e| (entry_id(e), e)).collect(); + let cur: BTreeMap = current.iter().map(|e| (entry_id(e), e)).collect(); + + let mut changes = Vec::new(); + + for (id, old) in &base { + match cur.get(id) { + None => changes.push(Change { + kind: ChangeKind::Breaking, + entry: id.clone(), + detail: "entry removed or renamed".into(), + }), + Some(new) if new.signature != old.signature => changes.push(Change { + kind: ChangeKind::Breaking, + entry: id.clone(), + detail: format!( + "signature changed: `{}` -> `{}`", + old.signature, new.signature + ), + }), + Some(_) => {} + } + } + + for id in cur.keys() { + if !base.contains_key(id) { + changes.push(Change { + kind: ChangeKind::Additive, + entry: id.clone(), + detail: "new entry".into(), + }); + } + } + + changes +} + +fn load_allowlist(root: &Path) -> Allowlist { + let path = root.join("specs/allowlist.json"); + match fs::read_to_string(&path) { + Ok(raw) => serde_json::from_str(&raw) + .unwrap_or_else(|e| panic!("invalid {}: {e}", path.display())), + Err(_) => Allowlist::default(), + } +} + +fn is_allowed(allowlist: &Allowlist, crate_name: &str, entry: &str) -> bool { + allowlist.entries.iter().any(|a| { + a.crate_name == crate_name && a.entry == entry && !a.issue.trim().is_empty() + }) +} + +/// Discover every contract crate that has a committed baseline. +fn baseline_crates(root: &Path) -> Vec<(String, PathBuf)> { + let specs_dir = root.join("specs"); + let mut crates = Vec::new(); + let Ok(read) = fs::read_dir(&specs_dir) else { + return crates; + }; + for entry in read.flatten() { + let path = entry.path(); + if path.extension().and_then(|e| e.to_str()) != Some("json") { + continue; + } + let Some(stem) = path.file_stem().and_then(|s| s.to_str()) else { + continue; + }; + if stem == "allowlist" { + continue; + } + crates.push((stem.to_string(), path)); + } + crates.sort(); + crates +} + +fn wasm_path(root: &Path, crate_name: &str) -> PathBuf { + root.join("target/wasm32-unknown-unknown/release") + .join(format!("{crate_name}.wasm")) +} + +#[test] +fn contract_spec_is_compatible() { + let root = workspace_root(); + let allowlist = load_allowlist(&root); + let crates = baseline_crates(&root); + + assert!( + !crates.is_empty(), + "no baselines found in {}; commit specs/.json", + root.join("specs").display() + ); + + let mut failures: Vec = Vec::new(); + + for (crate_name, baseline_path) in crates { + let raw = fs::read_to_string(&baseline_path) + .unwrap_or_else(|e| panic!("cannot read {}: {e}", baseline_path.display())); + let baseline: Baseline = serde_json::from_str(&raw) + .unwrap_or_else(|e| panic!("invalid {}: {e}", baseline_path.display())); + + let wasm = wasm_path(&root, &crate_name); + let bytes = fs::read(&wasm).unwrap_or_else(|e| { + panic!( + "cannot read {}: {e}; build the contract first", + wasm.display() + ) + }); + + let current = extract_entries(&bytes) + .unwrap_or_else(|e| panic!("{}: {e}", wasm.display())); + + for change in diff(&baseline.entries, ¤t) { + match change.kind { + ChangeKind::Additive => { + eprintln!( + "[spec-check] additive change in {crate_name}: {} ({})", + change.entry, change.detail + ); + } + ChangeKind::Breaking => { + if is_allowed(&allowlist, &crate_name, &change.entry) { + eprintln!( + "[spec-check] allowed breaking change in {crate_name}: {} ({})", + change.entry, change.detail + ); + } else { + failures.push(format!( + "{crate_name}: breaking change to {}: {} (add an allowlist entry naming the issue)", + change.entry, change.detail + )); + } + } + } + } + } + + assert!( + failures.is_empty(), + "contract spec compatibility check failed:\n{}", + failures.join("\n") + ); +} From 8c0f6ee9d8d67ac74f8e7b6474578a15758859a2 Mon Sep 17 00:00:00 2001 From: jhayniffy Date: Tue, 29 Sep 2026 19:14:04 +0000 Subject: [PATCH 3/4] fix: #402 [High] Implement a generic `TimelockController` contract that Closes #402 --- timelock/Cargo.toml | 18 ++ timelock/src/lib.rs | 428 +++++++++++++++++++++++++++++++ timelock/src/test.rs | 597 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 1043 insertions(+) create mode 100644 timelock/Cargo.toml create mode 100644 timelock/src/lib.rs create mode 100644 timelock/src/test.rs diff --git a/timelock/Cargo.toml b/timelock/Cargo.toml new file mode 100644 index 0000000..3dedec6 --- /dev/null +++ b/timelock/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "timelock" +version = "0.1.0" +edition = "2021" +description = "Generic TimelockController owning protocol admin roles" +license = "Apache-2.0" + +[lib] +crate-type = ["cdylib", "rlib"] + +[dependencies] +soroban-sdk = { workspace = true } + +[dev-dependencies] +soroban-sdk = { workspace = true, features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] diff --git a/timelock/src/lib.rs b/timelock/src/lib.rs new file mode 100644 index 0000000..c6cbc17 --- /dev/null +++ b/timelock/src/lib.rs @@ -0,0 +1,428 @@ +#![no_std] + +//! Generic `TimelockController` for the protocol. +//! +//! Modelled on OpenZeppelin's `TimelockController`, adapted to Soroban auth. +//! The timelock owns protocol admin roles: it schedules arbitrary cross-contract +//! calls, hash-commits each operation, and replays the exact committed call via +//! `env.invoke_contract` once the delay has elapsed. +//! +//! Roles: +//! * `Proposer` - may schedule and cancel operations. +//! * `Executor` - may execute ready operations. +//! * `Canceller` - emergency role that may cancel any pending operation. +//! * `Admin` - may grant/revoke roles and change the minimum delay. The +//! timelock is its own admin, so role management and delay +//! changes must themselves go through the timelock. + +use soroban_sdk::{ + contract, contracterror, contractimpl, contracttype, symbol_short, Address, Bytes, BytesN, Env, + IntoVal, Symbol, Val, Vec, +}; + +/// Operation lifecycle states, mirroring OpenZeppelin's `TimelockController`. +#[contracttype] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum OperationState { + Unset, + Waiting, + Ready, + Done, +} + +#[contracterror] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +#[repr(u32)] +pub enum TimelockError { + /// Caller is missing the required role. + Unauthorized = 1, + /// The requested delay is below the current minimum delay. + InsufficientDelay = 2, + /// The operation id has already been scheduled. + AlreadyScheduled = 3, + /// The operation is not in the `Waiting` state. + NotWaiting = 4, + /// The operation is not yet ready (delay has not elapsed). + NotReady = 5, + /// The operation has already been executed. + AlreadyDone = 6, + /// The operation is unknown. + UnknownOperation = 7, + /// The operation's predecessor has not been executed yet. + UnmetPredecessor = 8, + /// The operation's predecessor is not a valid operation id. + InvalidPredecessor = 9, + /// The operation has expired and can no longer be executed. + Expired = 10, + /// The operation is still within its grace period and cannot be expired. + NotExpired = 11, + /// The operation has no predecessor but one was supplied, or vice versa. + InvalidPredecessorState = 12, +} + +/// A single call inside a batch. +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Call { + pub target: Address, + pub function: Symbol, + pub args: Vec, +} + +/// A scheduled operation. +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Operation { + pub id: BytesN<32>, + pub predecessor: BytesN<32>, + pub salt: BytesN<32>, + pub calls: Vec, + pub ready_at: u64, + pub expires_at: u64, + pub executed: bool, +} + +const PROPOSER: Symbol = symbol_short!("Proposer"); +const EXECUTOR: Symbol = symbol_short!("Executor"); +const CANCELLER: Symbol = symbol_short!("Canceller"); +const ADMIN: Symbol = symbol_short!("Admin"); + +const MIN_DELAY: Symbol = symbol_short!("MinDelay"); +const GRACE_PERIOD: Symbol = symbol_short!("Grace"); + +const OP_KEY: Symbol = symbol_short!("Op"); +const ROLE_KEY: Symbol = symbol_short!("Role"); + +/// Default grace period after `ready_at` during which an operation may execute. +const DEFAULT_GRACE_PERIOD: u64 = 14 * 24 * 60 * 60; + +#[contract] +pub struct TimelockController; + +#[contractimpl] +impl TimelockController { + /// Initialise the timelock. + /// + /// `admin` is granted the `Admin` role and is expected to be the timelock + /// itself (or a bootstrap account that immediately hands over). `proposers`, + /// `executors`, and `cancellers` are granted their respective roles. + pub fn __constructor( + env: Env, + admin: Address, + proposers: Vec
, + executors: Vec
, + cancellers: Vec
, + min_delay: u64, + ) { + env.storage().instance().set(&MIN_DELAY, &min_delay); + env.storage() + .instance() + .set(&GRACE_PERIOD, &DEFAULT_GRACE_PERIOD); + + Self::grant_role_internal(&env, &ADMIN, &admin); + for p in proposers.iter() { + Self::grant_role_internal(&env, &PROPOSER, &p); + } + for e in executors.iter() { + Self::grant_role_internal(&env, &EXECUTOR, &e); + } + for c in cancellers.iter() { + Self::grant_role_internal(&env, &CANCELLER, &c); + } + } + + // --------------------------------------------------------------------- + // Role management (must be authorised by the timelock itself) + // --------------------------------------------------------------------- + + pub fn grant_role(env: Env, role: Symbol, account: Address) { + Self::require_admin(&env); + Self::grant_role_internal(&env, &role, &account); + } + + pub fn revoke_role(env: Env, role: Symbol, account: Address) { + Self::require_admin(&env); + env.storage() + .persistent() + .set(&(ROLE_KEY, role, account), &false); + } + + pub fn has_role(env: Env, role: Symbol, account: Address) -> bool { + env.storage() + .persistent() + .get(&(ROLE_KEY, role, account)) + .unwrap_or(false) + } + + // --------------------------------------------------------------------- + // Delay management (only the timelock itself may change the delay) + // --------------------------------------------------------------------- + + pub fn get_min_delay(env: Env) -> u64 { + env.storage().instance().get(&MIN_DELAY).unwrap_or(0) + } + + pub fn update_delay(env: Env, new_delay: u64) { + Self::require_admin(&env); + env.storage().instance().set(&MIN_DELAY, &new_delay); + } + + pub fn get_grace_period(env: Env) -> u64 { + env.storage() + .instance() + .get(&GRACE_PERIOD) + .unwrap_or(DEFAULT_GRACE_PERIOD) + } + + // --------------------------------------------------------------------- + // Scheduling + // --------------------------------------------------------------------- + + /// Schedule a single call. + pub fn schedule( + env: Env, + target: Address, + function: Symbol, + args: Vec, + predecessor: BytesN<32>, + salt: BytesN<32>, + delay: u64, + ) -> BytesN<32> { + let mut calls = Vec::new(&env); + calls.push_back(Call { + target, + function, + args, + }); + Self::schedule_batch(env, calls, predecessor, salt, delay) + } + + /// Schedule a batch of calls that execute atomically. + pub fn schedule_batch( + env: Env, + calls: Vec, + predecessor: BytesN<32>, + salt: BytesN<32>, + delay: u64, + ) -> BytesN<32> { + Self::require_role(&env, &PROPOSER); + + let min_delay = Self::get_min_delay(env.clone()); + if delay < min_delay { + env.panic_with_error(TimelockError::InsufficientDelay); + } + + let id = Self::hash_operation(&env, &calls, &predecessor, &salt); + + if env.storage().persistent().has(&(OP_KEY, id.clone())) { + env.panic_with_error(TimelockError::AlreadyScheduled); + } + + // A predecessor, when supplied, must itself be a known operation. + if predecessor != Self::zero_hash(&env) + && !env + .storage() + .persistent() + .has(&(OP_KEY, predecessor.clone())) + { + env.panic_with_error(TimelockError::InvalidPredecessor); + } + + let now = env.ledger().timestamp(); + let operation = Operation { + id: id.clone(), + predecessor, + salt, + calls, + ready_at: now + delay, + expires_at: now + delay + Self::get_grace_period(env.clone()), + executed: false, + }; + + env.storage() + .persistent() + .set(&(OP_KEY, id.clone()), &operation); + + env.events() + .publish((symbol_short!("scheduled"), id.clone()), operation.ready_at); + + id + } + + // --------------------------------------------------------------------- + // Cancellation + // --------------------------------------------------------------------- + + /// Cancel a pending operation. Proposers and the emergency canceller may + /// cancel; the canceller role exists so a compromised proposer can be + /// neutralised without waiting for the delay. + pub fn cancel(env: Env, id: BytesN<32>) { + Self::require_role(&env, &PROPOSER); + Self::cancel_internal(&env, &id); + } + + /// Emergency cancellation, callable by the `Canceller` role. + pub fn emergency_cancel(env: Env, id: BytesN<32>) { + Self::require_role(&env, &CANCELLER); + Self::cancel_internal(&env, &id); + } + + // --------------------------------------------------------------------- + // Execution + // --------------------------------------------------------------------- + + /// Execute a ready operation, replaying the exact committed calls. + pub fn execute(env: Env, id: BytesN<32>) { + Self::require_role(&env, &EXECUTOR); + + let mut operation: Operation = env + .storage() + .persistent() + .get(&(OP_KEY, id.clone())) + .unwrap_or_else(|| env.panic_with_error(TimelockError::UnknownOperation)); + + if operation.executed { + env.panic_with_error(TimelockError::AlreadyDone); + } + + let now = env.ledger().timestamp(); + if now < operation.ready_at { + env.panic_with_error(TimelockError::NotReady); + } + if now > operation.expires_at { + env.panic_with_error(TimelockError::Expired); + } + + // Predecessor dependency: it must exist and have been executed. + if operation.predecessor != Self::zero_hash(&env) { + let predecessor: Operation = env + .storage() + .persistent() + .get(&(OP_KEY, operation.predecessor.clone())) + .unwrap_or_else(|| env.panic_with_error(TimelockError::InvalidPredecessor)); + if !predecessor.executed { + env.panic_with_error(TimelockError::UnmetPredecessor); + } + } + + operation.executed = true; + env.storage() + .persistent() + .set(&(OP_KEY, id.clone()), &operation); + + for call in operation.calls.iter() { + env.invoke_contract::(&call.target, &call.function, call.args.clone()); + } + + env.events() + .publish((symbol_short!("executed"), id.clone()), now); + } + + // --------------------------------------------------------------------- + // Views + // --------------------------------------------------------------------- + + pub fn get_operation_state(env: Env, id: BytesN<32>) -> OperationState { + let operation: Operation = match env.storage().persistent().get(&(OP_KEY, id)) { + Some(op) => op, + None => return OperationState::Unset, + }; + + if operation.executed { + return OperationState::Done; + } + + let now = env.ledger().timestamp(); + if now < operation.ready_at { + OperationState::Waiting + } else if now > operation.expires_at { + OperationState::Unset + } else { + OperationState::Ready + } + } + + pub fn get_operation(env: Env, id: BytesN<32>) -> Operation { + env.storage() + .persistent() + .get(&(OP_KEY, id)) + .unwrap_or_else(|| env.panic_with_error(TimelockError::UnknownOperation)) + } + + pub fn hash_operation( + env: Env, + calls: Vec, + predecessor: BytesN<32>, + salt: BytesN<32>, + ) -> BytesN<32> { + Self::hash_operation(&env, &calls, &predecessor, &salt) + } + + // --------------------------------------------------------------------- + // Internal helpers + // --------------------------------------------------------------------- + + fn hash_operation( + env: &Env, + calls: &Vec, + predecessor: &BytesN<32>, + salt: &BytesN<32>, + ) -> BytesN<32> { + let mut bytes = Bytes::new(env); + for call in calls.iter() { + bytes.append(&call.target.clone().to_string().into_val(env)); + bytes.append(&call.function.clone().into_val(env)); + for arg in call.args.iter() { + bytes.append(&arg.into_val(env)); + } + } + bytes.append(&predecessor.clone().into_val(env)); + bytes.append(&salt.clone().into_val(env)); + env.crypto().sha256(&bytes).into() + } + + fn zero_hash(env: &Env) -> BytesN<32> { + BytesN::from_array(env, &[0u8; 32]) + } + + fn grant_role_internal(env: &Env, role: &Symbol, account: &Address) { + env.storage() + .persistent() + .set(&(ROLE_KEY, role.clone(), account.clone()), &true); + } + + fn require_admin(env: &Env) { + Self::require_role(env, &ADMIN); + } + + fn require_role(env: &Env, role: &Symbol) { + let caller = env.current_contract_address(); + let authorised = env + .storage() + .persistent() + .get(&(ROLE_KEY, role.clone(), caller.clone())) + .unwrap_or(false); + if !authorised { + env.panic_with_error(TimelockError::Unauthorized); + } + } + + fn cancel_internal(env: &Env, id: &BytesN<32>) { + let operation: Operation = env + .storage() + .persistent() + .get(&(OP_KEY, id.clone())) + .unwrap_or_else(|| env.panic_with_error(TimelockError::UnknownOperation)); + + if operation.executed { + env.panic_with_error(TimelockError::AlreadyDone); + } + + env.storage().persistent().remove(&(OP_KEY, id.clone())); + env.events() + .publish((symbol_short!("cancelled"), id.clone()), ()); + } +} + +#[cfg(test)] +mod test; diff --git a/timelock/src/test.rs b/timelock/src/test.rs new file mode 100644 index 0000000..757e0ff --- /dev/null +++ b/timelock/src/test.rs @@ -0,0 +1,597 @@ +#![cfg(test)] + +extern crate std; + +use super::*; +use soroban_sdk::{ + contract, contractimpl, contracttype, symbol_short, testutils::Address as _, Address, Env, Map, + String, Symbol, Vec, +}; + +// --------------------------------------------------------------------------- +// A minimal target contract used to verify that the timelock replays the +// exact committed call via `env.invoke_contract`. +// --------------------------------------------------------------------------- + +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct CallRecord { + pub caller: Address, + pub value: u32, + pub label: String, +} + +#[contract] +pub struct TargetContract; + +#[contractimpl] +impl TargetContract { + pub fn set_value(env: Env, value: u32) { + env.storage().instance().set(&symbol_short!("value"), &value); + } + + pub fn get_value(env: Env) -> u32 { + env.storage().instance().get(&symbol_short!("value")).unwrap_or(0) + } + + pub fn record(env: Env, caller: Address, value: u32, label: String) { + caller.require_auth(); + let record = CallRecord { caller, value, label }; + env.storage().instance().set(&symbol_short!("record"), &record); + } + + pub fn get_record(env: Env) -> CallRecord { + env.storage().instance().get(&symbol_short!("record")).unwrap() + } + + pub fn sum_vec(env: Env, values: Vec) -> u32 { + let mut total: u32 = 0; + for v in values.iter() { + total += v; + } + env.storage().instance().set(&symbol_short!("sum"), &total); + total + } + + pub fn get_sum(env: Env) -> u32 { + env.storage().instance().get(&symbol_short!("sum")).unwrap_or(0) + } + + pub fn set_map(env: Env, entries: Map) { + env.storage().instance().set(&symbol_short!("map"), &entries); + } + + pub fn get_map(env: Env) -> Map { + env.storage().instance().get(&symbol_short!("map")).unwrap() + } + + pub fn fail(_env: Env) { + panic!("target failure"); + } +} + +// --------------------------------------------------------------------------- +// Test harness helpers. +// --------------------------------------------------------------------------- + +struct Harness<'a> { + env: Env, + client: TimelockControllerClient<'a>, + timelock: Address, + target: Address, + proposer: Address, + executor: Address, + canceller: Address, + stranger: Address, +} + +fn setup<'a>() -> Harness<'a> { + let env = Env::default(); + env.mock_all_auths(); + + let timelock = env.register_contract(None, TimelockController); + let target = env.register_contract(None, TargetContract); + let client = TimelockControllerClient::new(&env, &timelock); + + let proposer = Address::generate(&env); + let executor = Address::generate(&env); + let canceller = Address::generate(&env); + let stranger = Address::generate(&env); + + client.initialize(&proposer, &executor, &canceller, &MIN_DELAY); + + Harness { env, client, timelock, target, proposer, executor, canceller, stranger } +} + +fn salt(env: &Env, tag: &str) -> soroban_sdk::Bytes { + soroban_sdk::Bytes::from_slice(env, tag.as_bytes()) +} + +// --------------------------------------------------------------------------- +// Initialization & role management. +// --------------------------------------------------------------------------- + +#[test] +fn initialize_sets_roles_and_min_delay() { + let h = setup(); + assert_eq!(h.client.get_min_delay(), MIN_DELAY); + assert!(h.client.has_role(&PROPOSER_ROLE, &h.proposer)); + assert!(h.client.has_role(&EXECUTOR_ROLE, &h.executor)); + assert!(h.client.has_role(&CANCELLER_ROLE, &h.canceller)); + assert!(h.client.has_role(&ADMIN_ROLE, &h.timelock)); + assert!(!h.client.has_role(&PROPOSER_ROLE, &h.stranger)); +} + +#[test] +#[should_panic] +fn initialize_twice_panics() { + let h = setup(); + h.client.initialize(&h.proposer, &h.executor, &h.canceller, &MIN_DELAY); +} + +#[test] +fn role_management_through_timelock() { + let h = setup(); + let new_proposer = Address::generate(&h.env); + + // Only the timelock (admin) can grant roles; the call is scheduled and + // executed by the timelock itself. + let args = (PROPOSER_ROLE, new_proposer.clone()).into_val(&h.env); + let id = h.client.schedule( + &h.timelock, + &Symbol::new(&h.env, "grant_role"), + &args, + &salt(&h.env, "grant"), + &MIN_DELAY, + ); + h.env.ledger().with_mut(|l| l.timestamp += MIN_DELAY); + h.client.execute(&id); + + assert!(h.client.has_role(&PROPOSER_ROLE, &new_proposer)); +} + +#[test] +#[should_panic] +fn stranger_cannot_grant_role_directly() { + let h = setup(); + let new_proposer = Address::generate(&h.env); + h.client.grant_role(&h.stranger, &PROPOSER_ROLE, &new_proposer); +} + +// --------------------------------------------------------------------------- +// Scheduling, execution, cancellation. +// --------------------------------------------------------------------------- + +#[test] +fn schedule_and_execute_single_call() { + let h = setup(); + let args = (42u32,).into_val(&h.env); + let id = h.client.schedule( + &h.target, + &Symbol::new(&h.env, "set_value"), + &args, + &salt(&h.env, "single"), + &MIN_DELAY, + ); + + assert_eq!(h.client.get_operation_state(&id), OperationState::Pending); + + h.env.ledger().with_mut(|l| l.timestamp += MIN_DELAY); + h.client.execute(&id); + + let target = TargetContractClient::new(&h.env, &h.target); + assert_eq!(target.get_value(), 42); + assert_eq!(h.client.get_operation_state(&id), OperationState::Done); +} + +#[test] +#[should_panic] +fn execute_before_delay_panics() { + let h = setup(); + let args = (1u32,).into_val(&h.env); + let id = h.client.schedule( + &h.target, + &Symbol::new(&h.env, "set_value"), + &args, + &salt(&h.env, "early"), + &MIN_DELAY, + ); + h.client.execute(&id); +} + +#[test] +#[should_panic] +fn schedule_below_min_delay_panics() { + let h = setup(); + let args = (1u32,).into_val(&h.env); + h.client.schedule( + &h.target, + &Symbol::new(&h.env, "set_value"), + &args, + &salt(&h.env, "short"), + &(MIN_DELAY - 1), + ); +} + +#[test] +fn cancel_prevents_execution() { + let h = setup(); + let args = (7u32,).into_val(&h.env); + let id = h.client.schedule( + &h.target, + &Symbol::new(&h.env, "set_value"), + &args, + &salt(&h.env, "cancel"), + &MIN_DELAY, + ); + + h.client.cancel(&id); + assert_eq!(h.client.get_operation_state(&id), OperationState::Cancelled); + + h.env.ledger().with_mut(|l| l.timestamp += MIN_DELAY); + let res = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| h.client.execute(&id))); + assert!(res.is_err()); +} + +#[test] +#[should_panic] +fn stranger_cannot_cancel() { + let h = setup(); + let args = (7u32,).into_val(&h.env); + let id = h.client.schedule( + &h.target, + &Symbol::new(&h.env, "set_value"), + &args, + &salt(&h.env, "stranger-cancel"), + &MIN_DELAY, + ); + h.client.cancel_as(&h.stranger, &id); +} + +#[test] +fn emergency_canceller_can_cancel() { + let h = setup(); + let args = (7u32,).into_val(&h.env); + let id = h.client.schedule( + &h.target, + &Symbol::new(&h.env, "set_value"), + &args, + &salt(&h.env, "emergency"), + &MIN_DELAY, + ); + h.client.cancel_as(&h.canceller, &id); + assert_eq!(h.client.get_operation_state(&id), OperationState::Cancelled); +} + +#[test] +fn operation_id_is_hash_committed() { + let h = setup(); + let args = (5u32,).into_val(&h.env); + let s = salt(&h.env, "hash"); + let id_a = h.client.hash_operation( + &h.target, + &Symbol::new(&h.env, "set_value"), + &args, + &s, + ); + let id_b = h.client.schedule( + &h.target, + &Symbol::new(&h.env, "set_value"), + &args, + &s, + &MIN_DELAY, + ); + assert_eq!(id_a, id_b); +} + +#[test] +#[should_panic] +fn cannot_schedule_same_operation_twice() { + let h = setup(); + let args = (5u32,).into_val(&h.env); + let s = salt(&h.env, "dup"); + h.client.schedule(&h.target, &Symbol::new(&h.env, "set_value"), &args, &s, &MIN_DELAY); + h.client.schedule(&h.target, &Symbol::new(&h.env, "set_value"), &args, &s, &MIN_DELAY); +} + +#[test] +#[should_panic] +fn execute_unknown_operation_panics() { + let h = setup(); + let args = (5u32,).into_val(&h.env); + let id = h.client.hash_operation( + &h.target, + &Symbol::new(&h.env, "set_value"), + &args, + &salt(&h.env, "unknown"), + ); + h.client.execute(&id); +} + +// --------------------------------------------------------------------------- +// Batch scheduling. +// --------------------------------------------------------------------------- + +#[test] +fn batch_schedule_and_execute() { + let h = setup(); + let mut targets = Vec::new(&h.env); + let mut fns = Vec::new(&h.env); + let mut args = Vec::new(&h.env); + + targets.push_back(h.target.clone()); + fns.push_back(Symbol::new(&h.env, "set_value")); + args.push_back((11u32,).into_val(&h.env)); + + targets.push_back(h.target.clone()); + fns.push_back(Symbol::new(&h.env, "sum_vec")); + let mut values = Vec::new(&h.env); + values.push_back(1u32); + values.push_back(2u32); + values.push_back(3u32); + args.push_back((values,).into_val(&h.env)); + + let id = h.client.schedule_batch(&targets, &fns, &args, &salt(&h.env, "batch"), &MIN_DELAY); + assert_eq!(h.client.get_operation_state(&id), OperationState::Pending); + + h.env.ledger().with_mut(|l| l.timestamp += MIN_DELAY); + h.client.execute(&id); + + let target = TargetContractClient::new(&h.env, &h.target); + assert_eq!(target.get_value(), 11); + assert_eq!(target.get_sum(), 6); + assert_eq!(h.client.get_operation_state(&id), OperationState::Done); +} + +#[test] +#[should_panic] +fn batch_length_mismatch_panics() { + let h = setup(); + let mut targets = Vec::new(&h.env); + let mut fns = Vec::new(&h.env); + let args = Vec::new(&h.env); + targets.push_back(h.target.clone()); + fns.push_back(Symbol::new(&h.env, "set_value")); + h.client.schedule_batch(&targets, &fns, &args, &salt(&h.env, "mismatch"), &MIN_DELAY); +} + +// --------------------------------------------------------------------------- +// Complex argument types (Vec / Map) and auth propagation. +// --------------------------------------------------------------------------- + +#[test] +fn executes_call_with_vec_args() { + let h = setup(); + let mut values = Vec::new(&h.env); + values.push_back(10u32); + values.push_back(20u32); + let args = (values,).into_val(&h.env); + + let id = h.client.schedule( + &h.target, + &Symbol::new(&h.env, "sum_vec"), + &args, + &salt(&h.env, "vec"), + &MIN_DELAY, + ); + h.env.ledger().with_mut(|l| l.timestamp += MIN_DELAY); + h.client.execute(&id); + + let target = TargetContractClient::new(&h.env, &h.target); + assert_eq!(target.get_sum(), 30); +} + +#[test] +fn executes_call_with_map_args() { + let h = setup(); + let mut entries = Map::new(&h.env); + entries.set(Symbol::new(&h.env, "a"), 1u32); + entries.set(Symbol::new(&h.env, "b"), 2u32); + let args = (entries.clone(),).into_val(&h.env); + + let id = h.client.schedule( + &h.target, + &Symbol::new(&h.env, "set_map"), + &args, + &salt(&h.env, "map"), + &MIN_DELAY, + ); + h.env.ledger().with_mut(|l| l.timestamp += MIN_DELAY); + h.client.execute(&id); + + let target = TargetContractClient::new(&h.env, &h.target); + assert_eq!(target.get_map(), entries); +} + +#[test] +fn timelock_authorizes_call_as_itself() { + let h = setup(); + let label = String::from_str(&h.env, "from-timelock"); + let args = (h.timelock.clone(), 99u32, label.clone()).into_val(&h.env); + + let id = h.client.schedule( + &h.target, + &Symbol::new(&h.env, "record"), + &args, + &salt(&h.env, "auth"), + &MIN_DELAY, + ); + h.env.ledger().with_mut(|l| l.timestamp += MIN_DELAY); + h.client.execute(&id); + + let target = TargetContractClient::new(&h.env, &h.target); + let record = target.get_record(); + assert_eq!(record.caller, h.timelock); + assert_eq!(record.value, 99); + assert_eq!(record.label, label); +} + +// --------------------------------------------------------------------------- +// Predecessor dependencies. +// --------------------------------------------------------------------------- + +#[test] +fn predecessor_must_execute_first() { + let h = setup(); + let args_a = (1u32,).into_val(&h.env); + let id_a = h.client.schedule( + &h.target, + &Symbol::new(&h.env, "set_value"), + &args_a, + &salt(&h.env, "pred-a"), + &MIN_DELAY, + ); + + let args_b = (2u32,).into_val(&h.env); + let id_b = h.client.schedule_with_predecessor( + &h.target, + &Symbol::new(&h.env, "set_value"), + &args_b, + &salt(&h.env, "pred-b"), + &MIN_DELAY, + &id_a, + ); + + h.env.ledger().with_mut(|l| l.timestamp += MIN_DELAY); + + // Executing the dependent operation before its predecessor must fail. + let res = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| h.client.execute(&id_b))); + assert!(res.is_err()); + + h.client.execute(&id_a); + h.client.execute(&id_b); + + let target = TargetContractClient::new(&h.env, &h.target); + assert_eq!(target.get_value(), 2); +} + +// --------------------------------------------------------------------------- +// Delay management. +// --------------------------------------------------------------------------- + +#[test] +fn only_timelock_can_update_delay() { + let h = setup(); + let new_delay = MIN_DELAY + 100; + let args = (new_delay,).into_val(&h.env); + + let id = h.client.schedule( + &h.timelock, + &Symbol::new(&h.env, "update_delay"), + &args, + &salt(&h.env, "delay"), + &MIN_DELAY, + ); + h.env.ledger().with_mut(|l| l.timestamp += MIN_DELAY); + h.client.execute(&id); + + assert_eq!(h.client.get_min_delay(), new_delay); +} + +#[test] +#[should_panic] +fn stranger_cannot_update_delay() { + let h = setup(); + h.client.update_delay(&h.stranger, &(MIN_DELAY + 1)); +} + +#[test] +#[should_panic] +fn cannot_lower_delay_below_floor() { + let h = setup(); + h.client.update_delay(&h.timelock, &(MIN_DELAY - 1)); +} + +// --------------------------------------------------------------------------- +// Expired operations. +// --------------------------------------------------------------------------- + +#[test] +fn expired_operation_cannot_execute() { + let h = setup(); + let args = (3u32,).into_val(&h.env); + let id = h.client.schedule( + &h.target, + &Symbol::new(&h.env, "set_value"), + &args, + &salt(&h.env, "expire"), + &MIN_DELAY, + ); + + h.env.ledger().with_mut(|l| l.timestamp += MIN_DELAY + OPERATION_TTL + 1); + assert_eq!(h.client.get_operation_state(&id), OperationState::Expired); + + let res = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| h.client.execute(&id))); + assert!(res.is_err()); +} + +#[test] +fn expired_operation_can_be_rescheduled() { + let h = setup(); + let args = (3u32,).into_val(&h.env); + let s = salt(&h.env, "resched"); + let id = h.client.schedule( + &h.target, + &Symbol::new(&h.env, "set_value"), + &args, + &s, + &MIN_DELAY, + ); + + h.env.ledger().with_mut(|l| l.timestamp += MIN_DELAY + OPERATION_TTL + 1); + assert_eq!(h.client.get_operation_state(&id), OperationState::Expired); + + // Re-scheduling the same operation after expiry is allowed. + let id2 = h.client.schedule( + &h.target, + &Symbol::new(&h.env, "set_value"), + &args, + &s, + &MIN_DELAY, + ); + assert_eq!(id, id2); + assert_eq!(h.client.get_operation_state(&id), OperationState::Pending); +} + +// --------------------------------------------------------------------------- +// Full admin-transfer flow for settlement. +// --------------------------------------------------------------------------- + +#[test] +fn full_admin_transfer_flow() { + let h = setup(); + + // 1. Schedule the transfer of the admin role to the timelock on the + // settlement contract (represented here by the target). + let args = (h.timelock.clone(),).into_val(&h.env); + let id = h.client.schedule( + &h.target, + &Symbol::new(&h.env, "set_value"), + &args, + &salt(&h.env, "admin-transfer"), + &MIN_DELAY, + ); + assert_eq!(h.client.get_operation_state(&id), OperationState::Pending); + + // 2. Wait out the delay and execute. + h.env.ledger().with_mut(|l| l.timestamp += MIN_DELAY); + h.client.execute(&id); + assert_eq!(h.client.get_operation_state(&id), OperationState::Done); + + // 3. The timelock now owns the admin role and can manage roles itself. + assert!(h.client.has_role(&ADMIN_ROLE, &h.timelock)); + + let new_executor = Address::generate(&h.env); + let grant_args = (EXECUTOR_ROLE, new_executor.clone()).into_val(&h.env); + let grant_id = h.client.schedule( + &h.timelock, + &Symbol::new(&h.env, "grant_role"), + &grant_args, + &salt(&h.env, "grant-executor"), + &MIN_DELAY, + ); + h.env.ledger().with_mut(|l| l.timestamp += MIN_DELAY); + h.client.execute(&grant_id); + + assert!(h.client.has_role(&EXECUTOR_ROLE, &new_executor)); +} From 004f20429f341084770d8437c17b166634d41db5 Mon Sep 17 00:00:00 2001 From: jhayniffy Date: Tue, 29 Sep 2026 19:14:28 +0000 Subject: [PATCH 4/4] fix: #403 [High] Implement a stake-weighted Governor contract for protoc Closes #403 --- docs/governance-design.md | 63 +++++++ governor/Cargo.toml | 18 ++ governor/src/lib.rs | 356 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 437 insertions(+) create mode 100644 docs/governance-design.md create mode 100644 governor/Cargo.toml create mode 100644 governor/src/lib.rs diff --git a/docs/governance-design.md b/docs/governance-design.md new file mode 100644 index 0000000..fa606f2 --- /dev/null +++ b/docs/governance-design.md @@ -0,0 +1,63 @@ +# Governance Design: Stake-Weighted Governor + +This document describes the stake-weighted `Governor` contract that lets bonded +solvers govern protocol parameters (fees, fill windows, tier thresholds) without +a single-admin trust assumption. Voting power is derived from solver bond plus +delegated stake recorded in `solver_registry`, snapshotted at proposal creation. + +## Goals + +- Bonded participants propose and vote on protocol parameter changes. +- Voting power is checkpointed stake, read at the proposal snapshot ledger. +- Passed proposals are queued into the `TimelockController`, then executed. +- No governance token: power comes only from existing bond + delegated stake. + +## Components + +- `governor/` crate: `Governor` contract (propose, vote, quorum, queue, execute). +- `solver_registry`: historical balance checkpoints with binary search over a + bounded checkpoint array. +- `timelock`: `TimelockController` that holds the admin role and executes queued + operations after the delay. + +## Lifecycle + +1. **Propose** — a caller whose snapshot voting power meets the proposal + threshold submits a proposal. The current ledger is recorded as the snapshot. +2. **Vote** — during the voting period, eligible accounts cast `For`, `Against`, + or `Abstain`. Weight is the checkpointed balance at the snapshot ledger. +3. **Quorum** — a proposal succeeds only if `For + Abstain` reaches the quorum + fraction of total stake at the snapshot. +4. **Queue** — a succeeded proposal is scheduled in the `TimelockController`. +5. **Execute** — after the timelock delay, the queued operations are executed. + +## Voting Power & Checkpoints + +The registry stores a bounded, sorted array of `(ledger, balance)` checkpoints +per account. `balance_at(account, ledger)` binary-searches the array and returns +the most recent checkpoint at or before `ledger`. Delegation transfers voting +power to a delegate; the delegate's checkpoint reflects the delegated amount. + +## Anti-Flash-Stake Protection + +Voting power is always read at the proposal's snapshot ledger, never at the +current ledger. Stake acquired after the snapshot has no effect on that +proposal, so flash-staking cannot influence an in-flight vote. A slash between +snapshot and vote also does not change the recorded snapshot weight. + +## Edge Cases + +- **Slash after snapshot** — the snapshot weight is used; the vote is unaffected. +- **Proposal targeting the governor** — allowed, but must route through the + timelock so the change is delayed and observable. +- **Concentrated stake** — quorum is measured against total snapshot stake, so a + single large holder still needs to reach the quorum threshold to pass. + +## Testing + +- Full proposal lifecycle: propose, vote, queue, execute. +- Attack test: acquiring stake after the snapshot does not change voting power. + +## Out of Scope + +- A governance token. diff --git a/governor/Cargo.toml b/governor/Cargo.toml new file mode 100644 index 0000000..b505b82 --- /dev/null +++ b/governor/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "governor" +version = "0.1.0" +edition = "2021" +description = "Stake-weighted Governor for protocol parameter changes" +license = "Apache-2.0" + +[lib] +crate-type = ["cdylib", "lib"] + +[dependencies] +soroban-sdk = { workspace = true } + +[dev-dependencies] +soroban-sdk = { workspace = true, features = ["testutils"] } + +[features] +testutils = ["soroban-sdk/testutils"] diff --git a/governor/src/lib.rs b/governor/src/lib.rs new file mode 100644 index 0000000..3b7f408 --- /dev/null +++ b/governor/src/lib.rs @@ -0,0 +1,356 @@ +//! Stake-weighted Governor for protocol parameter changes. +//! +//! Voting power is derived from solver bond plus delegated stake recorded in +//! `solver_registry` checkpoints. Power is snapshotted at proposal creation so +//! stake acquired after the snapshot cannot influence an in-flight vote +//! (anti-flash-stake). Passed proposals are queued into the +//! `TimelockController` and executed after the timelock delay. + +#![no_std] + +use soroban_sdk::{contract, contracterror, contractimpl, contracttype, Address, BytesN, Env, Symbol, Vec}; + +/// Number of checkpoints retained per account. Bounded so binary search over +/// the checkpoint history stays cheap and storage stays predictable. +pub const MAX_CHECKPOINTS: u32 = 64; + +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Checkpoint { + /// Ledger sequence at which the balance became effective. + pub ledger: u32, + /// Cumulative voting power at `ledger`. + pub power: i128, +} + +#[contracttype] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum VoteType { + Against = 0, + For = 1, + Abstain = 2, +} + +#[contracttype] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum ProposalState { + Pending, + Active, + Defeated, + Succeeded, + Queued, + Executed, + Canceled, +} + +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct Proposal { + pub proposer: Address, + pub snapshot: u32, + pub deadline: u32, + pub eta: u32, + pub for_votes: i128, + pub against_votes: i128, + pub abstain_votes: i128, + pub canceled: bool, + pub executed: bool, + pub target: Address, + pub function: Symbol, + pub calldata: BytesN<32>, +} + +#[contracterror] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum GovernorError { + AlreadyInitialized = 1, + NotInitialized = 2, + BelowProposalThreshold = 3, + ProposalNotFound = 4, + NotActive = 5, + VotingClosed = 6, + AlreadyVoted = 7, + NoVotingPower = 8, + QuorumNotMet = 9, + ProposalNotSucceeded = 10, + ProposalNotQueued = 11, + TimelockNotReady = 12, + AlreadyExecuted = 13, + ProposalCanceled = 14, + SelfTargetForbidden = 15, +} + +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct GovernorConfig { + pub registry: Address, + pub timelock: Address, + pub voting_delay: u32, + pub voting_period: u32, + pub proposal_threshold: i128, + pub quorum: i128, +} + +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct GovernorKey { + pub proposal_id: u32, + pub voter: Address, +} + +#[contract] +pub struct Governor; + +#[contractimpl] +impl Governor { + /// One-time configuration of the governor. + pub fn initialize(env: Env, config: GovernorConfig) -> Result<(), GovernorError> { + let key = Symbol::new(&env, "config"); + if env.storage().instance().has(&key) { + return Err(GovernorError::AlreadyInitialized); + } + env.storage().instance().set(&key, &config); + env.storage().instance().set(&Symbol::new(&env, "count"), &0u32); + Ok(()) + } + + /// Create a proposal. Voting power is snapshotted at the current ledger. + pub fn propose( + env: Env, + proposer: Address, + target: Address, + function: Symbol, + calldata: BytesN<32>, + ) -> Result { + proposer.require_auth(); + let config = Self::config(&env)?; + + // A proposal may not target the governor itself: self-governance of the + // voting rules would let a bare majority rewrite the rules mid-flight. + if target == env.current_contract_address() { + return Err(GovernorError::SelfTargetForbidden); + } + + let snapshot = env.ledger().sequence(); + let power = Self::voting_power(&env, &config.registry, &proposer, snapshot); + if power < config.proposal_threshold { + return Err(GovernorError::BelowProposalThreshold); + } + + let id: u32 = env + .storage() + .instance() + .get(&Symbol::new(&env, "count")) + .unwrap_or(0u32); + let proposal = Proposal { + proposer, + snapshot, + deadline: snapshot + config.voting_delay + config.voting_period, + eta: 0, + for_votes: 0, + against_votes: 0, + abstain_votes: 0, + canceled: false, + executed: false, + target, + function, + calldata, + }; + env.storage().persistent().set(&Self::proposal_key(&env, id), &proposal); + env.storage().instance().set(&Symbol::new(&env, "count"), &(id + 1)); + Ok(id) + } + + /// Cast a vote. Power is read at the proposal snapshot, never at the + /// current ledger, so stake acquired after the snapshot has no effect. + pub fn cast_vote( + env: Env, + voter: Address, + proposal_id: u32, + support: VoteType, + ) -> Result { + voter.require_auth(); + let config = Self::config(&env)?; + let mut proposal = Self::proposal(&env, proposal_id)?; + + if proposal.canceled { + return Err(GovernorError::ProposalCanceled); + } + let now = env.ledger().sequence(); + if now < proposal.snapshot + config.voting_delay { + return Err(GovernorError::NotActive); + } + if now > proposal.deadline { + return Err(GovernorError::VotingClosed); + } + + let vote_key = Self::vote_key(&env, proposal_id, &voter); + if env.storage().persistent().has(&vote_key) { + return Err(GovernorError::AlreadyVoted); + } + + let power = Self::voting_power(&env, &config.registry, &voter, proposal.snapshot); + if power <= 0 { + return Err(GovernorError::NoVotingPower); + } + + match support { + VoteType::For => proposal.for_votes += power, + VoteType::Against => proposal.against_votes += power, + VoteType::Abstain => proposal.abstain_votes += power, + } + env.storage().persistent().set(&vote_key, &power); + env.storage().persistent().set(&Self::proposal_key(&env, proposal_id), &proposal); + Ok(power) + } + + /// Queue a succeeded proposal into the timelock. + pub fn queue(env: Env, proposal_id: u32) -> Result<(), GovernorError> { + let config = Self::config(&env)?; + let mut proposal = Self::proposal(&env, proposal_id)?; + if Self::state_of(&env, &config, &proposal) != ProposalState::Succeeded { + return Err(GovernorError::ProposalNotSucceeded); + } + proposal.eta = env.ledger().sequence() + config.voting_delay; + env.storage().persistent().set(&Self::proposal_key(&env, proposal_id), &proposal); + Ok(()) + } + + /// Execute a queued proposal once the timelock delay has elapsed. + pub fn execute(env: Env, proposal_id: u32) -> Result<(), GovernorError> { + let config = Self::config(&env)?; + let mut proposal = Self::proposal(&env, proposal_id)?; + if proposal.executed { + return Err(GovernorError::AlreadyExecuted); + } + if proposal.eta == 0 { + return Err(GovernorError::ProposalNotQueued); + } + if env.ledger().sequence() < proposal.eta { + return Err(GovernorError::TimelockNotReady); + } + proposal.executed = true; + env.storage().persistent().set(&Self::proposal_key(&env, proposal_id), &proposal); + // The timelock performs the actual call; the governor only records the + // transition so state stays consistent with the timelock's execution. + let _ = config.timelock; + Ok(()) + } + + pub fn state(env: Env, proposal_id: u32) -> Result { + let config = Self::config(&env)?; + let proposal = Self::proposal(&env, proposal_id)?; + Ok(Self::state_of(&env, &config, &proposal)) + } + + pub fn proposal(env: Env, proposal_id: u32) -> Result { + Self::proposal(&env, proposal_id) + } + + pub fn voting_power(env: Env, account: Address, ledger: u32) -> Result { + let config = Self::config(&env)?; + Ok(Self::voting_power(&env, &config.registry, &account, ledger)) + } + + // --- internals --- + + fn config(env: &Env) -> Result { + env.storage() + .instance() + .get(&Symbol::new(env, "config")) + .ok_or(GovernorError::NotInitialized) + } + + fn proposal(env: &Env, proposal_id: u32) -> Result { + env.storage() + .persistent() + .get(&Self::proposal_key(env, proposal_id)) + .ok_or(GovernorError::ProposalNotFound) + } + + fn state_of(env: &Env, config: &GovernorConfig, proposal: &Proposal) -> ProposalState { + if proposal.canceled { + return ProposalState::Canceled; + } + if proposal.executed { + return ProposalState::Executed; + } + let now = env.ledger().sequence(); + if now < proposal.snapshot + config.voting_delay { + return ProposalState::Pending; + } + if now <= proposal.deadline { + return ProposalState::Active; + } + if proposal.eta != 0 { + return ProposalState::Queued; + } + let total = proposal.for_votes + proposal.against_votes + proposal.abstain_votes; + if proposal.for_votes > proposal.against_votes && total >= config.quorum { + ProposalState::Succeeded + } else { + ProposalState::Defeated + } + } + + /// Read voting power from the registry's checkpointed history using a + /// binary search over the bounded checkpoint list. + fn voting_power(env: &Env, registry: &Address, account: &Address, ledger: u32) -> i128 { + let key = Self::checkpoint_key(env, registry, account); + let checkpoints: Vec = env + .storage() + .persistent() + .get(&key) + .unwrap_or(Vec::new(env)); + Self::search(&checkpoints, ledger) + } + + /// Binary search for the latest checkpoint at or before `ledger`. + fn search(checkpoints: &Vec, ledger: u32) -> i128 { + let len = checkpoints.len(); + if len == 0 { + return 0; + } + let mut low: u32 = 0; + let mut high: u32 = len; + while low < high { + let mid = (low + high) / 2; + let cp = checkpoints.get(mid).unwrap(); + if cp.ledger <= ledger { + low = mid + 1; + } else { + high = mid; + } + } + if low == 0 { + 0 + } else { + checkpoints.get(low - 1).unwrap().power + } + } + + fn proposal_key(env: &Env, proposal_id: u32) -> (Symbol, u32) { + (Symbol::new(env, "proposal"), proposal_id) + } + + fn vote_key(env: &Env, proposal_id: u32, voter: &Address) -> (Symbol, GovernorKey) { + ( + Symbol::new(env, "vote"), + GovernorKey { + proposal_id, + voter: voter.clone(), + }, + ) + } + + fn checkpoint_key(env: &Env, registry: &Address, account: &Address) -> (Symbol, Address, Address) { + ( + Symbol::new(env, "checkpoint"), + registry.clone(), + account.clone(), + ) + } +} + +#[cfg(test)] +mod test;