Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Allow operational recovery path if on_initialize use fullblock. #6089

Merged
merged 2 commits into from
May 21, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ impl pallet_democracy::Trait for Runtime {
type VetoOrigin = pallet_collective::EnsureMember<AccountId, TechnicalCollective>;
type CooloffPeriod = CooloffPeriod;
type PreimageByteDeposit = PreimageByteDeposit;
type OperationalPreimageOrigin = pallet_collective::EnsureMember<AccountId, CouncilCollective>;
type Slash = Treasury;
type Scheduler = Scheduler;
type MaxVotes = MaxVotes;
Expand Down
158 changes: 107 additions & 51 deletions frame/democracy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,10 @@
//! Preimage actions:
//! - `note_preimage` - Registers the preimage for an upcoming proposal, requires
//! a deposit that is returned once the proposal is enacted.
//! - `note_preimage_operational` - same but provided by `T::OperationalPreimageOrigin`.
//! - `note_imminent_preimage` - Registers the preimage for an upcoming proposal.
//! Does not require a deposit, but the proposal must be in the dispatch queue.
//! - `note_imminent_preimage_operational` - same but provided by `T::OperationalPreimageOrigin`.
//! - `reap_preimage` - Removes the preimage for an expired proposal. Will only
//! work under the condition that it's the same account that noted it and
//! after the voting period, OR it's a different account after the enactment period.
Expand Down Expand Up @@ -285,6 +287,9 @@ pub trait Trait: frame_system::Trait + Sized {
/// The amount of balance that must be deposited per byte of preimage stored.
type PreimageByteDeposit: Get<BalanceOf<Self>>;

/// An origin that can provide a preimage using operational extrinsics.
type OperationalPreimageOrigin: EnsureOrigin<Self::Origin, Success=Self::AccountId>;

/// Handler for the unbalanced reduction when slashing a preimage deposit.
type Slash: OnUnbalanced<NegativeImbalanceOf<Self>>;

Expand Down Expand Up @@ -542,7 +547,7 @@ mod weight_for {
/// - Db writes per votes: `ReferendumInfoOf`
/// - Base Weight: 65.78 + 8.229 * R µs
// NOTE: weight must cover an incorrect voting of origin with 100 votes.
pub(crate) fn delegate<T: Trait>(votes: Weight) -> Weight {
pub fn delegate<T: Trait>(votes: Weight) -> Weight {
T::DbWeight::get().reads_writes(votes.saturating_add(3), votes.saturating_add(3))
.saturating_add(66_000_000)
.saturating_add(votes.saturating_mul(8_100_000))
Expand All @@ -554,7 +559,7 @@ mod weight_for {
/// - Db reads per votes: `ReferendumInfoOf`
/// - Db writes per votes: `ReferendumInfoOf`
/// - Base Weight: 33.29 + 8.104 * R µs
pub(crate) fn undelegate<T: Trait>(votes: Weight) -> Weight {
pub fn undelegate<T: Trait>(votes: Weight) -> Weight {
T::DbWeight::get().reads_writes(votes.saturating_add(2), votes.saturating_add(2))
.saturating_add(33_000_000)
.saturating_add(votes.saturating_mul(8_000_000))
Expand All @@ -565,7 +570,7 @@ mod weight_for {
/// - Db reads: `Proxy`, `proxy account`
/// - Db writes: `proxy account`
/// - Base Weight: 68.61 + 8.039 * R µs
pub(crate) fn proxy_delegate<T: Trait>(votes: Weight) -> Weight {
pub fn proxy_delegate<T: Trait>(votes: Weight) -> Weight {
T::DbWeight::get().reads_writes(votes.saturating_add(5), votes.saturating_add(4))
.saturating_add(69_000_000)
.saturating_add(votes.saturating_mul(8_000_000))
Expand All @@ -575,11 +580,37 @@ mod weight_for {
/// same as `undelegate with additional:
/// Db reads: `Proxy`
/// Base Weight: 39 + 7.958 * R µs
pub(crate) fn proxy_undelegate<T: Trait>(votes: Weight) -> Weight {
pub fn proxy_undelegate<T: Trait>(votes: Weight) -> Weight {
T::DbWeight::get().reads_writes(votes.saturating_add(3), votes.saturating_add(2))
.saturating_add(40_000_000)
.saturating_add(votes.saturating_mul(8_000_000))
}

/// Calculate the weight for `note_preimage`.
/// # <weight>
/// - Complexity: `O(E)` with E size of `encoded_proposal` (protected by a required deposit).
/// - Db reads: `Preimages`
/// - Db writes: `Preimages`
/// - Base Weight: 37.93 + .004 * b µs
/// # </weight>
pub fn note_preimage<T: Trait>(encoded_proposal_len: Weight) -> Weight {
T::DbWeight::get().reads_writes(1, 1)
.saturating_add(38_000_000)
.saturating_add(encoded_proposal_len.saturating_mul(4_000))
}

/// Calculate the weight for `note_imminent_preimage`.
/// # <weight>
/// - Complexity: `O(E)` with E size of `encoded_proposal` (protected by a required deposit).
/// - Db reads: `Preimages`
/// - Db writes: `Preimages`
/// - Base Weight: 28.04 + .003 * b µs
/// # </weight>
pub fn note_imminent_preimage<T: Trait>(encoded_proposal_len: Weight) -> Weight {
T::DbWeight::get().reads_writes(1, 1)
.saturating_add(28_000_000)
.saturating_add(encoded_proposal_len.saturating_mul(3_000))
}
}

decl_module! {
Expand Down Expand Up @@ -1157,33 +1188,21 @@ decl_module! {
/// Emits `PreimageNoted`.
///
/// # <weight>
/// - Complexity: `O(E)` with E size of `encoded_proposal` (protected by a required deposit).
/// - Db reads: `Preimages`
/// - Db writes: `Preimages`
/// - Base Weight: 37.93 + .004 * b µs
/// see `weight_for::note_preimage`
/// # </weight>
#[weight = 38_000_000 + 4_000 * Weight::from(encoded_proposal.len() as u32)
+ T::DbWeight::get().reads_writes(1, 1)]
#[weight = weight_for::note_preimage::<T>((encoded_proposal.len() as u32).into())]
fn note_preimage(origin, encoded_proposal: Vec<u8>) {
let who = ensure_signed(origin)?;
let proposal_hash = T::Hashing::hash(&encoded_proposal[..]);
ensure!(!<Preimages<T>>::contains_key(&proposal_hash), Error::<T>::DuplicatePreimage);

let deposit = <BalanceOf<T>>::from(encoded_proposal.len() as u32)
.saturating_mul(T::PreimageByteDeposit::get());
T::Currency::reserve(&who, deposit)?;

let now = <frame_system::Module<T>>::block_number();
let a = PreimageStatus::Available {
data: encoded_proposal,
provider: who.clone(),
deposit,
since: now,
expiry: None,
};
<Preimages<T>>::insert(proposal_hash, a);
Self::note_preimage_inner(ensure_signed(origin)?, encoded_proposal)?;
}

Self::deposit_event(RawEvent::PreimageNoted(proposal_hash, who, deposit));
/// Same as `note_preimage` but origin is `OperationalPreimageOrigin`.
#[weight = (
weight_for::note_preimage::<T>((encoded_proposal.len() as u32).into()),
DispatchClass::Operational,
)]
fn note_preimage_operational(origin, encoded_proposal: Vec<u8>) {
let who = T::OperationalPreimageOrigin::ensure_origin(origin)?;
Self::note_preimage_inner(who, encoded_proposal)?;
}

/// Register the preimage for an upcoming proposal. This requires the proposal to be
Expand All @@ -1196,32 +1215,21 @@ decl_module! {
/// Emits `PreimageNoted`.
///
/// # <weight>
/// - Complexity: `O(E)` with E size of `encoded_proposal` (protected by a required deposit).
/// - Db reads: `Preimages`
/// - Db writes: `Preimages`
/// - Base Weight: 28.04 + .003 * b µs
/// see `weight_for::note_preimage`
/// # </weight>
#[weight = 28_000_000 + 3_000 * Weight::from(encoded_proposal.len() as u32)
+ T::DbWeight::get().reads_writes(1, 1)]
#[weight = weight_for::note_imminent_preimage::<T>((encoded_proposal.len() as u32).into())]
fn note_imminent_preimage(origin, encoded_proposal: Vec<u8>) {
let who = ensure_signed(origin)?;
let proposal_hash = T::Hashing::hash(&encoded_proposal[..]);
Self::check_pre_image_is_missing(proposal_hash)?;
let status = Preimages::<T>::get(&proposal_hash).ok_or(Error::<T>::NotImminent)?;
let expiry = status.to_missing_expiry().ok_or(Error::<T>::DuplicatePreimage)?;

let now = <frame_system::Module<T>>::block_number();
let free = <BalanceOf<T>>::zero();
let a = PreimageStatus::Available {
data: encoded_proposal,
provider: who.clone(),
deposit: Zero::zero(),
since: now,
expiry: Some(expiry),
};
<Preimages<T>>::insert(proposal_hash, a);
Self::note_imminent_preimage_inner(ensure_signed(origin)?, encoded_proposal)?;
}

Self::deposit_event(RawEvent::PreimageNoted(proposal_hash, who, free));
/// Same as `note_imminent_preimage` but origin is `OperationalPreimageOrigin`.
#[weight = (
weight_for::note_imminent_preimage::<T>((encoded_proposal.len() as u32).into()),
DispatchClass::Operational,
)]
fn note_imminent_preimage_operational(origin, encoded_proposal: Vec<u8>) {
let who = T::OperationalPreimageOrigin::ensure_origin(origin)?;
Self::note_imminent_preimage_inner(who, encoded_proposal)?;
}

/// Remove an expired proposal preimage and collect the deposit.
Expand Down Expand Up @@ -2030,6 +2038,54 @@ impl<T: Trait> Module<T> {

Ok(len)
}

// See `note_preimage`
fn note_preimage_inner(who: T::AccountId, encoded_proposal: Vec<u8>) -> DispatchResult {
let proposal_hash = T::Hashing::hash(&encoded_proposal[..]);
ensure!(!<Preimages<T>>::contains_key(&proposal_hash), Error::<T>::DuplicatePreimage);

let deposit = <BalanceOf<T>>::from(encoded_proposal.len() as u32)
.saturating_mul(T::PreimageByteDeposit::get());
T::Currency::reserve(&who, deposit)?;

let now = <frame_system::Module<T>>::block_number();
let a = PreimageStatus::Available {
data: encoded_proposal,
provider: who.clone(),
deposit,
since: now,
expiry: None,
};
<Preimages<T>>::insert(proposal_hash, a);

Self::deposit_event(RawEvent::PreimageNoted(proposal_hash, who, deposit));

Ok(())
}

// See `note_imminent_preimage`
fn note_imminent_preimage_inner(who: T::AccountId, encoded_proposal: Vec<u8>) -> DispatchResult
{
gavofyork marked this conversation as resolved.
Show resolved Hide resolved
let proposal_hash = T::Hashing::hash(&encoded_proposal[..]);
Self::check_pre_image_is_missing(proposal_hash)?;
let status = Preimages::<T>::get(&proposal_hash).ok_or(Error::<T>::NotImminent)?;
let expiry = status.to_missing_expiry().ok_or(Error::<T>::DuplicatePreimage)?;

let now = <frame_system::Module<T>>::block_number();
let free = <BalanceOf<T>>::zero();
let a = PreimageStatus::Available {
data: encoded_proposal,
provider: who.clone(),
deposit: Zero::zero(),
since: now,
expiry: Some(expiry),
};
<Preimages<T>>::insert(proposal_hash, a);

Self::deposit_event(RawEvent::PreimageNoted(proposal_hash, who, free));

Ok(())
}
}

/// Decode `Compact<u32>` from the trie at given key.
Expand Down
7 changes: 7 additions & 0 deletions frame/democracy/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ impl super::Trait for Test {
type InstantAllowed = InstantAllowed;
type Scheduler = Scheduler;
type MaxVotes = MaxVotes;
type OperationalPreimageOrigin = EnsureSignedBy<Six, u64>;
}

pub fn new_test_ext() -> sp_io::TestExternalities {
Expand All @@ -199,6 +200,12 @@ pub fn new_test_ext() -> sp_io::TestExternalities {
ext
}

/// Execute the function two times, with `true` and with `false`.
pub fn new_test_ext_execute_with_cond(execute: impl FnOnce(bool) -> () + Clone) {
new_test_ext().execute_with(|| (execute.clone())(false));
new_test_ext().execute_with(|| execute(true));
}

type System = frame_system::Module<Test>;
type Balances = pallet_balances::Module<Test>;
type Scheduler = pallet_scheduler::Module<Test>;
Expand Down
40 changes: 24 additions & 16 deletions frame/democracy/src/tests/preimage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,14 @@ fn missing_preimage_should_fail() {

#[test]
fn preimage_deposit_should_be_required_and_returned() {
new_test_ext().execute_with(|| {
new_test_ext_execute_with_cond(|operational| {
// fee of 100 is too much.
PREIMAGE_BYTE_DEPOSIT.with(|v| *v.borrow_mut() = 100);
assert_noop!(
Democracy::note_preimage(Origin::signed(6), vec![0; 500]),
BalancesError::<Test, _>::InsufficientBalance,
);
if operational { Democracy::note_preimage_operational(Origin::signed(6), vec![0; 500]) }
else { Democracy::note_preimage(Origin::signed(6), vec![0; 500]) },
BalancesError::<Test, _>::InsufficientBalance,
);
// fee of 1 is reasonable.
PREIMAGE_BYTE_DEPOSIT.with(|v| *v.borrow_mut() = 1);
let r = Democracy::inject_referendum(
Expand All @@ -69,17 +70,20 @@ fn preimage_deposit_should_be_required_and_returned() {

#[test]
fn preimage_deposit_should_be_reapable_earlier_by_owner() {
new_test_ext().execute_with(|| {
new_test_ext_execute_with_cond(|operational| {
PREIMAGE_BYTE_DEPOSIT.with(|v| *v.borrow_mut() = 1);
assert_ok!(Democracy::note_preimage(Origin::signed(6), set_balance_proposal(2)));
assert_ok!(
if operational { Democracy::note_preimage_operational(Origin::signed(6), set_balance_proposal(2)) }
else { Democracy::note_preimage(Origin::signed(6), set_balance_proposal(2)) }
);

assert_eq!(Balances::reserved_balance(6), 12);

next_block();
assert_noop!(
Democracy::reap_preimage(Origin::signed(6), set_balance_proposal_hash(2), u32::max_value()),
Error::<Test>::TooEarly
);
Democracy::reap_preimage(Origin::signed(6), set_balance_proposal_hash(2), u32::max_value()),
Error::<Test>::TooEarly
);
next_block();
assert_ok!(Democracy::reap_preimage(Origin::signed(6), set_balance_proposal_hash(2), u32::max_value()));

Expand All @@ -90,14 +94,17 @@ fn preimage_deposit_should_be_reapable_earlier_by_owner() {

#[test]
fn preimage_deposit_should_be_reapable() {
new_test_ext().execute_with(|| {
new_test_ext_execute_with_cond(|operational| {
assert_noop!(
Democracy::reap_preimage(Origin::signed(5), set_balance_proposal_hash(2), u32::max_value()),
Error::<Test>::PreimageMissing
);

PREIMAGE_BYTE_DEPOSIT.with(|v| *v.borrow_mut() = 1);
assert_ok!(Democracy::note_preimage(Origin::signed(6), set_balance_proposal(2)));
assert_ok!(
if operational { Democracy::note_preimage_operational(Origin::signed(6), set_balance_proposal(2)) }
else { Democracy::note_preimage(Origin::signed(6), set_balance_proposal(2)) }
);
assert_eq!(Balances::reserved_balance(6), 12);

next_block();
Expand All @@ -118,7 +125,7 @@ fn preimage_deposit_should_be_reapable() {

#[test]
fn noting_imminent_preimage_for_free_should_work() {
new_test_ext().execute_with(|| {
new_test_ext_execute_with_cond(|operational| {
PREIMAGE_BYTE_DEPOSIT.with(|v| *v.borrow_mut() = 1);

let r = Democracy::inject_referendum(
Expand All @@ -130,14 +137,15 @@ fn noting_imminent_preimage_for_free_should_work() {
assert_ok!(Democracy::vote(Origin::signed(1), r, aye(1)));

assert_noop!(
Democracy::note_imminent_preimage(Origin::signed(7), set_balance_proposal(2)),
Error::<Test>::NotImminent
);
if operational { Democracy::note_imminent_preimage_operational(Origin::signed(6), set_balance_proposal(2)) }
else { Democracy::note_imminent_preimage(Origin::signed(6), set_balance_proposal(2)) },
Error::<Test>::NotImminent
);

next_block();

// Now we're in the dispatch queue it's all good.
assert_ok!(Democracy::note_imminent_preimage(Origin::signed(7), set_balance_proposal(2)));
assert_ok!(Democracy::note_imminent_preimage(Origin::signed(6), set_balance_proposal(2)));

next_block();

Expand Down