diff --git a/Cargo.lock b/Cargo.lock index 60aa134f0..648f87837 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -266,7 +266,6 @@ dependencies = [ "gcloud-sdk", "serde", "serial_test", - "thiserror 2.0.18", "tracing", ] @@ -603,8 +602,8 @@ dependencies = [ "alloy-signer-local", "async-trait", "derive_more 2.1.1", + "eyre", "rstest", - "thiserror 2.0.18", "tokio", "tracing", ] diff --git a/crates/agglayer-gcp-kms/Cargo.toml b/crates/agglayer-gcp-kms/Cargo.toml index 57f69ed56..7da17012c 100644 --- a/crates/agglayer-gcp-kms/Cargo.toml +++ b/crates/agglayer-gcp-kms/Cargo.toml @@ -10,7 +10,6 @@ async-trait.workspace = true eyre.workspace = true gcloud-sdk.workspace = true serde = { workspace = true, features = ["derive"] } -thiserror.workspace = true tracing.workspace = true agglayer-config.workspace = true diff --git a/crates/agglayer-gcp-kms/src/error.rs b/crates/agglayer-gcp-kms/src/error.rs deleted file mode 100644 index 3745ee1e4..000000000 --- a/crates/agglayer-gcp-kms/src/error.rs +++ /dev/null @@ -1,19 +0,0 @@ -//! The [`enum@Error`] enum represents errors that can occur in the KMS -//! operations. -//! -//! It includes errors from the KMS provider and configuration errors. - -use thiserror::Error; - -/// Represents errors that can occur in the KMS operations. -#[derive(Debug, Error)] -pub enum Error { - /// This variant is used when a required key or environment variable is - /// missing. - #[error("KMS configuration error: missing key or env {0}")] - KmsConfig(String), - - /// An error occurred while interacting with the KMS provider. - #[error("KMS error: {0}")] - KmsError(eyre::Error), -} diff --git a/crates/agglayer-gcp-kms/src/lib.rs b/crates/agglayer-gcp-kms/src/lib.rs index c601a457b..1c3d948d5 100644 --- a/crates/agglayer-gcp-kms/src/lib.rs +++ b/crates/agglayer-gcp-kms/src/lib.rs @@ -4,16 +4,15 @@ use agglayer_config::GcpKmsConfig; use alloy::signers::gcp::{GcpKeyRingRef, GcpSigner, KeySpecifier}; +use eyre::Context as _; use gcloud_sdk::{ google::cloud::kms::v1::key_management_service_client::KeyManagementServiceClient, GoogleApi, }; use serde::Deserialize; -pub(crate) mod error; pub(crate) mod params; pub(crate) mod signer; -pub use error::Error; pub use signer::KmsSigner; use tracing::debug; @@ -59,15 +58,15 @@ impl KMS { /// /// # Returns /// - /// * `Result` - A result containing the KmsSigner on - /// success, or an Error on failure. + /// * `eyre::Result` - A result containing the KmsSigner on + /// success, or an error report on failure. /// /// # Errors /// /// This function will return an error if it fails to retrieve the required /// environment variables or if there is an issue creating the GCP KMS /// signer. - pub async fn gcp_kms_signers(&self) -> Result { + pub async fn gcp_kms_signers(&self) -> eyre::Result { let params = KMSParameters::try_from(&self.config)?; debug!("Using GCP KMS with parameters: {:?}", params); @@ -89,19 +88,13 @@ impl KMS { let client = GoogleApi::from_function(KeyManagementServiceClient::new, GOOGLE_API_URL, None) .await - .map_err(|e| { - Error::KmsError( - eyre::Error::new(e).wrap_err("Unable to create GoogleApiClient"), - ) - })?; + .wrap_err("Unable to create GoogleApiClient")?; // Use GcpSigner::new with the proper client type let pp_settlement_gcp_signer = GcpSigner::new(client.clone(), pp_settlement_specifier, Some(self.chain_id)) .await - .map_err(|e| { - Error::KmsError(eyre::Error::new(e).wrap_err("Unable to create GcpSigner")) - })?; + .wrap_err("Unable to create PP settlement GcpSigner")?; let is_the_same_key = params.key_name_pp_settlement == params.key_name_tx_settlement && params.key_version_pp_settlement == params.key_version_tx_settlement; @@ -112,9 +105,7 @@ impl KMS { Some( GcpSigner::new(client, tx_settlement_specifier, Some(self.chain_id)) .await - .map_err(|e| { - Error::KmsError(eyre::Error::new(e).wrap_err("Unable to create GcpSigner")) - })?, + .wrap_err("Unable to create tx settlement GcpSigner")?, ) }; diff --git a/crates/agglayer-gcp-kms/src/params.rs b/crates/agglayer-gcp-kms/src/params.rs index 6b9784d6e..7bf384acb 100644 --- a/crates/agglayer-gcp-kms/src/params.rs +++ b/crates/agglayer-gcp-kms/src/params.rs @@ -5,10 +5,9 @@ use std::str::FromStr; use agglayer_config::GcpKmsConfig; +use eyre::eyre; use tracing::warn; -pub use crate::error::Error; - const GOOGLE_PROJECT_ID: &str = "GOOGLE_PROJECT_ID"; const GOOGLE_LOCATION: &str = "GOOGLE_LOCATION"; const GOOGLE_KEYRING: &str = "GOOGLE_KEYRING"; @@ -42,20 +41,20 @@ fn from_env_or_conf( env_key: &str, env_key_fallback: Option<&str>, config_value: &Option, -) -> Result +) -> eyre::Result where T: FromStr + Clone, { if let Ok(value) = std::env::var(env_key) { return value .parse() - .map_err(|_| Error::KmsConfig(env_key.to_string())); + .map_err(|_| eyre!("KMS configuration error: invalid value for key or env {env_key}")); } else if let Some(env_key_fallback) = env_key_fallback { if let Ok(value) = std::env::var(env_key_fallback) { warn!("Fallback KMS env:{env_key}=>env:{env_key_fallback}"); - return value - .parse() - .map_err(|_| Error::KmsConfig(env_key_fallback.to_string())); + return value.parse().map_err(|_| { + eyre!("KMS configuration error: invalid value for key or env {env_key_fallback}") + }); } } @@ -63,11 +62,13 @@ where return Ok(config_value.clone()); } - Err(Error::KmsConfig(env_key.to_string())) + Err(eyre!( + "KMS configuration error: missing key or env {env_key}" + )) } impl TryFrom<&GcpKmsConfig> for KMSParameters { - type Error = Error; + type Error = eyre::Report; fn try_from(config: &GcpKmsConfig) -> Result { // Get configuration values from environment variables or config let project_id = from_env_or_conf(GOOGLE_PROJECT_ID, None, &config.project_id)?; @@ -223,4 +224,62 @@ mod test { assert_eq!(params.key_name_tx_settlement, "conf_key_name_tx"); assert_eq!(params.key_version_tx_settlement, 2); } + + #[test] + #[serial] + fn missing_required_value_reports_kms_context() { + let _raii = set_env(false, false); + let config = GcpKmsConfig { + project_id: None, + location: None, + keyring: None, + pp_settlement_key_name: None, + pp_settlement_key_version: None, + tx_settlement_key_name: None, + tx_settlement_key_version: None, + }; + + let error = crate::KMSParameters::try_from(&config).unwrap_err(); + + assert_eq!( + error.to_string(), + "KMS configuration error: missing key or env GOOGLE_PROJECT_ID" + ); + } + + #[test] + #[serial] + fn invalid_primary_env_value_reports_kms_context() { + let _raii = ( + set_env(false, false), + SetEnvGuard::new("GOOGLE_KEY_VERSION_PP_SETTLEMENT", Some("invalid")), + ); + let config = config(); + + let error = crate::KMSParameters::try_from(&config).unwrap_err(); + + assert_eq!( + error.to_string(), + "KMS configuration error: invalid value for key or env \ + GOOGLE_KEY_VERSION_PP_SETTLEMENT" + ); + } + + #[test] + #[serial] + fn invalid_fallback_env_value_reports_kms_context() { + let _raii = ( + set_env(false, false), + SetEnvGuard::new("GOOGLE_KEY_VERSION", Some("invalid")), + ); + let mut config = config(); + config.pp_settlement_key_version = None; + + let error = crate::KMSParameters::try_from(&config).unwrap_err(); + + assert_eq!( + error.to_string(), + "KMS configuration error: invalid value for key or env GOOGLE_KEY_VERSION" + ); + } } diff --git a/crates/agglayer-gcp-kms/src/signer.rs b/crates/agglayer-gcp-kms/src/signer.rs index 80aaafe6f..3e393fe60 100644 --- a/crates/agglayer-gcp-kms/src/signer.rs +++ b/crates/agglayer-gcp-kms/src/signer.rs @@ -8,8 +8,7 @@ use alloy::{ }; use alloy_primitives::{Address, ChainId, Signature, B256}; use async_trait::async_trait; - -use crate::Error; +use eyre::Context as _; /// A wrapper around [`GcpSigner`] providing additional functionality /// for signing messages and transactions. @@ -29,24 +28,22 @@ impl KmsSigner { pub async fn sign_message>( &self, message: S, - ) -> Result { + ) -> eyre::Result { self.signer .sign_message(message.as_ref()) .await - .map_err(|e| Error::KmsError(eyre::Error::new(e).wrap_err("Unable to sign message"))) + .wrap_err("Unable to sign message") } /// Signs a transaction using the internal signer, this method can fail if /// the signer fails to create the digest. - pub async fn sign_transaction(&self, tx: &TypedTransaction) -> Result { + pub async fn sign_transaction(&self, tx: &TypedTransaction) -> eyre::Result { // Convert the TypedTransaction to a mutable dyn SignableTransaction let mut tx_clone = tx.clone(); self.signer .sign_transaction(&mut tx_clone) .await - .map_err(|e| { - Error::KmsError(eyre::Error::new(e).wrap_err("Unable to sign transaction")) - }) + .wrap_err("Unable to sign transaction") } /// Returns the address associated with the signer. diff --git a/crates/agglayer-signer/Cargo.toml b/crates/agglayer-signer/Cargo.toml index be59a8caa..abc01be5b 100644 --- a/crates/agglayer-signer/Cargo.toml +++ b/crates/agglayer-signer/Cargo.toml @@ -5,7 +5,7 @@ edition = "2021" [dependencies] async-trait.workspace = true -thiserror.workspace = true +eyre.workspace = true alloy.workspace = true alloy-primitives.workspace = true diff --git a/crates/agglayer-signer/src/error.rs b/crates/agglayer-signer/src/error.rs deleted file mode 100644 index ce10a67fd..000000000 --- a/crates/agglayer-signer/src/error.rs +++ /dev/null @@ -1,19 +0,0 @@ -use agglayer_gcp_kms::Error as GcpKmsError; -use alloy::signers::{local::LocalSignerError, Error as SignerError}; -use thiserror::Error; - -/// Errors that can occur when using a -/// [`ConfiguredSigner`](enum@super::ConfiguredSigner). -/// -/// This is a union of signer errors, local signer errors, and GCP KMS errors. -#[derive(Debug, Error)] -pub enum Error { - #[error("no private keys specified in the configuration")] - NoPk, - #[error("signer error: {0}")] - Signer(#[from] SignerError), - #[error("local signer error: {0}")] - LocalSigner(#[from] LocalSignerError), - #[error("GcpKMS error: {0}")] - GcpKms(#[from] GcpKmsError), -} diff --git a/crates/agglayer-signer/src/lib.rs b/crates/agglayer-signer/src/lib.rs index 9e0425ce1..5375cc52b 100644 --- a/crates/agglayer-signer/src/lib.rs +++ b/crates/agglayer-signer/src/lib.rs @@ -13,10 +13,7 @@ use alloy::{ }; use alloy_primitives::{Address, ChainId, Signature, B256}; use async_trait::async_trait; - -mod error; - -pub use error::Error; +use eyre::{eyre, Context as _}; /// A an alloy [`Signer`] that can house either a local keystore or a KMS /// signer. @@ -33,19 +30,23 @@ pub enum ConfiguredSigner { impl ConfiguredSigner { /// Decrypt the first local keystore specified in the configuration. - #[allow(clippy::result_large_err)] pub(crate) fn local_wallet( chain_id: u64, local: &LocalConfig, - ) -> Result<(PrivateKeySigner, Option), Error> { - let pk1 = local.private_keys.first().ok_or(Error::NoPk)?; - let signer1 = PrivateKeySigner::decrypt_keystore(&pk1.path, &pk1.password)? + ) -> eyre::Result<(PrivateKeySigner, Option)> { + let pk1 = local + .private_keys + .first() + .ok_or_else(|| eyre!("no private keys specified in the configuration"))?; + let signer1 = PrivateKeySigner::decrypt_keystore(&pk1.path, &pk1.password) + .wrap_err("local signer error")? .with_chain_id(Some(chain_id)); let mut signer2 = None; if let Some(pk2) = local.private_keys.get(1) { signer2 = Some( - PrivateKeySigner::decrypt_keystore(&pk2.path, &pk2.password)? + PrivateKeySigner::decrypt_keystore(&pk2.path, &pk2.password) + .wrap_err("local signer error")? .with_chain_id(Some(chain_id)), ) }; @@ -81,7 +82,7 @@ pub struct ConfiguredSigners { impl ConfiguredSigners { /// Get either a local wallet or GCP KMS signer based on the configuration. - pub async fn new(config: &Config) -> Result { + pub async fn new(config: &Config) -> eyre::Result { match &config.auth { AuthConfig::GcpKms(ref kms) => { let kms = KMS::new(config.l1.chain_id, kms.clone()); @@ -189,13 +190,13 @@ impl ConfiguredSigner { async fn sign_transaction_local( wallet: &PrivateKeySigner, tx: &TypedTransaction, - ) -> Result { + ) -> eyre::Result { // Convert the TypedTransaction to a mutable dyn SignableTransaction let mut tx_clone = tx.clone(); wallet .sign_transaction(&mut tx_clone) .await - .map_err(Error::Signer) + .wrap_err("signer error") } /// Signs a transaction using the appropriate signer. @@ -203,11 +204,11 @@ impl ConfiguredSigner { /// This method provides transaction signing functionality that delegates /// to the underlying signer implementation. #[inline] - pub async fn sign_transaction_typed(&self, tx: &TypedTransaction) -> Result { + pub async fn sign_transaction_typed(&self, tx: &TypedTransaction) -> eyre::Result { match self { ConfiguredSigner::Local(wallet) => Self::sign_transaction_local(wallet, tx).await, ConfiguredSigner::Kms(signer) => { - signer.sign_transaction(tx).await.map_err(Error::GcpKms) + signer.sign_transaction(tx).await.wrap_err("GcpKMS error") } } } diff --git a/crates/agglayer-signer/src/tests.rs b/crates/agglayer-signer/src/tests.rs index a69157d4c..ce116208d 100644 --- a/crates/agglayer-signer/src/tests.rs +++ b/crates/agglayer-signer/src/tests.rs @@ -1,3 +1,4 @@ +use agglayer_config::{AuthConfig, Config, LocalConfig}; use alloy::{ consensus::{SignableTransaction, TxEip1559, TypedTransaction}, signers::Signer, @@ -86,3 +87,18 @@ fn constructor_methods_work() { assert!(configured_signer.is_local()); assert!(!configured_signer.is_kms()); } + +#[tokio::test] +async fn configured_signers_reports_missing_private_key() { + let mut config = Config::new(std::path::Path::new("/tmp/agglayer")); + config.auth = AuthConfig::Local(LocalConfig { + private_keys: Vec::new(), + }); + + let error = ConfiguredSigners::new(&config).await.unwrap_err(); + + assert_eq!( + error.to_string(), + "no private keys specified in the configuration" + ); +}