Check current signers still validators (#1097)

* Check current signers still validators * update * tests * end of day push * working reshare * clean * remove mutable * test * update * threhsold limit * fix test * remove old signer * clean * metadata * fmt * test * update benchmarks * working benchmarks * fix benches * Update crates/shared/src/types.rs Co-authored-by: peg <[email protected]> * correct truncation * remove hash comparison * Apply suggestions from code review Co-authored-by: Hernando Castano <[email protected]> * fix * fix * fix benchmark * fix benchmark * fmt * fix * comment * Update crates/threshold-signature-server/src/validator/api.rs Co-authored-by: David <[email protected]> * fix type --------- Co-authored-by: peg <[email protected]> Co-authored-by: Hernando Castano <[email protected]> Co-authored-by: David <[email protected]>
entropyxyz · Oct 24, 2024 · 9eab023 · 9eab023
1 parent 75ae3d1
commit 9eab023
Show file tree

Hide file tree

Showing 16 changed files with 301 additions and 203 deletions.
diff --git a/crates/client/entropy_metadata.scale b/crates/client/entropy_metadata.scale
diff --git a/crates/protocol/src/execute_protocol.rs b/crates/protocol/src/execute_protocol.rs
@@ -334,6 +334,7 @@ pub async fn execute_reshare(
     chans: Channels,
     threshold_pair: &sr25519::Pair,
     inputs: KeyResharingInputs<KeyParams, PartyId>,
+    verifiers: &BTreeSet<PartyId>,
     aux_info_option: Option<AuxInfo<KeyParams, PartyId>>,
 ) -> Result<
     (ThresholdKeyShare<KeyParams, PartyId>, AuxInfo<KeyParams, PartyId>),
@@ -350,7 +351,7 @@ pub async fn execute_reshare(
         &mut OsRng,
         SynedrionSessionId::from_seed(session_id_hash.as_slice()),
         pair,
-        &inputs.new_holders,
+        verifiers,
         inputs.clone(),
     )
     .map_err(ProtocolExecutionErr::SessionCreation)?;

diff --git a/crates/protocol/tests/helpers/mod.rs b/crates/protocol/tests/helpers/mod.rs
@@ -144,7 +144,8 @@ pub async fn server(
                 new_threshold: old_key.threshold(),
             };
 
-            let new_keyshare = execute_reshare(session_id, channels, &pair, inputs, None).await?;
+            let new_keyshare =
+                execute_reshare(session_id, channels, &pair, inputs, &party_ids, None).await?;
             Ok(ProtocolOutput::Reshare(new_keyshare.0))
         },
         SessionId::Dkg { .. } => {

diff --git a/crates/shared/src/types.rs b/crates/shared/src/types.rs
@@ -37,8 +37,8 @@ pub type BlockNumber = u32;
 #[derive(Clone, Encode, Decode, Debug, Eq, PartialEq, TypeInfo)]
 pub struct ValidatorInfo {
     pub x25519_public_key: X25519PublicKey,
-    pub ip_address: codec::alloc::vec::Vec<u8>,
-    pub tss_account: codec::alloc::vec::Vec<u8>,
+    pub ip_address: Vec<u8>,
+    pub tss_account: Vec<u8>,
 }
 
 /// Offchain worker message for initiating the initial jumpstart DKG
@@ -55,8 +55,8 @@ pub struct OcwMessageDkg {
 #[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
 #[derive(Clone, Encode, Decode, Debug, Eq, PartialEq, TypeInfo)]
 pub struct OcwMessageReshare {
-    // Stash address of new signer
-    pub new_signer: Vec<u8>,
+    // Stash addresses of new signers
+    pub new_signers: Vec<Vec<u8>>,
     pub block_number: BlockNumber,
 }
 

diff --git a/crates/threshold-signature-server/src/signing_client/api.rs b/crates/threshold-signature-server/src/signing_client/api.rs
@@ -205,7 +205,8 @@ pub async fn do_proactive_refresh(
     .await?;
 
     let result =
-        execute_reshare(session_id, channels, signer.signer(), inputs, Some(aux_info)).await?;
+        execute_reshare(session_id, channels, signer.signer(), inputs, &party_ids, Some(aux_info))
+            .await?;
     Ok(result)
 }
 

diff --git a/crates/threshold-signature-server/src/validator/api.rs b/crates/threshold-signature-server/src/validator/api.rs
@@ -106,7 +106,7 @@ pub async fn new_reshare(
         .map_err(|e| ValidatorErr::UserError(e.to_string()))?;
 
     let old_holder: Option<OldHolder<KeyParams, PartyId>> =
-        if data.new_signer == my_stash_address.encode() {
+        if data.new_signers.contains(&my_stash_address.encode()) {
             None
         } else {
             let kvdb_result = app_state.kv_store.kv().get(&hex::encode(NETWORK_PARENT_KEY)).await?;
@@ -116,14 +116,15 @@ pub async fn new_reshare(
             Some(OldHolder { key_share: key_share.0 })
         };
 
-    let party_ids: BTreeSet<PartyId> =
+    // new_holders -> From chain next_signers (old_holders (currently forced to be t) + new_holders)
+    // also acts as verifiers as is everyone in the party
+    let new_holders: BTreeSet<PartyId> =
         validators_info.iter().cloned().map(|x| PartyId::new(x.tss_account)).collect();
-
-    let pruned_old_holders =
-        prune_old_holders(&api, &rpc, data.new_signer, validators_info.clone()).await?;
-
+    // old holders -> next_signers - new_signers (will be at least t)
+    let old_holders =
+        &prune_old_holders(&api, &rpc, data.new_signers, validators_info.clone()).await?;
     let old_holders: BTreeSet<PartyId> =
-        pruned_old_holders.into_iter().map(|x| PartyId::new(x.tss_account)).collect();
+        old_holders.iter().map(|x| PartyId::new(x.tss_account.clone())).collect();
 
     let new_holder = NewHolder {
         verifying_key: decoded_verifying_key,
@@ -139,7 +140,7 @@ pub async fn new_reshare(
     let inputs = KeyResharingInputs {
         old_holder,
         new_holder: Some(new_holder),
-        new_holders: party_ids.clone(),
+        new_holders: new_holders.clone(),
         new_threshold: threshold as usize,
     };
 
@@ -157,7 +158,6 @@ pub async fn new_reshare(
         converted_validator_info.push(validator_info.clone());
         tss_accounts.push(validator_info.tss_account.clone());
     }
-
     let channels = get_channels(
         &app_state.listener_state,
         converted_validator_info,
@@ -169,7 +169,8 @@ pub async fn new_reshare(
     .await?;
 
     let (new_key_share, aux_info) =
-        execute_reshare(session_id.clone(), channels, signer.signer(), inputs, None).await?;
+        execute_reshare(session_id.clone(), channels, signer.signer(), inputs, &new_holders, None)
+            .await?;
 
     let serialized_key_share = key_serialize(&(new_key_share, aux_info))
         .map_err(|_| ProtocolErr::KvSerialize("Kv Serialize Error".to_string()))?;
@@ -273,8 +274,8 @@ pub async fn validate_new_reshare(
         .await?
         .ok_or_else(|| ValidatorErr::ChainFetch("Not Currently in a reshare"))?;
 
-    if reshare_data.new_signer != chain_data.new_signer
-        || chain_data.block_number != reshare_data.block_number
+    if chain_data.block_number != reshare_data.block_number.saturating_sub(1)
+        || chain_data.new_signers != reshare_data.new_signers
     {
         return Err(ValidatorErr::InvalidData);
     }
@@ -365,20 +366,24 @@ pub fn check_forbidden_key(key: &str) -> Result<(), ValidatorErr> {
 pub async fn prune_old_holders(
     api: &OnlineClient<EntropyConfig>,
     rpc: &LegacyRpcMethods<EntropyConfig>,
-    new_signer: Vec<u8>,
+    new_signers: Vec<Vec<u8>>,
     validators_info: Vec<ValidatorInfo>,
 ) -> Result<Vec<ValidatorInfo>, ValidatorErr> {
-    Ok(if !new_signer.is_empty() {
-        let address_slice: &[u8; 32] = &new_signer.clone().try_into().unwrap();
-        let new_signer_address = AccountId32(*address_slice);
-        let new_signer_info = &get_validators_info(api, rpc, vec![new_signer_address])
-            .await
-            .map_err(|e| ValidatorErr::UserError(e.to_string()))?[0];
-        validators_info
-            .iter()
-            .filter(|x| x.tss_account != new_signer_info.tss_account)
-            .cloned()
-            .collect()
+    Ok(if !new_signers.is_empty() {
+        let mut filtered_validators_info = vec![];
+        for new_signer in new_signers {
+            let address_slice: &[u8; 32] = &new_signer.clone().try_into().unwrap();
+            let new_signer_address = AccountId32(*address_slice);
+            let new_signer_info = &get_validators_info(api, rpc, vec![new_signer_address])
+                .await
+                .map_err(|e| ValidatorErr::UserError(e.to_string()))?[0];
+            filtered_validators_info = validators_info
+                .iter()
+                .filter(|x| x.tss_account != new_signer_info.tss_account)
+                .cloned()
+                .collect::<Vec<_>>();
+        }
+        filtered_validators_info
     } else {
         validators_info.clone()
     })

diff --git a/crates/threshold-signature-server/src/validator/tests.rs b/crates/threshold-signature-server/src/validator/tests.rs
@@ -76,7 +76,7 @@ async fn test_reshare() {
     let (_validator_ips, _validator_ids) =
         spawn_testing_validators(ChainSpecType::Integration).await;
 
-    let validator_ports = vec![3001, 3002, 3003, 3004];
+    let validator_ports = vec![3002, 3003, 3004];
     let api = get_api(&cxt.ws_url).await.unwrap();
     let rpc = get_rpc(&cxt.ws_url).await.unwrap();
 
@@ -113,13 +113,15 @@ async fn test_reshare() {
     let new_signer = all_validators.iter().find(|v| !signer_stash_accounts.contains(v)).unwrap();
 
     let block_number = TEST_RESHARE_BLOCK_NUMBER;
-    let onchain_reshare_request =
-        OcwMessageReshare { new_signer: new_signer.0.to_vec(), block_number };
+    let onchain_reshare_request = OcwMessageReshare {
+        new_signers: vec![new_signer.0.to_vec()],
+        block_number: block_number - 1,
+    };
 
-    run_to_block(&rpc, block_number + 1).await;
+    run_to_block(&rpc, block_number).await;
     // Send the OCW message to all TS servers who don't have a chain node
     let response_results = join_all(
-        validator_ports[1..]
+        validator_ports
             .iter()
             .map(|port| {
                 client
@@ -323,7 +325,8 @@ async fn test_reshare_validation_fail() {
     let kv = setup_client().await;
 
     let block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number + 1;
-    let mut ocw_message = OcwMessageReshare { new_signer: dave.public().encode(), block_number };
+    let mut ocw_message =
+        OcwMessageReshare { new_signers: vec![dave.public().encode()], block_number };
 
     let err_stale_data =
         validate_new_reshare(&api, &rpc, &ocw_message, &kv).await.map_err(|e| e.to_string());
@@ -361,7 +364,8 @@ async fn test_reshare_validation_fail_not_in_reshare() {
     let kv = setup_client().await;
 
     let block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number + 1;
-    let ocw_message = OcwMessageReshare { new_signer: alice.public().encode(), block_number };
+    let ocw_message =
+        OcwMessageReshare { new_signers: vec![alice.public().encode()], block_number };
 
     run_to_block(&rpc, block_number + 1).await;
 

diff --git a/crates/threshold-signature-server/tests/register_sign_reshare_sign.rs b/crates/threshold-signature-server/tests/register_sign_reshare_sign.rs
@@ -153,19 +153,16 @@ async fn do_reshare(api: &OnlineClient<EntropyConfig>, rpc: &LegacyRpcMethods<En
         signers.push(server_info);
     }
 
-    // Get all validators
-    let validators_query = entropy::storage().session().validators();
-    let all_validators = query_chain(&api, &rpc, validators_query, None).await.unwrap().unwrap();
-
-    // Get stash account of a non-signer, to become the new signer
-    // Since we only have 4 nodes in our test setup, this will be the same one the chain chooses
-    let new_signer = all_validators.iter().find(|v| !signer_stash_accounts.contains(v)).unwrap();
+    let reshare_data_query = entropy::storage().staking_extension().reshare_data();
+    let reshare_data = query_chain(&api, &rpc, reshare_data_query, None).await.unwrap().unwrap();
 
     let block_number = TEST_RESHARE_BLOCK_NUMBER;
-    let onchain_reshare_request =
-        OcwMessageReshare { new_signer: new_signer.0.to_vec(), block_number };
+    let onchain_reshare_request = OcwMessageReshare {
+        new_signers: reshare_data.new_signers.into_iter().map(|s| s.to_vec()).collect(),
+        block_number: block_number - 1,
+    };
 
-    run_to_block(&rpc, block_number + 1).await;
+    run_to_block(&rpc, block_number).await;
     // Send the OCW message to all TS servers who don't have a chain node
     let client = reqwest::Client::new();
     let response_results = join_all(

diff --git a/node/cli/src/chain_spec/integration_tests.rs b/node/cli/src/chain_spec/integration_tests.rs
@@ -218,7 +218,7 @@ pub fn integration_tests_genesis_config(
                 ],
                 vec![EVE_VERIFYING_KEY.to_vec(), DAVE_VERIFYING_KEY.to_vec()],
             ),
-            mock_signer_rotate: (true, mock_signer_rotate_data, vec![get_account_id_from_seed::<sr25519::Public>("Charlie//stash")],),
+            mock_signer_rotate: (true, mock_signer_rotate_data, vec![get_account_id_from_seed::<sr25519::Public>("Charlie//stash")]),
         },
         "elections": ElectionsConfig {
             members: endowed_accounts

diff --git a/pallets/propagation/src/lib.rs b/pallets/propagation/src/lib.rs
@@ -174,7 +174,7 @@ pub mod pallet {
                 BlockNumberFor::<T>::try_into(block_number).unwrap_or_default();
 
             let req_body = OcwMessageReshare {
-                new_signer: reshare_data.new_signer,
+                new_signers: reshare_data.new_signers,
                 // subtract 1 from blocknumber since the request is from the last block
                 block_number: converted_block_number.saturating_sub(1),
             };

diff --git a/pallets/propagation/src/tests.rs b/pallets/propagation/src/tests.rs
@@ -59,7 +59,7 @@ fn knows_how_to_mock_several_http_calls() {
             uri: "http://localhost:3001/validator/reshare".into(),
             sent: true,
             response: Some([].to_vec()),
-            body: [32, 1, 0, 0, 0, 0, 0, 0, 0, 6, 0, 0, 0].to_vec(),
+            body: [4, 32, 1, 0, 0, 0, 0, 0, 0, 0, 6, 0, 0, 0].to_vec(),
             ..Default::default()
         });
         state.expect_request(testing::PendingRequest {
@@ -94,7 +94,7 @@ fn knows_how_to_mock_several_http_calls() {
         Propagation::post_reshare(7).unwrap();
         pallet_staking_extension::ReshareData::<Test>::put(ReshareInfo {
             block_number: 7,
-            new_signer: 1u64.encode(),
+            new_signers: vec![1u64.encode()],
         });
         // now triggers
         Propagation::post_reshare(7).unwrap();

diff --git a/pallets/staking/src/benchmarking.rs b/pallets/staking/src/benchmarking.rs
@@ -29,8 +29,8 @@ use frame_support::{
 use frame_system::{EventRecord, RawOrigin};
 use pallet_parameters::{SignersInfo, SignersSize};
 use pallet_staking::{
-    Event as FrameStakingEvent, MaxNominationsOf, Nominations, Pallet as FrameStaking,
-    RewardDestination, ValidatorPrefs,
+    Event as FrameStakingEvent, MaxNominationsOf, MaxValidatorsCount, Nominations,
+    Pallet as FrameStaking, RewardDestination, ValidatorPrefs,
 };
 use sp_std::{vec, vec::Vec};
 
@@ -272,7 +272,7 @@ benchmarks! {
     let block_number = 1;
     let nonce = NULL_ARR;
     let x25519_public_key = NULL_ARR;
-    let endpoint = b"http://localhost:3001".to_vec();
+    let endpoint = vec![];
     let validate_also = false;
 
     prep_bond_and_validate::<T>(
@@ -417,13 +417,27 @@ benchmarks! {
   new_session {
     let c in 1 .. MAX_SIGNERS as u32 - 1;
     let l in 0 .. MAX_SIGNERS as u32;
+    let v in 50 .. 100 as u32;
+    let r in 0 .. MAX_SIGNERS as u32;
+
+    // c -> current signer size
+    // l -> Add in new_signer rounds so next signer is in current signer re-run checks
+    // v -> number of validators, 100 is fine as a bounder, can add more
+    // r -> adds remove indexes in
 
     let caller: T::AccountId = whitelisted_caller();
+    let mut validator_ids = create_validators::<T>(v, 1);
+    let second_signer: T::AccountId = account("second_signer", 0, 10);
+    let second_signer_id =
+        <T as pallet_session::Config>::ValidatorId::try_from(second_signer.clone())
+            .or(Err(Error::<T>::InvalidValidatorId))
+            .unwrap();
+    let mut signers = vec![second_signer_id.clone(); c as usize];
 
     // For the purpose of the bench these values don't actually matter, we just care that there's a
     // storage entry available
     SignersInfo::<T>::put(SignersSize {
-        total_signers: MAX_SIGNERS,
+        total_signers: 5,
         threshold: 3,
         last_session_change: 0,
     });
@@ -432,23 +446,20 @@ benchmarks! {
         .or(Err(Error::<T>::InvalidValidatorId))
         .unwrap();
 
-    let second_signer: T::AccountId = account("second_signer", 0, SEED);
-    let second_signer_id =
-        <T as pallet_session::Config>::ValidatorId::try_from(second_signer.clone())
-            .or(Err(Error::<T>::InvalidValidatorId))
-            .unwrap();
-
-    // full signer list leaving room for one extra validator
-    let mut signers = vec![second_signer_id.clone(); c as usize];
-
-    Signers::<T>::put(signers.clone());
-    signers.push(second_signer_id.clone());
-
     // place new signer in the signers struct in different locations to calculate random selection
     // re-run
-    signers[l as usize % c as usize] = validator_id.clone();
+    // as well validators may be dropped before chosen
+    signers[l as usize % c as usize] = validator_ids[l as usize % c as usize].clone();
+
+    // place signers into validators so they won't get dropped
+    for i in 0 .. r {
+      if i > signers.len() as u32 && i > validator_ids.len() as u32 {
+        validator_ids[i as usize] = signers[i as usize].clone();
+      }
+    }
+    Signers::<T>::put(signers.clone());
   }:  {
-    let _ = Staking::<T>::new_session_handler(&signers);
+    let _ = Staking::<T>::new_session_handler(&validator_ids);
   }
   verify {
     assert!(NextSigners::<T>::get().is_some());