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

@@ -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<T: Trait> Module<T> {
 
 		let new_total = Self::total().checked_sub(&balance_due).ok_or(Error::<T>::PotUnderflow)?;
 
-		// Check if this claim should have a vesting schedule.
+		let vesting = Vesting::<T>::get(&signer);
-		if let Some(vs) = <Vesting<T>>::get(&signer) {
+		if vesting.is_some() && T::VestingSchedule::vesting_balance(&dest).is_some() {
-			// If this fails, destination account already has a vesting schedule
+			return Err(Error::<T>::VestedBalanceExists.into())
-			// 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::<T>::DestinationVesting)?;
 		}
 
+		// We first need to deposit the balance to ensure that the account exists.
 		CurrencyOf::<T>::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");
+		}
+
 		<Total<T>>::put(new_total);
 		<Claims<T>>::remove(&signer);
 		<Vesting<T>>::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::<Test>(&bob(), &69u64.encode(), &[][..])),
-				Error::<Test>::SignerHasNoClaim
+				Error::<Test>::SignerHasNoClaim,
 			);
 			assert_ok!(Claims::mint_claim(Origin::ROOT, eth(&bob()), 200, Some((50, 10, 1)), None));
 			assert_ok!(Claims::claim(Origin::NONE, 69, sig::<Test>(&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!(
+				<Balances as Currency<_>>::transfer(&69, &80, 180, ExistenceRequirement::AllowDeath),
+				balances::Error::<Test, _>::LiquidityRestrictions,
+			);
 		});
 	}
 
@@ -979,7 +992,7 @@ mod tests {
 			// They should not be able to claim
 			assert_noop!(
 				Claims::claim(Origin::NONE, 69, sig::<Test>(&bob(), &69u64.encode(), &[][..])),
-				Error::<Test>::DestinationVesting
+				Error::<Test>::VestedBalanceExists,
 			);
 		});
 	}

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,
+	spec_version: 1065,
-	impl_version: 1,
+	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 1,
 };

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",

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,

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::<Runtime>(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 = WEIGHT_PER_MICROS * 35 + 1 * 375 * WEIGHT_PER_NANOS + DbWeight::get().reads_writes(4, 1);
-	let expected_weight = 500_000_000;
+	let weight = PhragmenCall::submit_candidacy::<Runtime>(1).get_dispatch_info().weight;
-	let weight = PhragmenCall::submit_candidacy::<Runtime>().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 = 46 * WEIGHT_PER_MICROS + DbWeight::get().reads_writes(2, 2);
-	let expected_weight = 2_000_000_000;
+	let weight = PhragmenCall::renounce_candidacy::<Runtime>(elections_phragmen::Renouncing::Member)
-	let weight = PhragmenCall::renounce_candidacy::<Runtime>().get_dispatch_info().weight;
+		.get_dispatch_info().weight;
 
 	assert_eq!(weight, expected_weight);
 }

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,