Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ impl pallet_referenda::Config for Runtime {
type RuntimeCall = RuntimeCall;
type RuntimeEvent = RuntimeEvent;
type Scheduler = Scheduler;
type Currency = Balances;
type NativeBalance = Balances;
type SubmitOrigin = frame_system::EnsureSigned<AccountId>;
type CancelOrigin = EitherOf<EnsureRoot<AccountId>, ReferendumCanceller>;
type KillOrigin = EitherOf<EnsureRoot<AccountId>, ReferendumKiller>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ impl pallet_referenda::Config<AmbassadorReferendaInstance> for Runtime {
type RuntimeCall = RuntimeCall;
type RuntimeEvent = RuntimeEvent;
type Scheduler = Scheduler;
type Currency = Balances;
type NativeBalance = Balances;
// A proposal can be submitted by a member of the Ambassador Program of
// [ranks::SENIOR_AMBASSADOR_TIER_3] rank or higher.
type SubmitOrigin = pallet_ranked_collective::EnsureMember<
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl pallet_referenda::Config<FellowshipReferendaInstance> for Runtime {
type RuntimeCall = RuntimeCall;
type RuntimeEvent = RuntimeEvent;
type Scheduler = Scheduler;
type Currency = Balances;
type NativeBalance = Balances;
// Fellows can submit proposals.
type SubmitOrigin = EitherOf<
pallet_ranked_collective::EnsureMember<Runtime, FellowshipCollectiveInstance, 3>,
Expand Down
2 changes: 1 addition & 1 deletion polkadot/runtime/rococo/src/governance/fellowship.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ impl pallet_referenda::Config<FellowshipReferendaInstance> for Runtime {
type RuntimeCall = RuntimeCall;
type RuntimeEvent = RuntimeEvent;
type Scheduler = Scheduler;
type Currency = Balances;
type NativeBalance = Balances;
type SubmitOrigin =
pallet_ranked_collective::EnsureMember<Runtime, FellowshipCollectiveInstance, 1>;
type CancelOrigin = FellowshipExperts;
Expand Down
2 changes: 1 addition & 1 deletion polkadot/runtime/rococo/src/governance/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ impl pallet_referenda::Config for Runtime {
type RuntimeCall = RuntimeCall;
type RuntimeEvent = RuntimeEvent;
type Scheduler = Scheduler;
type Currency = Balances;
type NativeBalance = Balances;
type SubmitOrigin = frame_system::EnsureSigned<AccountId>;
type CancelOrigin = EitherOf<EnsureRoot<AccountId>, ReferendumCanceller>;
type KillOrigin = EitherOf<EnsureRoot<AccountId>, ReferendumKiller>;
Expand Down
2 changes: 1 addition & 1 deletion polkadot/runtime/westend/src/governance/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl pallet_referenda::Config for Runtime {
type RuntimeCall = RuntimeCall;
type RuntimeEvent = RuntimeEvent;
type Scheduler = Scheduler;
type Currency = Balances;
type NativeBalance = Balances;
type SubmitOrigin = frame_system::EnsureSigned<AccountId>;
type CancelOrigin = EitherOf<EnsureRoot<AccountId>, ReferendumCanceller>;
type KillOrigin = EitherOf<EnsureRoot<AccountId>, ReferendumKiller>;
Expand Down
2 changes: 1 addition & 1 deletion substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1051,7 +1051,7 @@ impl pallet_referenda::Config for Runtime {
type RuntimeCall = RuntimeCall;
type RuntimeEvent = RuntimeEvent;
type Scheduler = Scheduler;
type Currency = pallet_balances::Pallet<Self>;
type NativeBalance = pallet_balances::Pallet<Self>;
type SubmitOrigin = EnsureSigned<AccountId>;
type CancelOrigin = EnsureRoot<AccountId>;
type KillOrigin = EnsureRoot<AccountId>;
Expand Down
29 changes: 21 additions & 8 deletions substrate/frame/referenda/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ use frame_benchmarking::v1::{
};
use frame_support::{
assert_ok,
traits::{Currency, EnsureOrigin, EnsureOriginWithArg, UnfilteredDispatchable},
traits::{
tokens::{Fortitude, Preservation},
EnsureOrigin, EnsureOriginWithArg, UnfilteredDispatchable,
},
};
use frame_system::RawOrigin;
use sp_runtime::traits::Bounded as ArithBounded;
Expand All @@ -41,9 +44,16 @@ fn assert_last_event<T: Config<I>, I: 'static>(generic_event: <T as Config<I>>::
frame_system::Pallet::<T>::assert_last_event(generic_event.into());
}

fn funded_account<T: Config<I>, I: 'static>(name: &'static str, index: u32) -> T::AccountId {
fn funded_account<T: Config<I>, I: 'static>(
name: &'static str,
index: u32,
decision_deposit: BalanceOf<T, I>,
) -> T::AccountId {
let caller: T::AccountId = account(name, index, SEED);
T::Currency::make_free_balance_be(&caller, BalanceOf::<T, I>::max_value());
//Giving enough balance to cover the decision deposit + the submission deposit + buffer for
// fees
let amount = T::NativeBalance::minimum_balance() + decision_deposit.saturating_mul(2u32.into());
T::NativeBalance::set_balance(&caller, amount);
caller
}

Expand All @@ -55,7 +65,7 @@ fn dummy_call<T: Config<I>, I: 'static>() -> BoundedCallOf<T, I> {

fn create_referendum<T: Config<I>, I: 'static>(origin: T::RuntimeOrigin) -> ReferendumIndex {
if let Ok(caller) = frame_system::ensure_signed(origin.clone()) {
T::Currency::make_free_balance_be(&caller, BalanceOf::<T, I>::max_value());
T::NativeBalance::set_balance(&caller, BalanceOf::<T, I>::max_value() / 2u32.into());
whitelist_account!(caller);
}

Expand All @@ -69,8 +79,11 @@ fn create_referendum<T: Config<I>, I: 'static>(origin: T::RuntimeOrigin) -> Refe
}

fn place_deposit<T: Config<I>, I: 'static>(index: ReferendumIndex) {
let caller = funded_account::<T, I>("caller", 0);
let track_info = info::<T, I>(index);
let decision_deposit = track_info.decision_deposit;
let caller = funded_account::<T, I>("caller", 0, decision_deposit);
whitelist_account!(caller);

assert_ok!(Referenda::<T, I>::place_decision_deposit(RawOrigin::Signed(caller).into(), index));
}

Expand Down Expand Up @@ -202,7 +215,7 @@ benchmarks_instance_pallet! {
let origin =
T::SubmitOrigin::try_successful_origin(&RawOrigin::Root.into()).map_err(|_| BenchmarkError::Weightless)?;
if let Ok(caller) = frame_system::ensure_signed(origin.clone()) {
T::Currency::make_free_balance_be(&caller, BalanceOf::<T, I>::max_value());
T::NativeBalance::set_balance(&caller, BalanceOf::<T, I>::max_value()/2u32.into());
whitelist_account!(caller);
}
}: _<T::RuntimeOrigin>(
Expand Down Expand Up @@ -291,7 +304,7 @@ benchmarks_instance_pallet! {
T::SubmitOrigin::try_successful_origin(&RawOrigin::Root.into()).map_err(|_| BenchmarkError::Weightless)?;
let index = create_referendum::<T, I>(origin.clone());
let caller = frame_system::ensure_signed(origin.clone()).unwrap();
let balance = T::Currency::free_balance(&caller);
let balance = T::NativeBalance::reducible_balance(&caller, Preservation::Preserve, Fortitude::Polite);
assert_ok!(Referenda::<T, I>::cancel(
T::CancelOrigin::try_successful_origin()
.expect("CancelOrigin has no successful origin required for the benchmark"),
Expand All @@ -301,7 +314,7 @@ benchmarks_instance_pallet! {
}: _<T::RuntimeOrigin>(origin, index)
verify {
assert_matches!(ReferendumInfoFor::<T, I>::get(index), Some(ReferendumInfo::Cancelled(_, None, _)));
let new_balance = T::Currency::free_balance(&caller);
let new_balance = T::NativeBalance::reducible_balance(&caller, Preservation::Preserve, Fortitude::Polite);
// the deposit is zero or make sure it was unreserved.
assert!(T::SubmissionDeposit::get().is_zero() || new_balance > balance);
}
Expand Down
70 changes: 50 additions & 20 deletions substrate/frame/referenda/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,14 @@ use frame_support::{
dispatch::DispatchResult,
ensure,
traits::{
fungible::{Balanced, Inspect, InspectHold, Mutate, MutateHold},
schedule::{
v3::{Anon as ScheduleAnon, Named as ScheduleNamed},
DispatchTime,
},
Currency, LockIdentifier, OnUnbalanced, OriginTrait, PollStatus, Polling, QueryPreimage,
ReservableCurrency, StorePreimage, VoteTally,
tokens::{Fortitude, Precision},
LockIdentifier, OnUnbalanced, OriginTrait, PollStatus, Polling, QueryPreimage,
StorePreimage, VoteTally,
},
BoundedVec,
};
Expand All @@ -97,11 +99,10 @@ use self::branch::{BeginDecidingBranch, OneFewerDecidingBranch, ServiceBranch};
pub use self::{
pallet::*,
types::{
BalanceOf, BlockNumberFor, BoundedCallOf, CallOf, ConstTrackInfo, Curve, DecidingStatus,
DecidingStatusOf, Deposit, InsertSorted, NegativeImbalanceOf, PalletsOriginOf,
ReferendumIndex, ReferendumInfo, ReferendumInfoOf, ReferendumStatus, ReferendumStatusOf,
ScheduleAddressOf, StringLike, TallyOf, Track, TrackIdOf, TrackInfo, TrackInfoOf,
TracksInfo, VotesOf,
BalanceOf, BlockNumberFor, BoundedCallOf, CallOf, ConstTrackInfo, Curve, DebtOf,
DecidingStatus, DecidingStatusOf, Deposit, InsertSorted, PalletsOriginOf, ReferendumIndex,
ReferendumInfo, ReferendumInfoOf, ReferendumStatus, ReferendumStatusOf, ScheduleAddressOf,
StringLike, TallyOf, Track, TrackIdOf, TrackInfo, TrackInfoOf, TracksInfo, VotesOf,
},
weights::WeightInfo,
};
Expand Down Expand Up @@ -130,12 +131,20 @@ pub mod pallet {
};

/// The in-code storage version.
const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);
const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);

#[pallet::pallet]
#[pallet::storage_version(STORAGE_VERSION)]
pub struct Pallet<T, I = ()>(_);

/// A reason for holding funds.
/// Creates a hold reason for this pallet that is aggregated by `construct_runtime`.
#[pallet::composite_enum]
pub enum HoldReason<I: 'static = ()> {
/// The account has deposited tokens to place a decision deposit.
DecisionDeposit,
}

#[pallet::config]
pub trait Config<I: 'static = ()>: frame_system::Config + Sized {
// System level stuff.
Expand All @@ -162,7 +171,11 @@ pub mod pallet {
Hasher = Self::Hashing,
>;
/// Currency type for this pallet.
type Currency: ReservableCurrency<Self::AccountId>;
type NativeBalance: Inspect<Self::AccountId>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO better to keep the same old name Currency. It is still a kind of currency type, the underlying traits are fungible but this is fine. Most users won't see the difference as pallet-balances implements both. And if it breaks the compile error will be fine.

+ Mutate<Self::AccountId>
+ Balanced<Self::AccountId>
+ InspectHold<Self::AccountId, Reason: From<HoldReason<I>>>
+ MutateHold<Self::AccountId, Reason: From<HoldReason<I>>>;
// Origins and unbalances.
/// Origin from which proposals may be submitted.
type SubmitOrigin: EnsureOriginWithArg<
Expand All @@ -175,7 +188,7 @@ pub mod pallet {
/// Origin from which any vote may be killed.
type KillOrigin: EnsureOrigin<Self::RuntimeOrigin>;
/// Handler for the unbalanced reduction when slashing a preimage deposit.
type Slash: OnUnbalanced<NegativeImbalanceOf<Self, I>>;
type Slash: OnUnbalanced<DebtOf<Self, I>>;
/// The counting type for votes. Usually just balance.
type Votes: AtLeast32BitUnsigned + Copy + Parameter + Member + MaxEncodedLen;
/// The tallying type.
Expand Down Expand Up @@ -569,7 +582,7 @@ pub mod pallet {
.take_decision_deposit()
.map_err(|_| Error::<T, I>::Unfinished)?
.ok_or(Error::<T, I>::NoDeposit)?;
Self::refund_deposit(Some(deposit.clone()));
Self::refund_deposit(Some(deposit.clone()))?;
ReferendumInfoFor::<T, I>::insert(index, info);
let e = Event::<T, I>::DecisionDepositRefunded {
index,
Expand Down Expand Up @@ -621,8 +634,8 @@ pub mod pallet {
}
Self::note_one_fewer_deciding(status.track);
Self::deposit_event(Event::<T, I>::Killed { index, tally: status.tally });
Self::slash_deposit(Some(status.submission_deposit.clone()));
Self::slash_deposit(status.decision_deposit.clone());
Self::slash_deposit(Some(status.submission_deposit.clone()))?;
Self::slash_deposit(status.decision_deposit.clone())?;
Comment on lines +637 to +638
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like that this can return an error here. We don't want to block the kill if for some reason the slash fails. Also fungible API never really specifies why it could fail. In the other comment I say to change to BestEffort to make this scenario less likely but still strictly speaking it doesn't prevent the slash from failing, so maybe better to actually ignore the error.

Suggested change
Self::slash_deposit(Some(status.submission_deposit.clone()))?;
Self::slash_deposit(status.decision_deposit.clone())?;
let _ = Self::slash_deposit(Some(status.submission_deposit.clone())).inspect(/* log some error or something */);
let _ = Self::slash_deposit(status.decision_deposit.clone()).inspect(/* log some error or something */);

Self::do_clear_metadata(index);
let info = ReferendumInfo::Killed(T::BlockNumberProvider::current_block_number());
ReferendumInfoFor::<T, I>::insert(index, info);
Expand Down Expand Up @@ -707,7 +720,7 @@ pub mod pallet {
.take_submission_deposit()
.map_err(|_| Error::<T, I>::BadStatus)?
.ok_or(Error::<T, I>::NoDeposit)?;
Self::refund_deposit(Some(deposit.clone()));
Self::refund_deposit(Some(deposit.clone()))?;
ReferendumInfoFor::<T, I>::insert(index, info);
let e = Event::<T, I>::SubmissionDepositRefunded {
index,
Expand Down Expand Up @@ -1289,23 +1302,40 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
who: T::AccountId,
amount: BalanceOf<T, I>,
) -> Result<Deposit<T::AccountId, BalanceOf<T, I>>, DispatchError> {
T::Currency::reserve(&who, amount)?;
T::NativeBalance::hold(&HoldReason::<I>::DecisionDeposit.into(), &who, amount)?;
Ok(Deposit { who, amount })
}

/// Return a deposit, if `Some`.
fn refund_deposit(deposit: Option<Deposit<T::AccountId, BalanceOf<T, I>>>) {
fn refund_deposit(
deposit: Option<Deposit<T::AccountId, BalanceOf<T, I>>>,
) -> Result<(), DispatchError> {
if let Some(Deposit { who, amount }) = deposit {
T::Currency::unreserve(&who, amount);
T::NativeBalance::release(
&HoldReason::<I>::DecisionDeposit.into(),
&who,
amount,
Precision::BestEffort,
)?;
}
Ok(())
}

/// Slash a deposit, if `Some`.
fn slash_deposit(deposit: Option<Deposit<T::AccountId, BalanceOf<T, I>>>) {
fn slash_deposit(
deposit: Option<Deposit<T::AccountId, BalanceOf<T, I>>>,
) -> Result<(), DispatchError> {
if let Some(Deposit { who, amount }) = deposit {
T::Slash::on_unbalanced(T::Currency::slash_reserved(&who, amount).0);
Self::deposit_event(Event::<T, I>::DepositSlashed { who, amount });
let amount_burned = T::NativeBalance::burn_held(
&HoldReason::<I>::DecisionDeposit.into(),
&who,
amount,
Precision::BestEffort,
Fortitude::Force,
)?;
Self::deposit_event(Event::<T, I>::DepositSlashed { who, amount: amount_burned });
}
Ok(())
}

/// Determine whether the given `tally` would result in a referendum passing at `elapsed` blocks
Expand Down
Loading
Loading