feat(eigen-cllient-extra-features): Address more comments (#386)

* use `H160` for `DEFAULT_EIGENDA_SVC_MANAGER_ADDRESS`

* `parse` instead of `from_str`

* replace val with shif operation

* replace `.map_err` with `.context`

* remove needless parentheses

* use `div_ceil`

* remove needles `Clone` derives

* remove redundant `to_vec()`

* use `anyhow::bail!`

* remove needless `Option` and obscuring `anyhow::Result`

* move verifier test to separate file

* replace u64 constant with Duration

* use `ethabi::encode()`

* refactor `decode_bytes`

* wrap `G1Affine` inside of `Box` / fix clippy

* use `SensitiveUrl` for `eigenda_eth_rpc`

* refactor `save_point` fn

* remove retriable error wrapping

* replace `PKSigningClient` for simpler L1 Client

* remove chain id from `EigenConfig`

* use temp dir for kzg points

* refactor `VerifierClient` trait

* remove `VerifierConfig`

* fix eigen config not initializing correctly

* fix/refactor mock tests

* remove unwraps

* address comments/suggestions

* improve Error handling

* create temp file inside closure

* Remove unnecessary clone

* address comments

* address comments

* change `path()` fn to return `&std::path::Path`

* replace `Path(String)` for `Path(PathBuf)`

---------

Co-authored-by: Gianbelinche <39842759+gianbelinche@users.noreply.github.com>

Author: juan518munoz

Cargo.lock

@@ -550,7 +550,7 @@ impl MainNodeBuilder {
 
             (DAClientConfig::Eigen(mut config), DataAvailabilitySecrets::Eigen(secret)) => {
                 if config.eigenda_eth_rpc.is_none() {
-                    config.eigenda_eth_rpc = Some(l1_secrets.l1_rpc_url.expose_str().to_string());
+                    config.eigenda_eth_rpc = Some(l1_secrets.l1_rpc_url);
                 }
                 self.node.add_layer(EigenWiringLayer::new(config, secret));
             }

core/bin/zksync_server/src/node_builder.rs

@@ -1,10 +1,13 @@
 use std::str::FromStr;
 
 use serde::Deserialize;
-use zksync_basic_types::{secrets::PrivateKey, Address, H160};
+use zksync_basic_types::{secrets::PrivateKey, url::SensitiveUrl, Address, H160};
 
 /// Default address of the EigenDA service manager contract deployed on Holesky.
-const DEFAULT_EIGENDA_SVC_MANAGER_ADDRESS: &str = "0xD4A7E1Bd8015057293f0D0A557088c286942e84b";
+const DEFAULT_EIGENDA_SVC_MANAGER_ADDRESS: H160 = H160([
+    0xd4, 0xa7, 0xe1, 0xbd, 0x80, 0x15, 0x05, 0x72, 0x93, 0xf0, 0xd0, 0xa5, 0x57, 0x08, 0x8c, 0x28,
+    0x69, 0x42, 0xe8, 0x4b,
+]);
 
 /// Configuration for the EigenDA remote disperser client.
 #[derive(Clone, Debug, PartialEq, Deserialize)]
@@ -15,33 +18,33 @@ pub struct EigenConfig {
     /// a value less or equal to 0 means that the disperser will not wait for finalization
     pub settlement_layer_confirmation_depth: u32,
     /// URL of the Ethereum RPC server
-    pub eigenda_eth_rpc: Option<String>,
+    pub eigenda_eth_rpc: Option<SensitiveUrl>,
     /// Address of the service manager contract
     pub eigenda_svc_manager_address: Address,
     /// Wait for the blob to be finalized before returning the response
     pub wait_for_finalization: bool,
     /// Authenticated dispersal
     pub authenticated: bool,
+    /// Optional path to downloaded points directory
+    pub points_dir: Option<String>,
     /// Url to the file containing the G1 point used for KZG
     pub g1_url: String,
     /// Url to the file containing the G2 point used for KZG
     pub g2_url: String,
-    /// Chain ID of the Ethereum network
-    pub chain_id: u64,
 }
 
 impl Default for EigenConfig {
     fn default() -> Self {
         Self {
             disperser_rpc: "https://disperser-holesky.eigenda.xyz:443".to_string(),
             settlement_layer_confirmation_depth: 0,
-            eigenda_eth_rpc: Some("https://ethereum-holesky-rpc.publicnode.com".to_string()),
-            eigenda_svc_manager_address: H160::from_str(DEFAULT_EIGENDA_SVC_MANAGER_ADDRESS).unwrap_or_default(),
+            eigenda_eth_rpc: Some(SensitiveUrl::from_str("https://ethereum-holesky-rpc.publicnode.com").unwrap()), // Safe to unwrap, never fails
+            eigenda_svc_manager_address: DEFAULT_EIGENDA_SVC_MANAGER_ADDRESS,
             wait_for_finalization: false,
             authenticated: false,
+            points_dir: None,
             g1_url: "https://github.com/Layr-Labs/eigenda-proxy/raw/2fd70b99ef5bf137d7bbca3461cf9e1f2c899451/resources/g1.point".to_string(),
             g2_url: "https://github.com/Layr-Labs/eigenda-proxy/raw/2fd70b99ef5bf137d7bbca3461cf9e1f2c899451/resources/g2.point.powerOf2".to_string(),
-            chain_id: 19000,
         }
     }
 }

core/lib/config/src/configs/da_client/eigen.rs

@@ -92,7 +92,7 @@ impl FromEnv for DataAvailabilitySecrets {
 mod tests {
     use std::str::FromStr;
 
-    use zksync_basic_types::H160;
+    use zksync_basic_types::url::SensitiveUrl;
     use zksync_config::{
         configs::{
             da_client::{
@@ -257,9 +257,9 @@ mod tests {
             DA_EIGENDA_SVC_MANAGER_ADDRESS="0x0000000000000000000000000000000000000123"
             DA_WAIT_FOR_FINALIZATION=true
             DA_AUTHENTICATED=false
+            DA_POINTS_DIR="resources/"
             DA_G1_URL="resources1"
             DA_G2_URL="resources2"
-            DA_CHAIN_ID=1
         "#;
         lock.set_env(config);
 
@@ -269,16 +269,15 @@ mod tests {
             DAClientConfig::Eigen(EigenConfig {
                 disperser_rpc: "http://localhost:8080".to_string(),
                 settlement_layer_confirmation_depth: 0,
-                eigenda_eth_rpc: Some("http://localhost:8545".to_string()),
-                eigenda_svc_manager_address: H160::from_str(
-                    "0x0000000000000000000000000000000000000123"
-                )
-                .unwrap(),
+                eigenda_eth_rpc: Some(SensitiveUrl::from_str("http://localhost:8545").unwrap()),
+                eigenda_svc_manager_address: "0x0000000000000000000000000000000000000123"
+                    .parse()
+                    .unwrap(),
                 wait_for_finalization: true,
                 authenticated: false,
+                points_dir: Some("resources/".to_string()),
                 g1_url: "resources1".to_string(),
                 g2_url: "resources2".to_string(),
-                chain_id: 1
             })
         );
     }

core/lib/env_config/src/da_client.rs

@@ -1,3 +1,5 @@
+use std::str::FromStr;
+
 use anyhow::Context;
 use zksync_config::configs::{
     self,
@@ -9,6 +11,7 @@ use zksync_config::configs::{
     },
 };
 use zksync_protobuf::{required, ProtoRepr};
+use zksync_types::url::SensitiveUrl;
 
 use crate::{
     parse_h160,
@@ -66,16 +69,18 @@ impl ProtoRepr for proto::DataAvailabilityClient {
                     &conf.settlement_layer_confirmation_depth,
                 )
                 .context("settlement_layer_confirmation_depth")?,
-                eigenda_eth_rpc: required(&conf.eigenda_eth_rpc).ok().cloned(),
+                eigenda_eth_rpc: Some(SensitiveUrl::from_str(
+                    required(&conf.eigenda_eth_rpc).context("eigenda_eth_rpc")?,
+                )?),
                 eigenda_svc_manager_address: required(&conf.eigenda_svc_manager_address)
                     .and_then(|x| parse_h160(x))
                     .context("eigenda_svc_manager_address")?,
                 wait_for_finalization: *required(&conf.wait_for_finalization)
                     .context("wait_for_finalization")?,
                 authenticated: *required(&conf.authenticated).context("authenticated")?,
+                points_dir: conf.points_dir.clone(),
                 g1_url: required(&conf.g1_url).context("g1_url")?.clone(),
                 g2_url: required(&conf.g2_url).context("g2_url")?.clone(),
-                chain_id: *required(&conf.chain_id).context("chain_id")?,
             }),
             proto::data_availability_client::Config::ObjectStore(conf) => {
                 ObjectStore(object_store_proto::ObjectStore::read(conf)?)
@@ -118,7 +123,10 @@ impl ProtoRepr for proto::DataAvailabilityClient {
                 settlement_layer_confirmation_depth: Some(
                     config.settlement_layer_confirmation_depth,
                 ),
-                eigenda_eth_rpc: config.eigenda_eth_rpc.clone(),
+                eigenda_eth_rpc: config
+                    .eigenda_eth_rpc
+                    .as_ref()
+                    .map(|a| a.expose_str().to_string()),
                 eigenda_svc_manager_address: Some(format!(
                     "{:?}",
                     config.eigenda_svc_manager_address
@@ -127,7 +135,7 @@ impl ProtoRepr for proto::DataAvailabilityClient {
                 authenticated: Some(config.authenticated),
                 g1_url: Some(config.g1_url.clone()),
                 g2_url: Some(config.g2_url.clone()),
-                chain_id: Some(config.chain_id),
+                points_dir: config.points_dir.as_ref().map(|a| a.to_string()),
             }),
             ObjectStore(config) => proto::data_availability_client::Config::ObjectStore(
                 object_store_proto::ObjectStore::build(config),

core/lib/protobuf_config/src/da_client.rs

@@ -45,7 +45,7 @@ message EigenConfig {
   optional bool authenticated = 8;
   optional string g1_url = 9;
   optional string g2_url = 10;
-  optional uint64 chain_id = 11;
+  optional string points_dir = 11;
   reserved 1,2;
   reserved "rpc_node_url","inclusion_polling_interval_ms";
 }

core/lib/protobuf_config/src/proto/config/da_client.proto

@@ -66,6 +66,7 @@ zksync_web3_decl.workspace = true
 zksync_eth_client.workspace = true
 url.workspace = true
 thiserror.workspace = true
+tempfile.workspace = true
 
 [dev-dependencies]
 serial_test.workspace = true

core/node/da_clients/Cargo.toml

@@ -90,19 +90,13 @@ mod tests {
     use backon::{ConstantBuilder, Retryable};
     use serial_test::file_serial;
     use zksync_config::{configs::da_client::eigen::EigenSecrets, EigenConfig};
-    use zksync_da_client::{
-        types::{DAError, DispatchResponse},
-        DataAvailabilityClient,
-    };
+    use zksync_da_client::{types::DispatchResponse, DataAvailabilityClient};
     use zksync_types::secrets::PrivateKey;
 
     use crate::eigen::{blob_info::BlobInfo, EigenClient, GetBlobData};
 
     impl EigenClient {
-        async fn get_blob_data(
-            &self,
-            blob_id: BlobInfo,
-        ) -> anyhow::Result<Option<Vec<u8>>, DAError> {
+        async fn get_blob_data(&self, blob_id: BlobInfo) -> anyhow::Result<Vec<u8>> {
             self.client.get_blob_data(blob_id).await
         }
 
@@ -111,8 +105,8 @@ mod tests {
         }
     }
 
-    const STATUS_QUERY_TIMEOUT: u64 = 1800000; // 30 minutes
-    const STATUS_QUERY_INTERVAL: u64 = 5; // 5 ms
+    const STATUS_QUERY_INTERVAL: Duration = Duration::from_millis(5);
+    const MAX_RETRY_ATTEMPTS: usize = 1800000; // With this value we retry for a duration of 30 minutes
 
     async fn get_blob_info(
         client: &EigenClient,
@@ -127,8 +121,8 @@ mod tests {
         })
         .retry(
             &ConstantBuilder::default()
-                .with_delay(Duration::from_millis(STATUS_QUERY_INTERVAL))
-                .with_max_times((STATUS_QUERY_TIMEOUT / STATUS_QUERY_INTERVAL) as usize),
+                .with_delay(STATUS_QUERY_INTERVAL)
+                .with_max_times(MAX_RETRY_ATTEMPTS),
         )
         .when(|e| e.to_string().contains("Blob not found"))
         .await?;
@@ -177,7 +171,7 @@ mod tests {
             .data;
         assert_eq!(expected_inclusion_data, actual_inclusion_data);
         let retrieved_data = client.get_blob_data(blob_info).await.unwrap();
-        assert_eq!(retrieved_data.unwrap(), data);
+        assert_eq!(retrieved_data, data);
     }
 
     #[ignore = "depends on external RPC"]
@@ -205,7 +199,7 @@ mod tests {
             .data;
         assert_eq!(expected_inclusion_data, actual_inclusion_data);
         let retrieved_data = client.get_blob_data(blob_info).await.unwrap();
-        assert_eq!(retrieved_data.unwrap(), data);
+        assert_eq!(retrieved_data, data);
     }
 
     #[ignore = "depends on external RPC"]
@@ -235,7 +229,7 @@ mod tests {
             .data;
         assert_eq!(expected_inclusion_data, actual_inclusion_data);
         let retrieved_data = client.get_blob_data(blob_info).await.unwrap();
-        assert_eq!(retrieved_data.unwrap(), data);
+        assert_eq!(retrieved_data, data);
     }
 
     #[ignore = "depends on external RPC"]
@@ -263,7 +257,7 @@ mod tests {
             .data;
         assert_eq!(expected_inclusion_data, actual_inclusion_data);
         let retrieved_data = client.get_blob_data(blob_info).await.unwrap();
-        assert_eq!(retrieved_data.unwrap(), data);
+        assert_eq!(retrieved_data, data);
     }
 
     #[ignore = "depends on external RPC"]
@@ -292,6 +286,6 @@ mod tests {
             .data;
         assert_eq!(expected_inclusion_data, actual_inclusion_data);
         let retrieved_data = client.get_blob_data(blob_info).await.unwrap();
-        assert_eq!(retrieved_data.unwrap(), data);
+        assert_eq!(retrieved_data, data);
     }
 }

core/node/da_clients/src/eigen/client.rs

@@ -0,0 +1,111 @@
+use ark_bn254::G1Affine;
+use tonic::{transport::Error as TonicError, Status};
+use zksync_eth_client::EnrichedClientError;
+
+use super::blob_info::BlobQuorumParam;
+
+/// Errors returned by this crate
+#[derive(Debug, thiserror::Error)]
+pub enum EigenClientError {
+    #[error(transparent)]
+    EthClient(#[from] EthClientError),
+    #[error(transparent)]
+    Verification(#[from] VerificationError),
+    #[error(transparent)]
+    Communication(#[from] CommunicationError),
+    #[error(transparent)]
+    BlobStatus(#[from] BlobStatusError),
+    #[error(transparent)]
+    Conversion(#[from] ConversionError),
+    #[error(transparent)]
+    Config(#[from] ConfigError),
+}
+
+#[derive(Debug, thiserror::Error)]
+pub enum ConfigError {
+    #[error(transparent)]
+    Secp(#[from] secp256k1::Error),
+    #[error(transparent)]
+    Tonic(#[from] TonicError),
+}
+
+#[derive(Debug, thiserror::Error)]
+pub enum CommunicationError {
+    #[error(transparent)]
+    Secp(#[from] secp256k1::Error),
+    #[error(transparent)]
+    Hex(#[from] hex::FromHexError),
+    #[error(transparent)]
+    GetBlobData(#[from] Box<dyn std::error::Error + Send + Sync>),
+}
+
+#[derive(Debug, thiserror::Error)]
+pub enum BlobStatusError {
+    #[error(transparent)]
+    Prost(#[from] prost::DecodeError),
+    #[error(transparent)]
+    Status(#[from] Status),
+}
+
+/// Errors specific to conversion
+#[derive(Debug, thiserror::Error)]
+pub enum ConversionError {}
+
+/// Errors for the EthClient
+#[derive(Debug, thiserror::Error)]
+pub enum EthClientError {
+    #[error(transparent)]
+    HTTPClient(#[from] reqwest::Error),
+    #[error(transparent)]
+    SerdeJSON(#[from] serde_json::Error),
+    #[error("RPC: {0}")]
+    Rpc(String),
+}
+
+#[derive(Debug, thiserror::Error)]
+pub enum KzgError {
+    #[error("Kzg setup error: {0}")]
+    Setup(String),
+    #[error(transparent)]
+    Internal(#[from] rust_kzg_bn254::errors::KzgError),
+}
+
+#[derive(Debug, thiserror::Error)]
+pub enum ServiceManagerError {
+    #[error(transparent)]
+    EnrichedClient(#[from] EnrichedClientError),
+    #[error("Decoding error: {0}")]
+    Decoding(String),
+}
+
+/// Errors for the Verifier
+#[derive(Debug, thiserror::Error)]
+pub enum VerificationError {
+    #[error(transparent)]
+    ServiceManager(#[from] ServiceManagerError),
+    #[error(transparent)]
+    Kzg(#[from] KzgError),
+    #[error("Wrong proof")]
+    WrongProof,
+    #[error("Different commitments: expected {expected:?}, got {actual:?}")]
+    DifferentCommitments {
+        expected: Box<G1Affine>,
+        actual: Box<G1Affine>,
+    },
+    #[error("Different roots: expected {expected:?}, got {actual:?}")]
+    DifferentRoots { expected: String, actual: String },
+    #[error("Empty hashes")]
+    EmptyHash,
+    #[error("Different hashes: expected {expected:?}, got {actual:?}")]
+    DifferentHashes { expected: String, actual: String },
+    #[error("Wrong quorum params: {blob_quorum_params:?}")]
+    WrongQuorumParams { blob_quorum_params: BlobQuorumParam },
+    #[error("Quorum not confirmed")]
+    QuorumNotConfirmed,
+    #[error("Commitment not on curve: {0}")]
+    CommitmentNotOnCurve(G1Affine),
+    #[error("Commitment not on correct subgroup: {0}")]
+    CommitmentNotOnCorrectSubgroup(G1Affine),
+    #[error("Point download error: {0}")]
+    PointDownloadError(String),
+}

core/node/da_clients/src/eigen/errors.rs

@@ -1,5 +1,6 @@
 mod blob_info;
 mod client;
+mod errors;
 mod sdk;
 mod verifier;