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

Commit

Permalink
Pallets: Treasury deprecate propose_spend dispatchable (#14538)
Browse files Browse the repository at this point in the history 
* treasury deprecate dispatchables

* allow deprecated

* allow deprecated for benchmarks

* allow deprecated in tests

* allow deprecated for bounties tests

* deprecation month
  • Loading branch information
@muharem @Ank4n
muharem authored and Ank4n committed Aug 20, 2023
1 parent 954a3a6 commit 38827d0
Show file tree
Hide file tree
Showing 4 changed files with 258 additions and 60 deletions.
135 changes: 108 additions & 27 deletions frame/bounties/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,10 @@ fn minting_works() {
#[test]
fn spend_proposal_takes_min_deposit() {
new_test_ext().execute_with(|| {
assert_ok!(Treasury::propose_spend(RuntimeOrigin::signed(0), 1, 3));
assert_ok!({
#[allow(deprecated)]
Treasury::propose_spend(RuntimeOrigin::signed(0), 1, 3)
});
assert_eq!(Balances::free_balance(0), 99);
assert_eq!(Balances::reserved_balance(0), 1);
});
Expand All @@ -237,7 +240,10 @@ fn spend_proposal_takes_min_deposit() {
#[test]
fn spend_proposal_takes_proportional_deposit() {
new_test_ext().execute_with(|| {
assert_ok!(Treasury::propose_spend(RuntimeOrigin::signed(0), 100, 3));
assert_ok!({
#[allow(deprecated)]
Treasury::propose_spend(RuntimeOrigin::signed(0), 100, 3)
});
assert_eq!(Balances::free_balance(0), 95);
assert_eq!(Balances::reserved_balance(0), 5);
});
Expand All @@ -247,7 +253,10 @@ fn spend_proposal_takes_proportional_deposit() {
fn spend_proposal_fails_when_proposer_poor() {
new_test_ext().execute_with(|| {
assert_noop!(
Treasury::propose_spend(RuntimeOrigin::signed(2), 100, 3),
{
#[allow(deprecated)]
Treasury::propose_spend(RuntimeOrigin::signed(2), 100, 3)
},
TreasuryError::InsufficientProposersBalance,
);
});
Expand All @@ -258,8 +267,14 @@ fn accepted_spend_proposal_ignored_outside_spend_period() {
new_test_ext().execute_with(|| {
Balances::make_free_balance_be(&Treasury::account_id(), 101);

assert_ok!(Treasury::propose_spend(RuntimeOrigin::signed(0), 100, 3));
assert_ok!(Treasury::approve_proposal(RuntimeOrigin::root(), 0));
assert_ok!({
#[allow(deprecated)]
Treasury::propose_spend(RuntimeOrigin::signed(0), 100, 3)
});
assert_ok!({
#[allow(deprecated)]
Treasury::approve_proposal(RuntimeOrigin::root(), 0)
});

<Treasury as OnInitialize<u64>>::on_initialize(1);
assert_eq!(Balances::free_balance(3), 0);
Expand All @@ -285,8 +300,14 @@ fn rejected_spend_proposal_ignored_on_spend_period() {
new_test_ext().execute_with(|| {
Balances::make_free_balance_be(&Treasury::account_id(), 101);

assert_ok!(Treasury::propose_spend(RuntimeOrigin::signed(0), 100, 3));
assert_ok!(Treasury::reject_proposal(RuntimeOrigin::root(), 0));
assert_ok!({
#[allow(deprecated)]
Treasury::propose_spend(RuntimeOrigin::signed(0), 100, 3)
});
assert_ok!({
#[allow(deprecated)]
Treasury::reject_proposal(RuntimeOrigin::root(), 0)
});

<Treasury as OnInitialize<u64>>::on_initialize(2);
assert_eq!(Balances::free_balance(3), 0);
Expand All @@ -299,10 +320,19 @@ fn reject_already_rejected_spend_proposal_fails() {
new_test_ext().execute_with(|| {
Balances::make_free_balance_be(&Treasury::account_id(), 101);

assert_ok!(Treasury::propose_spend(RuntimeOrigin::signed(0), 100, 3));
assert_ok!(Treasury::reject_proposal(RuntimeOrigin::root(), 0));
assert_ok!({
#[allow(deprecated)]
Treasury::propose_spend(RuntimeOrigin::signed(0), 100, 3)
});
assert_ok!({
#[allow(deprecated)]
Treasury::reject_proposal(RuntimeOrigin::root(), 0)
});
assert_noop!(
Treasury::reject_proposal(RuntimeOrigin::root(), 0),
{
#[allow(deprecated)]
Treasury::reject_proposal(RuntimeOrigin::root(), 0)
},
TreasuryError::InvalidIndex
);
});
Expand All @@ -312,7 +342,10 @@ fn reject_already_rejected_spend_proposal_fails() {
fn reject_non_existent_spend_proposal_fails() {
new_test_ext().execute_with(|| {
assert_noop!(
Treasury::reject_proposal(RuntimeOrigin::root(), 0),
{
#[allow(deprecated)]
Treasury::reject_proposal(RuntimeOrigin::root(), 0)
},
pallet_treasury::Error::<Test>::InvalidIndex
);
});
Expand All @@ -322,7 +355,10 @@ fn reject_non_existent_spend_proposal_fails() {
fn accept_non_existent_spend_proposal_fails() {
new_test_ext().execute_with(|| {
assert_noop!(
Treasury::approve_proposal(RuntimeOrigin::root(), 0),
{
#[allow(deprecated)]
Treasury::approve_proposal(RuntimeOrigin::root(), 0)
},
TreasuryError::InvalidIndex
);
});
Expand All @@ -333,10 +369,19 @@ fn accept_already_rejected_spend_proposal_fails() {
new_test_ext().execute_with(|| {
Balances::make_free_balance_be(&Treasury::account_id(), 101);

assert_ok!(Treasury::propose_spend(RuntimeOrigin::signed(0), 100, 3));
assert_ok!(Treasury::reject_proposal(RuntimeOrigin::root(), 0));
assert_ok!({
#[allow(deprecated)]
Treasury::propose_spend(RuntimeOrigin::signed(0), 100, 3)
});
assert_ok!({
#[allow(deprecated)]
Treasury::reject_proposal(RuntimeOrigin::root(), 0)
});
assert_noop!(
Treasury::approve_proposal(RuntimeOrigin::root(), 0),
{
#[allow(deprecated)]
Treasury::approve_proposal(RuntimeOrigin::root(), 0)
},
TreasuryError::InvalidIndex
);
});
Expand All @@ -348,8 +393,14 @@ fn accepted_spend_proposal_enacted_on_spend_period() {
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Treasury::pot(), 100);

assert_ok!(Treasury::propose_spend(RuntimeOrigin::signed(0), 100, 3));
assert_ok!(Treasury::approve_proposal(RuntimeOrigin::root(), 0));
assert_ok!({
#[allow(deprecated)]
Treasury::propose_spend(RuntimeOrigin::signed(0), 100, 3)
});
assert_ok!({
#[allow(deprecated)]
Treasury::approve_proposal(RuntimeOrigin::root(), 0)
});

<Treasury as OnInitialize<u64>>::on_initialize(2);
assert_eq!(Balances::free_balance(3), 100);
Expand All @@ -363,8 +414,14 @@ fn pot_underflow_should_not_diminish() {
Balances::make_free_balance_be(&Treasury::account_id(), 101);
assert_eq!(Treasury::pot(), 100);

assert_ok!(Treasury::propose_spend(RuntimeOrigin::signed(0), 150, 3));
assert_ok!(Treasury::approve_proposal(RuntimeOrigin::root(), 0));
assert_ok!({
#[allow(deprecated)]
Treasury::propose_spend(RuntimeOrigin::signed(0), 150, 3)
});
assert_ok!({
#[allow(deprecated)]
Treasury::approve_proposal(RuntimeOrigin::root(), 0)
});

<Treasury as OnInitialize<u64>>::on_initialize(2);
assert_eq!(Treasury::pot(), 100); // Pot hasn't changed
Expand All @@ -385,14 +442,26 @@ fn treasury_account_doesnt_get_deleted() {
assert_eq!(Treasury::pot(), 100);
let treasury_balance = Balances::free_balance(&Treasury::account_id());

assert_ok!(Treasury::propose_spend(RuntimeOrigin::signed(0), treasury_balance, 3));
assert_ok!(Treasury::approve_proposal(RuntimeOrigin::root(), 0));
assert_ok!({
#[allow(deprecated)]
Treasury::propose_spend(RuntimeOrigin::signed(0), treasury_balance, 3)
});
assert_ok!({
#[allow(deprecated)]
Treasury::approve_proposal(RuntimeOrigin::root(), 0)
});

<Treasury as OnInitialize<u64>>::on_initialize(2);
assert_eq!(Treasury::pot(), 100); // Pot hasn't changed

assert_ok!(Treasury::propose_spend(RuntimeOrigin::signed(0), Treasury::pot(), 3));
assert_ok!(Treasury::approve_proposal(RuntimeOrigin::root(), 1));
assert_ok!({
#[allow(deprecated)]
Treasury::propose_spend(RuntimeOrigin::signed(0), Treasury::pot(), 3)
});
assert_ok!({
#[allow(deprecated)]
Treasury::approve_proposal(RuntimeOrigin::root(), 1)
});

<Treasury as OnInitialize<u64>>::on_initialize(4);
assert_eq!(Treasury::pot(), 0); // Pot is emptied
Expand All @@ -415,10 +484,22 @@ fn inexistent_account_works() {
assert_eq!(Balances::free_balance(Treasury::account_id()), 0); // Account does not exist
assert_eq!(Treasury::pot(), 0); // Pot is empty

assert_ok!(Treasury::propose_spend(RuntimeOrigin::signed(0), 99, 3));
assert_ok!(Treasury::approve_proposal(RuntimeOrigin::root(), 0));
assert_ok!(Treasury::propose_spend(RuntimeOrigin::signed(0), 1, 3));
assert_ok!(Treasury::approve_proposal(RuntimeOrigin::root(), 1));
assert_ok!({
#[allow(deprecated)]
Treasury::propose_spend(RuntimeOrigin::signed(0), 99, 3)
});
assert_ok!({
#[allow(deprecated)]
Treasury::approve_proposal(RuntimeOrigin::root(), 0)
});
assert_ok!({
#[allow(deprecated)]
Treasury::propose_spend(RuntimeOrigin::signed(0), 1, 3)
});
assert_ok!({
#[allow(deprecated)]
Treasury::approve_proposal(RuntimeOrigin::root(), 1)
});
<Treasury as OnInitialize<u64>>::on_initialize(2);
assert_eq!(Treasury::pot(), 0); // Pot hasn't changed
assert_eq!(Balances::free_balance(3), 0); // Balance of `3` hasn't changed
Expand Down
6 changes: 6 additions & 0 deletions frame/treasury/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,10 @@ fn setup_proposal<T: Config<I>, I: 'static>(
fn create_approved_proposals<T: Config<I>, I: 'static>(n: u32) -> Result<(), &'static str> {
for i in 0..n {
let (caller, value, lookup) = setup_proposal::<T, I>(i);
#[allow(deprecated)]
Treasury::<T, I>::propose_spend(RawOrigin::Signed(caller).into(), value, lookup)?;
let proposal_id = <ProposalCount<T, I>>::get() - 1;
#[allow(deprecated)]
Treasury::<T, I>::approve_proposal(RawOrigin::Root.into(), proposal_id)?;
}
ensure!(<Approvals<T, I>>::get().len() == n as usize, "Not all approved");
Expand Down Expand Up @@ -93,6 +95,7 @@ benchmarks_instance_pallet! {

reject_proposal {
let (caller, value, beneficiary_lookup) = setup_proposal::<T, _>(SEED);
#[allow(deprecated)]
Treasury::<T, _>::propose_spend(
RawOrigin::Signed(caller).into(),
value,
Expand All @@ -107,6 +110,7 @@ benchmarks_instance_pallet! {
let p in 0 .. T::MaxApprovals::get() - 1;
create_approved_proposals::<T, _>(p)?;
let (caller, value, beneficiary_lookup) = setup_proposal::<T, _>(SEED);
#[allow(deprecated)]
Treasury::<T, _>::propose_spend(
RawOrigin::Signed(caller).into(),
value,
Expand All @@ -119,12 +123,14 @@ benchmarks_instance_pallet! {

remove_approval {
let (caller, value, beneficiary_lookup) = setup_proposal::<T, _>(SEED);
#[allow(deprecated)]
Treasury::<T, _>::propose_spend(
RawOrigin::Signed(caller).into(),
value,
beneficiary_lookup
)?;
let proposal_id = Treasury::<T, _>::proposal_count() - 1;
#[allow(deprecated)]
Treasury::<T, I>::approve_proposal(RawOrigin::Root.into(), proposal_id)?;
let reject_origin =
T::RejectOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?;
Expand Down
12 changes: 12 additions & 0 deletions frame/treasury/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,10 @@ pub mod pallet {
/// - O(1)
#[pallet::call_index(0)]
#[pallet::weight(T::WeightInfo::propose_spend())]
#[allow(deprecated)]
#[deprecated(
note = "`propose_spend` will be removed in February 2024. Use `spend` instead."
)]
pub fn propose_spend(
origin: OriginFor<T>,
#[pallet::compact] value: BalanceOf<T, I>,
Expand Down Expand Up @@ -364,6 +368,10 @@ pub mod pallet {
/// - O(1)
#[pallet::call_index(1)]
#[pallet::weight((T::WeightInfo::reject_proposal(), DispatchClass::Operational))]
#[allow(deprecated)]
#[deprecated(
note = "`reject_proposal` will be removed in February 2024. Use `spend` instead."
)]
pub fn reject_proposal(
origin: OriginFor<T>,
#[pallet::compact] proposal_id: ProposalIndex,
Expand Down Expand Up @@ -392,6 +400,10 @@ pub mod pallet {
/// - O(1).
#[pallet::call_index(2)]
#[pallet::weight((T::WeightInfo::approve_proposal(T::MaxApprovals::get()), DispatchClass::Operational))]
#[allow(deprecated)]
#[deprecated(
note = "`approve_proposal` will be removed in February 2024. Use `spend` instead."
)]
pub fn approve_proposal(
origin: OriginFor<T>,
#[pallet::compact] proposal_id: ProposalIndex,
Expand Down
Loading

0 comments on commit 38827d0

Please sign in to comment.