Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions contracts/governance/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ pub enum Error {
MaxProposals,
NotASigner,
ProposalExpired,
/// Signer roster changes are blocked while proposals are actively voting.
SignerChangesLocked,
}

impl core::fmt::Display for Error {
Expand All @@ -34,6 +36,9 @@ impl core::fmt::Display for Error {
Error::MaxProposals => write!(f, "Maximum active proposals reached"),
Error::NotASigner => write!(f, "Caller is not a signer"),
Error::ProposalExpired => write!(f, "Proposal has expired"),
Error::SignerChangesLocked => {
write!(f, "Signer roster is locked while proposals are actively voting")
}
}
}
}
Expand All @@ -54,6 +59,7 @@ impl ContractError for Error {
Error::MaxProposals => governance_codes::GOVERNANCE_MAX_PROPOSALS,
Error::NotASigner => governance_codes::GOVERNANCE_NOT_A_SIGNER,
Error::ProposalExpired => governance_codes::GOVERNANCE_PROPOSAL_EXPIRED,
Error::SignerChangesLocked => governance_codes::GOVERNANCE_SIGNER_CHANGES_LOCKED,
}
}

Expand All @@ -72,6 +78,9 @@ impl ContractError for Error {
Error::MaxProposals => "Cannot create proposal: active limit reached",
Error::NotASigner => "Only signers can perform this action",
Error::ProposalExpired => "The proposal voting period has expired",
Error::SignerChangesLocked => {
"Signer roster is locked while proposals are actively voting"
}
}
}

Expand Down
25 changes: 25 additions & 0 deletions contracts/governance/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -761,10 +761,25 @@ pub mod governance {
}

/// Adds a new signer. Only admin may call.
///
/// # Roster freeze policy (Issue #1024)
///
/// The signer roster is frozen while any proposal is `Active`. `vote`
/// and `get_proposal_participation` read the **live** signer count, so
/// changing the roster mid-vote would silently alter a proposal's
/// quorum math (a removed signer can shrink `total_signers` and force
/// an early rejection; an added signer can keep a doomed proposal
/// alive). Admin roster changes are therefore rejected until every
/// active proposal has closed; use `emergency_override` for urgent
/// cases.
#[ink(message)]
pub fn add_signer(&mut self, new_signer: AccountId) -> Result<(), Error> {
self.ensure_admin()?;

if self.active_proposal_count > 0 {
return Err(Error::SignerChangesLocked);
}

if self.signers.contains(&new_signer) {
return Err(Error::SignerExists);
}
Expand All @@ -784,10 +799,20 @@ pub mod governance {
}

/// Removes a signer. Only admin may call.
///
/// # Roster freeze policy (Issue #1024)
///
/// Same freeze as `add_signer`: the roster cannot change while any
/// proposal is `Active`, so a proposal's pass/reject outcome never
/// depends on when the admin happened to edit the signer set.
#[ink(message)]
pub fn remove_signer(&mut self, signer: AccountId) -> Result<(), Error> {
self.ensure_admin()?;

if self.active_proposal_count > 0 {
return Err(Error::SignerChangesLocked);
}

if self.signers.len() as u32 <= constants::GOVERNANCE_MIN_SIGNERS {
return Err(Error::MinSigners);
}
Expand Down
43 changes: 43 additions & 0 deletions contracts/governance/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,49 @@ mod tests {
assert_eq!(gov.get_signers().len(), 3);
}

#[ink::test]
fn signer_changes_locked_while_proposal_active() {
let mut gov = create_governance();
let accounts = default_accounts();
set_caller(accounts.alice);

gov.create_proposal(dummy_hash(), GovernanceAction::ModifyProperty, None)
.unwrap();
assert_eq!(gov.get_active_proposal_count(), 1);

// The roster is frozen while a proposal is actively voting, so a
// mid-vote roster change cannot alter the proposal's quorum math.
assert_eq!(gov.add_signer(accounts.django), Err(Error::SignerChangesLocked));
assert_eq!(gov.remove_signer(accounts.charlie), Err(Error::SignerChangesLocked));
assert_eq!(gov.get_signers().len(), 3);
}

#[ink::test]
fn signer_changes_allowed_after_proposal_closes() {
let mut gov = create_governance();
let accounts = default_accounts();
set_caller(accounts.alice);

gov.create_proposal(dummy_hash(), GovernanceAction::ModifyProperty, None)
.unwrap();

// Reject the proposal: alice and bob vote no, leaving no path to the
// threshold of 2 with 3 signers.
gov.vote(0, false).unwrap();
set_caller(accounts.bob);
gov.vote(0, false).unwrap();
assert_eq!(gov.get_proposal(0).unwrap().status, ProposalStatus::Rejected);
assert_eq!(gov.get_active_proposal_count(), 0);

// With no active proposals the roster can be edited again.
set_caller(accounts.alice);
gov.add_signer(accounts.django).unwrap();
gov.remove_signer(accounts.charlie).unwrap();
assert_eq!(gov.get_signers().len(), 3);
assert!(gov.get_signers().contains(&accounts.django));
assert!(!gov.get_signers().contains(&accounts.charlie));
}

#[ink::test]
fn cannot_remove_below_min_signers() {
let accounts = default_accounts();
Expand Down
2 changes: 2 additions & 0 deletions contracts/traits/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,8 @@ pub mod governance_codes {
pub const GOVERNANCE_MAX_PROPOSALS: u32 = 8011;
pub const GOVERNANCE_NOT_A_SIGNER: u32 = 8012;
pub const GOVERNANCE_PROPOSAL_EXPIRED: u32 = 8013;
/// Signer roster changes are blocked while proposals are actively voting.
pub const GOVERNANCE_SIGNER_CHANGES_LOCKED: u32 = 8014;
}

/// Staking error codes (9000-9999)
Expand Down
Loading