diff --git a/substrate/frame/bounties/src/benchmarking.rs b/substrate/frame/bounties/src/benchmarking.rs index 5a323ff0aa..21f68b0781 100644 --- a/substrate/frame/bounties/src/benchmarking.rs +++ b/substrate/frame/bounties/src/benchmarking.rs @@ -77,7 +77,7 @@ fn create_bounty() -> Result<( Ok((curator_lookup, bounty_id)) } -fn setup_pod_account() { +fn setup_pot_account() { let pot_account = Bounties::::account_id(); let value = T::Currency::minimum_balance().saturating_mul(1_000_000_000u32.into()); let _ = T::Currency::make_free_balance_be(&pot_account, value); @@ -109,7 +109,7 @@ benchmarks! { }: _(RawOrigin::Root, bounty_id) propose_curator { - setup_pod_account::(); + setup_pot_account::(); let (caller, curator, fee, value, reason) = setup_bounty::(0, MAX_BYTES); let curator_lookup = T::Lookup::unlookup(curator.clone()); Bounties::::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?; @@ -120,7 +120,7 @@ benchmarks! { // Worst case when curator is inactive and any sender unassigns the curator. unassign_curator { - setup_pod_account::(); + setup_pot_account::(); let (curator_lookup, bounty_id) = create_bounty::()?; Bounties::::on_initialize(T::BlockNumber::zero()); let bounty_id = BountyCount::get() - 1; @@ -129,7 +129,7 @@ benchmarks! { }: _(RawOrigin::Signed(caller), bounty_id) accept_curator { - setup_pod_account::(); + setup_pot_account::(); let (caller, curator, fee, value, reason) = setup_bounty::(0, MAX_BYTES); let curator_lookup = T::Lookup::unlookup(curator.clone()); Bounties::::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?; @@ -140,7 +140,7 @@ benchmarks! { }: _(RawOrigin::Signed(curator), bounty_id) award_bounty { - setup_pod_account::(); + setup_pot_account::(); let (curator_lookup, bounty_id) = create_bounty::()?; Bounties::::on_initialize(T::BlockNumber::zero()); @@ -150,7 +150,7 @@ benchmarks! { }: _(RawOrigin::Signed(curator), bounty_id, beneficiary) claim_bounty { - setup_pod_account::(); + setup_pot_account::(); let (curator_lookup, bounty_id) = create_bounty::()?; Bounties::::on_initialize(T::BlockNumber::zero()); @@ -170,14 +170,14 @@ benchmarks! { } close_bounty_proposed { - setup_pod_account::(); + setup_pot_account::(); let (caller, curator, fee, value, reason) = setup_bounty::(0, 0); Bounties::::propose_bounty(RawOrigin::Signed(caller).into(), value, reason)?; let bounty_id = BountyCount::get() - 1; }: close_bounty(RawOrigin::Root, bounty_id) close_bounty_active { - setup_pod_account::(); + setup_pot_account::(); let (curator_lookup, bounty_id) = create_bounty::()?; Bounties::::on_initialize(T::BlockNumber::zero()); let bounty_id = BountyCount::get() - 1; @@ -187,7 +187,7 @@ benchmarks! { } extend_bounty_expiry { - setup_pod_account::(); + setup_pot_account::(); let (curator_lookup, bounty_id) = create_bounty::()?; Bounties::::on_initialize(T::BlockNumber::zero()); @@ -200,7 +200,7 @@ benchmarks! { spend_funds { let b in 1 .. 100; - setup_pod_account::(); + setup_pot_account::(); create_approved_bounties::(b)?; let mut budget_remaining = BalanceOf::::max_value(); diff --git a/substrate/frame/tips/README.md b/substrate/frame/tips/README.md index 457e5b3bd0..36148e276e 100644 --- a/substrate/frame/tips/README.md +++ b/substrate/frame/tips/README.md @@ -30,3 +30,4 @@ any finders fee, in case of a public (and bonded) original report. - `tip_new` - Report an item worthy of a tip and declare a specific amount to tip. - `tip` - Declare or redeclare an amount to tip for a particular reason. - `close_tip` - Close and pay out a tip. +- `slash_tip` - Remove and slash an already-open tip. \ No newline at end of file diff --git a/substrate/frame/tips/src/benchmarking.rs b/substrate/frame/tips/src/benchmarking.rs index 71f9002b9b..4f0338b9c5 100644 --- a/substrate/frame/tips/src/benchmarking.rs +++ b/substrate/frame/tips/src/benchmarking.rs @@ -79,7 +79,7 @@ fn create_tips(t: u32, hash: T::Hash, value: BalanceOf) -> Ok(()) } -fn setup_pod_account() { +fn setup_pot_account() { let pot_account = TipsMod::::account_id(); let value = T::Currency::minimum_balance().saturating_mul(1_000_000_000u32.into()); let _ = T::Currency::make_free_balance_be(&pot_account, value); @@ -148,7 +148,7 @@ benchmarks! { let t in 1 .. MAX_TIPPERS; // Make sure pot is funded - setup_pod_account::(); + setup_pot_account::(); // Set up a new tip proposal let (member, reason, beneficiary, value) = setup_tip::(0, t)?; @@ -164,6 +164,7 @@ benchmarks! { let reason_hash = T::Hashing::hash(&reason[..]); let hash = T::Hashing::hash_of(&(&reason_hash, &beneficiary)); ensure!(Tips::::contains_key(hash), "tip does not exist"); + create_tips::(t, hash.clone(), value)?; let caller = account("caller", t, SEED); @@ -172,6 +173,26 @@ benchmarks! { frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into()); }: _(RawOrigin::Signed(caller), hash) + slash_tip { + let t in 1 .. MAX_TIPPERS; + + // Make sure pot is funded + setup_pot_account::(); + + // Set up a new tip proposal + let (member, reason, beneficiary, value) = setup_tip::(0, t)?; + let value = T::Currency::minimum_balance().saturating_mul(100u32.into()); + TipsMod::::tip_new( + RawOrigin::Signed(member).into(), + reason.clone(), + beneficiary.clone(), + value + )?; + + let reason_hash = T::Hashing::hash(&reason[..]); + let hash = T::Hashing::hash_of(&(&reason_hash, &beneficiary)); + ensure!(Tips::::contains_key(hash), "tip does not exist"); + }: _(RawOrigin::Root, hash) } #[cfg(test)] @@ -188,6 +209,7 @@ mod tests { assert_ok!(test_benchmark_tip_new::()); assert_ok!(test_benchmark_tip::()); assert_ok!(test_benchmark_close_tip::()); + assert_ok!(test_benchmark_slash_tip::()); }); } } diff --git a/substrate/frame/tips/src/lib.rs b/substrate/frame/tips/src/lib.rs index 3507b220d5..eaa785a563 100644 --- a/substrate/frame/tips/src/lib.rs +++ b/substrate/frame/tips/src/lib.rs @@ -58,8 +58,6 @@ mod tests; mod benchmarking; pub mod weights; -use sp_std::if_std; - use sp_std::prelude::*; use frame_support::{decl_module, decl_storage, decl_event, ensure, decl_error, Parameter}; use frame_support::traits::{ @@ -70,8 +68,7 @@ use frame_support::traits::{ use sp_runtime::{ Percent, RuntimeDebug, traits::{ Zero, AccountIdConversion, Hash, BadOrigin }}; - -use frame_support::traits::{Contains, ContainsLengthBound}; +use frame_support::traits::{Contains, ContainsLengthBound, OnUnbalanced, EnsureOrigin}; use codec::{Encode, Decode}; use frame_system::{self as system, ensure_signed}; pub use weights::WeightInfo; @@ -170,6 +167,8 @@ decl_event!( TipClosed(Hash, AccountId, Balance), /// A tip suggestion has been retracted. \[tip_hash\] TipRetracted(Hash), + /// A tip suggestion has been slashed. \[tip_hash, finder, deposit\] + TipSlashed(Hash, AccountId, Balance), } ); @@ -408,6 +407,32 @@ decl_module! { Tips::::remove(hash); Self::payout_tip(hash, tip); } + + /// Remove and slash an already-open tip. + /// + /// May only be called from `T::RejectOrigin`. + /// + /// As a result, the finder is slashed and the deposits are lost. + /// + /// Emits `TipSlashed` if successful. + /// + /// # + /// `T` is charged as upper bound given by `ContainsLengthBound`. + /// The actual cost depends on the implementation of `T::Tippers`. + /// # + #[weight = ::WeightInfo::slash_tip(T::Tippers::max_len() as u32)] + fn slash_tip(origin, hash: T::Hash) { + T::RejectOrigin::ensure_origin(origin)?; + + let tip = Tips::::take(hash).ok_or(Error::::UnknownTip)?; + + if !tip.deposit.is_zero() { + let imbalance = T::Currency::slash_reserved(&tip.finder, tip.deposit).0; + T::OnSlash::on_unbalanced(imbalance); + } + Reasons::::remove(&tip.reason); + Self::deposit_event(RawEvent::TipSlashed(hash, tip.finder, tip.deposit)); + } } } @@ -523,10 +548,6 @@ impl Module { use frame_support::{Twox64Concat, migration::StorageKeyIterator}; - if_std! { - println!("Inside migrate_retract_tip_for_tip_new()!"); - } - for (hash, old_tip) in StorageKeyIterator::< T::Hash, OldOpenTip, T::BlockNumber, T::Hash>, @@ -534,25 +555,11 @@ impl Module { >::new(b"Treasury", b"Tips").drain() { - if_std! { - println!("Inside loop migrate_retract_tip_for_tip_new()!"); - } - let (finder, deposit, finders_fee) = match old_tip.finder { Some((finder, deposit)) => { - if_std! { - // This code is only being compiled and executed when the `std` feature is enabled. - println!("OK case!"); - println!("value is: {:#?},{:#?}", finder, deposit); - } (finder, deposit, true) }, None => { - if_std! { - // This code is only being compiled and executed when the `std` feature is enabled. - println!("None case!"); - // println!("value is: {:#?},{:#?}", T::AccountId::default(), Zero::zero()); - } (T::AccountId::default(), Zero::zero(), false) }, }; @@ -567,10 +574,5 @@ impl Module { }; Tips::::insert(hash, new_tip) } - - if_std! { - println!("Exit migrate_retract_tip_for_tip_new()!"); - } - } } diff --git a/substrate/frame/tips/src/tests.rs b/substrate/frame/tips/src/tests.rs index e6f9cd4e66..3bfecafeaf 100644 --- a/substrate/frame/tips/src/tests.rs +++ b/substrate/frame/tips/src/tests.rs @@ -286,6 +286,40 @@ fn close_tip_works() { }); } +#[test] +fn slash_tip_works() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + Balances::make_free_balance_be(&Treasury::account_id(), 101); + assert_eq!(Treasury::pot(), 100); + + assert_eq!(Balances::reserved_balance(0), 0); + assert_eq!(Balances::free_balance(0), 100); + + assert_ok!(TipsModTestInst::report_awesome(Origin::signed(0), b"awesome.dot".to_vec(), 3)); + + assert_eq!(Balances::reserved_balance(0), 12); + assert_eq!(Balances::free_balance(0), 88); + + let h = tip_hash(); + assert_eq!(last_event(), RawEvent::NewTip(h)); + + // can't remove from any origin + assert_noop!( + TipsModTestInst::slash_tip(Origin::signed(0), h.clone()), + BadOrigin, + ); + + // can remove from root. + assert_ok!(TipsModTestInst::slash_tip(Origin::root(), h.clone())); + assert_eq!(last_event(), RawEvent::TipSlashed(h, 0, 12)); + + // tipper slashed + assert_eq!(Balances::reserved_balance(0), 0); + assert_eq!(Balances::free_balance(0), 88); + }); +} + #[test] fn retract_tip_works() { new_test_ext().execute_with(|| { @@ -409,12 +443,8 @@ fn test_last_reward_migration() { s.top = data.into_iter().collect(); - println!("Executing the test!"); - sp_io::TestExternalities::new(s).execute_with(|| { - println!("Calling migrate_retract_tip_for_tip_new()!"); - TipsModTestInst::migrate_retract_tip_for_tip_new(); // Test w/ finder diff --git a/substrate/frame/tips/src/weights.rs b/substrate/frame/tips/src/weights.rs index ad2d3104ca..c1d9982910 100644 --- a/substrate/frame/tips/src/weights.rs +++ b/substrate/frame/tips/src/weights.rs @@ -18,11 +18,11 @@ //! Autogenerated weights for pallet_tips //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 2.0.0 -//! DATE: 2020-12-16, STEPS: [50, ], REPEAT: 20, LOW RANGE: [], HIGH RANGE: [] +//! DATE: 2020-12-20, STEPS: [50, ], REPEAT: 20, LOW RANGE: [], HIGH RANGE: [] //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 128 // Executed Command: -// ./target/release/substrate +// target/release/substrate // benchmark // --chain=dev // --steps=50 @@ -49,83 +49,98 @@ pub trait WeightInfo { fn tip_new(r: u32, t: u32, ) -> Weight; fn tip(t: u32, ) -> Weight; fn close_tip(t: u32, ) -> Weight; + fn slash_tip(t: u32, ) -> Weight; } /// Weights for pallet_tips using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { fn report_awesome(r: u32, ) -> Weight { - (74_814_000 as Weight) + (73_795_000 as Weight) // Standard Error: 0 .saturating_add((2_000 as Weight).saturating_mul(r as Weight)) .saturating_add(T::DbWeight::get().reads(2 as Weight)) .saturating_add(T::DbWeight::get().writes(2 as Weight)) } fn retract_tip() -> Weight { - (62_962_000 as Weight) + (61_753_000 as Weight) .saturating_add(T::DbWeight::get().reads(1 as Weight)) .saturating_add(T::DbWeight::get().writes(2 as Weight)) } fn tip_new(r: u32, t: u32, ) -> Weight { - (48_132_000 as Weight) + (47_731_000 as Weight) // Standard Error: 0 .saturating_add((2_000 as Weight).saturating_mul(r as Weight)) // Standard Error: 0 - .saturating_add((155_000 as Weight).saturating_mul(t as Weight)) + .saturating_add((154_000 as Weight).saturating_mul(t as Weight)) .saturating_add(T::DbWeight::get().reads(2 as Weight)) .saturating_add(T::DbWeight::get().writes(2 as Weight)) } fn tip(t: u32, ) -> Weight { - (36_168_000 as Weight) + (35_215_000 as Weight) // Standard Error: 1_000 - .saturating_add((695_000 as Weight).saturating_mul(t as Weight)) + .saturating_add((712_000 as Weight).saturating_mul(t as Weight)) .saturating_add(T::DbWeight::get().reads(2 as Weight)) .saturating_add(T::DbWeight::get().writes(1 as Weight)) } fn close_tip(t: u32, ) -> Weight { - (119_313_000 as Weight) + (117_027_000 as Weight) // Standard Error: 1_000 - .saturating_add((372_000 as Weight).saturating_mul(t as Weight)) + .saturating_add((375_000 as Weight).saturating_mul(t as Weight)) .saturating_add(T::DbWeight::get().reads(3 as Weight)) .saturating_add(T::DbWeight::get().writes(3 as Weight)) } + fn slash_tip(t: u32, ) -> Weight { + (37_184_000 as Weight) + // Standard Error: 0 + .saturating_add((11_000 as Weight).saturating_mul(t as Weight)) + .saturating_add(T::DbWeight::get().reads(1 as Weight)) + .saturating_add(T::DbWeight::get().writes(2 as Weight)) + } } // For backwards compatibility and tests impl WeightInfo for () { fn report_awesome(r: u32, ) -> Weight { - (74_814_000 as Weight) + (73_795_000 as Weight) // Standard Error: 0 .saturating_add((2_000 as Weight).saturating_mul(r as Weight)) .saturating_add(RocksDbWeight::get().reads(2 as Weight)) .saturating_add(RocksDbWeight::get().writes(2 as Weight)) } fn retract_tip() -> Weight { - (62_962_000 as Weight) + (61_753_000 as Weight) .saturating_add(RocksDbWeight::get().reads(1 as Weight)) .saturating_add(RocksDbWeight::get().writes(2 as Weight)) } fn tip_new(r: u32, t: u32, ) -> Weight { - (48_132_000 as Weight) + (47_731_000 as Weight) // Standard Error: 0 .saturating_add((2_000 as Weight).saturating_mul(r as Weight)) // Standard Error: 0 - .saturating_add((155_000 as Weight).saturating_mul(t as Weight)) + .saturating_add((154_000 as Weight).saturating_mul(t as Weight)) .saturating_add(RocksDbWeight::get().reads(2 as Weight)) .saturating_add(RocksDbWeight::get().writes(2 as Weight)) } fn tip(t: u32, ) -> Weight { - (36_168_000 as Weight) + (35_215_000 as Weight) // Standard Error: 1_000 - .saturating_add((695_000 as Weight).saturating_mul(t as Weight)) + .saturating_add((712_000 as Weight).saturating_mul(t as Weight)) .saturating_add(RocksDbWeight::get().reads(2 as Weight)) .saturating_add(RocksDbWeight::get().writes(1 as Weight)) } fn close_tip(t: u32, ) -> Weight { - (119_313_000 as Weight) + (117_027_000 as Weight) // Standard Error: 1_000 - .saturating_add((372_000 as Weight).saturating_mul(t as Weight)) + .saturating_add((375_000 as Weight).saturating_mul(t as Weight)) .saturating_add(RocksDbWeight::get().reads(3 as Weight)) .saturating_add(RocksDbWeight::get().writes(3 as Weight)) } + fn slash_tip(t: u32, ) -> Weight { + (37_184_000 as Weight) + // Standard Error: 0 + .saturating_add((11_000 as Weight).saturating_mul(t as Weight)) + .saturating_add(RocksDbWeight::get().reads(1 as Weight)) + .saturating_add(RocksDbWeight::get().writes(2 as Weight)) + } } diff --git a/substrate/frame/treasury/src/benchmarking.rs b/substrate/frame/treasury/src/benchmarking.rs index 16ed1b01ae..39b398ab89 100644 --- a/substrate/frame/treasury/src/benchmarking.rs +++ b/substrate/frame/treasury/src/benchmarking.rs @@ -59,7 +59,7 @@ fn create_approved_proposals, I: Instance>(n: u32) -> Result<(), &' Ok(()) } -fn setup_pod_account, I: Instance>() { +fn setup_pot_account, I: Instance>() { let pot_account = Treasury::::account_id(); let value = T::Currency::minimum_balance().saturating_mul(1_000_000_000u32.into()); let _ = T::Currency::make_free_balance_be(&pot_account, value); @@ -97,7 +97,7 @@ benchmarks_instance! { on_initialize_proposals { let p in 0 .. 100; - setup_pod_account::(); + setup_pot_account::(); create_approved_proposals::(p)?; }: { Treasury::::on_initialize(T::BlockNumber::zero());