feat: adding reconstruction threshold in node #3640
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,8 +7,9 @@ use blstrs::{G1Projective, Scalar}; | |
| use e2e_tests::{CLUSTER_WAIT_TIMEOUT, MpcCluster, MpcClusterConfig, metrics}; | ||
| use group::Group; | ||
| use near_mpc_contract_interface::types::{ | ||
| Bls12381G2PublicKey, CKDAppPublicKey, Curve, DomainConfig, DomainId, DomainPurpose, Protocol, | ||
| ProtocolContractState, PublicKey, PublicKeyExtended, ReconstructionThreshold, | ||
| RunningContractState, | ||
| }; | ||
| use near_mpc_crypto_types::Bls12381G1PublicKey; | ||
| use serde_json::json; | ||
|
|
@@ -36,6 +37,7 @@ pub const CONTRACT_UPGRADE_COMPATIBILITY_TESTNET_PORT_SEED: u16 = 19; | |
| pub const TIMEOUT_METRIC_PORT_SEED: u16 = 20; | ||
| pub const MIGRATION_BACK_PORT_SEED: u16 = 21; | ||
| pub const SIGTERM_HANDLER_PORT_SEED: u16 = 22; | ||
| pub const DISTINCT_RECONSTRUCTION_THRESHOLDS_PORT_SEED: u16 = 23; | ||
|
|
||
| /// Start a cluster, wait for Running state and presignatures to buffer. | ||
| /// | ||
|
|
@@ -376,6 +378,28 @@ pub fn must_get_bls_public_key( | |
| } | ||
| } | ||
|
|
||
| /// Builds a `DamgardEtAl` signing domain with reconstruction threshold `t`, which needs `2t - 1` signers. | ||
| pub fn damgard_etal_domain(id: u64, t: u64) -> DomainConfig { | ||
| DomainConfig { | ||
| id: DomainId(id), | ||
| protocol: Protocol::DamgardEtAl, | ||
| reconstruction_threshold: ReconstructionThreshold::new(t), | ||
| purpose: DomainPurpose::Sign, | ||
| } | ||
| } | ||
|
|
||
| /// Returns the domain running `protocol_type`, panicking if absent. Each | ||
| /// protocol appears at most once per registry, so it identifies a unique domain. | ||
|
SimonRastikian marked this conversation as resolved.
Outdated
|
||
| pub fn must_get_domain(running: &RunningContractState, protocol_type: Protocol) -> DomainConfig { | ||
| running | ||
| .domains | ||
| .domains | ||
| .iter() | ||
| .find(|d| d.protocol == protocol_type) | ||
| .unwrap_or_else(|| panic!("no domain with protocol {protocol_type:?}")) | ||
| .clone() | ||
| } | ||
|
|
||
| /// Send a sign request and assert the network produced a successful response. | ||
| /// | ||
| /// Panics if the request can't be submitted to the contract — the test cannot | ||
|
|
@@ -406,6 +430,34 @@ pub async fn send_sign_request( | |
| Ok(()) | ||
| } | ||
|
|
||
| /// Sign with every scheme in `running`, asserting each request succeeds. | ||
| pub async fn sign_all_schemes( | ||
| cluster: &MpcCluster, | ||
| running: &RunningContractState, | ||
| rng: &mut impl rand::Rng, | ||
| ) { | ||
|
Contributor
There was a problem hiding this comment. nit: the doc comment here is useless, as it just paraphrases the function name. non-nit: I think instead of signing for all schemes, we may want to sign for all domains (that do signatures) in the contract? Such that, in case we have two cait-sith domains, they both get a request.
Contributor
There was a problem hiding this comment. @SimonRastikian I don't think this comment was addressed.
Contributor
Author
There was a problem hiding this comment. Sorry for the miss, there was a lot of comments |
||
| for (label, protocol) in [ | ||
| ("ECDSA", Protocol::CaitSith), | ||
| ("Damgard et al", Protocol::DamgardEtAl), | ||
| ("EdDSA", Protocol::Frost), | ||
| ] { | ||
| let domain = must_get_domain(running, protocol); | ||
| let payload = match Curve::from(protocol) { | ||
| Curve::Edwards25519 => generate_eddsa_payload(rng), | ||
| _ => generate_ecdsa_payload(rng), | ||
| }; | ||
| let outcome = cluster | ||
| .send_sign_request(domain.id, payload, cluster.default_user_account()) | ||
| .await | ||
| .expect("sign request failed"); | ||
| assert!( | ||
| outcome.is_success(), | ||
| "{label} sign request failed: {:?}", | ||
| outcome.failure_message() | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /// Send a CKD request and assert the network produced a successful response. | ||
| /// | ||
| /// Panics if the request can't be submitted to the contract — the test cannot | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| use crate::common::{ | ||
| DISTINCT_RECONSTRUCTION_THRESHOLDS_PORT_SEED, damgard_etal_domain, generate_ckd_app_public_key, | ||
| must_get_domain, must_setup_cluster, sign_all_schemes, | ||
| }; | ||
|
|
||
| use near_mpc_contract_interface::types::Protocol; | ||
| use rand::SeedableRng; | ||
|
|
||
| /// Each domain signs using its own reconstruction threshold rather than the | ||
| /// governance threshold. The governance threshold is 4 with a total of 6 nodes. | ||
| /// The signing procedure succeeds despite Damgard et al. requiring at least 7 | ||
| /// participants under a governance-threshold signing model. | ||
| /// Therefore, signing is performed using the reconstruction threshold, | ||
| /// not the governance threshold. | ||
|
Contributor
There was a problem hiding this comment. although this is correct, it is a very indirect way to check that the per domain thresholds are being used. Here we just need to check that the signatures are produced correctly, integration tests in the node should be able to figure that the correct threshold is used. Therefore this part of the comment can be removed.
Contributor
Author
There was a problem hiding this comment. I would still vote to keep it as it is indeed part of the logic of the test. But I guess your comment anw was not a blocker
Contributor
There was a problem hiding this comment.
A more direct approach for testing this would be to shut down some of the nodes. E.g. you have governance and cat-sith threshold of 6, but set damgard-et-al reconstruction threshold to 3 (e.g. signing threshold of 5), then you bring down all but 5 nodes, now, we expect cait-sith signatures to fail, but damgard to be successful.
Contributor
Author
There was a problem hiding this comment. @kevindeforth I killed nodes in the test 0156843
Contributor
Author
There was a problem hiding this comment. Does this sound good to you? |
||
| #[tokio::test] | ||
| #[expect(non_snake_case)] | ||
| async fn distinct_reconstruction_thresholds__should_sign_for_every_scheme() { | ||
| // Given | ||
| let (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 = 4; | ||
| c.triples_to_buffer = 2; | ||
| c.presignatures_to_buffer = 2; | ||
|
Comment on lines
+26
to
+27
Contributor
There was a problem hiding this comment. shouldn't triples be at least 4 in this case?
Contributor
Author
There was a problem hiding this comment. 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. 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 | ||
| .push(damgard_etal_domain(c.domains.len() as u64, 3)); | ||
| }) | ||
| .await; | ||
|
|
||
| let ckd_domain = must_get_domain(&contract_state, Protocol::ConfidentialKeyDerivation); | ||
|
|
||
| // When / Then | ||
| let mut rng = rand::rngs::StdRng::seed_from_u64(0); | ||
|
SimonRastikian marked this conversation as resolved.
Outdated
|
||
| sign_all_schemes(&cluster, &contract_state, &mut rng).await; | ||
|
|
||
| let outcome = cluster | ||
| .send_ckd_request( | ||
| ckd_domain.id, | ||
| generate_ckd_app_public_key(&mut rng), | ||
| cluster.default_user_account(), | ||
| ) | ||
| .await | ||
| .expect("ckd request failed"); | ||
| assert!( | ||
| outcome.is_success(), | ||
| "ckd request failed: {:?}", | ||
| outcome.failure_message() | ||
| ); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.