Skip to content

Commit

Permalink
Fix for regression #1229 using new type ics02::TrustThreshold
Browse files Browse the repository at this point in the history
  • Loading branch information
adizere committed Aug 2, 2021
1 parent 5ff02a0 commit f9e0d5f
Show file tree
Hide file tree
Showing 15 changed files with 183 additions and 56 deletions.
17 changes: 12 additions & 5 deletions modules/src/ics02_client/client_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ use std::time::Duration;

use prost_types::Any;
use serde::{Deserialize, Serialize};
use tendermint::trust_threshold::TrustThresholdFraction;
use tendermint_proto::Protobuf;

use ibc_proto::ibc::core::client::v1::IdentifiedClientState;

use crate::ics02_client::client_type::ClientType;
use crate::ics02_client::error::Error;
use crate::ics02_client::trust_threshold::TrustThreshold;
use crate::ics07_tendermint::client_state;
use crate::ics24_host::error::ValidationError;
use crate::ics24_host::identifier::{ChainId, ClientId};
Expand Down Expand Up @@ -59,7 +59,7 @@ impl AnyClientState {
}
}

pub fn trust_threshold(&self) -> Option<TrustThresholdFraction> {
pub fn trust_threshold(&self) -> Option<TrustThreshold> {
match self {
AnyClientState::Tendermint(state) => Some(state.trust_level),

Expand Down Expand Up @@ -102,13 +102,20 @@ impl TryFrom<Any> for AnyClientState {
type Error = Error;

fn try_from(raw: Any) -> Result<Self, Self::Error> {
eprintln!("trying to convert from {:#?}", raw.type_url);
match raw.type_url.as_str() {
"" => Err(Error::empty_client_state_response()),

TENDERMINT_CLIENT_STATE_TYPE_URL => Ok(AnyClientState::Tendermint(
TENDERMINT_CLIENT_STATE_TYPE_URL => {
eprintln!("trying to convert from TM TYPE");
return Ok(AnyClientState::Tendermint(
client_state::ClientState::decode_vec(&raw.value)
.map_err(Error::decode_raw_client_state)?,
)),
.map_err( |e|{
eprintln!("found error :( {:?}", e);

Error::decode_raw_client_state(e) })?,
));
}

#[cfg(any(test, feature = "mocks"))]
MOCK_CLIENT_STATE_TYPE_URL => Ok(AnyClientState::Mock(
Expand Down
11 changes: 10 additions & 1 deletion modules/src/ics02_client/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::Height;
use std::num::TryFromIntError;
use tendermint_proto::Error as TendermintError;

use flex_error::{define_error, TraceError};
use flex_error::{define_error, DisplayOnly, TraceError};

define_error! {
Error {
Expand Down Expand Up @@ -49,6 +49,15 @@ define_error! {
{ reason: String }
| e | { format_args!("header verification failed with reason: {}", e.reason) },

InvalidTrustThreshold
{ numerator: u64, denominator: u64 }
| e | { format_args!("failed to build trust threshold from fraction: {}/{}", e.numerator, e.denominator) },

FailedTrustThresholdConversion
{ numerator: u64, denominator: u64 }
[ DisplayOnly<Box<dyn std::error::Error + Send + Sync>> ]
| e | { format_args!("failed to build Tendermint-domain type trust threshold from fraction: {}/{}", e.numerator, e.denominator) },

UnknownClientStateType
{ client_state_type: String }
| e | { format_args!("unknown client state type: {0}", e.client_state_type) },
Expand Down
3 changes: 1 addition & 2 deletions modules/src/ics02_client/handler/create_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@ mod tests {
use std::time::Duration;
use test_env_log::test;

use tendermint::trust_threshold::TrustThresholdFraction as TrustThreshold;

use crate::events::IbcEvent;
use crate::handler::HandlerOutput;
use crate::ics02_client::client_consensus::AnyConsensusState;
Expand All @@ -71,6 +69,7 @@ mod tests {
use crate::ics02_client::handler::{dispatch, ClientResult};
use crate::ics02_client::msgs::create_client::MsgCreateAnyClient;
use crate::ics02_client::msgs::ClientMsg;
use crate::ics02_client::trust_threshold::TrustThreshold;
use crate::ics07_tendermint::client_state::{AllowUpdate, ClientState};
use crate::ics07_tendermint::header::test_util::get_dummy_tendermint_header;
use crate::ics24_host::identifier::ClientId;
Expand Down
1 change: 1 addition & 0 deletions modules/src/ics02_client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ pub mod header;
pub mod height;
pub mod misbehaviour;
pub mod msgs;
pub mod trust_threshold;
128 changes: 128 additions & 0 deletions modules/src/ics02_client/trust_threshold.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
//! IBC Domain type definition for [`TrustThreshold`]
//! represented as a fraction with valid values in the
//! range `[0, 1)`.
use std::{convert::TryFrom, fmt};

use serde::{Deserialize, Serialize};
use tendermint_proto::Protobuf;

use ibc_proto::ibc::lightclients::tendermint::v1::Fraction;
use tendermint::trust_threshold::TrustThresholdFraction;

use crate::ics02_client::error::Error;

/// [`TrustThreshold`] defines the level of trust that a client has
/// towards a set of validators of a chain.
///
/// A trust threshold is represented as a fraction, i.e., a numerator and
/// and a denominator.
/// A typical trust threshold is 1/3 in practice.
/// This type accepts even a value of 0, (numerator = 0, denominator = 0),
/// which is used in the client state of an upgrading client.
#[derive(Copy, Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub struct TrustThreshold {
numerator: u64,
denominator: u64,
}

impl TrustThreshold {
/// Constant for a trust threshold of 1/3.
pub const ONE_THIRD: Self = Self {
numerator: 1,
denominator: 3,
};

/// Constant for a trust threshold of 2/3.
pub const TWO_THIRDS: Self = Self {
numerator: 2,
denominator: 3,
};

/// Constant for a trust threshold of 0/0.
pub const ZERO: Self = Self {
numerator: 0,
denominator: 0,
};

/// Instantiate a TrustThreshold with the given denominator and
/// numerator.
///
/// The constructor succeeds if long as the resulting fraction
/// is in the range`[0, 1)`.
pub fn new(numerator: u64, denominator: u64) -> Result<Self, Error> {
// The two parameters cannot yield a fraction that is bigger or equal to 1
if (numerator > denominator) || (denominator == 0 && numerator != 0) ||
(numerator == denominator && numerator != 0) {
return Err(Error::invalid_trust_threshold(numerator, denominator));
}

Ok(Self {
numerator,
denominator,
})
}

/// The numerator of the fraction underlying this trust threshold.
pub fn numerator(&self) -> u64 {
self.numerator
}

/// The denominator of the fraction underlying this trust threshold.
pub fn denominator(&self) -> u64 {
self.denominator
}
}

/// Conversion from Tendermint domain type into
/// IBC domain type.
impl From<TrustThresholdFraction> for TrustThreshold {
fn from(t: TrustThresholdFraction) -> Self {
Self {
numerator: t.numerator(),
denominator: t.denominator(),
}
}
}

/// Conversion from IBC domain type into
/// Tendermint domain type.
impl TryFrom<TrustThreshold> for TrustThresholdFraction {
type Error = Error;

fn try_from(t: TrustThreshold) -> Result<TrustThresholdFraction, Error> {
Self::new(t.numerator, t.denominator)
.map_err(|e| Error::failed_trust_threshold_conversion(t.numerator, t.denominator, e))
}
}

impl Protobuf<Fraction> for TrustThreshold {}

impl From<TrustThreshold> for Fraction {
fn from(t: TrustThreshold) -> Self {
Self {
numerator: t.numerator,
denominator: t.denominator,
}
}
}

impl TryFrom<Fraction> for TrustThreshold {
type Error = Error;

fn try_from(value: Fraction) -> Result<Self, Self::Error> {
Self::new(value.numerator, value.denominator)
}
}

impl Default for TrustThreshold {
fn default() -> Self {
Self::ONE_THIRD
}
}

impl fmt::Display for TrustThreshold {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}/{}", self.numerator, self.denominator)
}
}
20 changes: 8 additions & 12 deletions modules/src/ics07_tendermint/client_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,13 @@ use std::str::FromStr;
use std::time::Duration;

use serde::{Deserialize, Serialize};
use tendermint::trust_threshold::{
TrustThresholdFraction as TrustThreshold, TrustThresholdFraction,
};
use tendermint_proto::Protobuf;

use ibc_proto::ibc::lightclients::tendermint::v1::{ClientState as RawClientState, Fraction};
use ibc_proto::ibc::lightclients::tendermint::v1::ClientState as RawClientState;

use crate::ics02_client::client_state::AnyClientState;
use crate::ics02_client::client_type::ClientType;
use crate::ics02_client::trust_threshold::TrustThreshold;
use crate::ics07_tendermint::error::Error;
use crate::ics07_tendermint::header::Header;
use crate::ics23_commitment::specs::ProofSpecs;
Expand Down Expand Up @@ -111,11 +109,11 @@ impl ClientState {
}
}

/// Helper function to verify the upgrade client procedure.
/// Helper function for the upgrade chain & client procedures.
/// Resets all fields except the blockchain-specific ones.
pub fn zero_custom_fields(mut client_state: Self) -> Self {
client_state.trusting_period = ZERO_DURATION;
client_state.trust_level = TrustThresholdFraction::default();
client_state.trust_level = TrustThreshold::ZERO;
client_state.allow_update.after_expiry = false;
client_state.allow_update.after_misbehaviour = false;
client_state.frozen_height = Height::zero();
Expand Down Expand Up @@ -170,7 +168,8 @@ impl TryFrom<RawClientState> for ClientState {
Ok(Self {
chain_id: ChainId::from_str(raw.chain_id.as_str())
.map_err(Error::invalid_chain_identifier)?,
trust_level: TrustThreshold::new(trust_level.numerator, trust_level.denominator)
trust_level: trust_level
.try_into()
.map_err(|e| Error::invalid_trust_threshold(format!("{}", e)))?,
trusting_period: raw
.trusting_period
Expand Down Expand Up @@ -208,10 +207,7 @@ impl From<ClientState> for RawClientState {
fn from(value: ClientState) -> Self {
RawClientState {
chain_id: value.chain_id.to_string(),
trust_level: Some(Fraction {
numerator: value.trust_level.numerator(),
denominator: value.trust_level.denominator(),
}),
trust_level: Some(value.trust_level.into()),
trusting_period: Some(value.trusting_period.into()),
unbonding_period: Some(value.unbonding_period.into()),
max_clock_drift: Some(value.max_clock_drift.into()),
Expand All @@ -230,9 +226,9 @@ mod tests {
use std::time::Duration;
use test_env_log::test;

use tendermint::trust_threshold::TrustThresholdFraction as TrustThreshold;
use tendermint_rpc::endpoint::abci_query::AbciQuery;

use crate::ics02_client::trust_threshold::TrustThreshold;
use crate::ics07_tendermint::client_state::{AllowUpdate, ClientState};
use crate::ics24_host::identifier::ChainId;
use crate::test::test_serialization_roundtrip;
Expand Down
16 changes: 0 additions & 16 deletions relayer-cli/src/commands/tx/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,7 @@ pub struct TxUpgradeClientCmd {
}

impl Runnable for TxUpgradeClientCmd {
#[allow(unreachable_code)]
fn run(&self) {
tracing::error!("This command is currently disabled due to a regression in Hermes v0.6.1.");
tracing::error!("Please track the following issue for background and progress:");
tracing::error!("");
tracing::error!(" https://github.com/informalsystems/ibc-rs/issues/1229");

std::process::exit(1);

let config = app_config();

let dst_chain = match spawn_chain_runtime(&config, &self.chain_id) {
Expand Down Expand Up @@ -187,15 +179,7 @@ pub struct TxUpgradeClientsCmd {
}

impl Runnable for TxUpgradeClientsCmd {
#[allow(unreachable_code)]
fn run(&self) {
tracing::error!("This command is currently disabled due to a regression in Hermes v0.6.1.");
tracing::error!("Please track the following issue for background and progress:");
tracing::error!("");
tracing::error!(" https://github.com/informalsystems/ibc-rs/issues/1229");

std::process::exit(1);

let config = app_config();
let src_chain = match spawn_chain_runtime(&config, &self.src_chain_id) {
Ok(handle) => handle,
Expand Down
8 changes: 0 additions & 8 deletions relayer-cli/src/commands/tx/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,7 @@ impl TxUpgradeChainCmd {
}

impl Runnable for TxUpgradeChainCmd {
#[allow(unreachable_code)]
fn run(&self) {
tracing::error!("This command is currently disabled due to a regression in Hermes v0.6.1.");
tracing::error!("Please track the following issue for background and progress:");
tracing::error!("");
tracing::error!(" https://github.com/informalsystems/ibc-rs/issues/1229");

std::process::exit(1);

let config = app_config();

let opts = match self.validate_options(&config) {
Expand Down
4 changes: 2 additions & 2 deletions relayer/src/chain/cosmos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -973,7 +973,7 @@ impl Chain for CosmosSdkChain {
.upgraded_client_state
.ok_or_else(Error::empty_upgraded_client_state)?;
let client_state =
AnyClientState::try_from(upgraded_client_state_raw).map_err(Error::ics02)?;
AnyClientState::try_from(upgraded_client_state_raw).map_err(Error::invalid_upgraded_client_state)?;

// TODO: Better error kinds here.
let tm_client_state = downcast!(client_state.clone() => AnyClientState::Tendermint)
Expand Down Expand Up @@ -1687,7 +1687,7 @@ impl Chain for CosmosSdkChain {
// Build the client state.
ClientState::new(
self.id().clone(),
self.config.trust_threshold,
self.config.trust_threshold.into(),
self.config.trusting_period,
self.unbonding_period()?,
self.config.clock_drift,
Expand Down
2 changes: 1 addition & 1 deletion relayer/src/chain/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ impl Chain for MockChain {
fn build_client_state(&self, height: Height) -> Result<Self::ClientState, Error> {
let client_state = TendermintClientState::new(
self.id().clone(),
self.config.trust_threshold,
self.config.trust_threshold.into(),
self.config.trusting_period,
self.config.trusting_period.add(Duration::from_secs(1000)),
Duration::from_millis(3000),
Expand Down
10 changes: 9 additions & 1 deletion relayer/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ define_error! {
[ TraceError<LightClientError> ]
|e| { format!("Light client error for RPC address {0}", e.address) },

LightClientState
[ client_error::Error ]
|_| { "Light client encountered error due to client state".to_string() },

LightClientIo
{ address: String }
[ TraceError<LightClientIoError> ]
Expand All @@ -128,6 +132,10 @@ define_error! {
EmptyUpgradedClientState
|_| { "The upgrade plan specifies no upgraded client state" },

InvalidUpgradedClientState
[ client_error::Error ]
|e| { format!("the upgrade plan specifies an invalid upgraded client state: {}", e.source) },

EmptyResponseValue
|_| { "Empty response value" },

Expand Down Expand Up @@ -239,7 +247,7 @@ define_error! {

Ics02
[ client_error::Error ]
|_| { "ICS 02 error" },
|e| { format!("ICS 02 error: {}", e.source) },

Ics03
[ connection_error::Error ]
Expand Down
Loading

0 comments on commit f9e0d5f

Please sign in to comment.