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

Refund referendum submission deposit #12788

Merged
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions frame/referenda/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ frame-system = { version = "4.0.0-dev", default-features = false, path = "../sys
sp-io = { version = "7.0.0", default-features = false, path = "../../primitives/io" }
sp-runtime = { version = "7.0.0", default-features = false, path = "../../primitives/runtime" }
sp-std = { version = "5.0.0", default-features = false, path = "../../primitives/std" }
log = { version = "0.4.17", default-features = false }

[dev-dependencies]
assert_matches = { version = "1.5" }
Expand Down
13 changes: 13 additions & 0 deletions frame/referenda/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,19 @@ benchmarks_instance_pallet! {
assert_matches!(ReferendumInfoFor::<T, I>::get(index), Some(ReferendumInfo::Cancelled(_, _, None)));
}

refund_submission_deposit {
let (origin, index) = create_referendum::<T, I>();
let caller = frame_system::ensure_signed(origin.clone()).unwrap();
let balance = T::Currency::free_balance(&caller);
assert_ok!(Referenda::<T, I>::cancel(T::CancelOrigin::successful_origin(), index));
assert_matches!(ReferendumInfoFor::<T, I>::get(index), Some(ReferendumInfo::Cancelled(_, Some(_), _)));
}: _<T::RuntimeOrigin>(origin, index)
verify {
assert_matches!(ReferendumInfoFor::<T, I>::get(index), Some(ReferendumInfo::Cancelled(_, None, _)));
let new_balance = T::Currency::free_balance(&caller);
assert!(new_balance > balance);
gavofyork marked this conversation as resolved.
Show resolved Hide resolved
}

cancel {
let (_origin, index) = create_referendum::<T, I>();
place_deposit::<T, I>(index);
Expand Down
58 changes: 52 additions & 6 deletions frame/referenda/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ use sp_runtime::{
use sp_std::{fmt::Debug, prelude::*};

mod branch;
pub mod migration;
mod types;
pub mod weights;

Expand Down Expand Up @@ -140,8 +141,12 @@ pub mod pallet {
use frame_support::pallet_prelude::*;
use frame_system::pallet_prelude::*;

/// The current storage version.
const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);

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

#[pallet::config]
Expand Down Expand Up @@ -342,6 +347,15 @@ pub mod pallet {
/// The final tally of votes in this referendum.
tally: T::Tally,
},
/// The submission deposit has been refunded.
SubmissionDepositRefunded {
/// Index of the referendum.
index: ReferendumIndex,
/// The account who placed the deposit.
who: T::AccountId,
/// The amount placed by the account.
amount: BalanceOf<T, I>,
},
}

#[pallet::error]
Expand All @@ -368,6 +382,8 @@ pub mod pallet {
NoPermission,
/// The deposit cannot be refunded since none was made.
NoDeposit,
/// The referendum status is invalid for this operation.
BadStatus,
}

#[pallet::call]
Expand Down Expand Up @@ -495,7 +511,7 @@ pub mod pallet {
Self::deposit_event(Event::<T, I>::Cancelled { index, tally: status.tally });
let info = ReferendumInfo::Cancelled(
frame_system::Pallet::<T>::block_number(),
status.submission_deposit,
Some(status.submission_deposit),
status.decision_deposit,
);
ReferendumInfoFor::<T, I>::insert(index, info);
Expand Down Expand Up @@ -579,6 +595,36 @@ pub mod pallet {
};
Ok(Some(branch.weight::<T, I>()).into())
}

/// Refund the Submission Deposit for a closed referendum back to the depositor.
///
/// - `origin`: must be `Signed` or `Root`.
/// - `index`: The index of a closed referendum whose Submission Deposit has not yet been
/// refunded.
///
/// Emits `SubmissionDepositRefunded`.
#[pallet::weight(T::WeightInfo::refund_submission_deposit())]
pub fn refund_submission_deposit(
origin: OriginFor<T>,
index: ReferendumIndex,
) -> DispatchResult {
ensure_signed_or_root(origin)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

using some newlines could make your code generally a bit more easy on the eyes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

controversial topic, like spaces vs tabs.
I prefer no new lines for example, it makes me not jump between paragraphs, and read line by line.
So functions only as a separation of concept.

let mut info =
ReferendumInfoFor::<T, I>::get(index).ok_or(Error::<T, I>::BadReferendum)?;
let deposit = info
.take_submission_deposit()
.map_err(|_| Error::<T, I>::BadStatus)?
.ok_or(Error::<T, I>::NoDeposit)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.ok_or(Error::<T, I>::NoDeposit)?;
.ok_or(Error::<T, I>::NoDeposit)?;

Self::refund_deposit(Some(deposit.clone()));
ReferendumInfoFor::<T, I>::insert(index, info);
let e = Event::<T, I>::SubmissionDepositRefunded {
index,
who: deposit.who,
amount: deposit.amount,
};
Self::deposit_event(e);
Comment on lines +620 to +625
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let e = Event::<T, I>::SubmissionDepositRefunded {
index,
who: deposit.who,
amount: deposit.amount,
};
Self::deposit_event(e);
Self::deposit_event(Event::<T, I>::SubmissionDepositRefunded {
index,
who: deposit.who,
amount: deposit.amount,
});

Ok(())
}
}
}

Expand Down Expand Up @@ -671,9 +717,9 @@ impl<T: Config<I>, I: 'static> Polling<T::Tally> for Pallet<T, I> {
Self::note_one_fewer_deciding(status.track);
let now = frame_system::Pallet::<T>::block_number();
let info = if approved {
ReferendumInfo::Approved(now, status.submission_deposit, status.decision_deposit)
ReferendumInfo::Approved(now, Some(status.submission_deposit), status.decision_deposit)
} else {
ReferendumInfo::Rejected(now, status.submission_deposit, status.decision_deposit)
ReferendumInfo::Rejected(now, Some(status.submission_deposit), status.decision_deposit)
};
ReferendumInfoFor::<T, I>::insert(index, info);
Ok(())
Expand Down Expand Up @@ -995,7 +1041,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
return (
ReferendumInfo::TimedOut(
now,
status.submission_deposit,
Some(status.submission_deposit),
status.decision_deposit,
),
true,
Expand Down Expand Up @@ -1027,7 +1073,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
return (
ReferendumInfo::Approved(
now,
status.submission_deposit,
Some(status.submission_deposit),
status.decision_deposit,
),
true,
Expand All @@ -1052,7 +1098,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
return (
ReferendumInfo::Rejected(
now,
status.submission_deposit,
Some(status.submission_deposit),
status.decision_deposit,
),
true,
Expand Down
232 changes: 232 additions & 0 deletions frame/referenda/src/migration.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,232 @@
// This file is part of Substrate.

// Copyright (C) 2017-2022 Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: Apache-2.0

// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

//! Storage migrations for the referenda pallet.

use super::*;
use codec::{Decode, Encode, EncodeLike, MaxEncodedLen};
use frame_support::{pallet_prelude::*, storage_alias, traits::OnRuntimeUpgrade};
use log;

/// Initial version of storage types.
pub mod v0 {
use super::*;
// ReferendumStatus and its dependency types referenced from the latest version while staying
// unchanged. [`super::test::referendum_status_v0()`] checks its immutability between v0 and
// latest version.
#[cfg(test)]
pub(super) use super::{ReferendumStatus, ReferendumStatusOf};

pub type ReferendumInfoOf<T, I> = ReferendumInfo<
TrackIdOf<T, I>,
PalletsOriginOf<T>,
<T as frame_system::Config>::BlockNumber,
BoundedCallOf<T, I>,
BalanceOf<T, I>,
TallyOf<T, I>,
<T as frame_system::Config>::AccountId,
ScheduleAddressOf<T, I>,
>;

/// Info regarding a referendum, present or past.
#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
pub enum ReferendumInfo<
TrackId: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone,
RuntimeOrigin: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone,
Moment: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone + EncodeLike,
Call: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone,
Balance: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone,
Tally: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone,
AccountId: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone,
ScheduleAddress: Eq + PartialEq + Debug + Encode + Decode + TypeInfo + Clone,
> {
/// Referendum has been submitted and is being voted on.
Ongoing(
ReferendumStatus<
TrackId,
RuntimeOrigin,
Moment,
Call,
Balance,
Tally,
AccountId,
ScheduleAddress,
>,
),
/// Referendum finished with approval. Submission deposit is held.
Approved(Moment, Deposit<AccountId, Balance>, Option<Deposit<AccountId, Balance>>),
/// Referendum finished with rejection. Submission deposit is held.
Rejected(Moment, Deposit<AccountId, Balance>, Option<Deposit<AccountId, Balance>>),
/// Referendum finished with cancellation. Submission deposit is held.
Cancelled(Moment, Deposit<AccountId, Balance>, Option<Deposit<AccountId, Balance>>),
/// Referendum finished and was never decided. Submission deposit is held.
TimedOut(Moment, Deposit<AccountId, Balance>, Option<Deposit<AccountId, Balance>>),
/// Referendum finished with a kill.
Killed(Moment),
}

#[storage_alias]
pub type ReferendumInfoFor<T: Config<I>, I: 'static> =
StorageMap<Pallet<T, I>, Blake2_128Concat, ReferendumIndex, ReferendumInfoOf<T, I>>;
}

pub mod v1 {
use super::*;

/// The log target.
const TARGET: &'static str = "runtime::democracy::migration::v1";

/// Transforms a submission deposit of ReferendumInfo(Approved|Rejected|Cancelled|TimedOut) to
/// optional value, making it refundable.
pub struct MigrateV0ToV1<T, I = ()>(PhantomData<(T, I)>);
impl<T: Config<I>, I: 'static> OnRuntimeUpgrade for MigrateV0ToV1<T, I> {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
let onchain_version = Pallet::<T, I>::on_chain_storage_version();
assert_eq!(onchain_version, 0, "migration from version 0 to 1.");
let referendum_count = v0::ReferendumInfoFor::<T, I>::iter().count();
log::info!(
target: TARGET,
"pre-upgrade state contains '{}' referendums.",
referendum_count
);
Ok((referendum_count as u32).encode())
}

fn on_runtime_upgrade() -> Weight {
let current_version = Pallet::<T, I>::current_storage_version();
let onchain_version = Pallet::<T, I>::on_chain_storage_version();
let mut weight = T::DbWeight::get().reads(1);
log::info!(
target: TARGET,
"running migration with current storage version {:?} / onchain {:?}.",
current_version,
onchain_version
);
if onchain_version != 0 {
log::warn!(target: TARGET, "skipping migration from v0 to v1.");
return weight
}
v0::ReferendumInfoFor::<T, I>::iter().for_each(|(key, value)| {
let maybe_new_value = match value {
v0::ReferendumInfo::Ongoing(_) | v0::ReferendumInfo::Killed(_) => None,
v0::ReferendumInfo::Approved(e, s, d) =>
Some(ReferendumInfo::Approved(e, Some(s), d)),
v0::ReferendumInfo::Rejected(e, s, d) =>
Some(ReferendumInfo::Rejected(e, Some(s), d)),
v0::ReferendumInfo::Cancelled(e, s, d) =>
Some(ReferendumInfo::Cancelled(e, Some(s), d)),
v0::ReferendumInfo::TimedOut(e, s, d) =>
Some(ReferendumInfo::TimedOut(e, Some(s), d)),
};
if let Some(new_value) = maybe_new_value {
weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1));
log::info!(target: TARGET, "migrating referendum #{:?}", &key);
ReferendumInfoFor::<T, I>::insert(key, new_value);
} else {
weight.saturating_accrue(T::DbWeight::get().reads(1));
}
});
StorageVersion::new(1).put::<Pallet<T, I>>();
weight.saturating_accrue(T::DbWeight::get().writes(1));
weight
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(state: Vec<u8>) -> Result<(), &'static str> {
let onchain_version = Pallet::<T, I>::on_chain_storage_version();
assert_eq!(onchain_version, 1, "must upgrade from version 0 to 1.");
let pre_referendum_count: u32 = Decode::decode(&mut &state[..])
.expect("failed to decode the state from pre-upgrade.");
let post_referendum_count = ReferendumInfoFor::<T, I>::iter().count() as u32;
assert_eq!(
post_referendum_count, pre_referendum_count,
"must migrate all referendums."
);
log::info!(target: TARGET, "migrated all referendums.");
Ok(())
}
}
}

#[cfg(test)]
pub mod test {
use super::*;
use crate::mock::{Test as T, *};
use core::str::FromStr;

// create referendum status v0.
fn create_status_v0() -> v0::ReferendumStatusOf<T, ()> {
let origin: OriginCaller = frame_system::RawOrigin::Root.into();
let track = <T as Config<()>>::Tracks::track_for(&origin).unwrap();
v0::ReferendumStatusOf::<T, ()> {
track,
in_queue: true,
origin,
proposal: set_balance_proposal_bounded(1),
enactment: DispatchTime::At(1),
tally: TallyOf::<T, ()>::new(track),
submission_deposit: Deposit { who: 1, amount: 10 },
submitted: 1,
decision_deposit: None,
alarm: None,
deciding: None,
}
}

#[test]
pub fn referendum_status_v0() {
// make sure the bytes of the encoded referendum v0 is decodable.
let ongoing_encoded = sp_core::Bytes::from_str("0x00000000013001012a000000000000000400000100000000000000010000000000000001000000000000000a00000000000000000000000000000000000100").unwrap();
let ongoing_dec = v0::ReferendumInfoOf::<T, ()>::decode(&mut &*ongoing_encoded).unwrap();
let ongoing = v0::ReferendumInfoOf::<T, ()>::Ongoing(create_status_v0());
assert_eq!(ongoing, ongoing_dec);
}

#[test]
fn migration_v0_to_v1_works() {
new_test_ext().execute_with(|| {
// create and insert into the storage an ongoing referendum v0.
let status_v0 = create_status_v0();
let ongoing_v0 = v0::ReferendumInfoOf::<T, ()>::Ongoing(status_v0.clone());
v0::ReferendumInfoFor::<T, ()>::insert(2, ongoing_v0);
// create and insert into the storage an approved referendum v0.
let approved_v0 = v0::ReferendumInfoOf::<T, ()>::Approved(
123,
Deposit { who: 1, amount: 10 },
Some(Deposit { who: 2, amount: 20 }),
);
v0::ReferendumInfoFor::<T, ()>::insert(5, approved_v0);
// run migration from v0 to v1.
v1::MigrateV0ToV1::<T, ()>::on_runtime_upgrade();
// fetch and assert migrated into v1 the ongoing referendum.
let ongoing_v1 = ReferendumInfoFor::<T, ()>::get(2).unwrap();
// referendum status schema is the same for v0 and v1.
assert_eq!(ReferendumInfoOf::<T, ()>::Ongoing(status_v0), ongoing_v1);
// fetch and assert migrated into v1 the approved referendum.
let approved_v1 = ReferendumInfoFor::<T, ()>::get(5).unwrap();
assert_eq!(
approved_v1,
ReferendumInfoOf::<T, ()>::Approved(
123,
Some(Deposit { who: 1, amount: 10 }),
Some(Deposit { who: 2, amount: 20 })
)
);
});
}
}
Loading