diff --git a/Cargo.lock b/Cargo.lock index 89b24d0826f97..0df37db5edf92 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4665,6 +4665,7 @@ dependencies = [ "sp-io", "sp-runtime", "sp-std", + "sp-storage", ] [[package]] diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index c5c11fe577a3f..969e66653e421 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -97,8 +97,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // and set impl_version to 0. If only runtime // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. - spec_version: 254, - impl_version: 1, + spec_version: 255, + impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1, }; diff --git a/frame/treasury/Cargo.toml b/frame/treasury/Cargo.toml index 28f972d458c82..dfab1aca43b28 100644 --- a/frame/treasury/Cargo.toml +++ b/frame/treasury/Cargo.toml @@ -25,6 +25,7 @@ frame-benchmarking = { version = "2.0.0-rc4", default-features = false, path = " [dev-dependencies] sp-io ={ version = "2.0.0-rc4", path = "../../primitives/io" } sp-core = { version = "2.0.0-rc4", path = "../../primitives/core" } +sp-storage = { version = "2.0.0-rc4", path = "../../primitives/storage" } [features] default = ["std"] diff --git a/frame/treasury/src/lib.rs b/frame/treasury/src/lib.rs index e67ace54755c6..bb139c4cc6442 100644 --- a/frame/treasury/src/lib.rs +++ b/frame/treasury/src/lib.rs @@ -192,13 +192,17 @@ pub struct OpenTip< reason: Hash, /// The account to be tipped. who: AccountId, - /// The account who began this tip and the amount held on deposit. - finder: Option<(AccountId, Balance)>, + /// The account who began this tip. + finder: AccountId, + /// The amount held on deposit for this tip. + deposit: Balance, /// The block number at which this tip will close if `Some`. If `None`, then no closing is /// scheduled. closes: Option, /// The members who have voted for this tip. Sorted by AccountId. tips: Vec<(AccountId, Balance)>, + /// Whether this tip should result in the finder taking a fee. + finders_fee: bool, } decl_storage! { @@ -428,8 +432,15 @@ decl_module! { T::Currency::reserve(&finder, deposit)?; Reasons::::insert(&reason_hash, &reason); - let finder = Some((finder, deposit)); - let tip = OpenTip { reason: reason_hash, who, finder, closes: None, tips: vec![] }; + let tip = OpenTip { + reason: reason_hash, + who, + finder, + deposit, + closes: None, + tips: vec![], + finders_fee: true + }; Tips::::insert(&hash, tip); Self::deposit_event(RawEvent::NewTip(hash)); } @@ -457,12 +468,13 @@ decl_module! { fn retract_tip(origin, hash: T::Hash) { let who = ensure_signed(origin)?; let tip = Tips::::get(&hash).ok_or(Error::::UnknownTip)?; - let (finder, deposit) = tip.finder.ok_or(Error::::NotFinder)?; - ensure!(finder == who, Error::::NotFinder); + ensure!(tip.finder == who, Error::::NotFinder); Reasons::::remove(&tip.reason); Tips::::remove(&hash); - let _ = T::Currency::unreserve(&who, deposit); + if !tip.deposit.is_zero() { + let _ = T::Currency::unreserve(&who, tip.deposit); + } Self::deposit_event(RawEvent::TipRetracted(hash)); } @@ -501,8 +513,16 @@ decl_module! { Reasons::::insert(&reason_hash, &reason); Self::deposit_event(RawEvent::NewTip(hash.clone())); - let tips = vec![(tipper, tip_value)]; - let tip = OpenTip { reason: reason_hash, who, finder: None, closes: None, tips }; + let tips = vec![(tipper.clone(), tip_value)]; + let tip = OpenTip { + reason: reason_hash, + who, + finder: tipper, + deposit: Zero::zero(), + closes: None, + tips, + finders_fee: false, + }; Tips::::insert(&hash, tip); } @@ -667,15 +687,17 @@ impl Module { let treasury = Self::account_id(); let max_payout = Self::pot(); let mut payout = tips[tips.len() / 2].1.min(max_payout); - if let Some((finder, deposit)) = tip.finder { - let _ = T::Currency::unreserve(&finder, deposit); - if finder != tip.who { + if !tip.deposit.is_zero() { + let _ = T::Currency::unreserve(&tip.finder, tip.deposit); + } + if tip.finders_fee { + if tip.finder != tip.who { // pay out the finder's fee. let finders_fee = T::TipFindersFee::get() * payout; payout -= finders_fee; // this should go through given we checked it's at most the free balance, but still // we only make a best-effort. - let _ = T::Currency::transfer(&treasury, &finder, finders_fee, KeepAlive); + let _ = T::Currency::transfer(&treasury, &tip.finder, finders_fee, KeepAlive); } } // same as above: best-effort only. @@ -753,6 +775,55 @@ impl Module { // Must never be less than 0 but better be safe. .saturating_sub(T::Currency::minimum_balance()) } + + pub fn migrate_retract_tip_for_tip_new() { + /// An open tipping "motion". Retains all details of a tip including information on the finder + /// and the members who have voted. + #[derive(Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug)] + pub struct OldOpenTip< + AccountId: Parameter, + Balance: Parameter, + BlockNumber: Parameter, + Hash: Parameter, + > { + /// The hash of the reason for the tip. The reason should be a human-readable UTF-8 encoded string. A URL would be + /// sensible. + reason: Hash, + /// The account to be tipped. + who: AccountId, + /// The account who began this tip and the amount held on deposit. + finder: Option<(AccountId, Balance)>, + /// The block number at which this tip will close if `Some`. If `None`, then no closing is + /// scheduled. + closes: Option, + /// The members who have voted for this tip. Sorted by AccountId. + tips: Vec<(AccountId, Balance)>, + } + + use frame_support::{Twox64Concat, migration::StorageKeyIterator}; + + for (hash, old_tip) in StorageKeyIterator::< + T::Hash, + OldOpenTip, T::BlockNumber, T::Hash>, + Twox64Concat, + >::new(b"Treasury", b"Tips").drain() + { + let (finder, deposit, finders_fee) = match old_tip.finder { + Some((finder, deposit)) => (finder, deposit, true), + None => (T::AccountId::default(), Zero::zero(), false), + }; + let new_tip = OpenTip { + reason: old_tip.reason, + who: old_tip.who, + finder, + deposit, + closes: old_tip.closes, + tips: old_tip.tips, + finders_fee + }; + Tips::::insert(hash, new_tip) + } + } } impl OnUnbalanced> for Module { diff --git a/frame/treasury/src/tests.rs b/frame/treasury/src/tests.rs index 027e52c1bfb76..68820ffd5d2c9 100644 --- a/frame/treasury/src/tests.rs +++ b/frame/treasury/src/tests.rs @@ -293,6 +293,7 @@ fn close_tip_works() { #[test] fn retract_tip_works() { new_test_ext().execute_with(|| { + // with report awesome Balances::make_free_balance_be(&Treasury::account_id(), 101); assert_ok!(Treasury::report_awesome(Origin::signed(0), b"awesome.dot".to_vec(), 3)); let h = tip_hash(); @@ -303,6 +304,17 @@ fn retract_tip_works() { assert_ok!(Treasury::retract_tip(Origin::signed(0), h.clone())); System::set_block_number(2); assert_noop!(Treasury::close_tip(Origin::signed(0), h.into()), Error::::UnknownTip); + + // with tip new + Balances::make_free_balance_be(&Treasury::account_id(), 101); + assert_ok!(Treasury::tip_new(Origin::signed(10), b"awesome.dot".to_vec(), 3, 10)); + let h = tip_hash(); + assert_ok!(Treasury::tip(Origin::signed(11), h.clone(), 10)); + assert_ok!(Treasury::tip(Origin::signed(12), h.clone(), 10)); + assert_noop!(Treasury::retract_tip(Origin::signed(0), h.clone()), Error::::NotFinder); + assert_ok!(Treasury::retract_tip(Origin::signed(10), h.clone())); + System::set_block_number(2); + assert_noop!(Treasury::close_tip(Origin::signed(10), h.into()), Error::::UnknownTip); }); } @@ -544,3 +556,97 @@ fn inexistent_account_works() { assert_eq!(Balances::free_balance(3), 99); // Balance of `3` has changed }); } + +#[test] +fn test_last_reward_migration() { + use sp_storage::Storage; + + let mut s = Storage::default(); + + #[derive(Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug)] + pub struct OldOpenTip< + AccountId: Parameter, + Balance: Parameter, + BlockNumber: Parameter, + Hash: Parameter, + > { + /// The hash of the reason for the tip. The reason should be a human-readable UTF-8 encoded string. A URL would be + /// sensible. + reason: Hash, + /// The account to be tipped. + who: AccountId, + /// The account who began this tip and the amount held on deposit. + finder: Option<(AccountId, Balance)>, + /// The block number at which this tip will close if `Some`. If `None`, then no closing is + /// scheduled. + closes: Option, + /// The members who have voted for this tip. Sorted by AccountId. + tips: Vec<(AccountId, Balance)>, + } + + let reason1 = BlakeTwo256::hash(b"reason1"); + let hash1 = BlakeTwo256::hash_of(&(reason1, 10u64)); + + let old_tip_finder = OldOpenTip:: { + reason: reason1, + who: 10, + finder: Some((20, 30)), + closes: Some(13), + tips: vec![(40, 50), (60, 70)] + }; + + let reason2 = BlakeTwo256::hash(b"reason2"); + let hash2 = BlakeTwo256::hash_of(&(reason2, 20u64)); + + let old_tip_no_finder = OldOpenTip:: { + reason: reason2, + who: 20, + finder: None, + closes: Some(13), + tips: vec![(40, 50), (60, 70)] + }; + + let data = vec![ + ( + Tips::::hashed_key_for(hash1), + old_tip_finder.encode().to_vec() + ), + ( + Tips::::hashed_key_for(hash2), + old_tip_no_finder.encode().to_vec() + ), + ]; + + s.top = data.into_iter().collect(); + sp_io::TestExternalities::new(s).execute_with(|| { + Treasury::migrate_retract_tip_for_tip_new(); + + // Test w/ finder + assert_eq!( + Tips::::get(hash1), + Some(OpenTip { + reason: reason1, + who: 10, + finder: 20, + deposit: 30, + closes: Some(13), + tips: vec![(40, 50), (60, 70)], + finders_fee: true, + }) + ); + + // Test w/o finder + assert_eq!( + Tips::::get(hash2), + Some(OpenTip { + reason: reason2, + who: 20, + finder: Default::default(), + deposit: 0, + closes: Some(13), + tips: vec![(40, 50), (60, 70)], + finders_fee: false, + }) + ); + }); +}