chore!: add eip712 for new epoch (#471)

* chore: add eip712 for new epoch req in proto file

* feat: eip712 for new epoch in kms service

* chore: note on kms-init usage

* chore: add validation test

* chore: remove domain from key info

* chore: remove domain from crs info

Author: eudelins-zama

core-client/src/mpc_epoch.rs

@@ -7,7 +7,7 @@ use kms_grpc::{
     identifiers::EpochId,
     kms::v1::{DestroyMpcEpochRequest, FheParameter, KeyDigest, KeyInfo, PreviousEpochInfo},
     kms_service::v1::core_service_endpoint_client::CoreServiceEndpointClient,
-    rpc_types::{alloy_to_protobuf_domain, PubDataType},
+    rpc_types::PubDataType,
     RequestId,
 };
 use kms_lib::{
@@ -67,7 +67,6 @@ impl PreviousEpochParameters {
                 preproc_id: Some(previous_key_info.preproc_id.into()),
                 key_parameters: fhe_params.into(),
                 key_digests: digest,
-                domain: Some(alloy_to_protobuf_domain(&dummy_domain())?),
             });
         }
 
@@ -82,8 +81,7 @@ impl PreviousEpochParameters {
                         e
                     )
                 })?,
-                domain: Some(alloy_to_protobuf_domain(&dummy_domain())?),
-            });
+            })
         }
 
         let resp = PreviousEpochInfo {
@@ -124,8 +122,17 @@ pub(crate) async fn do_new_epoch(
         .as_ref()
         .map(|previous_epoch| previous_epoch.convert_to_grpc(fhe_params))
         .transpose()?;
-    let request =
-        internal_client.new_epoch_request(&new_context_id, &new_epoch_id, previous_epoch_grpc)?;
+    let domain = if new_epoch_params.previous_epoch_params.is_some() {
+        Some(dummy_domain())
+    } else {
+        None
+    };
+    let request = internal_client.new_epoch_request(
+        &new_context_id,
+        &new_epoch_id,
+        previous_epoch_grpc,
+        domain.as_ref(),
+    )?;
 
     // Send the request
     let mut req_tasks = JoinSet::new();

core/grpc/proto/kms.v1.proto

@@ -773,7 +773,6 @@ message KeyInfo {
   // The domain separator DSEP_PUBDATA_KEY="PDAT_KEY" is used when hashing the keys.
   // If there are no key_digests, the digest verification is skipped.
   repeated KeyDigest key_digests = 4;
-  Eip712DomainMsg domain = 5;
 }
 
 message CrsInfo {
@@ -783,9 +782,6 @@ message CrsInfo {
   // The digest of the generated CRS.
   // It is hashed with the domain separator DSEP_PUBDATA_CRS="PDAT_CRS".
   bytes crs_digest = 2;
-
-  // The EIP712 domain from the initial CrsGenRequest
-  Eip712DomainMsg domain = 4;
 }
 
 message PreviousEpochInfo {
@@ -811,6 +807,9 @@ message NewMpcEpochRequest {
   // Information about the previous epoch, used to perform resharing.
   // If not set, no keys will exist for the new epoch. (e.g. during init)
   PreviousEpochInfo previous_epoch = 3;
+  // The EIP712 domain used for signing the reshared key results.
+  // Should be set when previous_epoch is set (resharing produces signed results).
+  Eip712DomainMsg domain = 4;
 }
 
 message EpochResultResponse {

core/service/src/bin/kms-init.rs

@@ -52,6 +52,9 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
                 epoch_id: Some(req_id.clone()),
                 context_id: None,
                 previous_epoch: None,
+                // WARNING: domain is set to None here, as this CLI currently only supports
+                // initializing the KMS for the first epoch.
+                domain: None,
             };
             let _ = kms_client.new_mpc_epoch(request).await.unwrap();
 

core/service/src/client/key_gen.rs

@@ -122,17 +122,18 @@ impl Client {
         })
     }
 
-    #[allow(clippy::too_many_arguments)]
     pub fn new_epoch_request(
         &self,
         to_context_id: &ContextId,
         to_epoch_id: &EpochId,
         previous_epoch: Option<PreviousEpochInfo>,
+        domain: Option<&Eip712Domain>,
     ) -> anyhow::Result<NewMpcEpochRequest> {
         Ok(NewMpcEpochRequest {
             context_id: Some((*to_context_id).into()),
             epoch_id: Some((*to_epoch_id).into()),
             previous_epoch,
+            domain: domain.map(alloy_to_protobuf_domain).transpose()?,
         })
     }
 

core/service/src/client/tests/threshold/crs_gen_tests.rs

@@ -224,7 +224,6 @@ pub async fn wait_for_crsgen_result(
             })
             .collect();
         // domain should always exist
-        let domain_msg = req.domain.clone();
         let domain = protobuf_to_alloy_domain(&req.domain.clone().unwrap()).unwrap();
 
         // we need to setup the storage devices in the right order
@@ -275,7 +274,6 @@ pub async fn wait_for_crsgen_result(
         results.push(CrsInfo {
             crs_id: ref_response.request_id,
             crs_digest: ref_response.crs_digest,
-            domain: domain_msg,
         });
 
         // if there are only THRESHOLD results then we do not have consensus as at least THRESHOLD+1 is needed

core/service/src/client/tests/threshold/misc_tests.rs

@@ -127,6 +127,7 @@ async fn test_threshold_health_endpoint_availability() {
                     epoch_id: Some(req_id.into()),
                     context_id: None,
                     previous_epoch: None,
+                    domain: None,
                 }))
                 .await
         });

core/service/src/client/tests/threshold/misc_tests_isolated.rs

@@ -122,6 +122,7 @@ async fn test_threshold_health_endpoint_availability_isolated() -> Result<()> {
                     epoch_id: Some(req_id.into()),
                     context_id: None,
                     previous_epoch: None,
+                    domain: None,
                 }))
                 .await
         });

core/service/src/client/tests/threshold/mpc_epoch_tests.rs

@@ -5,7 +5,7 @@ use kms_grpc::{
     identifiers::EpochId,
     kms::v1::{EpochResultResponse, FheParameter, KeyGenResult, KeyInfo, PreviousEpochInfo},
     kms_service::v1::core_service_endpoint_client::CoreServiceEndpointClient,
-    rpc_types::{alloy_to_protobuf_domain, protobuf_to_alloy_domain, PubDataType},
+    rpc_types::PubDataType,
     ContextId, RequestId,
 };
 use serial_test::serial;
@@ -40,6 +40,7 @@ use crate::{
     engine::{
         base::{derive_request_id, safe_serialize_hash_element_versioned, DSEP_PUBDATA_KEY},
         threshold::service::ThresholdFheKeys,
+        validation::ResharingParams,
     },
     util::{
         key_setup::{
@@ -210,7 +211,6 @@ pub(crate) async fn new_epoch_with_reshare_and_crs(
                     digest: public_key_digest,
                 },
             ],
-            domain: Some(alloy_to_protobuf_domain(&dummy_domain()).unwrap()),
         });
         keysets.push((
             key_req_id,
@@ -239,11 +239,14 @@ pub(crate) async fn new_epoch_with_reshare_and_crs(
     }
 
     assert_eq!(keys_info.len(), num_keys);
-    let previous_epoch = Some(PreviousEpochInfo {
-        context_id: Some((*DEFAULT_MPC_CONTEXT).into()),
-        epoch_id: Some((*DEFAULT_EPOCH_ID).into()),
-        keys_info,
-        crs_info,
+    let resharing = Some(ResharingParams {
+        previous_epoch: PreviousEpochInfo {
+            context_id: Some((*DEFAULT_MPC_CONTEXT).into()),
+            epoch_id: Some((*DEFAULT_EPOCH_ID).into()),
+            keys_info,
+            crs_info,
+        },
+        signing_domain: dummy_domain(),
     });
 
     // Create the new epoch and reshare from previous one
@@ -257,7 +260,7 @@ pub(crate) async fn new_epoch_with_reshare_and_crs(
         &internal_client,
         new_context_id,
         new_epoch_id,
-        previous_epoch,
+        resharing,
     )
     .await
     .unwrap();
@@ -378,13 +381,18 @@ async fn run_new_epoch(
     internal_client: &Client,
     new_context_id: ContextId,
     new_epoch_id: EpochId,
-    previous_epoch: Option<PreviousEpochInfo>,
+    resharing: Option<ResharingParams>,
 ) -> Option<Vec<(TestKeyGenResult, HashMap<Role, ThresholdFheKeys>)>> {
-    let num_keys = previous_epoch
+    let num_keys = resharing
         .as_ref()
-        .map_or(0, |epoch| epoch.keys_info.len());
+        .map_or(0, |r| r.previous_epoch.keys_info.len());
     let reshare_request = internal_client
-        .new_epoch_request(&new_context_id, &new_epoch_id, previous_epoch.clone())
+        .new_epoch_request(
+            &new_context_id,
+            &new_epoch_id,
+            resharing.as_ref().map(|r| r.previous_epoch.clone()),
+            resharing.as_ref().map(|r| &r.signing_domain),
+        )
         .unwrap();
 
     // Execute reshare
@@ -395,7 +403,7 @@ async fn run_new_epoch(
         tasks_reshare.spawn(async move { client.new_mpc_epoch(req).await });
     }
 
-    if let Some(previous_epoch) = previous_epoch {
+    if let Some(resharing_params) = resharing {
         tasks_reshare.join_all().await.into_iter().for_each(|res| {
             assert!(res.is_ok(), "Reshare party failed: {:?}", res.err());
         });
@@ -446,7 +454,7 @@ async fn run_new_epoch(
                                             .iter()
                                             .find(|kg_result| {
                                                 kg_result.request_id
-                                                    == previous_epoch.keys_info[key_idx].key_id
+                                                    == resharing_params.previous_epoch.keys_info[key_idx].key_id
                                             })
                                             .unwrap_or_else(|| panic!("Each party should have a response for the key {}",
                                                 key_idx))
@@ -462,10 +470,11 @@ async fn run_new_epoch(
 
         assert_eq!(responses_as_dkg.len(), num_keys);
 
-        let crs_info = previous_epoch.crs_info.clone();
+        let crs_info = resharing_params.previous_epoch.crs_info.clone();
 
         let mut outs = Vec::new();
-        for (key_info, responses) in previous_epoch
+        for (key_info, responses) in resharing_params
+            .previous_epoch
             .keys_info
             .into_iter()
             .zip_eq(responses_as_dkg)
@@ -475,7 +484,6 @@ async fn run_new_epoch(
                 preproc_id,
                 key_parameters: _,
                 key_digests: _,
-                domain: _,
             } = key_info;
 
             let preproc_id = preproc_id.as_ref().unwrap().try_into().unwrap();
@@ -486,7 +494,7 @@ async fn run_new_epoch(
                 internal_client,
                 &preproc_id,
                 &key_id,
-                &dummy_domain(),
+                &resharing_params.signing_domain,
                 amount_parties,
                 Some(new_epoch_id.into()),
                 false, // compressed
@@ -504,7 +512,6 @@ async fn run_new_epoch(
 
         for crs in &crs_info {
             let crs_id: RequestId = crs.crs_id.as_ref().unwrap().try_into().unwrap();
-            let domain = protobuf_to_alloy_domain(crs.domain.as_ref().unwrap()).unwrap();
 
             let res_storage: Vec<_> = crs_responses_per_party
                 .iter()
@@ -528,7 +535,7 @@ async fn run_new_epoch(
                 .process_distributed_crs_result(
                     &crs_id,
                     res_storage,
-                    &domain,
+                    &resharing_params.signing_domain,
                     vec![],
                     min_agree_count,
                 )

core/service/src/engine/centralized/service/initiator.rs

@@ -43,17 +43,16 @@ pub async fn init_impl<
     request: Request<NewMpcEpochRequest>,
 ) -> Result<Response<Empty>, MetricedError> {
     let inner = request.into_inner();
-    let (context_id, epoch_id, _previous_epoch_info) =
-        validate_new_mpc_epoch_request(inner).await?;
+    let verified_request = validate_new_mpc_epoch_request(inner)?;
 
     if !service
         .context_manager
-        .mpc_context_exists_and_consistent(&context_id)
+        .mpc_context_exists_and_consistent(&verified_request.context_id)
         .await
         .map_err(|e| {
             MetricedError::new(
                 OP_NEW_EPOCH,
-                Some(context_id.into()),
+                Some(verified_request.context_id.into()),
                 e,
                 tonic::Code::Internal,
             )
@@ -62,8 +61,8 @@ pub async fn init_impl<
     {
         return Err(MetricedError::new(
             OP_NEW_EPOCH,
-            Some(context_id.into()),
-            format!("Context {context_id} not found"),
+            Some(verified_request.context_id.into()),
+            format!("Context {} not found", verified_request.context_id),
             tonic::Code::NotFound,
         ));
     }
@@ -79,26 +78,26 @@ pub async fn init_impl<
         {
             return Err(MetricedError::new(
                 OP_NEW_EPOCH,
-                Some(context_id.into()),
+                Some(verified_request.context_id.into()),
                 "Initialization already complete".to_string(),
                 tonic::Code::AlreadyExists,
             ));
         }
     }
     add_req_to_meta_store(
         &mut service.epoch_ids.write().await,
-        &epoch_id.into(),
+        &verified_request.epoch_id.into(),
         OP_NEW_EPOCH,
     )?;
     update_req_in_meta_store::<(), anyhow::Error>(
         &mut service.epoch_ids.write().await,
-        &epoch_id.into(),
+        &verified_request.epoch_id.into(),
         Ok(()),
         OP_NEW_EPOCH,
     );
     tracing::warn!(
         "Init called on centralized KMS with ID {} - no action taken",
-        epoch_id
+        verified_request.epoch_id
     );
     Ok(Response::new(Empty {}))
 }
@@ -123,6 +122,7 @@ mod tests {
             context_id: None,
             epoch_id: Some(req_id.into()),
             previous_epoch: None,
+            domain: None,
         };
         let result = init_impl(&kms, Request::new(preproc_req)).await;
         let _ = result.unwrap();
@@ -139,6 +139,7 @@ mod tests {
             context_id: None,
             epoch_id: Some(req_id1.into()),
             previous_epoch: None,
+            domain: None,
         };
         let result1 = init_impl(&kms, Request::new(preproc_req1)).await;
         let _ = result1.unwrap();
@@ -148,6 +149,7 @@ mod tests {
             context_id: None,
             epoch_id: Some(req_id1.into()),
             previous_epoch: None,
+            domain: None,
         };
         let result2 = init_impl(&kms, Request::new(preproc_req2)).await;
         let status = result2.unwrap_err();
@@ -162,6 +164,7 @@ mod tests {
             context_id: None,
             epoch_id: None,
             previous_epoch: None,
+            domain: None,
         };
         let result = init_impl(&kms, Request::new(preproc_req)).await;
         let status = result.unwrap_err();