-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enhancement : Refactor Settings #106
Changes from all commits
d14b790
620fe49
247dfb3
3d19e90
471d6e8
c9e81d1
f194492
5185ccc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,34 +1,19 @@ | ||
use std::str::FromStr; | ||
use serde::{Deserialize, Serialize}; | ||
use utils::settings::Settings; | ||
|
||
use alloy::{network::Ethereum, providers::ProviderBuilder, rpc::client::RpcClient}; | ||
use async_trait::async_trait; | ||
use da_client_interface::DaConfig; | ||
use url::Url; | ||
use utils::env_utils::get_env_var_or_panic; | ||
|
||
use crate::EthereumDaClient; | ||
|
||
#[derive(Clone, Debug)] | ||
#[derive(Clone, Debug, Serialize, Deserialize)] | ||
pub struct EthereumDaConfig { | ||
pub rpc_url: String, | ||
pub memory_pages_contract: String, | ||
pub private_key: String, | ||
} | ||
|
||
#[async_trait] | ||
impl DaConfig<EthereumDaClient> for EthereumDaConfig { | ||
fn new_from_env() -> Self { | ||
Self { | ||
rpc_url: get_env_var_or_panic("SETTLEMENT_RPC_URL"), | ||
memory_pages_contract: get_env_var_or_panic("MEMORY_PAGES_CONTRACT_ADDRESS"), | ||
private_key: get_env_var_or_panic("PRIVATE_KEY"), | ||
} | ||
} | ||
async fn build_client(&self) -> EthereumDaClient { | ||
let client = | ||
RpcClient::new_http(Url::from_str(self.rpc_url.as_str()).expect("Failed to parse SETTLEMENT_RPC_URL")); | ||
let provider = ProviderBuilder::<_, Ethereum>::new().on_client(client); | ||
|
||
EthereumDaClient { provider } | ||
impl EthereumDaConfig { | ||
pub fn new_with_settings(settings: &impl Settings) -> color_eyre::Result<Self> { | ||
Ok(Self { | ||
rpc_url: settings.get_settings("SETTLEMENT_RPC_URL")?, | ||
memory_pages_contract: settings.get_settings("MEMORY_PAGES_CONTRACT_ADDRESS")?, | ||
private_key: settings.get_settings("PRIVATE_KEY")?, | ||
}) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,23 @@ | ||
#![allow(missing_docs)] | ||
#![allow(clippy::missing_docs_in_private_items)] | ||
|
||
use crate::config::EthereumDaConfig; | ||
use alloy::network::Ethereum; | ||
use alloy::providers::RootProvider; | ||
use alloy::providers::{ProviderBuilder, RootProvider}; | ||
use alloy::rpc::client::RpcClient; | ||
use alloy::transports::http::Http; | ||
use async_trait::async_trait; | ||
use color_eyre::Result; | ||
use da_client_interface::{DaClient, DaVerificationStatus}; | ||
use mockall::automock; | ||
use mockall::predicate::*; | ||
use reqwest::Client; | ||
use std::str::FromStr; | ||
use url::Url; | ||
use utils::settings::Settings; | ||
|
||
pub const DA_SETTINGS_NAME: &str = "ethereum"; | ||
|
||
pub mod config; | ||
pub struct EthereumDaClient { | ||
#[allow(dead_code)] | ||
|
@@ -37,3 +45,14 @@ impl DaClient for EthereumDaClient { | |
131072 | ||
} | ||
} | ||
|
||
impl EthereumDaClient { | ||
pub fn new_with_settings(settings: &impl Settings) -> Self { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should restore the build_client one as mentioned above There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
let config = EthereumDaConfig::new_with_settings(settings) | ||
.expect("Not able to create EthereumDaClient from given settings."); | ||
let client = | ||
RpcClient::new_http(Url::from_str(config.rpc_url.as_str()).expect("Failed to parse SETTLEMENT_RPC_URL")); | ||
let provider = ProviderBuilder::<_, Ethereum>::new().on_client(client); | ||
EthereumDaClient { provider } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
use serde::{Deserialize, Serialize}; | ||
use utils::settings::Settings; | ||
|
||
#[derive(Clone, Serialize, Deserialize)] | ||
pub struct AWSSNSConfig { | ||
/// AWS SNS ARN | ||
pub sns_arn: String, | ||
/// AWS SNS region | ||
pub sns_arn_region: String, | ||
} | ||
|
||
impl AWSSNSConfig { | ||
pub fn new_with_settings(settings: &impl Settings) -> color_eyre::Result<Self> { | ||
Ok(Self { | ||
sns_arn: settings.get_settings("AWS_SNS_ARN")?, | ||
sns_arn_region: settings.get_settings("AWS_SNS_REGION")?, | ||
}) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,27 +1,35 @@ | ||
mod config; | ||
|
||
use crate::alerts::aws_sns::config::AWSSNSConfig; | ||
use crate::alerts::Alerts; | ||
use crate::config::ProviderConfig; | ||
use async_trait::async_trait; | ||
use aws_sdk_sns::config::Region; | ||
use aws_sdk_sns::Client; | ||
use utils::env_utils::get_env_var_or_panic; | ||
use utils::settings::Settings; | ||
|
||
pub const AWS_SNS_SETTINGS_NAME: &str = "sns"; | ||
|
||
pub struct AWSSNS { | ||
client: Client, | ||
topic_arn: String, | ||
apoorvsadana marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
impl AWSSNS { | ||
/// To create a new SNS client | ||
pub async fn new() -> Self { | ||
let sns_region = get_env_var_or_panic("AWS_SNS_REGION"); | ||
let config = aws_config::from_env().region(Region::new(sns_region)).load().await; | ||
AWSSNS { client: Client::new(&config) } | ||
pub async fn new_with_settings(settings: &impl Settings, provider_config: ProviderConfig) -> Self { | ||
match provider_config { | ||
ProviderConfig::AWS(aws_config) => { | ||
let sns_config = AWSSNSConfig::new_with_settings(settings) | ||
.expect("Not able to get Aws sns config from provided settings"); | ||
Self { client: Client::new(&aws_config), topic_arn: sns_config.sns_arn } | ||
} | ||
} | ||
} | ||
} | ||
|
||
#[async_trait] | ||
impl Alerts for AWSSNS { | ||
async fn send_alert_message(&self, message_body: String) -> color_eyre::Result<()> { | ||
let topic_arn = get_env_var_or_panic("AWS_SNS_ARN"); | ||
self.client.publish().topic_arn(topic_arn).message(message_body).send().await?; | ||
self.client.publish().topic_arn(self.topic_arn.clone()).message(message_body).send().await?; | ||
Ok(()) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so we actually did this before but then later in the celestia PR we realised that we wanted it to be async and this was the better way to do it, over here #46
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for this to be async ? this is config which should not be an async operation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in celestia it wasn't possible to do it sync, can you check with @heemankv once?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let me check