Skip to content

Improve ChainId implementation; this is API breaking change #698

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions crates/ibc/src/clients/ics07_tendermint/client_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,9 @@ impl TryFrom<RawTmClientState> for ClientState {
type Error = Error;

fn try_from(raw: RawTmClientState) -> Result<Self, Self::Error> {
let chain_id = ChainId::from_string(raw.chain_id.as_str());
let chain_id = ChainId::try_from(&raw.chain_id).map_err(|_| Error::InvalidRawChainId {
chain_id: raw.chain_id,
})?;

let trust_level = {
let trust_level = raw
Expand Down Expand Up @@ -853,15 +855,15 @@ mod tests {
Test {
name: "Valid long (50 chars) chain-id".to_string(),
params: ClientStateParams {
id: ChainId::new("a".repeat(48), 0),
id: ChainId::new(&"a".repeat(48), 0),
..default_params.clone()
},
want_pass: true,
},
Test {
name: "Invalid too-long (51 chars) chain-id".to_string(),
params: ClientStateParams {
id: ChainId::new("a".repeat(49), 0),
id: ChainId::new(&"a".repeat(49), 0),
..default_params.clone()
},
want_pass: false,
Expand Down Expand Up @@ -964,7 +966,7 @@ mod tests {
cs_result.is_ok(),
"ClientState::new() failed for test {}, \nmsg{:?} with error {:?}",
test.name,
test.params.clone(),
&test.params,
cs_result.err(),
);
}
Expand All @@ -974,7 +976,7 @@ mod tests {
fn client_state_verify_height() {
// Define a "default" set of parameters to reuse throughout these tests.
let default_params: ClientStateParams = ClientStateParams {
id: ChainId::new("ibc".to_string(), 1),
id: ChainId::new("ibc", 1),
trust_level: TrustThreshold::ONE_THIRD,
trusting_period: Duration::new(64000, 0),
unbonding_period: Duration::new(128000, 0),
Expand Down Expand Up @@ -1127,7 +1129,7 @@ pub mod test_util {

pub fn new_dummy_from_header(tm_header: Header) -> ClientState {
ClientState::new(
tm_header.chain_id.clone().into(),
tm_header.chain_id.clone().try_into().unwrap(),
Default::default(),
Duration::from_secs(64000),
Duration::from_secs(128000),
Expand All @@ -1151,7 +1153,7 @@ pub mod test_util {
pub fn get_dummy_raw_tm_client_state(frozen_height: RawHeight) -> RawTmClientState {
#[allow(deprecated)]
RawTmClientState {
chain_id: ChainId::new("ibc".to_string(), 0).to_string(),
chain_id: ChainId::new("ibc", 1).to_string(),
trust_level: Some(Fraction {
numerator: 1,
denominator: 3,
Expand Down
4 changes: 3 additions & 1 deletion crates/ibc/src/clients/ics07_tendermint/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use tendermint_light_client_verifier::Verdict;
/// The main error type
#[derive(Debug, Display)]
pub enum Error {
/// chain-id is (`{chain_id}`) is too long, got: `{len}`, max allowed: `{max_len}`
/// chain-id (`{chain_id}`) is too long, got: `{len}`, max allowed: `{max_len}`
ChainIdTooLong {
chain_id: ChainId,
len: usize,
Expand Down Expand Up @@ -101,6 +101,8 @@ pub enum Error {
MisbehaviourHeadersNotAtSameHeight,
/// invalid raw client id: `{client_id}`
InvalidRawClientId { client_id: String },
/// chain-id (`{chain_id}`) has no epoch version
InvalidRawChainId { chain_id: String },
}

#[cfg(feature = "std")]
Expand Down
26 changes: 13 additions & 13 deletions crates/ibc/src/core/ics02_client/handler/update_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,10 @@ mod tests {
let client_id = ClientId::new(tm_client_type(), 0).unwrap();
let client_height = Height::new(1, 20).unwrap();
let update_height = Height::new(1, 21).unwrap();
let chain_id_b = ChainId::new("mockgaiaB".to_string(), 1);
let chain_id_b = ChainId::new("mockgaiaB", 1);

let mut ctx = MockContext::new(
ChainId::new("mockgaiaA".to_string(), 1),
ChainId::new("mockgaiaA", 1),
HostType::Mock,
5,
Height::new(1, 1).unwrap(),
Expand Down Expand Up @@ -227,10 +227,10 @@ mod tests {
let client_id = ClientId::new(tm_client_type(), 0).unwrap();
let client_height = Height::new(1, 20).unwrap();
let update_height = Height::new(1, 21).unwrap();
let chain_id_b = ChainId::new("mockgaiaB".to_string(), 1);
let chain_id_b = ChainId::new("mockgaiaB", 1);

let mut ctx = MockContext::new(
ChainId::new("mockgaiaA".to_string(), 1),
ChainId::new("mockgaiaA", 1),
HostType::Mock,
5,
Height::new(1, 1).unwrap(),
Expand Down Expand Up @@ -274,8 +274,8 @@ mod tests {
let client_id = ClientId::new(tm_client_type(), 0).unwrap();
let client_height = Height::new(1, 20).unwrap();

let ctx_a_chain_id = ChainId::new("mockgaiaA".to_string(), 1);
let ctx_b_chain_id = ChainId::new("mockgaiaB".to_string(), 1);
let ctx_a_chain_id = ChainId::new("mockgaiaA", 1);
let ctx_b_chain_id = ChainId::new("mockgaiaB", 1);
let start_height = Height::new(1, 11).unwrap();

let mut ctx_a = MockContext::new(ctx_a_chain_id, HostType::Mock, 5, start_height)
Expand Down Expand Up @@ -329,7 +329,7 @@ mod tests {
let client_state = {
#[allow(deprecated)]
let raw_client_state = RawTmClientState {
chain_id: ChainId::from(tm_block.header().chain_id.clone()).to_string(),
chain_id: tm_block.header().chain_id.to_string(),
trust_level: Some(Fraction {
numerator: 1,
denominator: 3,
Expand Down Expand Up @@ -399,7 +399,7 @@ mod tests {
let chain_start_height = Height::new(1, 11).unwrap();

let ctx = MockContext::new(
ChainId::new("mockgaiaA".to_string(), 1),
ChainId::new("mockgaiaA", 1),
HostType::Mock,
5,
chain_start_height,
Expand All @@ -412,7 +412,7 @@ mod tests {
);

let ctx_b = MockContext::new(
ChainId::new("mockgaiaB".to_string(), 1),
ChainId::new("mockgaiaB", 1),
HostType::SyntheticTendermint,
5,
client_height,
Expand Down Expand Up @@ -538,11 +538,11 @@ mod tests {
let client_id = ClientId::new(tm_client_type(), 0).unwrap();
let client_height = Height::new(1, 20).unwrap();
let misbehaviour_height = Height::new(1, 21).unwrap();
let chain_id_b = ChainId::new("mockgaiaB".to_string(), 1);
let chain_id_b = ChainId::new("mockgaiaB", 1);

// Create a mock context for chain-A with a synthetic tendermint light client for chain-B
let mut ctx_a = MockContext::new(
ChainId::new("mockgaiaA".to_string(), 1),
ChainId::new("mockgaiaA", 1),
HostType::Mock,
5,
Height::new(1, 1).unwrap(),
Expand Down Expand Up @@ -599,11 +599,11 @@ mod tests {
let client_id = ClientId::new(tm_client_type(), 0).unwrap();
let client_height = Height::new(1, 20).unwrap();
let misbehaviour_height = Height::new(1, 21).unwrap();
let chain_id_b = ChainId::new("mockgaiaB".to_string(), 1);
let chain_id_b = ChainId::new("mockgaiaB", 1);

// Create a mock context for chain-A with a synthetic tendermint light client for chain-B
let mut ctx_a = MockContext::new(
ChainId::new("mockgaiaA".to_string(), 1),
ChainId::new("mockgaiaA", 1),
HostType::Mock,
5,
Height::new(1, 1).unwrap(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ mod tests {

let ctx_default = MockContext::default();
let ctx_new = MockContext::new(
ChainId::new("mockgaia".to_string(), latest_height.revision_number()),
ChainId::new("mockgaia", latest_height.revision_number()),
HostType::Mock,
max_history_size,
latest_height,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ mod tests {
};

let ctx_new = MockContext::new(
ChainId::new("mockgaia".to_string(), 0),
ChainId::new("mockgaia", 0),
HostType::Mock,
max_history_size,
host_chain_height,
Expand Down
Loading