From 3bfcdeb2502141a5b9016240eb0e5fa22826df66 Mon Sep 17 00:00:00 2001
From: thiolliere <gui.thiolliere@gmail.com>
Date: Wed, 2 Oct 2019 12:36:50 +0200
Subject: [PATCH] Fix quantization from OnDilution in treasury (#3736)
* fix
* bump version
* remove println
---
substrate/node/runtime/src/lib.rs | 2 +-
substrate/srml/treasury/src/lib.rs | 74 +++++++++++++++---------------
2 files changed, 37 insertions(+), 39 deletions(-)
diff --git a/substrate/node/runtime/src/lib.rs b/substrate/node/runtime/src/lib.rs
index 7163d2c7365..f4f71e37f36 100644
--- a/substrate/node/runtime/src/lib.rs
+++ b/substrate/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,
};
diff --git a/substrate/srml/treasury/src/lib.rs b/substrate/srml/treasury/src/lib.rs
index 0fd561fca00..db8b0f34c90 100644
--- a/substrate/srml/treasury/src/lib.rs
+++ b/substrate/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(), || {
--
GitLab