From b04ce6e85ba983938c95c7a24ab64495036b90a1 Mon Sep 17 00:00:00 2001 From: srdtrk <59252793+srdtrk@users.noreply.github.com> Date: Tue, 16 Jul 2024 20:06:25 +0200 Subject: [PATCH] refactor(operator): use extension trait pattern instead of the new type pattern (#65) * refactor: TendermintRpcExt * refactor: LightBlockExt * docs: removed unneeded rustdoc --- operator/src/helpers/light_block.rs | 72 +++++++++---------- operator/src/lib.rs | 2 +- operator/src/rpc.rs | 56 ++++++--------- operator/src/runners/fixtures/membership.rs | 17 ++--- operator/src/runners/fixtures/uc_and_mem.rs | 27 +++---- .../src/runners/fixtures/update_client.rs | 25 +++---- operator/src/runners/genesis.rs | 17 +++-- operator/src/runners/operator.rs | 22 +++--- 8 files changed, 102 insertions(+), 136 deletions(-) diff --git a/operator/src/helpers/light_block.rs b/operator/src/helpers/light_block.rs index 95b28cb..662ecc5 100644 --- a/operator/src/helpers/light_block.rs +++ b/operator/src/helpers/light_block.rs @@ -8,29 +8,34 @@ use sp1_ics07_tendermint_solidity::sp1_ics07_tendermint::{ClientState, Height, T use std::str::FromStr; use tendermint_light_client_verifier::types::LightBlock; -/// A wrapper around a [`LightBlock`] that provides additional methods. +/// Extension trait for [`LightBlock`] that provides additional methods for converting to other +/// types. #[allow(clippy::module_name_repetitions)] -pub struct LightBlockWrapper(LightBlock); - -impl LightBlockWrapper { - /// Create a new instance of the `LightBlockWrapper`. +pub trait LightBlockExt { + /// Convert the [`LightBlock`] to a new solidity [`ClientState`]. + /// + /// # Errors + /// Returns an error if the chain identifier or height cannot be parsed. + fn to_sol_client_state(&self) -> anyhow::Result; + /// Convert the [`LightBlock`] to a new [`ConsensusState`]. #[must_use] - pub const fn new(light_block: LightBlock) -> Self { - Self(light_block) - } - - /// Get the inner `LightBlock`. + fn to_consensus_state(&self) -> ConsensusState; + /// Convert the [`LightBlock`] to a new [`Header`]. + /// + /// # Panics + /// Panics if the `trusted_height` is zero. #[must_use] - pub const fn as_light_block(&self) -> &LightBlock { - &self.0 - } - - /// Convert the [`LightBlockWrapper`] to a new solidity [`ClientState`]. + fn into_header(self, trusted_light_block: &LightBlock) -> Header; + /// Get the chain identifier from the [`LightBlock`]. /// /// # Errors - /// Returns an error if the chain identifier or height cannot be parsed. - pub fn to_sol_client_state(&self) -> anyhow::Result { - let chain_id = ChainId::from_str(self.0.signed_header.header.chain_id.as_str())?; + /// Returns an error if the chain identifier cannot be parsed. + fn chain_id(&self) -> Result; +} + +impl LightBlockExt for LightBlock { + fn to_sol_client_state(&self) -> anyhow::Result { + let chain_id = ChainId::from_str(self.signed_header.header.chain_id.as_str())?; let two_weeks_in_seconds = 14 * 24 * 60 * 60; Ok(ClientState { chain_id: chain_id.to_string(), @@ -40,7 +45,7 @@ impl LightBlockWrapper { }, latest_height: Height { revision_number: chain_id.revision_number().try_into()?, - revision_height: self.0.height().value().try_into()?, + revision_height: self.height().value().try_into()?, }, is_frozen: false, trusting_period: two_weeks_in_seconds, @@ -48,40 +53,29 @@ impl LightBlockWrapper { }) } - /// Convert the [`LightBlockWrapper`] to a new [`ConsensusState`]. - #[must_use] - pub fn to_consensus_state(&self) -> ConsensusState { + fn to_consensus_state(&self) -> ConsensusState { ConsensusState { - timestamp: self.0.signed_header.header.time, - root: CommitmentRoot::from_bytes(self.0.signed_header.header.app_hash.as_bytes()), - next_validators_hash: self.0.signed_header.header.next_validators_hash, + timestamp: self.signed_header.header.time, + root: CommitmentRoot::from_bytes(self.signed_header.header.app_hash.as_bytes()), + next_validators_hash: self.signed_header.header.next_validators_hash, } } - /// Convert the [`LightBlockWrapper`] to a new [`Header`]. - /// - /// # Panics - /// Panics if the `trusted_height` is zero. - #[must_use] - pub fn into_header(self, trusted_light_block: &LightBlock) -> Header { + fn into_header(self, trusted_light_block: &LightBlock) -> Header { let trusted_revision_number = ChainId::from_str(trusted_light_block.signed_header.header.chain_id.as_str()) .unwrap() .revision_number(); let trusted_block_height = trusted_light_block.height().value(); Header { - signed_header: self.0.signed_header, - validator_set: self.0.validators, + signed_header: self.signed_header, + validator_set: self.validators, trusted_height: IbcHeight::new(trusted_revision_number, trusted_block_height).unwrap(), trusted_next_validator_set: trusted_light_block.next_validators.clone(), } } - /// Get the chain identifier from the [`LightBlock`]. - /// - /// # Errors - /// Returns an error if the chain identifier cannot be parsed. - pub fn chain_id(&self) -> Result { - ChainId::from_str(self.0.signed_header.header.chain_id.as_str()) + fn chain_id(&self) -> Result { + ChainId::from_str(self.signed_header.header.chain_id.as_str()) } } diff --git a/operator/src/lib.rs b/operator/src/lib.rs index 1b33d52..9dc6be5 100644 --- a/operator/src/lib.rs +++ b/operator/src/lib.rs @@ -5,5 +5,5 @@ pub mod cli; pub mod helpers; pub mod programs; pub mod prover; -pub mod rpc; +pub(crate) mod rpc; pub mod runners; diff --git a/operator/src/rpc.rs b/operator/src/rpc.rs index ddd1338..d48e99b 100644 --- a/operator/src/rpc.rs +++ b/operator/src/rpc.rs @@ -9,53 +9,42 @@ use tendermint::{block::signed_header::SignedHeader, validator::Set}; use tendermint_light_client_verifier::types::{LightBlock, ValidatorSet}; use tendermint_rpc::{Client, HttpClient, Paging, Url}; -/// A wrapper around the [`HttpClient`] that provides additional methods for +/// An extension trait for [`HttpClient`] that provides additional methods for /// obtaining light blocks. -pub struct TendermintRPCClient(HttpClient); - -impl Default for TendermintRPCClient { - fn default() -> Self { - Self::new() - } -} - -impl TendermintRPCClient { +pub trait TendermintRpcExt { /// Creates a new instance of the Tendermint RPC client from the environment variables. /// /// # Panics /// Panics if the `TENDERMINT_RPC_URL` environment variable is not set or if the URL is /// invalid. #[must_use] - pub fn new() -> Self { - Self( - HttpClient::new::( - Url::from_str(&env::var("TENDERMINT_RPC_URL").expect("TENDERMINT_RPC_URL not set")) - .expect("Failed to parse URL"), - ) - .expect("Failed to create HTTP client"), - ) - } - - /// Get the inner tendermint [`HttpClient`]. - #[must_use] - pub const fn as_tm_client(&self) -> &HttpClient { - &self.0 - } - + fn from_env() -> Self; /// Gets a light block for a specific block height. /// If `block_height` is `None`, the latest block is fetched. /// /// # Errors /// Returns an error if the RPC request fails or if the response cannot be parsed. - pub async fn get_light_block(&self, block_height: Option) -> Result { - let peer_id = self.as_tm_client().status().await?.node_info.id; + async fn get_light_block(&self, block_height: Option) -> Result; +} + +impl TendermintRpcExt for HttpClient { + fn from_env() -> Self { + Self::new::( + Url::from_str(&env::var("TENDERMINT_RPC_URL").expect("TENDERMINT_RPC_URL not set")) + .expect("Failed to parse URL"), + ) + .expect("Failed to create HTTP client") + } + + async fn get_light_block(&self, block_height: Option) -> Result { + let peer_id = self.status().await?.node_info.id; let commit_response; let height; if let Some(block_height) = block_height { - commit_response = self.as_tm_client().commit(block_height).await?; + commit_response = self.commit(block_height).await?; height = block_height; } else { - commit_response = self.as_tm_client().latest_commit().await?; + commit_response = self.latest_commit().await?; height = commit_response .signed_header .header @@ -65,13 +54,10 @@ impl TendermintRPCClient { } let mut signed_header = commit_response.signed_header; - let validator_response = self.as_tm_client().validators(height, Paging::All).await?; + let validator_response = self.validators(height, Paging::All).await?; let validators = Set::new(validator_response.validators, None); - let next_validator_response = self - .as_tm_client() - .validators(height + 1, Paging::All) - .await?; + let next_validator_response = self.validators(height + 1, Paging::All).await?; let next_validators = Set::new(next_validator_response.validators, None); sort_signatures_by_validators_power_desc(&mut signed_header, &validators); diff --git a/operator/src/runners/fixtures/membership.rs b/operator/src/runners/fixtures/membership.rs index 066c50f..0726b67 100644 --- a/operator/src/runners/fixtures/membership.rs +++ b/operator/src/runners/fixtures/membership.rs @@ -2,12 +2,12 @@ use crate::{ cli::command::fixtures::MembershipCmd, - helpers::light_block::LightBlockWrapper, + helpers::light_block::LightBlockExt, programs::{ MembershipProgram, SP1Program, UpdateClientAndMembershipProgram, UpdateClientProgram, }, prover::SP1ICS07TendermintProver, - rpc::TendermintRPCClient, + rpc::TendermintRpcExt, }; use alloy_sol_types::SolValue; use ibc_core_commitment_types::merkle::MerkleProof; @@ -18,7 +18,7 @@ use sp1_ics07_tendermint_solidity::sp1_ics07_tendermint::{ use sp1_ics07_tendermint_utils::convert_tm_to_ics_merkle_proof; use sp1_sdk::HashableKey; use std::path::PathBuf; -use tendermint_rpc::Client; +use tendermint_rpc::{Client, HttpClient}; /// The fixture data to be used in [`UpdateClientProgram`] tests. #[derive(Debug, Clone, Deserialize, Serialize)] @@ -51,14 +51,12 @@ struct SP1ICS07MembershipFixture { pub async fn run(args: MembershipCmd) -> anyhow::Result<()> { assert!(!args.key_paths.is_empty()); - let tm_rpc_client = TendermintRPCClient::default(); + let tm_rpc_client = HttpClient::from_env(); let verify_mem_prover = SP1ICS07TendermintProver::::default(); - let trusted_light_block = LightBlockWrapper::new( - tm_rpc_client - .get_light_block(Some(args.trusted_block)) - .await?, - ); + let trusted_light_block = tm_rpc_client + .get_light_block(Some(args.trusted_block)) + .await?; let trusted_client_state = trusted_light_block.to_sol_client_state()?; let trusted_consensus_state = trusted_light_block.to_consensus_state(); @@ -67,7 +65,6 @@ pub async fn run(args: MembershipCmd) -> anyhow::Result<()> { let kv_proofs: Vec<(String, MerkleProof, Vec)> = futures::future::try_join_all(args.key_paths.into_iter().map(|key_path| async { let res = tm_rpc_client - .as_tm_client() .abci_query( Some("store/ibc/key".to_string()), key_path.as_bytes(), diff --git a/operator/src/runners/fixtures/uc_and_mem.rs b/operator/src/runners/fixtures/uc_and_mem.rs index 7206d0d..a390d48 100644 --- a/operator/src/runners/fixtures/uc_and_mem.rs +++ b/operator/src/runners/fixtures/uc_and_mem.rs @@ -2,12 +2,12 @@ use crate::{ cli::command::fixtures::UpdateClientAndMembershipCmd, - helpers::light_block::LightBlockWrapper, + helpers::light_block::LightBlockExt, programs::{ MembershipProgram, SP1Program, UpdateClientAndMembershipProgram, UpdateClientProgram, }, prover::SP1ICS07TendermintProver, - rpc::TendermintRPCClient, + rpc::TendermintRpcExt, }; use alloy_sol_types::SolValue; use ibc_core_commitment_types::merkle::MerkleProof; @@ -16,7 +16,7 @@ use sp1_ics07_tendermint_solidity::sp1_ics07_tendermint::{Env, UcAndMembershipOu use sp1_ics07_tendermint_utils::convert_tm_to_ics_merkle_proof; use sp1_sdk::HashableKey; use std::path::PathBuf; -use tendermint_rpc::Client; +use tendermint_rpc::{Client, HttpClient}; /// The fixture data to be used in [`UpdateClientProgram`] tests. #[derive(Debug, Clone, Deserialize, Serialize)] @@ -52,23 +52,19 @@ pub async fn run(args: UpdateClientAndMembershipCmd) -> anyhow::Result<()> { "The target block must be greater than the trusted block" ); - let tm_rpc_client = TendermintRPCClient::default(); + let tm_rpc_client = HttpClient::from_env(); let uc_mem_prover = SP1ICS07TendermintProver::::default(); - let trusted_light_block = LightBlockWrapper::new( - tm_rpc_client - .get_light_block(Some(args.trusted_block)) - .await?, - ); - let target_light_block = LightBlockWrapper::new( - tm_rpc_client - .get_light_block(Some(args.target_block)) - .await?, - ); + let trusted_light_block = tm_rpc_client + .get_light_block(Some(args.trusted_block)) + .await?; + let target_light_block = tm_rpc_client + .get_light_block(Some(args.target_block)) + .await?; let trusted_client_state = trusted_light_block.to_sol_client_state()?; let trusted_consensus_state = trusted_light_block.to_consensus_state().into(); - let proposed_header = target_light_block.into_header(trusted_light_block.as_light_block()); + let proposed_header = target_light_block.into_header(&trusted_light_block); let contract_env = Env { chain_id: trusted_light_block.chain_id()?.to_string(), trust_threshold: trusted_client_state.trust_level.clone(), @@ -81,7 +77,6 @@ pub async fn run(args: UpdateClientAndMembershipCmd) -> anyhow::Result<()> { let kv_proofs: Vec<(String, MerkleProof, Vec)> = futures::future::try_join_all(args.key_paths.into_iter().map(|key_path| async { let res = tm_rpc_client - .as_tm_client() .abci_query( Some("store/ibc/key".to_string()), key_path.as_bytes(), diff --git a/operator/src/runners/fixtures/update_client.rs b/operator/src/runners/fixtures/update_client.rs index 5bb9e51..f64c432 100644 --- a/operator/src/runners/fixtures/update_client.rs +++ b/operator/src/runners/fixtures/update_client.rs @@ -2,18 +2,19 @@ use crate::{ cli::command::fixtures::UpdateClientCmd, - helpers::light_block::LightBlockWrapper, + helpers::light_block::LightBlockExt, programs::{ MembershipProgram, SP1Program, UpdateClientAndMembershipProgram, UpdateClientProgram, }, prover::SP1ICS07TendermintProver, - rpc::TendermintRPCClient, + rpc::TendermintRpcExt, }; use alloy_sol_types::SolValue; use serde::{Deserialize, Serialize}; use sp1_ics07_tendermint_solidity::sp1_ics07_tendermint::{Env, UpdateClientOutput}; use sp1_sdk::HashableKey; use std::path::PathBuf; +use tendermint_rpc::HttpClient; /// The fixture data to be used in [`UpdateClientProgram`] tests. #[derive(Debug, Clone, Deserialize, Serialize)] @@ -47,23 +48,19 @@ pub async fn run(args: UpdateClientCmd) -> anyhow::Result<()> { "The target block must be greater than the trusted block" ); - let tendermint_rpc_client = TendermintRPCClient::default(); + let tendermint_rpc_client = HttpClient::from_env(); let uc_prover = SP1ICS07TendermintProver::::default(); - let trusted_light_block = LightBlockWrapper::new( - tendermint_rpc_client - .get_light_block(Some(args.trusted_block)) - .await?, - ); - let target_light_block = LightBlockWrapper::new( - tendermint_rpc_client - .get_light_block(Some(args.target_block)) - .await?, - ); + let trusted_light_block = tendermint_rpc_client + .get_light_block(Some(args.trusted_block)) + .await?; + let target_light_block = tendermint_rpc_client + .get_light_block(Some(args.target_block)) + .await?; let trusted_client_state = trusted_light_block.to_sol_client_state()?; let trusted_consensus_state = trusted_light_block.to_consensus_state().into(); - let proposed_header = target_light_block.into_header(trusted_light_block.as_light_block()); + let proposed_header = target_light_block.into_header(&trusted_light_block); let contract_env = Env { chain_id: trusted_light_block.chain_id()?.to_string(), trust_threshold: trusted_client_state.trust_level.clone(), diff --git a/operator/src/runners/genesis.rs b/operator/src/runners/genesis.rs index 1ce186b..5391a6a 100644 --- a/operator/src/runners/genesis.rs +++ b/operator/src/runners/genesis.rs @@ -2,16 +2,17 @@ use crate::{ cli::command::genesis::Args, - helpers::light_block::LightBlockWrapper, + helpers::light_block::LightBlockExt, programs::{ MembershipProgram, SP1Program, UpdateClientAndMembershipProgram, UpdateClientProgram, }, - rpc::TendermintRPCClient, + rpc::TendermintRpcExt, }; use alloy_sol_types::SolValue; use sp1_ics07_tendermint_solidity::sp1_ics07_tendermint::ConsensusState as SolConsensusState; use sp1_sdk::{utils::setup_logger, HashableKey}; use std::{env, path::PathBuf}; +use tendermint_rpc::HttpClient; /// The genesis data for the SP1 ICS07 Tendermint contract. #[derive(Debug, Clone, serde::Deserialize, serde::Serialize)] @@ -37,17 +38,15 @@ pub async fn run(args: Args) -> anyhow::Result<()> { log::warn!("No .env file found"); } - let tendermint_rpc_client = TendermintRPCClient::default(); + let tendermint_rpc_client = HttpClient::from_env(); - let trusted_light_block = LightBlockWrapper::new( - tendermint_rpc_client - .get_light_block(args.trusted_block) - .await?, - ); + let trusted_light_block = tendermint_rpc_client + .get_light_block(args.trusted_block) + .await?; if args.trusted_block.is_none() { log::info!( "Latest block height: {}", - trusted_light_block.as_light_block().height().value() + trusted_light_block.height().value() ); } diff --git a/operator/src/runners/operator.rs b/operator/src/runners/operator.rs index 94426b3..f5d841f 100644 --- a/operator/src/runners/operator.rs +++ b/operator/src/runners/operator.rs @@ -4,16 +4,17 @@ use std::env; use crate::{ cli::command::operator::Args, - helpers::{self, light_block::LightBlockWrapper}, + helpers::{self, light_block::LightBlockExt}, programs::UpdateClientProgram, prover::SP1ICS07TendermintProver, - rpc::TendermintRPCClient, + rpc::TendermintRpcExt, }; use alloy::providers::ProviderBuilder; use log::{debug, info}; use reqwest::Url; use sp1_ics07_tendermint_solidity::sp1_ics07_tendermint::{self, Env}; use sp1_sdk::utils::setup_logger; +use tendermint_rpc::HttpClient; /// An implementation of a Tendermint Light Client operator that will poll an onchain Tendermint /// light client and generate a proof of the transition from the latest block in the contract to the @@ -37,7 +38,7 @@ pub async fn run(args: Args) -> anyhow::Result<()> { .on_http(Url::parse(rpc_url.as_str())?); let contract = sp1_ics07_tendermint::new(contract_address.parse()?, provider); - let tendermint_rpc_client = TendermintRPCClient::default(); + let tendermint_rpc_client = HttpClient::from_env(); let prover = SP1ICS07TendermintProver::::default(); loop { @@ -50,21 +51,18 @@ pub async fn run(args: Args) -> anyhow::Result<()> { "No trusted height found on the contract. Something is wrong with the contract." ); - let trusted_light_block = LightBlockWrapper::new( - tendermint_rpc_client - .get_light_block(Some(trusted_block_height)) - .await?, - ); + let trusted_light_block = tendermint_rpc_client + .get_light_block(Some(trusted_block_height)) + .await?; // Get trusted consensus state from the trusted light block. let trusted_consensus_state = trusted_light_block.to_consensus_state().into(); - let target_light_block = - LightBlockWrapper::new(tendermint_rpc_client.get_light_block(None).await?); - let target_height = target_light_block.as_light_block().height().value(); + let target_light_block = tendermint_rpc_client.get_light_block(None).await?; + let target_height = target_light_block.height().value(); // Get the proposed header from the target light block. - let proposed_header = target_light_block.into_header(trusted_light_block.as_light_block()); + let proposed_header = target_light_block.into_header(&trusted_light_block); let contract_env = Env { chain_id: trusted_light_block.chain_id()?.to_string(),