Treasury rewards should pay the remainder of the 10% (#4026)

* Treasury rewards should pay the remainder of the 10%..

* Event for deposits coming into treasury

* Fix tests

* Remove OnDilution

substrate/node/executor/src/lib.rs

@@ -466,13 +466,18 @@ mod tests {
 					event: Event::system(system::Event::ExtrinsicSuccess),
 					topics: vec![],
 				},
+				EventRecord {
+					phase: Phase::ApplyExtrinsic(1),
+					event: Event::treasury(treasury::RawEvent::Deposit(1984800000000)),
+					topics: vec![],
+				},
 				EventRecord {
 					phase: Phase::ApplyExtrinsic(1),
 					event: Event::balances(balances::RawEvent::Transfer(
 						alice().into(),
 						bob().into(),
 						69 * DOLLARS,
-						1 * CENTS
+						1 * CENTS,
 					)),
 					topics: vec![],
 				},
@@ -510,6 +515,11 @@ mod tests {
 					event: Event::system(system::Event::ExtrinsicSuccess),
 					topics: vec![],
 				},
+				EventRecord {
+					phase: Phase::ApplyExtrinsic(1),
+					event: Event::treasury(treasury::RawEvent::Deposit(1984780231392)),
+					topics: vec![],
+				},
 				EventRecord {
 					phase: Phase::ApplyExtrinsic(1),
 					event: Event::balances(
@@ -527,6 +537,11 @@ mod tests {
 					event: Event::system(system::Event::ExtrinsicSuccess),
 					topics: vec![],
 				},
+				EventRecord {
+					phase: Phase::ApplyExtrinsic(2),
+					event: Event::treasury(treasury::RawEvent::Deposit(1984780231392)),
+					topics: vec![],
+				},
 				EventRecord {
 					phase: Phase::ApplyExtrinsic(2),
 					event: Event::balances(

substrate/node/runtime/src/lib.rs

@@ -241,7 +241,7 @@ impl session::historical::Trait for Runtime {
 srml_staking_reward_curve::build! {
 	const REWARD_CURVE: PiecewiseLinear<'static> = curve!(
 		min_inflation: 0_025_000,
-		max_inflation: 0_100_000,
+		max_inflation: 0_100_000,   // 10% - must be equal to MaxReward below.
 		ideal_stake: 0_500_000,
 		falloff: 0_050_000,
 		max_piece_count: 40,
@@ -253,13 +253,15 @@ parameter_types! {
 	pub const SessionsPerEra: sr_staking_primitives::SessionIndex = 6;
 	pub const BondingDuration: staking::EraIndex = 24 * 28;
 	pub const RewardCurve: &'static PiecewiseLinear<'static> = &REWARD_CURVE;
+	pub const MaxReward: Perbill = Perbill::from_percent(10);
+	// ^^^ 10% - must be equal to max_inflation, above.
 }
 
 impl staking::Trait for Runtime {
 	type Currency = Balances;
 	type Time = Timestamp;
 	type CurrencyToVote = CurrencyToVoteHandler;
-	type OnRewardMinted = Treasury;
+	type RewardRemainder = Treasury;
 	type Event = Event;
 	type Slash = Treasury; // send the slashed funds to the treasury.
 	type Reward = (); // rewards are minted from the void
@@ -267,6 +269,7 @@ impl staking::Trait for Runtime {
 	type BondingDuration = BondingDuration;
 	type SessionInterface = Self;
 	type RewardCurve = RewardCurve;
+	type MaxPossibleReward = MaxReward;
 }
 
 parameter_types! {

substrate/srml/staking/src/lib.rs

@@ -256,7 +256,7 @@ use codec::{HasCompact, Encode, Decode};
 use support::{
 	decl_module, decl_event, decl_storage, ensure,
 	traits::{
-		Currency, OnFreeBalanceZero, OnDilution, LockIdentifier, LockableCurrency,
+		Currency, OnFreeBalanceZero, LockIdentifier, LockableCurrency,
 		WithdrawReasons, OnUnbalanced, Imbalance, Get, Time
 	}
 };
@@ -501,8 +501,8 @@ pub trait Trait: system::Trait {
 	/// The post-processing needs it but will be moved to off-chain. TODO: #2908
 	type CurrencyToVote: Convert<BalanceOf<Self>, u64> + Convert<u128, BalanceOf<Self>>;
 
-	/// Some tokens minted.
-	type OnRewardMinted: OnDilution<BalanceOf<Self>>;
+	/// Tokens have been minted and are unused for validator-reward.
+	type RewardRemainder: OnUnbalanced<NegativeImbalanceOf<Self>>;
 
 	/// The overarching event type.
 	type Event: From<Event<Self>> + Into<<Self as system::Trait>::Event>;
@@ -524,6 +524,10 @@ pub trait Trait: system::Trait {
 
 	/// The NPoS reward curve to use.
 	type RewardCurve: Get<&'static PiecewiseLinear<'static>>;
+
+	/// The maximum possible reward (in proportion of total issued tokens) that can be paid in one
+	/// reward cycle.
+	type MaxPossibleReward: Get<Perbill>;
 }
 
 /// Mode of era-forcing.
@@ -652,8 +656,9 @@ decl_storage! {
 
 decl_event!(
 	pub enum Event<T> where Balance = BalanceOf<T>, <T as system::Trait>::AccountId {
-		/// All validators have been rewarded by the given balance.
-		Reward(Balance),
+		/// All validators have been rewarded by the first balance; the second is the remainder
+		/// from the maximum amount of reward.
+		Reward(Balance, Balance),
 		/// One validator (and its nominators) has been slashed by the given amount.
 		Slash(AccountId, Balance),
 		/// An old slashing report from a prior era was discarded because it could
@@ -1218,9 +1223,13 @@ impl<T: Trait> Module<T> {
 			let total_reward = total_imbalance.peek();
 			// assert!(total_reward <= total_payout)
 
-			Self::deposit_event(RawEvent::Reward(total_reward));
+			let max_reward = T::MaxPossibleReward::get() * T::Currency::total_issuance();
+			let rest_reward = max_reward.saturating_sub(total_reward);
+
+			Self::deposit_event(RawEvent::Reward(total_reward, rest_reward));
+
 			T::Reward::on_unbalanced(total_imbalance);
-			T::OnRewardMinted::on_dilution(total_reward, total_rewarded_stake);
+			T::RewardRemainder::on_unbalanced(T::Currency::issue(rest_reward));
 		}
 
 		// Increment current era.

substrate/srml/staking/src/mock.rs

@@ -192,12 +192,13 @@ parameter_types! {
 	pub const SessionsPerEra: SessionIndex = 3;
 	pub const BondingDuration: EraIndex = 3;
 	pub const RewardCurve: &'static PiecewiseLinear<'static> = &I_NPOS;
+	pub const MaxReward: Perbill = Perbill::from_percent(10);
 }
 impl Trait for Test {
 	type Currency = balances::Module<Self>;
 	type Time = timestamp::Module<Self>;
 	type CurrencyToVote = CurrencyToVoteHandler;
-	type OnRewardMinted = ();
+	type RewardRemainder = ();
 	type Event = ();
 	type Slash = ();
 	type Reward = ();
@@ -205,6 +206,7 @@ impl Trait for Test {
 	type BondingDuration = BondingDuration;
 	type SessionInterface = Self;
 	type RewardCurve = RewardCurve;
+	type MaxPossibleReward = MaxReward;
 }
 
 pub struct ExtBuilder {

substrate/srml/support/src/traits.rs

@@ -69,17 +69,6 @@ pub trait OnFreeBalanceZero<AccountId> {
 	fn on_free_balance_zero(who: &AccountId);
 }
 
-/// Trait for a hook to get called when some balance has been minted, causing dilution.
-pub trait OnDilution<Balance> {
-	/// Some `portion` of the total balance just "grew" by `minted`. `portion` is the pre-growth
-	/// amount (it doesn't take account of the recent growth).
-	fn on_dilution(minted: Balance, portion: Balance);
-}
-
-impl<Balance> OnDilution<Balance> for () {
-	fn on_dilution(_minted: Balance, _portion: Balance) {}
-}
-
 /// Outcome of a balance update.
 pub enum UpdateBalanceOutcome {
 	/// Account balance was simply updated.

substrate/srml/treasury/src/lib.rs

@@ -41,16 +41,6 @@
 //! respectively.
 //! - **Pot:** Unspent funds accumulated by the treasury module.
 //!
-//! ### Implementations
-//!
-//! The treasury module provides an implementation for the following trait:
-//!
-//! - `OnDilution` - When new funds are minted to reward the deployment of other existing funds,
-//! a corresponding amount of tokens are minted into the treasury so that the tokens being rewarded
-//! do not represent a higher portion of total supply. For example, in the default substrate node,
-//! when validators are rewarded new tokens for staking, they do not hold a higher portion of total
-//! tokens. Rather, tokens are added to the treasury to keep the portion of tokens staked constant.
-//!
 //! ## Interface
 //!
 //! ### Dispatchable Functions
@@ -72,12 +62,12 @@ use serde::{Serialize, Deserialize};
 use rstd::prelude::*;
 use support::{decl_module, decl_storage, decl_event, ensure, print};
 use support::traits::{
-	Currency, ExistenceRequirement, Get, Imbalance, OnDilution, OnUnbalanced,
+	Currency, ExistenceRequirement, Get, Imbalance, OnUnbalanced,
 	ReservableCurrency, WithdrawReason
 };
-use sr_primitives::{Permill, Perbill, ModuleId};
+use sr_primitives::{Permill, ModuleId};
 use sr_primitives::traits::{
-	Zero, EnsureOrigin, StaticLookup, AccountIdConversion, CheckedSub, Saturating
+	Zero, EnsureOrigin, StaticLookup, AccountIdConversion, Saturating
 };
 use sr_primitives::weights::SimpleDispatchInfo;
 use codec::{Encode, Decode};
@@ -257,6 +247,8 @@ decl_event!(
 		Burnt(Balance),
 		/// Spending has finished; this is the amount that rolls over until next spend.
 		Rollover(Balance),
+		/// Some funds have been deposited.
+		Deposit(Balance),
 	}
 );
 
@@ -346,29 +338,12 @@ impl<T: Trait> Module<T> {
 
 impl<T: Trait> OnUnbalanced<NegativeImbalanceOf<T>> for Module<T> {
 	fn on_unbalanced(amount: NegativeImbalanceOf<T>) {
+		let numeric_amount = amount.peek();
+
 		// Must resolve into existing but better to be safe.
 		let _ = T::Currency::resolve_creating(&Self::account_id(), amount);
-	}
-}
 
-/// Mint extra funds for the treasury to keep the ratio of portion to total_issuance equal
-/// pre dilution and post-dilution.
-///
-/// i.e.
-/// ```nocompile
-/// portion / total_issuance_before_dilution ==
-///   (portion + minted) / (total_issuance_before_dilution + minted_to_treasury + minted)
-/// ```
-impl<T: Trait> OnDilution<BalanceOf<T>> for Module<T> {
-	fn on_dilution(minted: BalanceOf<T>, portion: BalanceOf<T>) {
-		if !minted.is_zero() && !portion.is_zero() {
-			let total_issuance = T::Currency::total_issuance();
-			if let Some(funding) = total_issuance.checked_sub(&portion) {
-				let increase_ratio = Perbill::from_rational_approximation(minted, portion);
-				let funding = increase_ratio * funding;
-				Self::on_unbalanced(T::Currency::issue(funding));
-			}
-		}
+		Self::deposit_event(RawEvent::Deposit(numeric_amount));
 	}
 }
 
@@ -379,7 +354,7 @@ mod tests {
 	use support::{assert_noop, assert_ok, impl_outer_origin, parameter_types};
 	use primitives::H256;
 	use sr_primitives::{
-		traits::{BlakeTwo256, OnFinalize, IdentityLookup}, testing::Header, assert_eq_error_rate,
+		traits::{BlakeTwo256, OnFinalize, IdentityLookup}, testing::Header, Perbill
 	};
 
 	impl_outer_origin! {
@@ -470,37 +445,11 @@ mod tests {
 	fn minting_works() {
 		new_test_ext().execute_with(|| {
 			// Check that accumulate works when we have Some value in Dummy already.
-			Treasury::on_dilution(100, 100);
+			Balances::make_free_balance_be(&Treasury::account_id(), 101);
 			assert_eq!(Treasury::pot(), 100);
 		});
 	}
 
-	#[test]
-	fn minting_works_2() {
-		let tests = [(1, 10), (1, 20), (40, 130), (2, 66), (2, 67), (2, 100), (2, 101), (2, 134)];
-		for &(minted, portion) in &tests {
-			new_test_ext().execute_with(|| {
-				let init_total_issuance = Balances::total_issuance();
-				Treasury::on_dilution(minted, portion);
-
-				assert_eq!(
-					Treasury::pot(),
-					(((init_total_issuance - portion) * minted) as f32 / portion as f32)
-						.round() as u64
-				);
-
-				// Assert:
-				// portion / init_total_issuance
-				// == (portion + minted) / (init_total_issuance + Treasury::pot() + minted),
-				assert_eq_error_rate!(
-					portion * 1_000 / init_total_issuance,
-					(portion + minted) * 1_000 / (init_total_issuance + Treasury::pot() + minted),
-					2,
-				);
-			});
-		}
-	}
-
 	#[test]
 	fn spend_proposal_takes_min_deposit() {
 		new_test_ext().execute_with(|| {
@@ -529,7 +478,7 @@ mod tests {
 	#[test]
 	fn accepted_spend_proposal_ignored_outside_spend_period() {
 		new_test_ext().execute_with(|| {
-			Treasury::on_dilution(100, 100);
+			Balances::make_free_balance_be(&Treasury::account_id(), 101);
 
 			assert_ok!(Treasury::propose_spend(Origin::signed(0), 100, 3));
 			assert_ok!(Treasury::approve_proposal(Origin::ROOT, 0));
@@ -544,7 +493,7 @@ mod tests {
 	fn unused_pot_should_diminish() {
 		new_test_ext().execute_with(|| {
 			let init_total_issuance = Balances::total_issuance();
-			Treasury::on_dilution(100, 100);
+			Balances::make_free_balance_be(&Treasury::account_id(), 101);
 			assert_eq!(Balances::total_issuance(), init_total_issuance + 100);
 
 			<Treasury as OnFinalize<u64>>::on_finalize(2);
@@ -556,7 +505,7 @@ mod tests {
 	#[test]
 	fn rejected_spend_proposal_ignored_on_spend_period() {
 		new_test_ext().execute_with(|| {
-			Treasury::on_dilution(100, 100);
+			Balances::make_free_balance_be(&Treasury::account_id(), 101);
 
 			assert_ok!(Treasury::propose_spend(Origin::signed(0), 100, 3));
 			assert_ok!(Treasury::reject_proposal(Origin::ROOT, 0));
@@ -570,7 +519,7 @@ mod tests {
 	#[test]
 	fn reject_already_rejected_spend_proposal_fails() {
 		new_test_ext().execute_with(|| {
-			Treasury::on_dilution(100, 100);
+			Balances::make_free_balance_be(&Treasury::account_id(), 101);
 
 			assert_ok!(Treasury::propose_spend(Origin::signed(0), 100, 3));
 			assert_ok!(Treasury::reject_proposal(Origin::ROOT, 0));
@@ -595,7 +544,7 @@ mod tests {
 	#[test]
 	fn accept_already_rejected_spend_proposal_fails() {
 		new_test_ext().execute_with(|| {
-			Treasury::on_dilution(100, 100);
+			Balances::make_free_balance_be(&Treasury::account_id(), 101);
 
 			assert_ok!(Treasury::propose_spend(Origin::signed(0), 100, 3));
 			assert_ok!(Treasury::reject_proposal(Origin::ROOT, 0));
@@ -606,7 +555,7 @@ mod tests {
 	#[test]
 	fn accepted_spend_proposal_enacted_on_spend_period() {
 		new_test_ext().execute_with(|| {
-			Treasury::on_dilution(100, 100);
+			Balances::make_free_balance_be(&Treasury::account_id(), 101);
 			assert_eq!(Treasury::pot(), 100);
 
 			assert_ok!(Treasury::propose_spend(Origin::signed(0), 100, 3));
@@ -621,7 +570,7 @@ mod tests {
 	#[test]
 	fn pot_underflow_should_not_diminish() {
 		new_test_ext().execute_with(|| {
-			Treasury::on_dilution(100, 100);
+			Balances::make_free_balance_be(&Treasury::account_id(), 101);
 			assert_eq!(Treasury::pot(), 100);
 
 			assert_ok!(Treasury::propose_spend(Origin::signed(0), 150, 3));
@@ -630,10 +579,10 @@ mod tests {
 			<Treasury as OnFinalize<u64>>::on_finalize(2);
 			assert_eq!(Treasury::pot(), 100); // Pot hasn't changed
 
-			Treasury::on_dilution(100, 100);
+			Balances::deposit_into_existing(&Treasury::account_id(), 100);
 			<Treasury as OnFinalize<u64>>::on_finalize(4);
 			assert_eq!(Balances::free_balance(&3), 150); // Fund has been spent
-			assert_eq!(Treasury::pot(), 75); // Pot has finally changed
+			assert_eq!(Treasury::pot(), 25); // Pot has finally changed
 		});
 	}
 
@@ -642,7 +591,7 @@ mod tests {
 	#[test]
 	fn treasury_account_doesnt_get_deleted() {
 		new_test_ext().execute_with(|| {
-			Treasury::on_dilution(100, 100);
+			Balances::make_free_balance_be(&Treasury::account_id(), 101);
 			assert_eq!(Treasury::pot(), 100);
 			let treasury_balance = Balances::free_balance(&Treasury::account_id());
 
@@ -685,7 +634,7 @@ mod tests {
 			assert_eq!(Treasury::pot(), 0); // Pot hasn't changed
 			assert_eq!(Balances::free_balance(&3), 0); // Balance of `3` hasn't changed
 
-			Treasury::on_dilution(100, 100);
+			Balances::make_free_balance_be(&Treasury::account_id(), 100);
 			assert_eq!(Treasury::pot(), 99); // Pot now contains funds
 			assert_eq!(Balances::free_balance(&Treasury::account_id()), 100); // Account does exist