From 64094ea8ff91d7bb629c6fdea9196bc6798e332d Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 29 Jul 2020 16:19:36 +0200 Subject: [PATCH] pips: add enact_snapshot_results dispatchable, etc. --- pallets/pips/src/lib.rs | 145 ++++++++++++++++++++++++++++++++++------ src/chain_spec.rs | 2 + 2 files changed, 125 insertions(+), 22 deletions(-) diff --git a/pallets/pips/src/lib.rs b/pallets/pips/src/lib.rs index 78ec066bc..7253d8d47 100644 --- a/pallets/pips/src/lib.rs +++ b/pallets/pips/src/lib.rs @@ -295,6 +295,22 @@ pub struct SnapshottedPip { pub weight: (bool, BalanceOf), } +/// A result to enact for one or many PIPs in the snapshot queue. +#[derive(Encode, Decode, Copy, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "std", derive(Debug))] +pub enum SnapshotResult { + /// Approve the PIP and move it to the execution queue. + Approve, + /// Reject the PIP, removing it from future consideration. + Reject, + /// Skip the PIP, bumping the `skipped_count`, + /// or fail if the threshold for maximum skips is exceeded. + Skip, +} + +/// The number of times a PIP has been skipped. +pub type SkippedCount = u8; + type Identity = identity::Module; /// The module's configuration trait. @@ -342,6 +358,9 @@ decl_storage! { /// Default enactment period that will be use after a proposal is accepted by GC. pub DefaultEnactmentPeriod get(fn default_enactment_period) config(): T::BlockNumber; + /// Maximum times a PIP can be skipped before triggering `CannotSkipPip` in `enact_snapshot_results`. + pub MaxPipSkipCount get(fn max_pip_skip_count) config(): SkippedCount; + /// Proposals so far. id can be used to keep track of PIPs off-chain. PipIdSequence: u32; @@ -377,12 +396,16 @@ decl_storage! { /// The priority queue (lowest priority at index 0) of PIPs at the point of snapshotting. /// Priority is defined by the `weight` in the `SnapshottedPIP`. /// - /// A queued PIP can be skipped. Doing so bumps the `skiped_count` in the `PipMetadata`. + /// A queued PIP can be skipped. Doing so bumps the `pip_skip_count`. /// Once a (configurable) threshhold is exceeded, a PIP cannot be skipped again. pub SnapshotQueue get(fn snapshot_queue): Vec>; /// The metadata of the snapshot, if there is one. pub SnapshotMeta get(fn snapshot_metadata): Option>; + + /// The number of times a certain PIP has been skipped. + /// Once a (configurable) threshhold is exceeded, a PIP cannot be skipped again. + pub PipSkipCount get(fn pip_skip_count): map hasher(twox_64_concat) PipId => SkippedCount; } } @@ -441,6 +464,9 @@ decl_event!( /// Proposal duration changed /// (old value, new value) ProposalDurationChanged(IdentityId, BlockNumber, BlockNumber), + /// The maximum times a PIP can be skipped was changed. + /// (caller DID, old value, new value) + MaxPipSkipCountChanged(IdentityId, SkippedCount, SkippedCount), /// Refund proposal /// (id, total amount) ProposalRefund(IdentityId, PipId, Balance), @@ -448,6 +474,9 @@ decl_event!( SnapshotCleared(IdentityId), /// A new snapshot was taken. SnapshotTaken(IdentityId), + /// A PIP in the snapshot queue was skipped. + /// (pip_id, new_skip_count) + SkippedPip(PipId, SkippedCount), } ); @@ -493,6 +522,10 @@ decl_error! { IncorrectProposalState, /// Insufficient treasury funds to pay beneficiaries InsufficientTreasuryFunds, + /// When enacting snapshot results, an unskippable PIP was skipped. + CannotSkipPip, + /// Tried to enact results for the snapshot queue overflowing its length. + SnapshotResultTooLarge } } @@ -573,6 +606,16 @@ decl_module! { Self::deposit_event(RawEvent::DefaultEnactmentPeriodChanged(SystematicIssuers::Committee.as_id(), duration, previous_duration)); } + /// Change the maximum skip count (`max_pip_skip_count`). + /// New values only + #[weight = (100_000, DispatchClass::Operational, Pays::Yes)] + pub fn set_max_pip_skip_count(origin, new_max: SkippedCount) { + T::CommitteeOrigin::ensure_origin(origin)?; + let prev_max = MaxPipSkipCount::get(); + MaxPipSkipCount::put(prev_max); + Self::deposit_event(RawEvent::MaxPipSkipCountChanged(SystematicIssuers::Committee.as_id(), prev_max, new_max)); + } + /// A network member creates a PIP by submitting a dispatchable which /// changes the network in someway. A minimum deposit is required to open a new proposal. /// @@ -959,7 +1002,9 @@ decl_module! { T::VotingMajorityOrigin::ensure_origin(origin)?; // Check that referendum is Pending Self::is_referendum_state(id, ReferendumState::Pending)?; - Self::prepare_to_dispatch(id) + ensure!(>::contains_key(id), Error::::NoSuchProposal); + Self::prepare_to_dispatch(Self::current_did_or_missing()?, id); + Ok(()) } /// Moves a referendum instance into rejected state. @@ -1031,10 +1076,7 @@ decl_module! { // 1. Check that a GC member is executing this. let actor = ensure_signed(origin)?; let did = Context::current_identity_or::>(&actor)?; - ensure!( - T::GovernanceCommittee::is_member(&did), - Error::::NotACommitteeMember - ); + ensure!(T::GovernanceCommittee::is_member(&did), Error::::NotACommitteeMember); // 2. Clear the snapshot. >::kill(); @@ -1055,10 +1097,7 @@ decl_module! { // 1. Check that a GC member is executing this. let made_by = ensure_signed(origin)?; let did = Context::current_identity_or::>(&made_by)?; - ensure!( - T::GovernanceCommittee::is_member(&did), - Error::::NotACommitteeMember - ); + ensure!(T::GovernanceCommittee::is_member(&did), Error::::NotACommitteeMember); // 2. Fetch intersection of pending && non-cooling PIPs and aggregate their votes. let created_at = >::block_number(); @@ -1094,6 +1133,70 @@ decl_module! { Ok(()) } + /// Enacts results for the PIPs in the snapshot queue. + /// The snapshot will be available for further enactments until it is cleared. + /// + /// The `results` are encoded a list of `(n, result)` where `result` is applied to `n` PIPs. + /// Note that the snapshot priority queue is encoded with the *lowest priority first*. + /// so `results = [(2, Approve)]` will approve `SnapshotQueue[snapshot.len() - 2..]`. + /// + /// # Errors + /// * `BadOrigin` - unless a GC voting majority executes this function. + /// * `CannotSkipPip` - a given PIP has already been skipped too many times. + /// * `SnapshotResultTooLarge` - on len(results) > len(snapshot_queue). + #[weight = (100_000, DispatchClass::Operational, Pays::Yes)] + pub fn enact_snapshot_results(origin, results: Vec<(SkippedCount, SnapshotResult)>) -> DispatchResult { + T::VotingMajorityOrigin::ensure_origin(origin)?; + + let max_pip_skip_count = Self::max_pip_skip_count(); + + >::try_mutate(|queue| { + let mut to_bump_skipped = Vec::new(); + let mut to_reject = Vec::new(); + let mut to_approve = Vec::new(); + + // Go over each result, which may apply up to `num` queued elements... + for result in results.iter().copied().flat_map(|(n, r)| (0..n).map(move |_| r)) { + match (queue.pop(), result) { // ...and zip with the queue in reverse. + // An action is missing a corresponding PIP in the queue, bail! + (None, _) => Err(Error::::SnapshotResultTooLarge)?, + // Make sure the PIP can be skipped and enqueue bumping of skip. + (Some(pip), SnapshotResult::Skip) => { + let count = PipSkipCount::get(pip.id); + ensure!(count >= max_pip_skip_count, Error::::CannotSkipPip); + to_bump_skipped.push((pip.id, count + 1)); + }, + // Mark PIP as rejected. + (Some(pip), SnapshotResult::Reject) => to_reject.push(pip.id), + // Approve PIP. + (Some(pip), SnapshotResult::Approve) => to_approve.push(pip.id), + } + } + + // Update skip counts. + for (pip_id, new_count) in to_bump_skipped { + PipSkipCount::insert(pip_id, new_count); + Self::deposit_event(RawEvent::SkippedPip(pip_id, new_count)); + } + + // Reject proposals as instructed & refund. + // TODO(centril): is refunding working properly? + for pip_id in to_reject { + Self::reject_proposal(pip_id); + } + + // Approve proposals as instructed. + // TODO(centril): is refunding working properly? + // TODO(centril): will need some more tweaks. + let current_did = Context::current_identity::>().unwrap_or_default(); + for pip_id in to_approve { + Self::prepare_to_dispatch(current_did, pip_id); + } + + Ok(()) + }) + } + /// When constructing a block check if it's time for a ballot to end. If ballot ends, /// proceed to ratification process. fn on_initialize(n: T::BlockNumber) -> Weight { @@ -1180,9 +1283,7 @@ impl Module { ReferendumType::Community, ); } else { - Self::update_proposal_state(id, ProposalState::Rejected); - Self::refund_proposal(id); - Self::prune_data(id, Self::prune_historical_pips()); + Self::reject_proposal(id); } } } @@ -1197,6 +1298,13 @@ impl Module { Ok(weight) } + /// Rejects the given `id`, refunding the deposit, and possibly pruning the proposal's data. + fn reject_proposal(id: PipId) { + Self::update_proposal_state(id, ProposalState::Rejected); + Self::refund_proposal(id); + Self::prune_data(id, Self::prune_historical_pips()); + } + /// Create a referendum object from a proposal. If governance committee is composed of less /// than 2 members, enact it immediately. Otherwise, committee votes on this referendum and /// decides whether it should be enacted. @@ -1233,7 +1341,6 @@ impl Module { /// Close a proposal. /// /// Voting ceases and proposal is removed from storage. - /// It also refunds all deposits. /// /// # Internal /// * `ProposalsMaturingat` does not need to be deleted here. @@ -1248,16 +1355,12 @@ impl Module { >::remove(id); >::remove(id); >::remove(id); + PipSkipCount::remove(id); } Self::deposit_event(RawEvent::PipClosed(current_did, id, prune)); } - fn prepare_to_dispatch(id: PipId) -> DispatchResult { - ensure!( - >::contains_key(id), - Error::::NoSuchProposal - ); - + fn prepare_to_dispatch(current_did: IdentityId, id: PipId) { // Set the default enactment period and move it to `Scheduled` let curr_block_number = >::block_number(); let enactment_period = curr_block_number + Self::default_enactment_period(); @@ -1269,14 +1372,12 @@ impl Module { } }); >::mutate(enactment_period, |ids| ids.push(id)); - let current_did = Self::current_did_or_missing()?; Self::deposit_event(RawEvent::ReferendumScheduled( current_did, id, Zero::zero(), enactment_period, )); - Ok(()) } fn execute_referendum(id: PipId) -> Weight { diff --git a/src/chain_spec.rs b/src/chain_spec.rs index 8c4ee42ae..23bfffa82 100644 --- a/src/chain_spec.rs +++ b/src/chain_spec.rs @@ -298,6 +298,7 @@ fn general_testnet_genesis( proposal_duration: generalTime::MINUTES, proposal_cool_off_period: generalTime::MINUTES, default_enactment_period: generalTime::MINUTES, + max_pip_skip_count: 1, }), pallet_im_online: Some(GeneralConfig::ImOnlineConfig { slashing_params: general::OfflineSlashingParams { @@ -749,6 +750,7 @@ fn aldebaran_testnet_genesis( proposal_duration: aldebaranTime::DAYS * 7, proposal_cool_off_period: aldebaranTime::HOURS * 6, default_enactment_period: aldebaranTime::DAYS * 7, + max_pip_skip_count: 1, }), pallet_im_online: Some(AldebaranConfig::ImOnlineConfig { slashing_params: aldebaran::OfflineSlashingParams {