-
Notifications
You must be signed in to change notification settings - Fork 41
feat: adding reconstruction threshold in node #3640
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
Changes from all commits
02dd798
44e9e76
7d86856
b33f455
02c5313
a378c56
3da1652
aeed6e7
7257a16
a20a7a8
1afd958
db47b85
0cf1b53
27e40bf
7878ac8
1f28966
7bca4d8
263bc8d
f45ebf5
c8fec49
6e1ce47
5056109
5f5af82
fbf22be
b310e84
ca8b199
da2d190
7458432
4f52bdd
29613b8
3b329e2
f168468
cd557e1
e769a53
df26736
876bcfe
fc57a33
484d40e
ec871da
ee76a11
6a4fa9e
0ef09e5
bc928d4
e3d6c89
2c690bd
86bbbd2
158c838
6b08b91
fa67289
5c9a3fe
be0b517
db5692e
41184ee
cb5725f
02e0d13
eed4c8f
8423f2e
79f9666
05bdcfa
fb858bd
50cf59e
49b39a7
3c3912f
9041366
aa1b927
1597647
5055b0e
5f71e41
4874fb3
6c535c5
446f162
1395243
37cf136
392549d
3d94416
71392e0
a0c3637
7c6164b
d80827e
fad6726
5792061
0b8b201
4b2f853
f89e30d
766fef6
d759fc8
7733fd0
9cc8741
954a69a
18e1ed1
a2cc899
aac0973
ec959e4
955e669
e89f261
19f1ca5
133ba43
6b4107e
80de0a0
2481a55
feaa80a
46342fa
1905cd5
857ac6f
2066927
d22f3c1
aeedac8
1ec15b5
6b119e8
98b5c73
6eae2be
86aa75e
b2bc11a
cb69750
aec2f59
19954d8
996d9a3
06a9a47
a83a1f2
87c2d07
5c53876
d416017
0156843
316d15b
bc8bc72
75b82c4
6e5dfce
c3269ea
17d649e
96535d6
9ca92e1
e350511
5d3fedc
9231d99
4ab6098
3e3af3b
da56e05
97a24ae
c09f549
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,125 @@ | ||
| use crate::common::{ | ||
| DISTINCT_RECONSTRUCTION_THRESHOLDS_PORT_SEED, ckd_domain, damgard_etal_domain, | ||
| generate_ckd_app_public_key, generate_ecdsa_payload, generate_eddsa_payload, must_get_domain, | ||
| must_setup_cluster, wait_metric_on_nodes, | ||
| }; | ||
|
|
||
| use e2e_tests::{CLUSTER_WAIT_TIMEOUT, metrics}; | ||
| use near_mpc_contract_interface::types::{ | ||
| DomainConfig, DomainId, DomainPurpose, Protocol, ReconstructionThreshold, | ||
| }; | ||
| use rand::SeedableRng; | ||
|
|
||
| /// Each domain signs under its own reconstruction threshold, not the governance | ||
| /// threshold. With 6 nodes and 1 killed, Cait-Sith (needs all 6) can no longer sign | ||
| /// while Damgard et al. (`2t - 1 = 5`), CKD (`t = 5`) and Frost (`t = 5`) still can. | ||
| #[tokio::test] | ||
| #[expect(non_snake_case)] | ||
| async fn distinct_reconstruction_thresholds__should_use_per_domain_threshold_when_nodes_are_down() { | ||
| // Given | ||
| let mut rng = rand::rngs::StdRng::seed_from_u64(0); | ||
| let (mut cluster, contract_state) = | ||
| must_setup_cluster(DISTINCT_RECONSTRUCTION_THRESHOLDS_PORT_SEED, |c| { | ||
| c.num_nodes = 6; | ||
| c.initial_participant_indices = (0..6).collect(); | ||
| c.threshold = 6; | ||
| c.triples_to_buffer = 2; | ||
| c.presignatures_to_buffer = 2; | ||
|
Comment on lines
+26
to
+27
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't triples be at least 4 in this case?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's fine here as is. You need 2 triples per presignature and then you have a background triple generation process that continuously refills the buffer to 2 triples.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we usually put enough triples in the buffer for all the signatures in the test to try to avoid flakiness as much as possible. |
||
| c.domains = vec![ | ||
| DomainConfig { | ||
| id: DomainId(0), | ||
| protocol: Protocol::CaitSith, | ||
| reconstruction_threshold: ReconstructionThreshold::new(6), | ||
| purpose: DomainPurpose::Sign, | ||
| }, | ||
| damgard_etal_domain(1, 3), | ||
|
SimonRastikian marked this conversation as resolved.
|
||
| ckd_domain(2, 5), | ||
| DomainConfig { | ||
| id: DomainId(3), | ||
| protocol: Protocol::Frost, | ||
| reconstruction_threshold: ReconstructionThreshold::new(5), | ||
| purpose: DomainPurpose::Sign, | ||
| }, | ||
| ]; | ||
| }) | ||
| .await; | ||
|
|
||
| let caitsith_domain = must_get_domain(&contract_state, Protocol::CaitSith); | ||
| let damgard_domain = must_get_domain(&contract_state, Protocol::DamgardEtAl); | ||
| let ckd_domain = must_get_domain(&contract_state, Protocol::ConfidentialKeyDerivation); | ||
| let frost_domain = must_get_domain(&contract_state, Protocol::Frost); | ||
|
|
||
| // When | ||
| cluster.kill_nodes(&[5]).expect("failed to kill node 5"); | ||
|
|
||
| // Then Damgard et al. (needs 5 signers) still signs. | ||
| let outcome = cluster | ||
| .send_sign_request( | ||
| damgard_domain.id, | ||
| generate_ecdsa_payload(&mut rng), | ||
| cluster.default_user_account(), | ||
| ) | ||
| .await | ||
| .expect("failed to submit Damgard et al. sign request"); | ||
| assert!( | ||
| outcome.is_success(), | ||
| "Damgard et al. sign request failed with 5 of 6 nodes alive: {:?}", | ||
| outcome.failure_message() | ||
| ); | ||
|
|
||
| // And CKD (its own `t = 5`, not the governance threshold of 6) still derives. | ||
| let outcome = cluster | ||
| .send_ckd_request( | ||
| ckd_domain.id, | ||
| generate_ckd_app_public_key(&mut rng), | ||
| cluster.default_user_account(), | ||
| ) | ||
| .await | ||
| .expect("failed to submit CKD request"); | ||
| assert!( | ||
| outcome.is_success(), | ||
| "CKD request failed with 5 of its 5 required signers alive: {:?}", | ||
| outcome.failure_message() | ||
| ); | ||
|
|
||
| // And Frost (its own `t = 5`) still signs. | ||
| let outcome = cluster | ||
| .send_sign_request( | ||
| frost_domain.id, | ||
| generate_eddsa_payload(&mut rng), | ||
| cluster.default_user_account(), | ||
| ) | ||
| .await | ||
| .expect("failed to submit Frost sign request"); | ||
| assert!( | ||
| outcome.is_success(), | ||
| "Frost sign request failed with 5 of its 5 required signers alive: {:?}", | ||
| outcome.failure_message() | ||
| ); | ||
|
|
||
| // And Cait-Sith (needs all 6) is unanswerable. Its request never resolves on | ||
| // chain, and the yield auto-timeout outlives the JSON-RPC call, so we race the | ||
| // doomed request against the surviving nodes' timeout counter rather than | ||
| // awaiting it (see `timeout_metric.rs`). | ||
| tokio::select! { | ||
| res = wait_metric_on_nodes( | ||
| &cluster, | ||
| &[0, 1, 2, 3, 4], | ||
| metrics::TIMEOUTS_INDEXED, | ||
| |v| v >= 1, | ||
| CLUSTER_WAIT_TIMEOUT, | ||
| ) => res.unwrap_or_else(|_| panic!( | ||
| "{} did not reach 1 on the surviving nodes — Cait-Sith request was answered \ | ||
| despite only 5 of its 6 required signers being alive", | ||
| metrics::TIMEOUTS_INDEXED | ||
| )), | ||
|
Comment on lines
+104
to
+115
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How long will this take? The timeout of a signature, 60 seconds?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, it's the default 200 blocks of yield_timeout_length_in_blocks |
||
| _ = cluster.send_sign_request( | ||
| caitsith_domain.id, | ||
| generate_ecdsa_payload(&mut rng), | ||
| cluster.default_user_account(), | ||
| ) => panic!( | ||
| "Cait-Sith sign request returned before the timeout metric — it should be \ | ||
| unanswerable with only 5 of 6 required signers alive" | ||
| ), | ||
|
SimonRastikian marked this conversation as resolved.
|
||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.