Skip to content

Commit

Permalink
fix: review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
daniel-savu committed Dec 4, 2023
1 parent fb614f8 commit 5de6a05
Show file tree
Hide file tree
Showing 17 changed files with 89 additions and 160 deletions.
15 changes: 6 additions & 9 deletions rust/agents/relayer/src/relayer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use derive_more::AsRef;
use eyre::Result;
use hyperlane_base::{
db::{HyperlaneRocksDB, DB},
metrics::{AgentMetrics, Metrics},
metrics::{AgentMetrics, AgentMetricsUpdater},
run_all,
settings::ChainConf,
BaseAgent, ContractSyncMetrics, CoreMetrics, HyperlaneAgentCore, SequencedDataContractSync,
Expand Down Expand Up @@ -72,7 +72,7 @@ pub struct Relayer {
skip_transaction_gas_limit_for: HashSet<u32>,
allow_local_checkpoint_syncers: bool,
core_metrics: Arc<CoreMetrics>,
agent_metrics: Metrics,
agent_metrics: AgentMetrics,
}

impl Debug for Relayer {
Expand Down Expand Up @@ -101,7 +101,7 @@ impl BaseAgent for Relayer {
async fn from_settings(
settings: Self::Settings,
core_metrics: Arc<CoreMetrics>,
agent_metrics: Metrics,
agent_metrics: AgentMetrics,
) -> Result<Self>
where
Self: Sized,
Expand Down Expand Up @@ -275,11 +275,8 @@ impl BaseAgent for Relayer {
.agent_metrics_conf(Self::AGENT_NAME.to_string())
.await
.unwrap();
let agent_metrics_fetcher = dest_conf
.build_agent_metrics_fetcher(&self.core_metrics)
.await
.unwrap();
let agent_metrics = AgentMetrics::new(
let agent_metrics_fetcher = dest_conf.build_provider(&self.core_metrics).await.unwrap();
let agent_metrics = AgentMetricsUpdater::new(
self.agent_metrics.clone(),
agent_metrics_conf,
agent_metrics_fetcher,
Expand All @@ -291,7 +288,7 @@ impl BaseAgent for Relayer {
.await;
Ok(())
})
.instrument(info_span!("AgentMetricsFetcher"));
.instrument(info_span!("AgentMetrics"));
tasks.push(fetcher_task);
}

Expand Down
4 changes: 2 additions & 2 deletions rust/agents/scraper/src/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use std::{collections::HashMap, sync::Arc};
use async_trait::async_trait;
use derive_more::AsRef;
use hyperlane_base::{
metrics::Metrics as AgentMetrics, run_all, settings::IndexSettings, BaseAgent,
ContractSyncMetrics, CoreMetrics, HyperlaneAgentCore,
metrics::AgentMetrics, run_all, settings::IndexSettings, BaseAgent, ContractSyncMetrics,
CoreMetrics, HyperlaneAgentCore,
};
use hyperlane_core::HyperlaneDomain;
use tokio::task::JoinHandle;
Expand Down
2 changes: 1 addition & 1 deletion rust/agents/validator/src/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use tracing::{error, info, info_span, instrument::Instrumented, warn, Instrument

use hyperlane_base::{
db::{HyperlaneRocksDB, DB},
metrics::Metrics as AgentMetrics,
metrics::AgentMetrics,
run_all, BaseAgent, CheckpointSyncer, ContractSyncMetrics, CoreMetrics, HyperlaneAgentCore,
SequencedDataContractSync,
};
Expand Down
5 changes: 5 additions & 0 deletions rust/chains/hyperlane-cosmos/src/libs/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,11 @@ pub mod test {
addr.address(),
"neutron1kknekjxg0ear00dky5ykzs8wwp2gz62z9s6aaj"
);
// TODO: watch out for this edge case. This check will fail unless
// the first 12 bytes are removed from the digest.
// let digest = addr.digest();
// let addr2 = CosmosAddress::from_h256(digest, prefix).expect("Cosmos address creation failed");
// assert_eq!(addr.address(), addr2.address());
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion rust/chains/hyperlane-cosmos/src/mailbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ impl Debug for CosmosMailbox {
impl Mailbox for CosmosMailbox {
#[instrument(level = "debug", err, ret, skip(self))]
async fn count(&self, lag: Option<NonZeroU64>) -> ChainResult<u32> {
let block_height = get_block_height_for_lag(&self.provider.grpc(), lag).await?;
let block_height = get_block_height_for_lag(self.provider.grpc(), lag).await?;
self.nonce_at_block(block_height).await
}

Expand Down
6 changes: 3 additions & 3 deletions rust/chains/hyperlane-cosmos/src/merkle_tree_hook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl MerkleTreeHook for CosmosMerkleTreeHook {
tree: general::EmptyStruct {},
};

let block_height = get_block_height_for_lag(&self.provider.grpc(), lag).await?;
let block_height = get_block_height_for_lag(self.provider.grpc(), lag).await?;

let data = self
.provider
Expand Down Expand Up @@ -117,7 +117,7 @@ impl MerkleTreeHook for CosmosMerkleTreeHook {
count: general::EmptyStruct {},
};

let block_height = get_block_height_for_lag(&self.provider.grpc(), lag).await?;
let block_height = get_block_height_for_lag(self.provider.grpc(), lag).await?;

self.count_at_block(block_height).await
}
Expand All @@ -128,7 +128,7 @@ impl MerkleTreeHook for CosmosMerkleTreeHook {
check_point: general::EmptyStruct {},
};

let block_height = get_block_height_for_lag(&self.provider.grpc(), lag).await?;
let block_height = get_block_height_for_lag(self.provider.grpc(), lag).await?;

let data = self
.provider
Expand Down
27 changes: 12 additions & 15 deletions rust/chains/hyperlane-cosmos/src/providers/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use async_trait::async_trait;
use hyperlane_core::{
metrics::agent::AgentMetricsFetcher, BlockInfo, ChainResult, ContractLocator, HyperlaneChain,
HyperlaneDomain, HyperlaneProvider, TxnInfo, H256, U256,
BlockInfo, ChainResult, ContractLocator, HyperlaneChain, HyperlaneDomain, HyperlaneProvider,
TxnInfo, H256, U256,
};
use tendermint_rpc::{client::CompatMode, HttpClient};

Expand Down Expand Up @@ -51,8 +51,8 @@ impl CosmosProvider {
}

/// Get a grpc client
pub fn grpc(&self) -> WasmGrpcProvider {
self.grpc_client.clone()
pub fn grpc(&self) -> &WasmGrpcProvider {
&self.grpc_client
}

/// Get an rpc client
Expand All @@ -71,17 +71,6 @@ impl HyperlaneChain for CosmosProvider {
}
}

#[async_trait]
impl AgentMetricsFetcher for CosmosProvider {
async fn get_balance(&self, address: String) -> ChainResult<U256> {
Ok(self
.grpc_client
.get_balance(address, self.canonical_asset.clone())
.await?
.into())
}
}

#[async_trait]
impl HyperlaneProvider for CosmosProvider {
async fn get_block_by_hash(&self, _hash: &H256) -> ChainResult<BlockInfo> {
Expand All @@ -96,4 +85,12 @@ impl HyperlaneProvider for CosmosProvider {
// FIXME
Ok(true)
}

async fn get_balance(&self, address: String) -> ChainResult<U256> {
Ok(self
.grpc_client
.get_balance(address, self.canonical_asset.clone())
.await?
.into())
}
}
59 changes: 15 additions & 44 deletions rust/chains/hyperlane-ethereum/src/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use std::time::Duration;
use async_trait::async_trait;
use derive_new::new;
use ethers::prelude::Middleware;
use ethers_core::types::Address;
use hyperlane_core::{ethers_core_types, metrics::agent::AgentMetricsFetcher, U256};
use ethers_core::abi::Address;
use hyperlane_core::{ethers_core_types, U256};
use tokio::time::sleep;
use tracing::instrument;

Expand Down Expand Up @@ -45,25 +45,6 @@ where
}
}

#[async_trait]
impl<M> AgentMetricsFetcher for EthereumProvider<M>
where
M: Middleware + 'static,
{
#[instrument(err, skip(self))]
async fn get_balance(&self, address: String) -> ChainResult<U256> {
// Can't use the address directly as a string, because ethers interprets it
// as an ENS name rather than an address.
let addr: Address = address.parse()?;
let balance = self
.provider
.get_balance(addr, None)
.await
.map_err(ChainCommunicationError::from_other)?;
Ok(balance.into())
}
}

#[async_trait]
impl<M> HyperlaneProvider for EthereumProvider<M>
where
Expand Down Expand Up @@ -125,6 +106,19 @@ where
.map_err(ChainCommunicationError::from_other)?;
Ok(!code.is_empty())
}

#[instrument(err, skip(self))]
async fn get_balance(&self, address: String) -> ChainResult<U256> {
// Can't use the address directly as a string, because ethers interprets it
// as an ENS name rather than an address.
let addr: Address = address.parse()?;
let balance = self
.provider
.get_balance(addr, None)
.await
.map_err(ChainCommunicationError::from_other)?;
Ok(balance.into())
}
}

impl<M> EthereumProvider<M>
Expand Down Expand Up @@ -165,29 +159,6 @@ impl BuildableWithProvider for HyperlaneProviderBuilder {
}
}

/// Builder for the Agent Metrics Fetcher.
// TODO: Remove this when trait upcasting is stabilized and Box<dyn HyperlaneProvider> can be used
// as Box<dyn AgentMetricsFetcher>
// Tracking issue:
// https://github.com/rust-lang/rust/issues/65991
pub struct AgentMetricsFetcherBuilder {}

#[async_trait]
impl BuildableWithProvider for AgentMetricsFetcherBuilder {
type Output = Box<dyn AgentMetricsFetcher>;

async fn build_with_provider<M: Middleware + 'static>(
&self,
provider: M,
locator: &ContractLocator,
) -> Self::Output {
Box::new(EthereumProvider::new(
Arc::new(provider),
locator.domain.clone(),
))
}
}

/// Call a get function that returns a Result<Option<T>> and retry if the inner
/// option is None. This can happen because the provider has not discovered the
/// object we are looking for yet.
Expand Down
14 changes: 5 additions & 9 deletions rust/chains/hyperlane-fuel/src/provider.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use async_trait::async_trait;

use hyperlane_core::{
metrics::agent::AgentMetricsFetcher, BlockInfo, ChainResult, HyperlaneChain, HyperlaneDomain,
HyperlaneProvider, TxnInfo, H256, U256,
BlockInfo, ChainResult, HyperlaneChain, HyperlaneDomain, HyperlaneProvider, TxnInfo, H256, U256,
};

/// A wrapper around a fuel provider to get generic blockchain information.
Expand All @@ -19,13 +18,6 @@ impl HyperlaneChain for FuelProvider {
}
}

#[async_trait]
impl AgentMetricsFetcher for FuelProvider {
async fn get_balance(&self, address: String) -> ChainResult<U256> {
todo!()
}
}

#[async_trait]
impl HyperlaneProvider for FuelProvider {
async fn get_block_by_hash(&self, hash: &H256) -> ChainResult<BlockInfo> {
Expand All @@ -39,4 +31,8 @@ impl HyperlaneProvider for FuelProvider {
async fn is_contract(&self, address: &H256) -> ChainResult<bool> {
todo!()
}

async fn get_balance(&self, address: String) -> ChainResult<U256> {
todo!()
}
}
14 changes: 5 additions & 9 deletions rust/chains/hyperlane-sealevel/src/provider.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use async_trait::async_trait;

use hyperlane_core::{
metrics::agent::AgentMetricsFetcher, BlockInfo, ChainResult, HyperlaneChain, HyperlaneDomain,
HyperlaneProvider, TxnInfo, H256, U256,
BlockInfo, ChainResult, HyperlaneChain, HyperlaneDomain, HyperlaneProvider, TxnInfo, H256, U256,
};

/// A wrapper around a Sealevel provider to get generic blockchain information.
Expand Down Expand Up @@ -30,13 +29,6 @@ impl HyperlaneChain for SealevelProvider {
}
}

#[async_trait]
impl AgentMetricsFetcher for SealevelProvider {
async fn get_balance(&self, _address: String) -> ChainResult<U256> {
todo!() // FIXME
}
}

#[async_trait]
impl HyperlaneProvider for SealevelProvider {
async fn get_block_by_hash(&self, _hash: &H256) -> ChainResult<BlockInfo> {
Expand All @@ -51,4 +43,8 @@ impl HyperlaneProvider for SealevelProvider {
// FIXME
Ok(true)
}

async fn get_balance(&self, _address: String) -> ChainResult<U256> {
todo!() // FIXME
}
}
2 changes: 1 addition & 1 deletion rust/hyperlane-base/src/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use tokio::task::JoinHandle;
use tracing::{debug_span, instrument::Instrumented, Instrument};

use crate::{
metrics::{create_agent_metrics, CoreMetrics, Metrics as AgentMetrics},
metrics::{create_agent_metrics, AgentMetrics, CoreMetrics},
settings::Settings,
};

Expand Down
21 changes: 11 additions & 10 deletions rust/hyperlane-base/src/metrics/agent_metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ use derive_builder::Builder;
use derive_new::new;
use eyre::Result;
use hyperlane_core::metrics::agent::u256_as_scaled_f64;
use hyperlane_core::{metrics::agent::AgentMetricsFetcher, HyperlaneDomain};
use hyperlane_core::HyperlaneDomain;
use hyperlane_core::HyperlaneProvider;
use maplit::hashmap;
use prometheus::GaugeVec;
use tokio::time::MissedTickBehavior;
Expand All @@ -27,7 +28,7 @@ pub const WALLET_BALANCE_HELP: &str =

/// Agent-specific metrics
#[derive(Clone, Builder)]
pub struct Metrics {
pub struct AgentMetrics {
/// Current balance of native tokens for the
/// wallet address.
/// - `chain`: the chain name (or chain ID if the name is unknown) of the
Expand All @@ -41,8 +42,8 @@ pub struct Metrics {
wallet_balance: Option<GaugeVec>,
}

pub(crate) fn create_agent_metrics(metrics: &CoreMetrics) -> Result<Metrics> {
Ok(MetricsBuilder::default()
pub(crate) fn create_agent_metrics(metrics: &CoreMetrics) -> Result<AgentMetrics> {
Ok(AgentMetricsBuilder::default()
.wallet_balance(metrics.new_gauge(
"wallet_balance",
WALLET_BALANCE_HELP,
Expand All @@ -67,15 +68,15 @@ pub struct AgentMetricsConf {
pub name: String,
}

/// Utility struct to update agent metrics
/// Utility struct to update agent metrics for a given chain
#[derive(new)]
pub struct AgentMetrics {
metrics: Metrics,
pub struct AgentMetricsUpdater {
metrics: AgentMetrics,
conf: AgentMetricsConf,
fetcher: Box<dyn AgentMetricsFetcher>,
provider: Box<dyn HyperlaneProvider>,
}

impl AgentMetrics {
impl AgentMetricsUpdater {
async fn update_wallet_balances(&self) {
let Some(wallet_addr) = self.conf.address.clone() else {
return;
Expand All @@ -86,7 +87,7 @@ impl AgentMetrics {
};
let chain = self.conf.domain.name();

match self.fetcher.get_balance(wallet_addr.clone()).await {
match self.provider.get_balance(wallet_addr.clone()).await {
Ok(balance) => {
// Okay, so the native type is not a token, but whatever, close enough.
// Note: This is ETH for many chains, but not all so that is why we use `N` and `Native`
Expand Down
Loading

0 comments on commit 5de6a05

Please sign in to comment.