diff --git a/crates/shared/src/constants.rs b/crates/shared/src/constants.rs index ea5badf27..7b8ec635a 100644 --- a/crates/shared/src/constants.rs +++ b/crates/shared/src/constants.rs @@ -74,8 +74,5 @@ pub const MAX_SIGNERS: u8 = 15; /// Threshold for those signers pub const SIGNER_THRESHOLD: u8 = 2; -/// For testing to line up chain mock data and reshare_test -pub const TEST_RESHARE_BLOCK_NUMBER: u32 = 10; - /// Program version number, must be incremented if version number changes pub const PROGRAM_VERSION_NUMBER: u8 = 0; diff --git a/crates/threshold-signature-server/Cargo.toml b/crates/threshold-signature-server/Cargo.toml index be5cdf564..fa050d9aa 100644 --- a/crates/threshold-signature-server/Cargo.toml +++ b/crates/threshold-signature-server/Cargo.toml @@ -38,6 +38,7 @@ axum ={ version="0.7.9", features=["ws"] } subxt ="0.35.3" parity-scale-codec="3.6.12" sp-core ={ version="31.0.0", default-features=false } +sp-keyring ="34.0.0" # Entropy entropy-shared ={ version="0.3.0", path="../shared" } @@ -81,7 +82,6 @@ configfs-tsm={ version="0.0.1", optional=true } serial_test="3.2.0" hex-literal="0.4.1" project-root="0.2.2" -sp-keyring="34.0.0" more-asserts="0.3.1" lazy_static="1.5.0" blake3="1.5.5" diff --git a/crates/threshold-signature-server/src/helpers/tests.rs b/crates/threshold-signature-server/src/helpers/tests.rs index 029ddcf4c..97679c9aa 100644 --- a/crates/threshold-signature-server/src/helpers/tests.rs +++ b/crates/threshold-signature-server/src/helpers/tests.rs @@ -25,6 +25,7 @@ use crate::{ chain_api::{ entropy::{ self, + runtime_types::entropy_runtime::RuntimeCall, runtime_types::pallet_staking_extension::pallet::{JumpStartStatus, ServerInfo}, }, EntropyConfig, @@ -48,6 +49,7 @@ use entropy_protocol::PartyId; #[cfg(test)] use entropy_shared::EncodedVerifyingKey; use entropy_shared::NETWORK_PARENT_KEY; +use sp_keyring::AccountKeyring; use std::{fmt, net::SocketAddr, str, time::Duration}; use subxt::{ backend::legacy::LegacyRpcMethods, ext::sp_core::sr25519, tx::PairSigner, @@ -370,3 +372,18 @@ pub fn get_port(server_info: &ServerInfo) -> u32 { str::from_utf8(&server_info.endpoint).unwrap().parse().unwrap(); socket_address.port().into() } + +/// Calls set storage from sudo for testing, allows use to manipulate chain storage. +pub async fn call_set_storage( + api: &OnlineClient, + rpc: &LegacyRpcMethods, + call: RuntimeCall, +) { + let set_storage = entropy::tx().sudo().sudo(call); + let alice = AccountKeyring::Alice; + + let signature_request_pair_signer = + PairSigner::::new(alice.into()); + + submit_transaction(api, rpc, &signature_request_pair_signer, &set_storage, None).await.unwrap(); +} diff --git a/crates/threshold-signature-server/src/validator/api.rs b/crates/threshold-signature-server/src/validator/api.rs index 179342402..074629da9 100644 --- a/crates/threshold-signature-server/src/validator/api.rs +++ b/crates/threshold-signature-server/src/validator/api.rs @@ -295,7 +295,7 @@ pub async fn validate_new_reshare( .await? .ok_or_else(|| ValidatorErr::ChainFetch("Not Currently in a reshare"))?; - if chain_data.block_number != reshare_data.block_number.saturating_sub(1) + if chain_data.block_number != reshare_data.block_number || chain_data.new_signers != reshare_data.new_signers { return Err(ValidatorErr::InvalidData); diff --git a/crates/threshold-signature-server/src/validator/tests.rs b/crates/threshold-signature-server/src/validator/tests.rs index faf73f645..8d9489b37 100644 --- a/crates/threshold-signature-server/src/validator/tests.rs +++ b/crates/threshold-signature-server/src/validator/tests.rs @@ -17,8 +17,8 @@ use crate::{ helpers::{ launch::{FORBIDDEN_KEYS, LATEST_BLOCK_NUMBER_RESHARE}, tests::{ - get_port, initialize_test_logger, run_to_block, setup_client, spawn_testing_validators, - unsafe_get, + call_set_storage, get_port, initialize_test_logger, run_to_block, setup_client, + spawn_testing_validators, unsafe_get, }, }, validator::{ @@ -29,8 +29,13 @@ use crate::{ use entropy_client::{self as test_client}; use entropy_client::{ chain_api::{ - entropy, entropy::runtime_types::bounded_collections::bounded_vec::BoundedVec, - entropy::runtime_types::pallet_registry::pallet::ProgramInstance, get_api, get_rpc, + entropy, + entropy::runtime_types::bounded_collections::bounded_vec::BoundedVec, + entropy::runtime_types::entropy_runtime::RuntimeCall, + entropy::runtime_types::frame_system::pallet::Call as SystemsCall, + entropy::runtime_types::pallet_registry::pallet::ProgramInstance, + entropy::runtime_types::pallet_staking_extension::pallet::{NextSignerInfo, ReshareInfo}, + get_api, get_rpc, }, substrate::query_chain, Hasher, @@ -69,7 +74,8 @@ async fn test_reshare_basic() { .await; let api = get_api(&context[0].ws_url).await.unwrap(); let rpc = get_rpc(&context[0].ws_url).await.unwrap(); - + let alice_stash = AccountKeyring::AliceStash; + let dave_stash = AccountKeyring::DaveStash; let client = reqwest::Client::new(); // Get current signers @@ -78,7 +84,9 @@ async fn test_reshare_basic() { let old_signer_ids: HashSet<[u8; 32]> = HashSet::from_iter(signer_stash_accounts.clone().into_iter().map(|id| id.0)); let mut signers = Vec::new(); + let mut next_signers = vec![]; for signer in signer_stash_accounts.iter() { + next_signers.push(signer); let query = entropy::storage().staking_extension().threshold_servers(signer); let server_info = query_chain(&api, &rpc, query, None).await.unwrap().unwrap(); signers.push(server_info); @@ -89,6 +97,29 @@ async fn test_reshare_basic() { let key_share = unsafe_get(&client, hex::encode(NETWORK_PARENT_KEY), port).await; assert!(!key_share.is_empty()); } + next_signers.remove(0); + let binding = dave_stash.to_account_id().into(); + next_signers.push(&binding); + let block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number + 1; + let storage_address_next_signers = entropy::storage().staking_extension().next_signers(); + let value_next_signers = + NextSignerInfo { confirmations: vec![], next_signers: next_signers.clone() }; + // Add reshare + let call = RuntimeCall::System(SystemsCall::set_storage { + items: vec![(storage_address_next_signers.to_root_bytes(), value_next_signers.encode())], + }); + call_set_storage(&api, &rpc, call).await; + + let storage_address_reshare_data = entropy::storage().staking_extension().reshare_data(); + let value_reshare_info = + ReshareInfo { block_number, new_signers: vec![dave_stash.public().encode()] }; + // Add reshare + let call = RuntimeCall::System(SystemsCall::set_storage { + items: vec![(storage_address_reshare_data.to_root_bytes(), value_reshare_info.encode())], + }); + call_set_storage(&api, &rpc, call).await; + + let key_share_before = unsafe_get(&client, hex::encode(NETWORK_PARENT_KEY), 3002).await; let mut i = 0; // Wait up to 2min for reshare to complete: check once every second if we have a new set of signers. @@ -101,7 +132,7 @@ async fn test_reshare_basic() { if new_signer_ids != old_signer_ids { break Ok(new_signer_ids); } - if i > 120 { + if i > 240 { break Err("Timed out waiting for reshare"); } i += 1; @@ -109,6 +140,11 @@ async fn test_reshare_basic() { } .unwrap(); + // wait for roatate keyshare + tokio::time::sleep(std::time::Duration::from_secs(10)).await; + + let key_share_after = unsafe_get(&client, hex::encode(NETWORK_PARENT_KEY), 3002).await; + assert_ne!(key_share_before, key_share_after); // At this point the signing set has changed on-chain, but the keyshares haven't been rotated // but by the time we have stored a program and registered, the rotation should have happened @@ -173,6 +209,36 @@ async fn test_reshare_basic() { let key_share = unsafe_get(&client, hex::encode(NETWORK_PARENT_KEY), port).await; assert!(!key_share.is_empty()); } + let block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number + 1; + let key_share_before_2 = unsafe_get(&client, hex::encode(NETWORK_PARENT_KEY), 3003).await; + + next_signers.remove(0); + let binding = alice_stash.to_account_id().into(); + next_signers.push(&binding); + + let storage_address_next_signers = entropy::storage().staking_extension().next_signers(); + let value_next_signers = NextSignerInfo { confirmations: vec![], next_signers }; + // Add another reshare by adding next signer info + let call = RuntimeCall::System(SystemsCall::set_storage { + items: vec![(storage_address_next_signers.to_root_bytes(), value_next_signers.encode())], + }); + call_set_storage(&api, &rpc, call).await; + + let storage_address_reshare_data = entropy::storage().staking_extension().reshare_data(); + let value_reshare_info = + ReshareInfo { block_number, new_signers: vec![alice_stash.public().encode()] }; + // Same reshare needs reshare data too + let call = RuntimeCall::System(SystemsCall::set_storage { + items: vec![(storage_address_reshare_data.to_root_bytes(), value_reshare_info.encode())], + }); + call_set_storage(&api, &rpc, call).await; + + // wait for roatate keyshare + tokio::time::sleep(std::time::Duration::from_secs(60)).await; + + let key_share_after_2 = unsafe_get(&client, hex::encode(NETWORK_PARENT_KEY), 3003).await; + assert_ne!(key_share_before_2, key_share_after_2); + clean_tests(); } @@ -225,9 +291,17 @@ async fn test_reshare_validation_fail() { validate_new_reshare(&api, &rpc, &ocw_message, &kv).await.map_err(|e| e.to_string()); assert_eq!(err_stale_data, Err("Data is stale".to_string())); - let block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number + 1; + let block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number; + let storage_address_reshare_data = entropy::storage().staking_extension().reshare_data(); + let value_reshare_info = + ReshareInfo { block_number: block_number + 1, new_signers: vec![dave.public().encode()] }; + // Add reshare + let call = RuntimeCall::System(SystemsCall::set_storage { + items: vec![(storage_address_reshare_data.to_root_bytes(), value_reshare_info.encode())], + }); + ocw_message.block_number = block_number; - run_to_block(&rpc, block_number + 1).await; + call_set_storage(&api, &rpc, call).await; let err_incorrect_data = validate_new_reshare(&api, &rpc, &ocw_message, &kv).await.map_err(|e| e.to_string()); diff --git a/node/cli/src/chain_spec/dev.rs b/node/cli/src/chain_spec/dev.rs index 355785b55..833b1d839 100644 --- a/node/cli/src/chain_spec/dev.rs +++ b/node/cli/src/chain_spec/dev.rs @@ -255,7 +255,6 @@ pub fn development_genesis_config( }) .collect::>(), proactive_refresh_data: (vec![], vec![]), - mock_signer_rotate: (false, vec![], vec![]), jump_started_signers: None, }, "elections": ElectionsConfig { diff --git a/node/cli/src/chain_spec/integration_tests.rs b/node/cli/src/chain_spec/integration_tests.rs index 18a9523ea..192f7f646 100644 --- a/node/cli/src/chain_spec/integration_tests.rs +++ b/node/cli/src/chain_spec/integration_tests.rs @@ -66,10 +66,6 @@ pub fn integration_tests_config(jumpstarted: bool) -> ChainSpec { ], vec![], get_account_id_from_seed::("Alice"), - vec![ - get_account_id_from_seed::("Bob//stash"), - get_account_id_from_seed::("Charlie//stash"), - ], jump_started_signers, )) .build() @@ -87,7 +83,6 @@ pub fn integration_tests_genesis_config( )>, initial_nominators: Vec, root_key: AccountId, - mock_signer_rotate_data: Vec, jump_started_signers: Option>, ) -> serde_json::Value { // Note that any endowed_accounts added here will be included in the `elections` and @@ -229,7 +224,6 @@ pub fn integration_tests_genesis_config( ], vec![PREGENERATED_NETWORK_VERIFYING_KEY.to_vec(), DAVE_VERIFYING_KEY.to_vec()], ), - mock_signer_rotate: (true, mock_signer_rotate_data, vec![get_account_id_from_seed::("Dave//stash")]), jump_started_signers, }, "elections": ElectionsConfig { diff --git a/node/cli/src/chain_spec/testnet.rs b/node/cli/src/chain_spec/testnet.rs index 590e14be3..64874f068 100644 --- a/node/cli/src/chain_spec/testnet.rs +++ b/node/cli/src/chain_spec/testnet.rs @@ -427,7 +427,6 @@ pub fn testnet_genesis_config( }) .collect::>(), proactive_refresh_data: (vec![], vec![]), - mock_signer_rotate: (false, vec![], vec![]), jump_started_signers: None, }, "elections": ElectionsConfig { diff --git a/pallets/attestation/src/mock.rs b/pallets/attestation/src/mock.rs index 575c2cead..8c26ea362 100644 --- a/pallets/attestation/src/mock.rs +++ b/pallets/attestation/src/mock.rs @@ -372,7 +372,6 @@ pub fn new_test_ext() -> sp_io::TestExternalities { (5, (0, NULL_ARR, vec![20], pck_encoded.to_vec().try_into().unwrap())), ], proactive_refresh_data: (vec![], vec![]), - mock_signer_rotate: (false, vec![], vec![]), jump_started_signers: None, }; pallet_staking_extension.assimilate_storage(&mut t).unwrap(); diff --git a/pallets/propagation/src/lib.rs b/pallets/propagation/src/lib.rs index fcea66d47..372cae8bd 100644 --- a/pallets/propagation/src/lib.rs +++ b/pallets/propagation/src/lib.rs @@ -160,11 +160,11 @@ pub mod pallet { /// Submits a request to do a key refresh on the signers parent key. pub fn post_reshare(block_number: BlockNumberFor) -> Result<(), http::Error> { let reshare_data = pallet_staking_extension::Pallet::::reshare_data(); - if reshare_data.block_number != block_number { + if reshare_data.block_number + sp_runtime::traits::One::one() != block_number { return Ok(()); } - let deadline = sp_io::offchain::timestamp().add(Duration::from_millis(2_000)); + let deadline = sp_io::offchain::timestamp().add(Duration::from_millis(10_000)); let kind = sp_core::offchain::StorageKind::PERSISTENT; let from_local = sp_io::offchain::local_storage_get(kind, b"reshare_validators") .unwrap_or_else(|| b"http://localhost:3001/validator/reshare".to_vec()); diff --git a/pallets/propagation/src/mock.rs b/pallets/propagation/src/mock.rs index fac31d4ec..31de2ed29 100644 --- a/pallets/propagation/src/mock.rs +++ b/pallets/propagation/src/mock.rs @@ -418,7 +418,6 @@ pub fn new_test_ext() -> sp_io::TestExternalities { (2, (4, NULL_ARR, vec![11], BoundedVec::with_max_capacity())), ], proactive_refresh_data: (vec![], vec![]), - mock_signer_rotate: (false, vec![], vec![]), jump_started_signers: None, }; diff --git a/pallets/propagation/src/tests.rs b/pallets/propagation/src/tests.rs index 05b597633..0be5f1315 100644 --- a/pallets/propagation/src/tests.rs +++ b/pallets/propagation/src/tests.rs @@ -93,7 +93,7 @@ fn knows_how_to_mock_several_http_calls() { // doesn't trigger no reshare block Propagation::post_reshare(7).unwrap(); pallet_staking_extension::ReshareData::::put(ReshareInfo { - block_number: 7, + block_number: 6, new_signers: vec![1u64.encode()], }); // now triggers diff --git a/pallets/registry/src/mock.rs b/pallets/registry/src/mock.rs index c38c485da..c91d3f730 100644 --- a/pallets/registry/src/mock.rs +++ b/pallets/registry/src/mock.rs @@ -405,7 +405,6 @@ pub fn new_test_ext() -> sp_io::TestExternalities { (7, (4, NULL_ARR, vec![50], BoundedVec::with_max_capacity())), ], proactive_refresh_data: (vec![], vec![]), - mock_signer_rotate: (false, vec![], vec![]), jump_started_signers: None, }; diff --git a/pallets/staking/src/lib.rs b/pallets/staking/src/lib.rs index 5181e3307..516e6a3e6 100644 --- a/pallets/staking/src/lib.rs +++ b/pallets/staking/src/lib.rs @@ -59,7 +59,7 @@ use sp_staking::SessionIndex; pub mod pallet { use entropy_shared::{ QuoteContext, ValidatorInfo, VerifyQuoteError, X25519PublicKey, MAX_SIGNERS, - PREGENERATED_NETWORK_VERIFYING_KEY, TEST_RESHARE_BLOCK_NUMBER, VERIFICATION_KEY_LENGTH, + PREGENERATED_NETWORK_VERIFYING_KEY, VERIFICATION_KEY_LENGTH, }; use frame_support::{ dispatch::{DispatchResult, DispatchResultWithPostInfo}, @@ -256,8 +256,6 @@ pub mod pallet { pub threshold_servers: Vec>, /// validator info and accounts to take part in proactive refresh pub proactive_refresh_data: (Vec, Vec>), - /// validator info and account new signer to take part in a reshare - pub mock_signer_rotate: (bool, Vec, Vec), /// Whether to begin in an already jumpstarted state in order to be able to test signing /// using pre-generated keyshares pub jump_started_signers: Option>, @@ -289,22 +287,6 @@ pub mod pallet { proactive_refresh_keys: self.proactive_refresh_data.1.clone(), }; ProactiveRefresh::::put(refresh_info); - // mocks a signer rotation for tss new_reshare tests - if self.mock_signer_rotate.0 { - let next_signers = &mut self.mock_signer_rotate.1.clone(); - let mut new_signers = vec![]; - for new_signer in self.mock_signer_rotate.2.clone() { - next_signers.push(new_signer.clone()); - new_signers.push(new_signer.encode()) - } - let next_signers = next_signers.to_vec(); - NextSigners::::put(NextSignerInfo { next_signers, confirmations: vec![] }); - ReshareData::::put(ReshareInfo { - // To give enough time for test_reshare setup - block_number: TEST_RESHARE_BLOCK_NUMBER.into(), - new_signers, - }) - } if let Some(jump_started_signers) = &self.jump_started_signers { Signers::::put(jump_started_signers.clone()); @@ -922,7 +904,7 @@ pub mod pallet { // trigger reshare at next block let current_block_number = >::block_number(); let reshare_info = ReshareInfo { - block_number: current_block_number + sp_runtime::traits::One::one(), + block_number: current_block_number - sp_runtime::traits::One::one(), new_signers, }; diff --git a/pallets/staking/src/mock.rs b/pallets/staking/src/mock.rs index bd6faffef..85e217f74 100644 --- a/pallets/staking/src/mock.rs +++ b/pallets/staking/src/mock.rs @@ -477,7 +477,6 @@ pub fn new_test_ext() -> sp_io::TestExternalities { (6, (8, NULL_ARR, vec![40], BoundedVec::with_max_capacity())), ], proactive_refresh_data: (vec![], vec![]), - mock_signer_rotate: (false, vec![], vec![]), jump_started_signers: None, }; pallet_balances.assimilate_storage(&mut t).unwrap(); diff --git a/pallets/staking/src/tests.rs b/pallets/staking/src/tests.rs index 54074d087..4d971598f 100644 --- a/pallets/staking/src/tests.rs +++ b/pallets/staking/src/tests.rs @@ -510,11 +510,7 @@ fn it_tests_new_session_handler() { assert_ok!(Staking::new_session_handler(&[1, 5, 6])); // takes signers original (5,6) pops off one and adds in new validator assert_eq!(Staking::next_signers().unwrap().next_signers, vec![6, 1]); - assert_eq!( - Staking::reshare_data().block_number, - 101, - "Check reshare block start at 100 + 1" - ); + assert_eq!(Staking::reshare_data().block_number, 99, "Check reshare block start at 99 + 1"); assert_eq!( Staking::reshare_data().new_signers, vec![1u64.encode()], @@ -526,12 +522,6 @@ fn it_tests_new_session_handler() { "parent key threhsold updated" ); - assert_eq!( - Staking::reshare_data().block_number, - 101, - "Check reshare block start at 100 + 1" - ); - assert_ok!(Staking::new_session_handler(&[6, 5, 3])); // takes 3 and leaves 5 and 6 since already in signer group assert_eq!(Staking::next_signers().unwrap().next_signers, vec![6, 3]);