Merge branch 'mathias-CRP-1996-remove-nodepublickeydataerror' into 'm…

…aster'

chore(crypto): CRP-1996: Change usages of NodePublicKeyDataError to CspPublicKeyStoreError

Change usages of `NodePublicKeyDataError` to `CspPublicKeyStoreError` since the name `NodePublicKeyDataError` is no longer accurate, and to avoid an additional layer of errors that currently do not bring any value, but require an additional translation step when calling the related methods. The now-unused `NodePublicKeyDataError` is thereafter deleted. 

See merge request dfinity-lab/public/ic!11852

rs/crypto/internal/crypto_service_provider/src/api/keygen.rs

@@ -1,10 +1,7 @@
-mod errors;
-
-pub use errors::*;
 use ic_crypto_node_key_validation::ValidNodePublicKeys;
 
 use super::super::types::{CspPop, CspPublicKey};
-use crate::vault::api::ValidatePksAndSksError;
+use crate::vault::api::{CspPublicKeyStoreError, ValidatePksAndSksError};
 use crate::{ExternalPublicKeys, PksAndSksContainsErrors};
 use ic_crypto_tls_interfaces::TlsPublicKeyCert;
 use ic_types::crypto::{CryptoError, CurrentNodePublicKeys};
@@ -105,24 +102,24 @@ pub trait CspPublicKeyStore {
     /// Returns the node's current public keys where generation timestamps are stripped.
     ///
     /// # Errors
-    /// * [`NodePublicKeyDataError::TransientInternalError`] if there is a transient internal
+    /// * [`CspPublicKeyStoreError::TransientInternalError`] if there is a transient internal
     ///   error when calling the CSP vault.
-    fn current_node_public_keys(&self) -> Result<CurrentNodePublicKeys, NodePublicKeyDataError>;
+    fn current_node_public_keys(&self) -> Result<CurrentNodePublicKeys, CspPublicKeyStoreError>;
 
     /// Returns the node's current public keys with their associated timestamps.
     ///
     /// If timestamps are not needed, you should use [`Self::current_node_public_keys`].
     ///
     /// # Errors
-    /// * [`NodePublicKeyDataError::TransientInternalError`] if there is a transient internal
+    /// * [`CspPublicKeyStoreError::TransientInternalError`] if there is a transient internal
     ///   error when calling the CSP vault.
     fn current_node_public_keys_with_timestamps(
         &self,
-    ) -> Result<CurrentNodePublicKeys, NodePublicKeyDataError>;
+    ) -> Result<CurrentNodePublicKeys, CspPublicKeyStoreError>;
 
     /// Returns the number of iDKG dealing encryption public keys stored locally.
     ///
     /// # Errors
     /// * if a transient error (e.g., RPC timeout) occurs when accessing the public key store
-    fn idkg_dealing_encryption_pubkeys_count(&self) -> Result<usize, NodePublicKeyDataError>;
+    fn idkg_dealing_encryption_pubkeys_count(&self) -> Result<usize, CspPublicKeyStoreError>;
 }

rs/crypto/internal/crypto_service_provider/src/api/mod.rs

@@ -9,9 +9,7 @@ mod tls;
 pub use canister_threshold::{
     CspCreateMEGaKeyError, CspIDkgProtocol, CspThresholdEcdsaSigVerifier, CspThresholdEcdsaSigner,
 };
-pub use keygen::{
-    CspKeyGenerator, CspPublicAndSecretKeyStoreChecker, CspPublicKeyStore, NodePublicKeyDataError,
-};
+pub use keygen::{CspKeyGenerator, CspPublicAndSecretKeyStoreChecker, CspPublicKeyStore};
 pub use sign::{CspSigVerifier, CspSigner};
 pub use threshold::{
     threshold_sign_error::CspThresholdSignError, NiDkgCspClient, ThresholdSignatureCspClient,

rs/crypto/internal/crypto_service_provider/src/lib.rs

@@ -26,8 +26,7 @@ use crate::vault::remote_csp_vault::RemoteCspVault;
 use crate::api::{
     CspIDkgProtocol, CspKeyGenerator, CspPublicAndSecretKeyStoreChecker, CspPublicKeyStore,
     CspSigVerifier, CspSigner, CspThresholdEcdsaSigVerifier, CspThresholdEcdsaSigner,
-    CspTlsHandshakeSignerProvider, NiDkgCspClient, NodePublicKeyDataError,
-    ThresholdSignatureCspClient,
+    CspTlsHandshakeSignerProvider, NiDkgCspClient, ThresholdSignatureCspClient,
 };
 use crate::secret_key_store::SecretKeyStore;
 use crate::types::{CspPublicKey, ExternalPublicKeys};
@@ -232,21 +231,18 @@ impl Csp {
 }
 
 impl CspPublicKeyStore for Csp {
-    fn current_node_public_keys(&self) -> Result<CurrentNodePublicKeys, NodePublicKeyDataError> {
-        let pks = self.csp_vault.current_node_public_keys()?;
-        Ok(pks)
+    fn current_node_public_keys(&self) -> Result<CurrentNodePublicKeys, CspPublicKeyStoreError> {
+        self.csp_vault.current_node_public_keys()
     }
 
     fn current_node_public_keys_with_timestamps(
         &self,
-    ) -> Result<CurrentNodePublicKeys, NodePublicKeyDataError> {
-        let pks = self.csp_vault.current_node_public_keys_with_timestamps()?;
-        Ok(pks)
+    ) -> Result<CurrentNodePublicKeys, CspPublicKeyStoreError> {
+        self.csp_vault.current_node_public_keys_with_timestamps()
     }
 
-    fn idkg_dealing_encryption_pubkeys_count(&self) -> Result<usize, NodePublicKeyDataError> {
-        let idkg_key_count = self.csp_vault.idkg_dealing_encryption_pubkeys_count()?;
-        Ok(idkg_key_count)
+    fn idkg_dealing_encryption_pubkeys_count(&self) -> Result<usize, CspPublicKeyStoreError> {
+        self.csp_vault.idkg_dealing_encryption_pubkeys_count()
     }
 }
 

rs/crypto/internal/crypto_service_provider/src/tests.rs

@@ -11,11 +11,9 @@ mod csp_tests {
     use ic_types::crypto::AlgorithmId;
     use ic_types_test_utils::ids::node_test_id;
 
-    mod node_public_key_data {
+    mod csp_public_key_store {
         use super::*;
-        use crate::vault::api::CspPublicKeyStoreError;
-        use crate::{CspPublicKeyStore, NodePublicKeyDataError};
-        use assert_matches::assert_matches;
+        use crate::CspPublicKeyStore;
         use ic_types::crypto::CurrentNodePublicKeys;
 
         #[test]
@@ -48,16 +46,6 @@ mod csp_tests {
 
             assert_eq!(key_count, 0);
         }
-
-        #[test]
-        fn should_correctly_translate_error() {
-            let csp_error = CspPublicKeyStoreError::TransientInternalError("oh no!".to_string());
-            let node_public_key_data_error = NodePublicKeyDataError::from(csp_error);
-            assert_matches!(
-                node_public_key_data_error,
-                NodePublicKeyDataError::TransientInternalError(internal_error) if internal_error == "oh no!"
-            )
-        }
     }
 
     #[test]

rs/crypto/internal/crypto_service_provider/src/threshold/ni_dkg/mod.rs

@@ -4,9 +4,10 @@
 //! including the secret key store and random number generator, and the
 //! stateless crypto lib.
 
-use crate::api::{CspPublicKeyStore, NiDkgCspClient, NodePublicKeyDataError};
+use crate::api::{CspPublicKeyStore, NiDkgCspClient};
 use crate::key_id::KeyId;
 use crate::types::{CspPublicCoefficients, CspSecretKey};
+use crate::vault::api::CspPublicKeyStoreError;
 use crate::Csp;
 use ic_crypto_internal_threshold_sig_bls12381::api::dkg_errors::InternalError;
 use ic_crypto_internal_threshold_sig_bls12381::api::ni_dkg_errors;
@@ -273,7 +274,7 @@ fn dkg_dealing_encryption_key_id<T: CspPublicKeyStore>(
     let pk = CspFsEncryptionPublicKey::try_from(
         csp.current_node_public_keys()
             .map_err(|error| match error {
-                NodePublicKeyDataError::TransientInternalError(msg) => {
+                CspPublicKeyStoreError::TransientInternalError(msg) => {
                     DkgDealingEncryptionKeyIdRetrievalError::TransientInternalError(msg)
                 }
             })?

rs/crypto/internal/crypto_service_provider/src/vault/api.rs

@@ -18,6 +18,7 @@ use ic_crypto_internal_types::sign::threshold_sig::ni_dkg::{
 };
 use ic_crypto_node_key_validation::ValidNodePublicKeys;
 use ic_crypto_tls_interfaces::TlsPublicKeyCert;
+use ic_interfaces::crypto::CurrentNodePublicKeysError;
 use ic_types::crypto::canister_threshold_sig::error::{
     IDkgCreateDealingError, IDkgLoadTranscriptError, IDkgOpenTranscriptError, IDkgRetainKeysError,
     IDkgVerifyDealingPrivateError, ThresholdEcdsaSignShareError,
@@ -125,6 +126,16 @@ impl From<CspPublicKeyStoreError> for CryptoError {
     }
 }
 
+impl From<CspPublicKeyStoreError> for CurrentNodePublicKeysError {
+    fn from(e: CspPublicKeyStoreError) -> CurrentNodePublicKeysError {
+        match e {
+            CspPublicKeyStoreError::TransientInternalError(details) => {
+                CurrentNodePublicKeysError::TransientInternalError(details)
+            }
+        }
+    }
+}
+
 #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
 pub enum CspTlsKeygenError {
     InvalidNotAfterDate { message: String, not_after: String },

rs/crypto/src/keygen/mod.rs

@@ -3,10 +3,11 @@ mod tests;
 
 use crate::tls::{tls_cert_from_registry_raw, TlsCertFromRegistryError};
 use crate::{key_from_registry, CryptoComponentImpl};
-use ic_crypto_internal_csp::api::NodePublicKeyDataError;
 use ic_crypto_internal_csp::keygen::utils::idkg_dealing_encryption_pk_to_proto;
 use ic_crypto_internal_csp::types::ExternalPublicKeys;
-use ic_crypto_internal_csp::vault::api::{NodeKeysErrors, PksAndSksContainsErrors};
+use ic_crypto_internal_csp::vault::api::{
+    CspPublicKeyStoreError, NodeKeysErrors, PksAndSksContainsErrors,
+};
 use ic_crypto_internal_csp::CryptoServiceProvider;
 use ic_crypto_internal_logmon::metrics::{
     BooleanOperation, BooleanResult, KeyCounts, KeyRotationResult, MetricsResult,
@@ -344,7 +345,7 @@ impl<C: CryptoServiceProvider> CryptoComponentImpl<C> {
             .csp
             .current_node_public_keys()
             .map_err(
-                |NodePublicKeyDataError::TransientInternalError(internal_error)| {
+                |CspPublicKeyStoreError::TransientInternalError(internal_error)| {
                     IDkgDealingEncryptionKeyRotationError::TransientInternalError(internal_error)
                 },
             )?
@@ -354,7 +355,7 @@ impl<C: CryptoServiceProvider> CryptoComponentImpl<C> {
             .csp
             .current_node_public_keys_with_timestamps()
             .map_err(
-                |NodePublicKeyDataError::TransientInternalError(internal_error)| {
+                |CspPublicKeyStoreError::TransientInternalError(internal_error)| {
                     IDkgDealingEncryptionKeyRotationError::TransientInternalError(internal_error)
                 },
             )?
@@ -455,7 +456,7 @@ impl<C: CryptoServiceProvider> CryptoComponentImpl<C> {
                     MetricsResult::Ok,
                 );
             }
-            Err(NodePublicKeyDataError::TransientInternalError(internal_error)) => {
+            Err(CspPublicKeyStoreError::TransientInternalError(internal_error)) => {
                 warn!(
                     self.logger,
                     "Transient error retrieving local iDKG dealing encryption public key count: {}",

rs/crypto/src/keygen/tests.rs

@@ -5,7 +5,6 @@ use assert_matches::assert_matches;
 use ic_base_types::SubnetId;
 use ic_base_types::{NodeId, PrincipalId};
 use ic_crypto_internal_csp::api::CspCreateMEGaKeyError;
-use ic_crypto_internal_csp::api::NodePublicKeyDataError;
 use ic_crypto_internal_csp::vault::api::ExternalPublicKeyError;
 use ic_crypto_internal_csp::vault::api::LocalPublicKeyError;
 use ic_crypto_internal_csp::vault::api::NodeKeysError;
@@ -276,7 +275,7 @@ mod check_keys_with_registry {
                 .with_registry_public_keys(valid_current_node_public_keys())
                 .with_csp_pks_and_sks_contains_result(Ok(()))
                 .with_csp_idkg_dealing_encryption_public_keys_count_result(Err(
-                    NodePublicKeyDataError::TransientInternalError(
+                    CspPublicKeyStoreError::TransientInternalError(
                         "error calling remote csp vault".to_string(),
                     ),
                 ))
@@ -1247,7 +1246,7 @@ mod rotate_idkg_dealing_encryption_keys {
         const ERROR_MSG: &str = "transient error getting current node public keys";
         let setup = Setup::builder()
             .with_csp_current_node_public_keys_result(Err(
-                NodePublicKeyDataError::TransientInternalError(ERROR_MSG.to_string()),
+                CspPublicKeyStoreError::TransientInternalError(ERROR_MSG.to_string()),
             ))
             .with_ecdsa_subnet_config(EcdsaSubnetConfig::new(
                 subnet_id(),
@@ -1768,14 +1767,14 @@ impl Setup {
 
 struct SetupBuilder {
     csp_current_node_public_keys_result:
-        Option<Result<CurrentNodePublicKeys, NodePublicKeyDataError>>,
+        Option<Result<CurrentNodePublicKeys, CspPublicKeyStoreError>>,
     csp_current_node_public_keys_with_timestamps_result:
-        Option<Result<CurrentNodePublicKeys, NodePublicKeyDataError>>,
+        Option<Result<CurrentNodePublicKeys, CspPublicKeyStoreError>>,
     csp_pks_and_sks_contains_result: Option<Result<(), PksAndSksContainsErrors>>,
     registry_client_override: Option<Arc<dyn RegistryClient>>,
     registry_public_keys: Option<CurrentNodePublicKeys>,
     csp_idkg_dealing_encryption_public_keys_count_result:
-        Option<Result<usize, NodePublicKeyDataError>>,
+        Option<Result<usize, CspPublicKeyStoreError>>,
     csp_idkg_gen_dealing_encryption_key_pair_result:
         Option<Result<MEGaPublicKey, CspCreateMEGaKeyError>>,
     logger: Option<ReplicaLogger>,
@@ -1785,7 +1784,7 @@ struct SetupBuilder {
 impl SetupBuilder {
     fn with_csp_current_node_public_keys_result(
         mut self,
-        csp_current_node_public_keys: Result<CurrentNodePublicKeys, NodePublicKeyDataError>,
+        csp_current_node_public_keys: Result<CurrentNodePublicKeys, CspPublicKeyStoreError>,
     ) -> Self {
         self.csp_current_node_public_keys_result = Some(csp_current_node_public_keys);
         self
@@ -1795,7 +1794,7 @@ impl SetupBuilder {
         mut self,
         csp_current_node_public_keys_with_timestamps: Result<
             CurrentNodePublicKeys,
-            NodePublicKeyDataError,
+            CspPublicKeyStoreError,
         >,
     ) -> Self {
         self.csp_current_node_public_keys_with_timestamps_result =
@@ -1829,7 +1828,7 @@ impl SetupBuilder {
 
     fn with_csp_idkg_dealing_encryption_public_keys_count_result(
         mut self,
-        idkg_dealing_encryption_public_keys_count: Result<usize, NodePublicKeyDataError>,
+        idkg_dealing_encryption_public_keys_count: Result<usize, CspPublicKeyStoreError>,
     ) -> Self {
         self.csp_idkg_dealing_encryption_public_keys_count_result =
             Some(idkg_dealing_encryption_public_keys_count);

rs/crypto/test_utils/csp/src/lib.rs

@@ -1,5 +1,4 @@
 use ic_base_types::RegistryVersion;
-use ic_crypto_internal_csp::api::NodePublicKeyDataError;
 use ic_crypto_internal_csp::api::{
     CspCreateMEGaKeyError, CspIDkgProtocol, CspKeyGenerator, CspPublicAndSecretKeyStoreChecker,
     CspPublicKeyStore, CspSigVerifier, CspSigner, CspThresholdEcdsaSigVerifier,
@@ -9,6 +8,7 @@ use ic_crypto_internal_csp::api::{
 use ic_crypto_internal_csp::key_id::KeyId;
 use ic_crypto_internal_csp::types::ExternalPublicKeys;
 use ic_crypto_internal_csp::types::{CspPop, CspPublicCoefficients, CspPublicKey, CspSignature};
+use ic_crypto_internal_csp::vault::api::CspPublicKeyStoreError;
 use ic_crypto_internal_csp::vault::api::PksAndSksContainsErrors;
 use ic_crypto_internal_csp::vault::api::ValidatePksAndSksError;
 use ic_crypto_internal_csp::TlsHandshakeCspVault;
@@ -248,9 +248,9 @@ mock! {
     }
 
     pub trait CspPublicKeyStore {
-        fn current_node_public_keys(&self) -> Result<CurrentNodePublicKeys, NodePublicKeyDataError>;
-        fn current_node_public_keys_with_timestamps(&self) -> Result<CurrentNodePublicKeys, NodePublicKeyDataError>;
-        fn idkg_dealing_encryption_pubkeys_count(&self) -> Result<usize, NodePublicKeyDataError>;
+        fn current_node_public_keys(&self) -> Result<CurrentNodePublicKeys, CspPublicKeyStoreError>;
+        fn current_node_public_keys_with_timestamps(&self) -> Result<CurrentNodePublicKeys, CspPublicKeyStoreError>;
+        fn idkg_dealing_encryption_pubkeys_count(&self) -> Result<usize, CspPublicKeyStoreError>;
     }
 
     pub trait CspTlsHandshakeSignerProvider: Send + Sync {