From 94f407807e5735b21bd0ccbe035cf21a8961f367 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Thu, 28 May 2020 00:19:23 +0200 Subject: [PATCH] Make sure that vested claims are not transferable (#1160) * Make sure that vested claims are not transferable We need to deposit the claimed balance to the destination account before setting the locks through the vesting module. Otherwise we loose the locks and the vested claim is directly transferable. * Add comment * Enable missing feature for webbrowser check * Remove unneeded error variant * Increment `spec_version`'s * Fix tests --- polkadot/runtime/common/src/claims.rs | 35 +++++++++++++++------- polkadot/runtime/kusama/src/lib.rs | 4 +-- polkadot/runtime/polkadot/Cargo.toml | 1 + polkadot/runtime/polkadot/src/lib.rs | 2 +- polkadot/runtime/polkadot/tests/weights.rs | 13 ++++---- polkadot/runtime/westend/src/lib.rs | 2 +- 6 files changed, 35 insertions(+), 22 deletions(-) diff --git a/polkadot/runtime/common/src/claims.rs b/polkadot/runtime/common/src/claims.rs index f8c03ff5af..4207f56b63 100644 --- a/polkadot/runtime/common/src/claims.rs +++ b/polkadot/runtime/common/src/claims.rs @@ -144,13 +144,13 @@ decl_error! { SignerHasNoClaim, /// Account ID sending tx has no claim. SenderHasNoClaim, - /// The destination is already vesting and cannot be the target of a further claim. - DestinationVesting, /// There's not enough in the pot to pay out some unvested amount. Generally implies a logic /// error. PotUnderflow, /// A needed statement was not included. InvalidStatement, + /// The account already has a vested balance. + VestedBalanceExists, } } @@ -438,15 +438,22 @@ impl Module { let new_total = Self::total().checked_sub(&balance_due).ok_or(Error::::PotUnderflow)?; - // Check if this claim should have a vesting schedule. - if let Some(vs) = >::get(&signer) { - // If this fails, destination account already has a vesting schedule - // applied to it, and this claim should not be processed. - T::VestingSchedule::add_vesting_schedule(&dest, vs.0, vs.1, vs.2) - .map_err(|_| Error::::DestinationVesting)?; + let vesting = Vesting::::get(&signer); + if vesting.is_some() && T::VestingSchedule::vesting_balance(&dest).is_some() { + return Err(Error::::VestedBalanceExists.into()) } + // We first need to deposit the balance to ensure that the account exists. CurrencyOf::::deposit_creating(&dest, balance_due); + + // Check if this claim should have a vesting schedule. + if let Some(vs) = vesting { + // This can only fail if the account already has a vesting schedule, + // but this is checked above. + T::VestingSchedule::add_vesting_schedule(&dest, vs.0, vs.1, vs.2) + .expect("No other vesting schedule exists, as checked above; qed"); + } + >::put(new_total); >::remove(&signer); >::remove(&signer); @@ -611,7 +618,7 @@ mod tests { use sp_runtime::{Perbill, traits::{BlakeTwo256, IdentityLookup, Identity}, testing::Header}; use frame_support::{ impl_outer_origin, impl_outer_dispatch, assert_ok, assert_err, assert_noop, parameter_types, - weights::{Pays, GetDispatchInfo}, + weights::{Pays, GetDispatchInfo}, traits::ExistenceRequirement, }; use balances; use super::Call as ClaimsCall; @@ -902,12 +909,18 @@ mod tests { assert_eq!(Balances::free_balance(42), 0); assert_noop!( Claims::claim(Origin::NONE, 69, sig::(&bob(), &69u64.encode(), &[][..])), - Error::::SignerHasNoClaim + Error::::SignerHasNoClaim, ); assert_ok!(Claims::mint_claim(Origin::ROOT, eth(&bob()), 200, Some((50, 10, 1)), None)); assert_ok!(Claims::claim(Origin::NONE, 69, sig::(&bob(), &69u64.encode(), &[][..]))); assert_eq!(Balances::free_balance(&69), 200); assert_eq!(Vesting::vesting_balance(&69), Some(50)); + + // Make sure we can not transfer the vested balance. + assert_err!( + >::transfer(&69, &80, 180, ExistenceRequirement::AllowDeath), + balances::Error::::LiquidityRestrictions, + ); }); } @@ -979,7 +992,7 @@ mod tests { // They should not be able to claim assert_noop!( Claims::claim(Origin::NONE, 69, sig::(&bob(), &69u64.encode(), &[][..])), - Error::::DestinationVesting + Error::::VestedBalanceExists, ); }); } diff --git a/polkadot/runtime/kusama/src/lib.rs b/polkadot/runtime/kusama/src/lib.rs index 28be46c111..be040fe842 100644 --- a/polkadot/runtime/kusama/src/lib.rs +++ b/polkadot/runtime/kusama/src/lib.rs @@ -84,8 +84,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("kusama"), impl_name: create_runtime_str!("parity-kusama"), authoring_version: 2, - spec_version: 1064, - impl_version: 1, + spec_version: 1065, + impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1, }; diff --git a/polkadot/runtime/polkadot/Cargo.toml b/polkadot/runtime/polkadot/Cargo.toml index 32b0ece09e..ea496e55b2 100644 --- a/polkadot/runtime/polkadot/Cargo.toml +++ b/polkadot/runtime/polkadot/Cargo.toml @@ -136,6 +136,7 @@ std = [ "runtime-common/std", "sudo/std", "vesting/std", + "utility/std", ] runtime-benchmarks = [ "runtime-common/runtime-benchmarks", diff --git a/polkadot/runtime/polkadot/src/lib.rs b/polkadot/runtime/polkadot/src/lib.rs index adc189d149..8c0f84314c 100644 --- a/polkadot/runtime/polkadot/src/lib.rs +++ b/polkadot/runtime/polkadot/src/lib.rs @@ -88,7 +88,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("polkadot"), impl_name: create_runtime_str!("parity-polkadot"), authoring_version: 0, - spec_version: 0, + spec_version: 1, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 0, diff --git a/polkadot/runtime/polkadot/tests/weights.rs b/polkadot/runtime/polkadot/tests/weights.rs index 6bfe1dced4..533783a4e4 100644 --- a/polkadot/runtime/polkadot/tests/weights.rs +++ b/polkadot/runtime/polkadot/tests/weights.rs @@ -219,7 +219,7 @@ fn weight_of_democracy_enact_proposal_is_correct() { #[test] fn weight_of_phragmen_vote_is_correct() { // #[weight = 100_000_000] - let expected_weight = 100_000_000; + let expected_weight = 350_000_000; let weight = PhragmenCall::vote::(Default::default(), Default::default()).get_dispatch_info().weight; assert_eq!(weight, expected_weight); @@ -227,18 +227,17 @@ fn weight_of_phragmen_vote_is_correct() { #[test] fn weight_of_phragmen_submit_candidacy_is_correct() { - // #[weight = 500_000_000] - let expected_weight = 500_000_000; - let weight = PhragmenCall::submit_candidacy::().get_dispatch_info().weight; + let expected_weight = WEIGHT_PER_MICROS * 35 + 1 * 375 * WEIGHT_PER_NANOS + DbWeight::get().reads_writes(4, 1); + let weight = PhragmenCall::submit_candidacy::(1).get_dispatch_info().weight; assert_eq!(weight, expected_weight); } #[test] fn weight_of_phragmen_renounce_candidacy_is_correct() { - // #[weight = (2_000_000_000, DispatchClass::Operational)] - let expected_weight = 2_000_000_000; - let weight = PhragmenCall::renounce_candidacy::().get_dispatch_info().weight; + let expected_weight = 46 * WEIGHT_PER_MICROS + DbWeight::get().reads_writes(2, 2); + let weight = PhragmenCall::renounce_candidacy::(elections_phragmen::Renouncing::Member) + .get_dispatch_info().weight; assert_eq!(weight, expected_weight); } diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index c78bce2742..cb766cf526 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -82,7 +82,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("westend"), impl_name: create_runtime_str!("parity-westend"), authoring_version: 2, - spec_version: 10, + spec_version: 11, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1,