Skip to content

Commit 6d170f4

Browse files
adizereromacrnbguy
authored
Fix for regression #1229 using new type ics02::TrustThreshold (#1254)
* Fix for regression #1229 using new type ics02::TrustThreshold * Cleanup * changelog * Apply suggestions from code review Co-authored-by: Romain Ruetschi <[email protected]> * Add CI job for finding invalid links in markdown files (#1244) * Added markdown-link-check * Update changelog * Added config file * Added ignore pattern for crates.io, simplified output * Added ignore pattern for localhost * Fix ignore patterns in markdown-link-check config * Better Github relative link for directory * Fix Github relative links within the project * Fixed two broken links * Fixed last broken link * Reverted changelog * Removed pending changelog file Co-authored-by: Adi Seredinschi <[email protected]> Co-authored-by: Romain Ruetschi <[email protected]> Co-authored-by: Ranadeep Biswas <[email protected]>
1 parent f4aa4b4 commit 6d170f4

File tree

16 files changed

+178
-53
lines changed

16 files changed

+178
-53
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
- Fix for upgrade CLI regression using new type ics02::TrustThreshold ([#1229])
2+
3+
[#1229]: https://github.com/informalsystems/ibc-rs/issues/1229

modules/src/ics02_client/client_state.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@ use std::time::Duration;
44

55
use prost_types::Any;
66
use serde::{Deserialize, Serialize};
7-
use tendermint::trust_threshold::TrustThresholdFraction;
87
use tendermint_proto::Protobuf;
98

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

1211
use crate::ics02_client::client_type::ClientType;
1312
use crate::ics02_client::error::Error;
13+
use crate::ics02_client::trust_threshold::TrustThreshold;
1414
use crate::ics07_tendermint::client_state;
1515
use crate::ics24_host::error::ValidationError;
1616
use crate::ics24_host::identifier::{ChainId, ClientId};
@@ -59,7 +59,7 @@ impl AnyClientState {
5959
}
6060
}
6161

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

modules/src/ics02_client/error.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::Height;
77
use std::num::TryFromIntError;
88
use tendermint_proto::Error as TendermintError;
99

10-
use flex_error::{define_error, TraceError};
10+
use flex_error::{define_error, DisplayOnly, TraceError};
1111

1212
define_error! {
1313
Error {
@@ -49,6 +49,15 @@ define_error! {
4949
{ reason: String }
5050
| e | { format_args!("header verification failed with reason: {}", e.reason) },
5151

52+
InvalidTrustThreshold
53+
{ numerator: u64, denominator: u64 }
54+
| e | { format_args!("failed to build trust threshold from fraction: {}/{}", e.numerator, e.denominator) },
55+
56+
FailedTrustThresholdConversion
57+
{ numerator: u64, denominator: u64 }
58+
[ DisplayOnly<Box<dyn std::error::Error + Send + Sync>> ]
59+
| e | { format_args!("failed to build Tendermint domain type trust threshold from fraction: {}/{}", e.numerator, e.denominator) },
60+
5261
UnknownClientStateType
5362
{ client_state_type: String }
5463
| e | { format_args!("unknown client state type: {0}", e.client_state_type) },

modules/src/ics02_client/handler/create_client.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,6 @@ mod tests {
6161
use std::time::Duration;
6262
use test_env_log::test;
6363

64-
use tendermint::trust_threshold::TrustThresholdFraction as TrustThreshold;
65-
6664
use crate::events::IbcEvent;
6765
use crate::handler::HandlerOutput;
6866
use crate::ics02_client::client_consensus::AnyConsensusState;
@@ -71,6 +69,7 @@ mod tests {
7169
use crate::ics02_client::handler::{dispatch, ClientResult};
7270
use crate::ics02_client::msgs::create_client::MsgCreateAnyClient;
7371
use crate::ics02_client::msgs::ClientMsg;
72+
use crate::ics02_client::trust_threshold::TrustThreshold;
7473
use crate::ics07_tendermint::client_state::{AllowUpdate, ClientState};
7574
use crate::ics07_tendermint::header::test_util::get_dummy_tendermint_header;
7675
use crate::ics24_host::identifier::ClientId;

modules/src/ics02_client/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,4 @@ pub mod header;
1212
pub mod height;
1313
pub mod misbehaviour;
1414
pub mod msgs;
15+
pub mod trust_threshold;
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
//! IBC Domain type definition for [`TrustThreshold`]
2+
//! represented as a fraction with valid values in the
3+
//! range `[0, 1)`.
4+
5+
use std::{convert::TryFrom, fmt};
6+
7+
use serde::{Deserialize, Serialize};
8+
use tendermint_proto::Protobuf;
9+
10+
use ibc_proto::ibc::lightclients::tendermint::v1::Fraction;
11+
use tendermint::trust_threshold::TrustThresholdFraction;
12+
13+
use crate::ics02_client::error::Error;
14+
15+
/// [`TrustThreshold`] defines the level of trust that a client has
16+
/// towards a set of validators of a chain.
17+
///
18+
/// A trust threshold is represented as a fraction, i.e., a numerator and
19+
/// and a denominator.
20+
/// A typical trust threshold is 1/3 in practice.
21+
/// This type accepts even a value of 0, (numerator = 0, denominator = 0),
22+
/// which is used in the client state of an upgrading client.
23+
#[derive(Copy, Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
24+
pub struct TrustThreshold {
25+
numerator: u64,
26+
denominator: u64,
27+
}
28+
29+
impl TrustThreshold {
30+
/// Constant for a trust threshold of 1/3.
31+
pub const ONE_THIRD: Self = Self {
32+
numerator: 1,
33+
denominator: 3,
34+
};
35+
36+
/// Constant for a trust threshold of 2/3.
37+
pub const TWO_THIRDS: Self = Self {
38+
numerator: 2,
39+
denominator: 3,
40+
};
41+
42+
/// Constant for a trust threshold of 0/0.
43+
pub const ZERO: Self = Self {
44+
numerator: 0,
45+
denominator: 0,
46+
};
47+
48+
/// Instantiate a TrustThreshold with the given denominator and
49+
/// numerator.
50+
///
51+
/// The constructor succeeds if long as the resulting fraction
52+
/// is in the range`[0, 1)`.
53+
pub fn new(numerator: u64, denominator: u64) -> Result<Self, Error> {
54+
// The two parameters cannot yield a fraction that is bigger or equal to 1
55+
if (numerator > denominator)
56+
|| (denominator == 0 && numerator != 0)
57+
|| (numerator == denominator && numerator != 0)
58+
{
59+
return Err(Error::invalid_trust_threshold(numerator, denominator));
60+
}
61+
62+
Ok(Self {
63+
numerator,
64+
denominator,
65+
})
66+
}
67+
68+
/// The numerator of the fraction underlying this trust threshold.
69+
pub fn numerator(&self) -> u64 {
70+
self.numerator
71+
}
72+
73+
/// The denominator of the fraction underlying this trust threshold.
74+
pub fn denominator(&self) -> u64 {
75+
self.denominator
76+
}
77+
}
78+
79+
/// Conversion from Tendermint domain type into
80+
/// IBC domain type.
81+
impl From<TrustThresholdFraction> for TrustThreshold {
82+
fn from(t: TrustThresholdFraction) -> Self {
83+
Self {
84+
numerator: t.numerator(),
85+
denominator: t.denominator(),
86+
}
87+
}
88+
}
89+
90+
/// Conversion from IBC domain type into
91+
/// Tendermint domain type.
92+
impl TryFrom<TrustThreshold> for TrustThresholdFraction {
93+
type Error = Error;
94+
95+
fn try_from(t: TrustThreshold) -> Result<TrustThresholdFraction, Error> {
96+
Self::new(t.numerator, t.denominator)
97+
.map_err(|e| Error::failed_trust_threshold_conversion(t.numerator, t.denominator, e))
98+
}
99+
}
100+
101+
impl Protobuf<Fraction> for TrustThreshold {}
102+
103+
impl From<TrustThreshold> for Fraction {
104+
fn from(t: TrustThreshold) -> Self {
105+
Self {
106+
numerator: t.numerator,
107+
denominator: t.denominator,
108+
}
109+
}
110+
}
111+
112+
impl TryFrom<Fraction> for TrustThreshold {
113+
type Error = Error;
114+
115+
fn try_from(value: Fraction) -> Result<Self, Self::Error> {
116+
Self::new(value.numerator, value.denominator)
117+
}
118+
}
119+
120+
impl Default for TrustThreshold {
121+
fn default() -> Self {
122+
Self::ONE_THIRD
123+
}
124+
}
125+
126+
impl fmt::Display for TrustThreshold {
127+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
128+
write!(f, "{}/{}", self.numerator, self.denominator)
129+
}
130+
}

modules/src/ics07_tendermint/client_state.rs

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,13 @@ use std::str::FromStr;
33
use std::time::Duration;
44

55
use serde::{Deserialize, Serialize};
6-
use tendermint::trust_threshold::{
7-
TrustThresholdFraction as TrustThreshold, TrustThresholdFraction,
8-
};
96
use tendermint_proto::Protobuf;
107

11-
use ibc_proto::ibc::lightclients::tendermint::v1::{ClientState as RawClientState, Fraction};
8+
use ibc_proto::ibc::lightclients::tendermint::v1::ClientState as RawClientState;
129

1310
use crate::ics02_client::client_state::AnyClientState;
1411
use crate::ics02_client::client_type::ClientType;
12+
use crate::ics02_client::trust_threshold::TrustThreshold;
1513
use crate::ics07_tendermint::error::Error;
1614
use crate::ics07_tendermint::header::Header;
1715
use crate::ics23_commitment::specs::ProofSpecs;
@@ -111,11 +109,11 @@ impl ClientState {
111109
}
112110
}
113111

114-
/// Helper function to verify the upgrade client procedure.
112+
/// Helper function for the upgrade chain & client procedures.
115113
/// Resets all fields except the blockchain-specific ones.
116114
pub fn zero_custom_fields(mut client_state: Self) -> Self {
117115
client_state.trusting_period = ZERO_DURATION;
118-
client_state.trust_level = TrustThresholdFraction::default();
116+
client_state.trust_level = TrustThreshold::ZERO;
119117
client_state.allow_update.after_expiry = false;
120118
client_state.allow_update.after_misbehaviour = false;
121119
client_state.frozen_height = Height::zero();
@@ -170,7 +168,8 @@ impl TryFrom<RawClientState> for ClientState {
170168
Ok(Self {
171169
chain_id: ChainId::from_str(raw.chain_id.as_str())
172170
.map_err(Error::invalid_chain_identifier)?,
173-
trust_level: TrustThreshold::new(trust_level.numerator, trust_level.denominator)
171+
trust_level: trust_level
172+
.try_into()
174173
.map_err(|e| Error::invalid_trust_threshold(format!("{}", e)))?,
175174
trusting_period: raw
176175
.trusting_period
@@ -208,10 +207,7 @@ impl From<ClientState> for RawClientState {
208207
fn from(value: ClientState) -> Self {
209208
RawClientState {
210209
chain_id: value.chain_id.to_string(),
211-
trust_level: Some(Fraction {
212-
numerator: value.trust_level.numerator(),
213-
denominator: value.trust_level.denominator(),
214-
}),
210+
trust_level: Some(value.trust_level.into()),
215211
trusting_period: Some(value.trusting_period.into()),
216212
unbonding_period: Some(value.unbonding_period.into()),
217213
max_clock_drift: Some(value.max_clock_drift.into()),
@@ -230,9 +226,9 @@ mod tests {
230226
use std::time::Duration;
231227
use test_env_log::test;
232228

233-
use tendermint::trust_threshold::TrustThresholdFraction as TrustThreshold;
234229
use tendermint_rpc::endpoint::abci_query::AbciQuery;
235230

231+
use crate::ics02_client::trust_threshold::TrustThreshold;
236232
use crate::ics07_tendermint::client_state::{AllowUpdate, ClientState};
237233
use crate::ics24_host::identifier::ChainId;
238234
use crate::test::test_serialization_roundtrip;

relayer-cli/src/commands/tx/client.rs

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -131,15 +131,7 @@ pub struct TxUpgradeClientCmd {
131131
}
132132

133133
impl Runnable for TxUpgradeClientCmd {
134-
#[allow(unreachable_code)]
135134
fn run(&self) {
136-
tracing::error!("This command is currently disabled due to a regression in Hermes v0.6.1.");
137-
tracing::error!("Please track the following issue for background and progress:");
138-
tracing::error!("");
139-
tracing::error!(" https://github.com/informalsystems/ibc-rs/issues/1229");
140-
141-
std::process::exit(1);
142-
143135
let config = app_config();
144136

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

189181
impl Runnable for TxUpgradeClientsCmd {
190-
#[allow(unreachable_code)]
191182
fn run(&self) {
192-
tracing::error!("This command is currently disabled due to a regression in Hermes v0.6.1.");
193-
tracing::error!("Please track the following issue for background and progress:");
194-
tracing::error!("");
195-
tracing::error!(" https://github.com/informalsystems/ibc-rs/issues/1229");
196-
197-
std::process::exit(1);
198-
199183
let config = app_config();
200184
let src_chain = match spawn_chain_runtime(&config, &self.src_chain_id) {
201185
Ok(handle) => handle,

relayer-cli/src/commands/tx/upgrade.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,7 @@ impl TxUpgradeChainCmd {
6464
}
6565

6666
impl Runnable for TxUpgradeChainCmd {
67-
#[allow(unreachable_code)]
6867
fn run(&self) {
69-
tracing::error!("This command is currently disabled due to a regression in Hermes v0.6.1.");
70-
tracing::error!("Please track the following issue for background and progress:");
71-
tracing::error!("");
72-
tracing::error!(" https://github.com/informalsystems/ibc-rs/issues/1229");
73-
74-
std::process::exit(1);
75-
7668
let config = app_config();
7769

7870
let opts = match self.validate_options(&config) {

relayer/src/chain/cosmos.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -972,8 +972,8 @@ impl Chain for CosmosSdkChain {
972972
.ok_or_else(Error::empty_response_value)?
973973
.upgraded_client_state
974974
.ok_or_else(Error::empty_upgraded_client_state)?;
975-
let client_state =
976-
AnyClientState::try_from(upgraded_client_state_raw).map_err(Error::ics02)?;
975+
let client_state = AnyClientState::try_from(upgraded_client_state_raw)
976+
.map_err(Error::invalid_upgraded_client_state)?;
977977

978978
// TODO: Better error kinds here.
979979
let tm_client_state = downcast!(client_state.clone() => AnyClientState::Tendermint)
@@ -1687,7 +1687,7 @@ impl Chain for CosmosSdkChain {
16871687
// Build the client state.
16881688
ClientState::new(
16891689
self.id().clone(),
1690-
self.config.trust_threshold,
1690+
self.config.trust_threshold.into(),
16911691
self.config.trusting_period,
16921692
self.unbonding_period()?,
16931693
self.config.clock_drift,

0 commit comments

Comments
 (0)