From f881102f0a1679c1b7e428d4d84bd6b1d4e8a381 Mon Sep 17 00:00:00 2001 From: CleverWild Date: Wed, 26 Aug 2026 12:49:11 +0200 Subject: [PATCH] refactor(proposal): drop the expired status, enforce expiry on every vote --- .../2026-02-14-171124-0000_init/up.sql | 2 +- .../src/actors/proposal_manager.rs | 57 ++++--------------- server/crates/arbiter-server/src/db/models.rs | 3 - .../crates/arbiter-server/tests/governance.rs | 32 +++++++---- 4 files changed, 35 insertions(+), 59 deletions(-) diff --git a/server/crates/arbiter-server/migrations/2026-02-14-171124-0000_init/up.sql b/server/crates/arbiter-server/migrations/2026-02-14-171124-0000_init/up.sql index 1bd40c3..60244bd 100644 --- a/server/crates/arbiter-server/migrations/2026-02-14-171124-0000_init/up.sql +++ b/server/crates/arbiter-server/migrations/2026-02-14-171124-0000_init/up.sql @@ -225,7 +225,7 @@ create table if not exists proposal ( created_at integer not null default(unixepoch('now')), expires_at integer not null, status text not null default 'pending' - check (status in ('pending', 'approved', 'rejected', 'expired')) + check (status in ('pending', 'approved', 'rejected')) ) STRICT; create table if not exists proposal_vote ( diff --git a/server/crates/arbiter-server/src/actors/proposal_manager.rs b/server/crates/arbiter-server/src/actors/proposal_manager.rs index a4e655e..21e69f9 100644 --- a/server/crates/arbiter-server/src/actors/proposal_manager.rs +++ b/server/crates/arbiter-server/src/actors/proposal_manager.rs @@ -16,7 +16,7 @@ use crate::{ use chrono::Utc; use diesel::{ExpressionMethods as _, QueryDsl}; use diesel_async::RunQueryDsl; -use kameo::{actor::ActorRef, messages}; +use kameo::{Actor, actor::ActorRef, messages}; use strum::{Display, EnumString, IntoStaticStr}; use tracing::{error, warn}; @@ -184,6 +184,8 @@ pub enum Error { ProposalNotFound, #[error("Proposal is not pending")] ProposalNotPending, + #[error("Proposal has expired")] + ProposalExpired, #[error("Operator already voted on this proposal")] AlreadyVoted, #[error("Invalid vote signature")] @@ -216,6 +218,7 @@ pub struct ProposalSummary { pub reject_count: i64, } +#[derive(Actor)] pub struct ProposalManager { pub(crate) db: db::DatabasePool, pub(crate) vault: ActorRef, @@ -239,27 +242,6 @@ impl ProposalManager { } } -impl kameo::Actor for ProposalManager { - type Args = Self; - type Error = (); - - async fn on_start(args: Self::Args, actor_ref: ActorRef) -> Result { - let weak = actor_ref.downgrade(); - tokio::spawn(async move { - loop { - tokio::time::sleep(tokio::time::Duration::from_hours(1)).await; - match weak.upgrade() { - Some(r) => { - let _ = r.ask(ExpireStale).await; - } - None => break, - } - } - }); - Ok(args) - } -} - #[messages] impl ProposalManager { #[message] @@ -346,29 +328,6 @@ impl ProposalManager { summaries } - #[message] - pub async fn expire_stale(&mut self) -> usize { - #[expect( - clippy::cast_possible_truncation, - clippy::as_conversions, - reason = "fixme! #84; this will break in 2038" - )] - let now_ts = Utc::now().timestamp() as i32; - - let Ok(mut conn) = self.db.get().await else { - warn!("expire_stale: failed to acquire DB connection"); - return 0; - }; - - diesel::update(schema::proposal::table) - .filter(schema::proposal::status.eq(ProposalStatus::Pending)) - .filter(schema::proposal::expires_at.lt(now_ts)) - .set(schema::proposal::status.eq(ProposalStatus::Expired)) - .execute(&mut conn) - .await - .unwrap_or(0) - } - #[message] pub async fn cast_vote( &mut self, @@ -406,6 +365,10 @@ impl ProposalManager { return Err(Error::ProposalNotPending); } + if proposal.expires_at.0 <= Utc::now() { + return Err(Error::ProposalExpired); + } + // Load operator public key from operator_identity let pubkey_bytes: Vec = schema::operator_identity::table .find(operator_id) @@ -609,6 +572,10 @@ impl ProposalManager { return Err(Error::ProposalNotPending); } + if proposal.expires_at.0 <= Utc::now() { + return Err(Error::ProposalExpired); + } + let pubkey_bytes: Vec = schema::recovery_operator_identity::table .find(recovery_operator_id) .select(schema::recovery_operator_identity::public_key) diff --git a/server/crates/arbiter-server/src/db/models.rs b/server/crates/arbiter-server/src/db/models.rs index 11a9919..c0c466b 100644 --- a/server/crates/arbiter-server/src/db/models.rs +++ b/server/crates/arbiter-server/src/db/models.rs @@ -149,7 +149,6 @@ pub mod types { Pending, Approved, Rejected, - Expired, } impl ToSql for ProposalStatus { @@ -161,7 +160,6 @@ pub mod types { Self::Pending => "pending", Self::Approved => "approved", Self::Rejected => "rejected", - Self::Expired => "expired", }; >::to_sql(s, out) } @@ -176,7 +174,6 @@ pub mod types { "pending" => Ok(Self::Pending), "approved" => Ok(Self::Approved), "rejected" => Ok(Self::Rejected), - "expired" => Ok(Self::Expired), other => Err(format!("Unknown proposal status: {other}").into()), } } diff --git a/server/crates/arbiter-server/tests/governance.rs b/server/crates/arbiter-server/tests/governance.rs index e7e0c28..6f6c68e 100644 --- a/server/crates/arbiter-server/tests/governance.rs +++ b/server/crates/arbiter-server/tests/governance.rs @@ -4,7 +4,7 @@ use arbiter_server::{ GlobalActors, proposal_manager::{ CancelRecoveryWakeup, CastRecoveryVote, CastVote, CreateProposal, - Error as ProposalError, ExpireStale, ProposalKind, QueryPending, + Error as ProposalError, ProposalKind, QueryPending, RequestRecoveryWakeup, VoteOutcome, }, }, @@ -382,7 +382,7 @@ async fn query_pending_excludes_already_voted() { } #[tokio::test] -async fn expire_stale_marks_old_proposals_expired() { +async fn expired_proposal_is_hidden_and_unvotable() { let db = db::create_test_pool().await; let actors = GlobalActors::spawn(db.clone()).await.unwrap(); actors @@ -398,7 +398,7 @@ async fn expire_stale_marks_old_proposals_expired() { let client_id = insert_unapproved_client(&db, &client_key.public_key()).await; // Create proposal with ttl_secs = -1 so it's immediately expired - let _proposal_id = actors + let proposal_id = actors .proposal_manager .ask(CreateProposal { kind: ProposalKind::ApproveSdkClient { client_id }, @@ -408,19 +408,31 @@ async fn expire_stale_marks_old_proposals_expired() { .await .unwrap(); - let expired = actors - .proposal_manager - .ask(ExpireStale) - .await - .unwrap(); - assert_eq!(expired, 1); - + // The row keeps status 'pending' (nothing sweeps it), but reads must skip it. let pending = actors .proposal_manager .ask(QueryPending { operator_id: op }) .await .unwrap(); assert!(pending.is_empty()); + + // And the write path must refuse it rather than rely on a status flip. + let msg = make_vote_message(proposal_id, true); + let sig = signing_key.sign_message(&msg, GOVERNANCE_CONTEXT).unwrap(); + let result = actors + .proposal_manager + .ask(CastVote { + proposal_id, + operator_id: op, + approve: true, + signature: sig.to_bytes(), + }) + .await; + + assert!(matches!( + result, + Err(kameo::error::SendError::HandlerError(ProposalError::ProposalExpired)) + )); } #[tokio::test]