refactor(proposal): drop the expired status, enforce expiry on every vote
This commit is contained in:
@@ -225,7 +225,7 @@ create table if not exists proposal (
|
|||||||
created_at integer not null default(unixepoch('now')),
|
created_at integer not null default(unixepoch('now')),
|
||||||
expires_at integer not null,
|
expires_at integer not null,
|
||||||
status text not null default 'pending'
|
status text not null default 'pending'
|
||||||
check (status in ('pending', 'approved', 'rejected', 'expired'))
|
check (status in ('pending', 'approved', 'rejected'))
|
||||||
) STRICT;
|
) STRICT;
|
||||||
|
|
||||||
create table if not exists proposal_vote (
|
create table if not exists proposal_vote (
|
||||||
|
|||||||
@@ -16,7 +16,7 @@ use crate::{
|
|||||||
use chrono::Utc;
|
use chrono::Utc;
|
||||||
use diesel::{ExpressionMethods as _, QueryDsl};
|
use diesel::{ExpressionMethods as _, QueryDsl};
|
||||||
use diesel_async::RunQueryDsl;
|
use diesel_async::RunQueryDsl;
|
||||||
use kameo::{actor::ActorRef, messages};
|
use kameo::{Actor, actor::ActorRef, messages};
|
||||||
use strum::{Display, EnumString, IntoStaticStr};
|
use strum::{Display, EnumString, IntoStaticStr};
|
||||||
use tracing::{error, warn};
|
use tracing::{error, warn};
|
||||||
|
|
||||||
@@ -184,6 +184,8 @@ pub enum Error {
|
|||||||
ProposalNotFound,
|
ProposalNotFound,
|
||||||
#[error("Proposal is not pending")]
|
#[error("Proposal is not pending")]
|
||||||
ProposalNotPending,
|
ProposalNotPending,
|
||||||
|
#[error("Proposal has expired")]
|
||||||
|
ProposalExpired,
|
||||||
#[error("Operator already voted on this proposal")]
|
#[error("Operator already voted on this proposal")]
|
||||||
AlreadyVoted,
|
AlreadyVoted,
|
||||||
#[error("Invalid vote signature")]
|
#[error("Invalid vote signature")]
|
||||||
@@ -216,6 +218,7 @@ pub struct ProposalSummary {
|
|||||||
pub reject_count: i64,
|
pub reject_count: i64,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[derive(Actor)]
|
||||||
pub struct ProposalManager {
|
pub struct ProposalManager {
|
||||||
pub(crate) db: db::DatabasePool,
|
pub(crate) db: db::DatabasePool,
|
||||||
pub(crate) vault: ActorRef<Vault>,
|
pub(crate) vault: ActorRef<Vault>,
|
||||||
@@ -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<Self>) -> Result<Self, Self::Error> {
|
|
||||||
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]
|
#[messages]
|
||||||
impl ProposalManager {
|
impl ProposalManager {
|
||||||
#[message]
|
#[message]
|
||||||
@@ -346,29 +328,6 @@ impl ProposalManager {
|
|||||||
summaries
|
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]
|
#[message]
|
||||||
pub async fn cast_vote(
|
pub async fn cast_vote(
|
||||||
&mut self,
|
&mut self,
|
||||||
@@ -406,6 +365,10 @@ impl ProposalManager {
|
|||||||
return Err(Error::ProposalNotPending);
|
return Err(Error::ProposalNotPending);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if proposal.expires_at.0 <= Utc::now() {
|
||||||
|
return Err(Error::ProposalExpired);
|
||||||
|
}
|
||||||
|
|
||||||
// Load operator public key from operator_identity
|
// Load operator public key from operator_identity
|
||||||
let pubkey_bytes: Vec<u8> = schema::operator_identity::table
|
let pubkey_bytes: Vec<u8> = schema::operator_identity::table
|
||||||
.find(operator_id)
|
.find(operator_id)
|
||||||
@@ -609,6 +572,10 @@ impl ProposalManager {
|
|||||||
return Err(Error::ProposalNotPending);
|
return Err(Error::ProposalNotPending);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if proposal.expires_at.0 <= Utc::now() {
|
||||||
|
return Err(Error::ProposalExpired);
|
||||||
|
}
|
||||||
|
|
||||||
let pubkey_bytes: Vec<u8> = schema::recovery_operator_identity::table
|
let pubkey_bytes: Vec<u8> = schema::recovery_operator_identity::table
|
||||||
.find(recovery_operator_id)
|
.find(recovery_operator_id)
|
||||||
.select(schema::recovery_operator_identity::public_key)
|
.select(schema::recovery_operator_identity::public_key)
|
||||||
|
|||||||
@@ -149,7 +149,6 @@ pub mod types {
|
|||||||
Pending,
|
Pending,
|
||||||
Approved,
|
Approved,
|
||||||
Rejected,
|
Rejected,
|
||||||
Expired,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
impl ToSql<Text, Sqlite> for ProposalStatus {
|
impl ToSql<Text, Sqlite> for ProposalStatus {
|
||||||
@@ -161,7 +160,6 @@ pub mod types {
|
|||||||
Self::Pending => "pending",
|
Self::Pending => "pending",
|
||||||
Self::Approved => "approved",
|
Self::Approved => "approved",
|
||||||
Self::Rejected => "rejected",
|
Self::Rejected => "rejected",
|
||||||
Self::Expired => "expired",
|
|
||||||
};
|
};
|
||||||
<str as ToSql<Text, Sqlite>>::to_sql(s, out)
|
<str as ToSql<Text, Sqlite>>::to_sql(s, out)
|
||||||
}
|
}
|
||||||
@@ -176,7 +174,6 @@ pub mod types {
|
|||||||
"pending" => Ok(Self::Pending),
|
"pending" => Ok(Self::Pending),
|
||||||
"approved" => Ok(Self::Approved),
|
"approved" => Ok(Self::Approved),
|
||||||
"rejected" => Ok(Self::Rejected),
|
"rejected" => Ok(Self::Rejected),
|
||||||
"expired" => Ok(Self::Expired),
|
|
||||||
other => Err(format!("Unknown proposal status: {other}").into()),
|
other => Err(format!("Unknown proposal status: {other}").into()),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -4,7 +4,7 @@ use arbiter_server::{
|
|||||||
GlobalActors,
|
GlobalActors,
|
||||||
proposal_manager::{
|
proposal_manager::{
|
||||||
CancelRecoveryWakeup, CastRecoveryVote, CastVote, CreateProposal,
|
CancelRecoveryWakeup, CastRecoveryVote, CastVote, CreateProposal,
|
||||||
Error as ProposalError, ExpireStale, ProposalKind, QueryPending,
|
Error as ProposalError, ProposalKind, QueryPending,
|
||||||
RequestRecoveryWakeup, VoteOutcome,
|
RequestRecoveryWakeup, VoteOutcome,
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
@@ -382,7 +382,7 @@ async fn query_pending_excludes_already_voted() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[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 db = db::create_test_pool().await;
|
||||||
let actors = GlobalActors::spawn(db.clone()).await.unwrap();
|
let actors = GlobalActors::spawn(db.clone()).await.unwrap();
|
||||||
actors
|
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;
|
let client_id = insert_unapproved_client(&db, &client_key.public_key()).await;
|
||||||
|
|
||||||
// Create proposal with ttl_secs = -1 so it's immediately expired
|
// Create proposal with ttl_secs = -1 so it's immediately expired
|
||||||
let _proposal_id = actors
|
let proposal_id = actors
|
||||||
.proposal_manager
|
.proposal_manager
|
||||||
.ask(CreateProposal {
|
.ask(CreateProposal {
|
||||||
kind: ProposalKind::ApproveSdkClient { client_id },
|
kind: ProposalKind::ApproveSdkClient { client_id },
|
||||||
@@ -408,19 +408,31 @@ async fn expire_stale_marks_old_proposals_expired() {
|
|||||||
.await
|
.await
|
||||||
.unwrap();
|
.unwrap();
|
||||||
|
|
||||||
let expired = actors
|
// The row keeps status 'pending' (nothing sweeps it), but reads must skip it.
|
||||||
.proposal_manager
|
|
||||||
.ask(ExpireStale)
|
|
||||||
.await
|
|
||||||
.unwrap();
|
|
||||||
assert_eq!(expired, 1);
|
|
||||||
|
|
||||||
let pending = actors
|
let pending = actors
|
||||||
.proposal_manager
|
.proposal_manager
|
||||||
.ask(QueryPending { operator_id: op })
|
.ask(QueryPending { operator_id: op })
|
||||||
.await
|
.await
|
||||||
.unwrap();
|
.unwrap();
|
||||||
assert!(pending.is_empty());
|
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]
|
#[tokio::test]
|
||||||
|
|||||||
Reference in New Issue
Block a user