Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reshare e2e test #1254

Merged
merged 10 commits into from
Jan 31, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions RELEASE_CHECKLIST.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ tagged as the final release.

## Pre-Prep
- [ ] Inform relevant parties that you're preparing a release (e.g, by posting on Discord)
- [ ] Manually run e2e reshare test
Copy link
Collaborator

@HCastano HCastano Jan 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would try and avoid running this manually and instead make it a part of the release CI job. We have an on > push > release/... stage where we can add an extra testing stage which encompasses this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be doable but let me do in another PR so I can work with zach of vi on that

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(first compile node with features=reshare-test and then run
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took this to mean do this:

cargo build -p entropy --release --features reshare-test

but this gives me:

error: none of the selected packages contains these features: reshare-test

How are you compiling the runtime with this feature flag without having it present in node/cli/Cargo.toml and propagated down?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gonna add based on the other comment, but weird I don't see it locally or on CI

```cargo test --release --features=reshare-test -- --test test_reshare_e2e```)
JesseAbram marked this conversation as resolved.
Show resolved Hide resolved
- [ ] Create a release branch, e.g., for release candidate `1`: `release/vX.Y.Z-rc.1`.

## Prep the Runtime and Node
Expand Down
3 changes: 2 additions & 1 deletion crates/threshold-signature-server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -113,4 +113,5 @@ alice =[]
bob =[]
# Enable this feature to run the integration tests for the wasm API of entropy-protocol
# This requires the entropy-protocol node-js module to be built and so is not run by default
wasm_test=[]
wasm_test =[]
reshare-test=[]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add some comments explaining the feature here and in the other TOML files

58 changes: 58 additions & 0 deletions crates/threshold-signature-server/src/validator/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,64 @@ async fn test_reshare_basic() {
clean_tests();
}

#[cfg(feature = "reshare-test")]
#[tokio::test]
#[serial]
async fn test_reshare_e2e() {
initialize_test_logger().await;
clean_tests();

let (_validator_ips, _validator_ids) =
spawn_testing_validators(crate::helpers::tests::ChainSpecType::IntegrationJumpStarted)
.await;

let force_authoring = true;
let context =
test_node_process_testing_state(ChainSpecType::IntegrationJumpStarted, force_authoring)
.await;
let api = get_api(&context[0].ws_url).await.unwrap();
let rpc = get_rpc(&context[0].ws_url).await.unwrap();
let client = reqwest::Client::new();
Comment on lines +249 to +259
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can replace this with spawn_tss_nodes_and_start_chain()


// Get current signers
let signer_query = entropy::storage().staking_extension().signers();
let signer_stash_accounts = query_chain(&api, &rpc, signer_query, None).await.unwrap().unwrap();
let old_signer_ids: HashSet<[u8; 32]> =
HashSet::from_iter(signer_stash_accounts.clone().into_iter().map(|id| id.0));
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.
let _ = loop {
let new_signer_ids: HashSet<[u8; 32]> = {
let signer_query = entropy::storage().staking_extension().signers();
let signer_ids = query_chain(&api, &rpc, signer_query, None).await.unwrap().unwrap();
HashSet::from_iter(signer_ids.into_iter().map(|id| id.0))
};
if new_signer_ids != old_signer_ids {
break Ok(new_signer_ids);
}
if i > 240 {
break Err("Timed out waiting for reshare");
}
i += 1;
tokio::time::sleep(std::time::Duration::from_secs(1)).await;
}
.unwrap();

// wait for rotate 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);

// wait for rotate keyshare 2
tokio::time::sleep(std::time::Duration::from_secs(80)).await;

let key_share_after_2 = unsafe_get(&client, hex::encode(NETWORK_PARENT_KEY), 3002).await;
assert_ne!(key_share_after, key_share_after_2);
}

#[tokio::test]
#[serial]
async fn test_reshare_none_called() {
Expand Down
1 change: 1 addition & 0 deletions node/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,4 @@ try-runtime=["entropy-runtime/try-runtime", "try-runtime-cli/try-runtime"]
# Enables real PCK certificate chain verification - which means TSS nodes must be runnning on TDX
# hardware
production=["entropy-runtime/production"]
reshare-test=["entropy-runtime/reshare-test"]
4 changes: 3 additions & 1 deletion node/cli/src/chain_spec/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,9 @@ pub fn integration_tests_genesis_config(
"staking": StakingConfig {
validator_count: initial_authorities.len() as u32,
minimum_validator_count: 0,
invulnerables: vec![],
invulnerables: initial_authorities
.iter()
.map(|x| {x.0.clone()}).collect::<Vec<_>>(),
slash_reward_fraction: Perbill::from_percent(10),
stakers,
..Default::default()
Expand Down
4 changes: 3 additions & 1 deletion runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -275,4 +275,6 @@ try-runtime=[
]
# Enables real PCK certificate chain verification - which means TSS nodes must be runnning on TDX
# hardware
production=["pallet-attestation/production"]
production =["pallet-attestation/production"]
# Sets a shorter session duration for the entropy-tss test_reshare_e2e
reshare-test=[]
JesseAbram marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 3 additions & 0 deletions runtime/src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,10 @@ pub mod time {

// NOTE: Currently it is not possible to change the epoch duration after the chain has started.
// Attempting to do so will brick block production.
#[cfg(not(feature = "reshare-test"))]
pub const EPOCH_DURATION_IN_BLOCKS: BlockNumber = 4 * HOURS;
#[cfg(feature = "reshare-test")]
pub const EPOCH_DURATION_IN_BLOCKS: BlockNumber = 15 / (SECS_PER_BLOCK as BlockNumber);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why define this in terms of SECS_PER_BLOCK instead of having something fixed for tests? Say, 1 * MINUTE or something like that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because I want it less than one minuet and I I get an overflow problem that way

pub const EPOCH_DURATION_IN_SLOTS: u64 = {
const SLOT_FILL_RATE: f64 = MILLISECS_PER_BLOCK as f64 / SLOT_DURATION as f64;

Expand Down
Loading