From 1ed55f7b9d35ee32a3a711bddb89f0c32c25feb8 Mon Sep 17 00:00:00 2001 From: peg Date: Mon, 4 Mar 2024 12:19:00 +0100 Subject: [PATCH] Disallow using existing TSS account IDs in Staking pallet (#657) * Add check to staking pallet to disallow using existing TSS account IDs * Update CHANGELOG * Shorter description in changelog Co-authored-by: Hernando Castano * Move ensure assertion to beginning of fn * Move other ensure assertion to beginning of fn * Update / refactor tests --------- Co-authored-by: Hernando Castano --- CHANGELOG.md | 1 + pallets/staking/src/lib.rs | 12 +++++ pallets/staking/src/tests.rs | 93 +++++++++++++++++++++++++++++++++++- 3 files changed, 105 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c66ea171..ddacb94b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ At the moment this project **does not** adhere to - Change test-cli default access mode and update readme for recent changes ([#643](https://github.com/entropyxyz/entropy-core/pull/643)) - Test CLI - dont send hardcoded auxiliary data by default when signing ([#614](https://github.com/entropyxyz/entropy-core/pull/614)) - Add proactive refresh keys on-chain ([#629](https://github.com/entropyxyz/entropy-core/pull/629)) +- Disallow using existing TSS account IDs in Staking pallet #657 ([#657](https://github.com/entropyxyz/entropy-core/pull/657)) ## [0.0.10](https://github.com/entropyxyz/entropy-core/compare/release/v0.0.9...release/v0.0.10) - 2024-01-24 diff --git a/pallets/staking/src/lib.rs b/pallets/staking/src/lib.rs index f6c516f71..27c1f6d76 100644 --- a/pallets/staking/src/lib.rs +++ b/pallets/staking/src/lib.rs @@ -222,6 +222,7 @@ pub mod pallet { NoThresholdKey, InvalidValidatorId, SigningGroupError, + TssAccountAlreadyExists, } #[pallet::event] @@ -288,6 +289,11 @@ pub mod pallet { tss_account: T::AccountId, x25519_public_key: X25519PublicKey, ) -> DispatchResult { + ensure!( + !ThresholdToStash::::contains_key(&tss_account), + Error::::TssAccountAlreadyExists + ); + let who = ensure_signed(origin)?; let stash = Self::get_stash(&who)?; let validator_id = ::ValidatorId::try_from(stash) @@ -351,6 +357,12 @@ pub mod pallet { endpoint.len() as u32 <= T::MaxEndpointLength::get(), Error::::EndpointTooLong ); + + ensure!( + !ThresholdToStash::::contains_key(&tss_account), + Error::::TssAccountAlreadyExists + ); + let stash = Self::get_stash(&who)?; pallet_staking::Pallet::::validate(origin, prefs)?; let validator_id = ::ValidatorId::try_from(stash) diff --git a/pallets/staking/src/tests.rs b/pallets/staking/src/tests.rs index d660c475c..40a31c6fd 100644 --- a/pallets/staking/src/tests.rs +++ b/pallets/staking/src/tests.rs @@ -74,7 +74,7 @@ fn it_takes_in_an_endpoint() { RuntimeOrigin::signed(4), pallet_staking::ValidatorPrefs::default(), vec![20, 20], - 3, + 5, NULL_ARR ), Error::::NotController @@ -82,6 +82,41 @@ fn it_takes_in_an_endpoint() { }); } +#[test] +fn it_will_not_allow_validator_to_use_existing_tss_account() { + new_test_ext().execute_with(|| { + assert_ok!(FrameStaking::bond( + RuntimeOrigin::signed(1), + 100u64, + pallet_staking::RewardDestination::Account(1), + )); + assert_ok!(Staking::validate( + RuntimeOrigin::signed(1), + pallet_staking::ValidatorPrefs::default(), + vec![20], + 3, + NULL_ARR + )); + + // Attempt to call validate with a TSS account which already exists + assert_ok!(FrameStaking::bond( + RuntimeOrigin::signed(2), + 100u64, + pallet_staking::RewardDestination::Account(2), + )); + assert_noop!( + Staking::validate( + RuntimeOrigin::signed(2), + pallet_staking::ValidatorPrefs::default(), + vec![20], + 3, + NULL_ARR + ), + Error::::TssAccountAlreadyExists + ); + }); +} + #[test] fn it_changes_endpoint() { new_test_ext().execute_with(|| { @@ -132,6 +167,62 @@ fn it_changes_threshold_account() { Staking::change_threshold_accounts(RuntimeOrigin::signed(4), 5, NULL_ARR), Error::::NotController ); + + // Check that we cannot change to a TSS account which already exists + assert_ok!(FrameStaking::bond( + RuntimeOrigin::signed(2), + 100u64, + pallet_staking::RewardDestination::Account(2), + )); + assert_ok!(Staking::validate( + RuntimeOrigin::signed(2), + pallet_staking::ValidatorPrefs::default(), + vec![20], + 5, + NULL_ARR + )); + + assert_noop!( + Staking::change_threshold_accounts(RuntimeOrigin::signed(1), 5, NULL_ARR), + Error::::TssAccountAlreadyExists + ); + }); +} + +#[test] +fn it_will_not_allow_existing_tss_account_when_changing_threshold_account() { + new_test_ext().execute_with(|| { + assert_ok!(FrameStaking::bond( + RuntimeOrigin::signed(1), + 100u64, + pallet_staking::RewardDestination::Account(1), + )); + assert_ok!(Staking::validate( + RuntimeOrigin::signed(1), + pallet_staking::ValidatorPrefs::default(), + vec![20], + 3, + NULL_ARR + )); + + // Check that we cannot change to a TSS account which already exists + assert_ok!(FrameStaking::bond( + RuntimeOrigin::signed(2), + 100u64, + pallet_staking::RewardDestination::Account(2), + )); + assert_ok!(Staking::validate( + RuntimeOrigin::signed(2), + pallet_staking::ValidatorPrefs::default(), + vec![20], + 5, + NULL_ARR + )); + + assert_noop!( + Staking::change_threshold_accounts(RuntimeOrigin::signed(1), 5, NULL_ARR), + Error::::TssAccountAlreadyExists + ); }); }