fix(proposal): count recovery operators only in electorates they can vote in
This commit is contained in:
@@ -159,10 +159,7 @@ impl ProposalManager {
|
|||||||
.await?;
|
.await?;
|
||||||
|
|
||||||
let mut tally = self.store.tally(proposal_id).await?;
|
let mut tally = self.store.tally(proposal_id).await?;
|
||||||
// §3.5: recovery operators only join the electorate once they are awake.
|
self.narrow_electorate(&proposal, &mut tally).await?;
|
||||||
if !self.store.is_recovery_active().await? {
|
|
||||||
tally.total_recovery = 0;
|
|
||||||
}
|
|
||||||
|
|
||||||
self.settle(&proposal, &tally).await
|
self.settle(&proposal, &tally).await
|
||||||
}
|
}
|
||||||
@@ -240,7 +237,9 @@ impl ProposalManager {
|
|||||||
})
|
})
|
||||||
.await?;
|
.await?;
|
||||||
|
|
||||||
let tally = self.store.tally(proposal_id).await?;
|
let mut tally = self.store.tally(proposal_id).await?;
|
||||||
|
self.narrow_electorate(&proposal, &mut tally).await?;
|
||||||
|
|
||||||
self.settle(&proposal, &tally).await
|
self.settle(&proposal, &tally).await
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -257,6 +256,16 @@ impl ProposalManager {
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// §3.5/§3.6: recovery operators join the electorate only for the kinds they may vote on,
|
||||||
|
/// and only once the wake-up window has elapsed. Counting them anywhere else makes the
|
||||||
|
/// rejection threshold unreachable and, for full-quorum kinds, approval unreachable too.
|
||||||
|
async fn narrow_electorate(&self, proposal: &Proposal, tally: &mut Tally) -> Result<(), Error> {
|
||||||
|
if !proposal.kind.recovery_may_vote() || !self.store.is_recovery_active().await? {
|
||||||
|
tally.total_recovery = 0;
|
||||||
|
}
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
/// Pure quorum arithmetic — no I/O, so the rules can be tested directly (§3.3).
|
/// Pure quorum arithmetic — no I/O, so the rules can be tested directly (§3.3).
|
||||||
///
|
///
|
||||||
/// A proposal is rejected once approval has become unreachable: even if every voter
|
/// A proposal is rejected once approval has become unreachable: even if every voter
|
||||||
|
|||||||
@@ -180,11 +180,24 @@ async fn a_vote_short_of_quorum_does_not_touch_the_status() {
|
|||||||
assert_eq!(outcome, VoteOutcome::Pending);
|
assert_eq!(outcome, VoteOutcome::Pending);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// A sleeping recovery electorate must not raise the bar for an ordinary proposal.
|
/// Drives one `cast_vote` on a proposal of the given `kind` through a mocked store and
|
||||||
#[tokio::test]
|
/// returns the outcome. `recovery_active` decides what `is_recovery_active` reports;
|
||||||
async fn sleeping_recovery_operators_do_not_count_towards_quorum() {
|
/// `expected_status` is the status a settled outcome must be persisted under.
|
||||||
let id = ProposalId::from_raw(9);
|
///
|
||||||
let voter = OperatorIdentityId::from_raw(3);
|
/// `set_status` carries an argument matcher but no `.times()`: whichever outcome a caller
|
||||||
|
/// asserts is either `Approved` or `Rejected` (never `Pending`), so the write must happen
|
||||||
|
/// with the right status if it happens at all, but leaving the count unconstrained means a
|
||||||
|
/// regression that turns the outcome into `Pending` still fails on the caller's own
|
||||||
|
/// `assert_eq!` -- a readable diff -- rather than on a mockall cardinality panic that hides
|
||||||
|
/// what the actor actually computed.
|
||||||
|
async fn settle_vote_with(
|
||||||
|
kind: ProposalKindTag,
|
||||||
|
tally: Tally,
|
||||||
|
recovery_active: bool,
|
||||||
|
expected_status: ProposalStatus,
|
||||||
|
) -> VoteOutcome {
|
||||||
|
let id = ProposalId::from_raw(11);
|
||||||
|
let voter = OperatorIdentityId::from_raw(1);
|
||||||
let key = SigningKey::generate();
|
let key = SigningKey::generate();
|
||||||
let signature = key
|
let signature = key
|
||||||
.sign_message(&vote_message(id, true), SigningContext::GovernanceVote)
|
.sign_message(&vote_message(id, true), SigningContext::GovernanceVote)
|
||||||
@@ -194,29 +207,135 @@ async fn sleeping_recovery_operators_do_not_count_towards_quorum() {
|
|||||||
let mut store = MockProposalStore::new();
|
let mut store = MockProposalStore::new();
|
||||||
store
|
store
|
||||||
.expect_load()
|
.expect_load()
|
||||||
.returning(move |id| Ok(pending_proposal(id, ProposalKindTag::ApproveSdkClient)));
|
.returning(move |id| Ok(pending_proposal(id, kind)));
|
||||||
store.expect_has_voted().returning(|_, _| Ok(false));
|
store.expect_has_voted().returning(|_, _| Ok(false));
|
||||||
store
|
store
|
||||||
.expect_operator_public_key()
|
.expect_operator_public_key()
|
||||||
.returning(move |_| Ok(public_key.clone()));
|
.returning(move |_| Ok(public_key.clone()));
|
||||||
store.expect_record_vote().returning(|_| Ok(()));
|
store.expect_record_vote().returning(|_| Ok(()));
|
||||||
store.expect_is_recovery_active().returning(|| Ok(false));
|
store
|
||||||
// Two recovery operators exist but are asleep, so the threshold stays at 1 of 1.
|
.expect_is_recovery_active()
|
||||||
store.expect_tally().returning(|_| Ok(tally(1, 0, 1, 2)));
|
.returning(move || Ok(recovery_active));
|
||||||
store.expect_set_status().times(1).returning(|_, _| Ok(()));
|
store.expect_tally().returning(move |_| Ok(tally));
|
||||||
store.expect_load_kind().returning(|_, _| {
|
store
|
||||||
Ok(crate::db::proposal::ProposalKind::ApproveSdkClient(
|
.expect_set_status()
|
||||||
|
.withf(move |_, status| *status == expected_status)
|
||||||
|
.returning(|_, _| Ok(()));
|
||||||
|
store.expect_load_kind().returning(move |_, _| {
|
||||||
|
Ok(match kind {
|
||||||
|
ProposalKindTag::TriggerRekey => crate::db::proposal::ProposalKind::TriggerRekey,
|
||||||
|
ProposalKindTag::ApproveSdkClient => {
|
||||||
|
crate::db::proposal::ProposalKind::ApproveSdkClient(
|
||||||
crate::db::proposal::approve_sdk_client::Settings { client_id: 1 },
|
crate::db::proposal::approve_sdk_client::Settings { client_id: 1 },
|
||||||
))
|
)
|
||||||
|
}
|
||||||
|
ProposalKindTag::ReplaceOperator => crate::db::proposal::ProposalKind::ReplaceOperator(
|
||||||
|
crate::db::proposal::replace_operator::Settings {
|
||||||
|
old_operator_id: OperatorIdentityId::from_raw(1),
|
||||||
|
new_pubkey: vec![0u8; 32],
|
||||||
|
},
|
||||||
|
),
|
||||||
|
other => unreachable!("settle_vote_with has no load_kind fixture for {other:?}"),
|
||||||
|
})
|
||||||
});
|
});
|
||||||
|
|
||||||
let mut manager =
|
let mut manager =
|
||||||
ProposalManager::with_store(Arc::new(store), GlobalActors::spawn_message_bus());
|
ProposalManager::with_store(Arc::new(store), GlobalActors::spawn_message_bus());
|
||||||
|
|
||||||
let outcome = manager
|
manager
|
||||||
.cast_vote(id, voter, true, signature.to_bytes())
|
.cast_vote(id, voter, true, signature.to_bytes())
|
||||||
.await
|
.await
|
||||||
.expect("a valid vote must be accepted");
|
.expect("a valid vote must be accepted")
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The full-quorum rejection path is insensitive to electorate size by construction:
|
||||||
|
/// `threshold == total_eligible` there, so `total_eligible - threshold` is always 0 and any
|
||||||
|
/// single rejection settles the proposal, whether or not recovery operators are (wrongly)
|
||||||
|
/// counted. This does not exercise the electorate-narrowing fix -- see
|
||||||
|
/// `unanimous_ordinary_rejection_rejects_a_non_full_quorum_proposal_while_recovery_is_awake`
|
||||||
|
/// below for the test that does -- it just pins that `cast_vote` still writes `Rejected`
|
||||||
|
/// through `settle` for a full-quorum kind.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn unanimous_rejection_settles_a_full_quorum_rekey_via_cast_vote() {
|
||||||
|
let outcome = settle_vote_with(
|
||||||
|
ProposalKindTag::TriggerRekey,
|
||||||
|
Tally {
|
||||||
|
approve: 0,
|
||||||
|
reject: 3,
|
||||||
|
total_ordinary: 3,
|
||||||
|
total_recovery: 2,
|
||||||
|
},
|
||||||
|
/* recovery_active */ true,
|
||||||
|
ProposalStatus::Rejected,
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
|
||||||
|
assert_eq!(outcome, VoteOutcome::Rejected);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// §3.3 full quorum for a rekey means every *ordinary* operator, not every identity on file.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn unanimous_ordinary_approval_approves_a_rekey_while_recovery_is_awake() {
|
||||||
|
let outcome = settle_vote_with(
|
||||||
|
ProposalKindTag::TriggerRekey,
|
||||||
|
Tally {
|
||||||
|
approve: 3,
|
||||||
|
reject: 0,
|
||||||
|
total_ordinary: 3,
|
||||||
|
total_recovery: 2,
|
||||||
|
},
|
||||||
|
/* recovery_active */ true,
|
||||||
|
ProposalStatus::Approved,
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
|
||||||
|
assert_eq!(outcome, VoteOutcome::Approved);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// §3.5: recovery operators do not vote on `ApproveSdkClient`, so they must not inflate its
|
||||||
|
/// electorate. Before the fix, `total_eligible` counted them anyway (5, not 3), so the
|
||||||
|
/// rejection test `reject > total_eligible - threshold` became `3 > 5 - 2 = 3`, which is
|
||||||
|
/// false -- three unanimous rejections left the proposal `Pending` forever, since a fourth
|
||||||
|
/// vote could never arrive. After the fix, `total_eligible` is 3 and the same test becomes
|
||||||
|
/// `3 > 3 - 2 = 1`, which settles it.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn unanimous_ordinary_rejection_rejects_a_non_full_quorum_proposal_while_recovery_is_awake() {
|
||||||
|
let outcome = settle_vote_with(
|
||||||
|
ProposalKindTag::ApproveSdkClient,
|
||||||
|
Tally {
|
||||||
|
approve: 0,
|
||||||
|
reject: 3,
|
||||||
|
total_ordinary: 3,
|
||||||
|
total_recovery: 2,
|
||||||
|
},
|
||||||
|
/* recovery_active */ true,
|
||||||
|
ProposalStatus::Rejected,
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
|
||||||
|
assert_eq!(outcome, VoteOutcome::Rejected);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// §3.5/§3.6: `ReplaceOperator` is the one kind recovery may vote on, so it is the only kind
|
||||||
|
/// where whether recovery is awake is observable at all -- for every other kind
|
||||||
|
/// `narrow_electorate` zeroes `total_recovery` regardless of `is_recovery_active`, short-
|
||||||
|
/// circuiting before that call. A sleeping recovery electorate must not raise the bar here:
|
||||||
|
/// with 1 ordinary operator and 2 (asleep) recovery operators, the lone ordinary approval
|
||||||
|
/// must already reach full quorum.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn sleeping_recovery_operators_do_not_count_towards_quorum() {
|
||||||
|
let outcome = settle_vote_with(
|
||||||
|
ProposalKindTag::ReplaceOperator,
|
||||||
|
Tally {
|
||||||
|
approve: 1,
|
||||||
|
reject: 0,
|
||||||
|
total_ordinary: 1,
|
||||||
|
total_recovery: 2,
|
||||||
|
},
|
||||||
|
/* recovery_active */ false,
|
||||||
|
ProposalStatus::Approved,
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
|
||||||
assert_eq!(outcome, VoteOutcome::Approved);
|
assert_eq!(outcome, VoteOutcome::Approved);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -82,6 +82,12 @@ impl ProposalKindTag {
|
|||||||
pub const fn requires_full_quorum(self) -> bool {
|
pub const fn requires_full_quorum(self) -> bool {
|
||||||
matches!(self, Self::ReplaceOperator | Self::TriggerRekey)
|
matches!(self, Self::ReplaceOperator | Self::TriggerRekey)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// §3.5: recovery operators weigh in on operator replacement and nothing else.
|
||||||
|
#[must_use]
|
||||||
|
pub const fn recovery_may_vote(self) -> bool {
|
||||||
|
matches!(self, Self::ReplaceOperator)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Pins every implementation to the variant it is dispatched from. Without this a
|
/// Pins every implementation to the variant it is dispatched from. Without this a
|
||||||
|
|||||||
Reference in New Issue
Block a user