Fix quantization from OnDilution in treasury (#3736)

* fix

* bump version

* remove println

node/runtime/src/lib.rs

@@ -84,7 +84,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	// and set impl_version to equal spec_version. If only runtime
 	// implementation changes and behavior does not, then leave spec_version as
 	// is and increment impl_version.
-	spec_version: 168,
+	spec_version: 169,
 	impl_version: 169,
 	apis: RUNTIME_API_VERSIONS,
 };

srml/treasury/src/lib.rs

@@ -75,9 +75,9 @@ use support::traits::{
 	Currency, ExistenceRequirement, Get, Imbalance, OnDilution, OnUnbalanced,
 	ReservableCurrency, WithdrawReason
 };
-use sr_primitives::{Permill, ModuleId};
+use sr_primitives::{Permill, Perbill, ModuleId};
 use sr_primitives::traits::{
-	Zero, EnsureOrigin, StaticLookup, CheckedSub, CheckedMul, AccountIdConversion
+	Zero, EnsureOrigin, StaticLookup, AccountIdConversion, CheckedSub
 };
 use sr_primitives::weights::SimpleDispatchInfo;
 use codec::{Encode, Decode};
@@ -343,10 +343,9 @@ impl<T: Trait> OnDilution<BalanceOf<T>> for Module<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 funding = funding / portion;
-				if let Some(funding) = funding.checked_mul(&minted) {
-					Self::on_unbalanced(T::Currency::issue(funding));
-				}
+				let increase_ratio = Perbill::from_rational_approximation(minted, portion);
+				let funding = increase_ratio * funding;
+				Self::on_unbalanced(T::Currency::issue(funding));
 			}
 		}
 	}
@@ -359,7 +358,11 @@ mod tests {
 	use runtime_io::with_externalities;
 	use support::{assert_noop, assert_ok, impl_outer_origin, parameter_types};
 	use primitives::{H256, Blake2Hasher};
-	use sr_primitives::{Perbill, traits::{BlakeTwo256, OnFinalize, IdentityLookup}, testing::Header};
+	use sr_primitives::{
+		traits::{BlakeTwo256, OnFinalize, IdentityLookup},
+		testing::Header,
+		assert_eq_error_rate,
+	};
 
 	impl_outer_origin! {
 		pub enum Origin for Test {}
@@ -460,6 +463,32 @@ mod tests {
 		});
 	}
 
+	#[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 {
+			with_externalities(&mut new_test_ext(), || {
+				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() {
 		with_externalities(&mut new_test_ext(), || {
@@ -577,37 +606,6 @@ mod tests {
 		});
 	}
 
-	#[test]
-	// Note: This test demonstrates that `on_dilution` does not increase the pot with good resolution
-	// with large amounts of the network staked. https://github.com/paritytech/substrate/issues/2579
-	// A fix to 2579 should include a change of this test.
-	fn on_dilution_quantization_effects() {
-		with_externalities(&mut new_test_ext(), || {
-			// minted = 1% of total issuance for all cases
-			assert_eq!(Balances::total_issuance(), 200);
-
-			Treasury::on_dilution(2, 66);   // portion = 33% of total issuance
-			assert_eq!(Treasury::pot(), 4); // should increase by 4 (200 - 66) / 66 * 2
-			Balances::make_free_balance_be(&Treasury::account_id(), 0);
-
-			Treasury::on_dilution(2, 67);   // portion = 33+eps% of total issuance
-			assert_eq!(Treasury::pot(), 2); // should increase by 2 (200 - 67) / 67 * 2
-			Balances::make_free_balance_be(&Treasury::account_id(), 0);
-
-			Treasury::on_dilution(2, 100);  // portion = 50% of total issuance
-			assert_eq!(Treasury::pot(), 2); // should increase by 2 (200 - 100) / 100 * 2
-			Balances::make_free_balance_be(&Treasury::account_id(), 0);
-
-			// If any more than 50% of the network is staked (i.e. (2 * portion) > total_issuance)
-			// then the pot will not increase.
-			Treasury::on_dilution(2, 101);  // portion = 50+eps% of total issuance
-			assert_eq!(Treasury::pot(), 0); // should increase by 0 (200 - 101) / 101 * 2
-
-			Treasury::on_dilution(2, 134);  // portion = 67% of total issuance
-			assert_eq!(Treasury::pot(), 0); // should increase by 0 (200 - 134) / 134 * 2
-		});
-	}
-
 	#[test]
 	fn pot_underflow_should_not_diminish() {
 		with_externalities(&mut new_test_ext(), || {