Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Don’t needlessly take ownership of argument in ChainId::new #721

Merged
merged 1 commit into from
Jun 21, 2023
Merged

Don’t needlessly take ownership of argument in ChainId::new #721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions ...eleased/breaking-changes/721-dont-take-ownership-of-argument-in-chainid::new.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Revise the `ChainId::new` method so that rather than taking String argument
it borrows a str. ([#721](https://github.com/cosmos/ibc-rs/issues/721))
8 changes: 4 additions & 4 deletions crates/ibc/src/clients/ics07_tendermint/client_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -853,15 +853,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 @@ -974,7 +974,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 @@ -1151,7 +1151,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", 0).to_string(),
trust_level: Some(Fraction {
numerator: 1,
denominator: 3,
Expand Down
24 changes: 12 additions & 12 deletions crates/ibc/src/core/ics02_client/handler/update_client.rs
View file
Open in desktop
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 @@ -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
2 changes: 1 addition & 1 deletion crates/ibc/src/core/ics03_connection/handler/conn_open_ack.rs
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion crates/ibc/src/core/ics03_connection/handler/conn_open_try.rs
View file
Open in desktop
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
13 changes: 6 additions & 7 deletions crates/ibc/src/core/ics24_host/identifier.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,17 @@ impl ChainId {
/// Creates a new `ChainId` given a chain name and an epoch number.
///
/// The returned `ChainId` will have the format: `{chain name}-{epoch number}`.
///
/// ```
/// use ibc::core::ics24_host::identifier::ChainId;
///
/// let epoch_number = 10;
/// let id = ChainId::new("chainA".to_string(), epoch_number);
/// let id = ChainId::new("chainA", epoch_number);
/// assert_eq!(id.version(), epoch_number);
/// ```
pub fn new(name: String, version: u64) -> Self {
Self {
id: format!("{name}-{version}"),
version,
}
pub fn new(name: &str, version: u64) -> Self {
let id = format!("{name}-{version}");
Self { id, version }
}

pub fn from_string(id: &str) -> Self {
Expand Down Expand Up @@ -133,7 +132,7 @@ impl ChainId {
/// replaces it's version with the specified version
/// ```
/// use ibc::core::ics24_host::identifier::ChainId;
/// assert_eq!(ChainId::new("chainA".to_string(), 1).with_version(2), ChainId::new("chainA".to_string(), 2));
/// assert_eq!(ChainId::new("chainA", 1).with_version(2), ChainId::new("chainA", 2));
/// assert_eq!("chain1".parse::<ChainId>().unwrap().with_version(2), "chain1".parse::<ChainId>().unwrap());
/// ```
pub fn with_version(mut self, version: u64) -> Self {
Expand Down
24 changes: 12 additions & 12 deletions crates/ibc/src/mock/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ pub struct MockContext {
impl Default for MockContext {
fn default() -> Self {
Self::new(
ChainId::new("mockgaia".to_string(), 0),
ChainId::new("mockgaia", 0),
HostType::Mock,
5,
Height::new(0, 5).unwrap(),
Expand Down Expand Up @@ -1487,7 +1487,7 @@ mod tests {
Test {
name: "Empty history, small pruning window".to_string(),
ctx: MockContext::new(
ChainId::new("mockgaia".to_string(), cv),
ChainId::new("mockgaia", cv),
HostType::Mock,
2,
Height::new(cv, 1).unwrap(),
Expand All @@ -1496,7 +1496,7 @@ mod tests {
Test {
name: "[Synthetic TM host] Empty history, small pruning window".to_string(),
ctx: MockContext::new(
ChainId::new("mocksgaia".to_string(), cv),
ChainId::new("mocksgaia", cv),
HostType::SyntheticTendermint,
2,
Height::new(cv, 1).unwrap(),
Expand All @@ -1505,7 +1505,7 @@ mod tests {
Test {
name: "Large pruning window".to_string(),
ctx: MockContext::new(
ChainId::new("mockgaia".to_string(), cv),
ChainId::new("mockgaia", cv),
HostType::Mock,
30,
Height::new(cv, 2).unwrap(),
Expand All @@ -1514,7 +1514,7 @@ mod tests {
Test {
name: "[Synthetic TM host] Large pruning window".to_string(),
ctx: MockContext::new(
ChainId::new("mocksgaia".to_string(), cv),
ChainId::new("mocksgaia", cv),
HostType::SyntheticTendermint,
30,
Height::new(cv, 2).unwrap(),
Expand All @@ -1523,7 +1523,7 @@ mod tests {
Test {
name: "Small pruning window".to_string(),
ctx: MockContext::new(
ChainId::new("mockgaia".to_string(), cv),
ChainId::new("mockgaia", cv),
HostType::Mock,
3,
Height::new(cv, 30).unwrap(),
Expand All @@ -1532,7 +1532,7 @@ mod tests {
Test {
name: "[Synthetic TM host] Small pruning window".to_string(),
ctx: MockContext::new(
ChainId::new("mockgaia".to_string(), cv),
ChainId::new("mockgaia", cv),
HostType::SyntheticTendermint,
3,
Height::new(cv, 30).unwrap(),
Expand All @@ -1541,7 +1541,7 @@ mod tests {
Test {
name: "Small pruning window, small starting height".to_string(),
ctx: MockContext::new(
ChainId::new("mockgaia".to_string(), cv),
ChainId::new("mockgaia", cv),
HostType::Mock,
3,
Height::new(cv, 2).unwrap(),
Expand All @@ -1550,7 +1550,7 @@ mod tests {
Test {
name: "[Synthetic TM host] Small pruning window, small starting height".to_string(),
ctx: MockContext::new(
ChainId::new("mockgaia".to_string(), cv),
ChainId::new("mockgaia", cv),
HostType::SyntheticTendermint,
3,
Height::new(cv, 2).unwrap(),
Expand All @@ -1559,7 +1559,7 @@ mod tests {
Test {
name: "Large pruning window, large starting height".to_string(),
ctx: MockContext::new(
ChainId::new("mockgaia".to_string(), cv),
ChainId::new("mockgaia", cv),
HostType::Mock,
50,
Height::new(cv, 2000).unwrap(),
Expand All @@ -1568,7 +1568,7 @@ mod tests {
Test {
name: "[Synthetic TM host] Large pruning window, large starting height".to_string(),
ctx: MockContext::new(
ChainId::new("mockgaia".to_string(), cv),
ChainId::new("mockgaia", cv),
HostType::SyntheticTendermint,
50,
Height::new(cv, 2000).unwrap(),
Expand Down Expand Up @@ -1817,7 +1817,7 @@ mod tests {
}

let mut ctx = MockContext::new(
ChainId::new("mockgaia".to_string(), 1),
ChainId::new("mockgaia", 1),
HostType::Mock,
1,
Height::new(1, 1).unwrap(),
Expand Down
6 changes: 3 additions & 3 deletions crates/ibc/src/mock/ics18_relayer/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ mod tests {
let client_on_a_for_b = ClientId::new(tm_client_type(), 0).unwrap();
let client_on_b_for_a = ClientId::new(mock_client_type(), 0).unwrap();

let chain_id_a = ChainId::new("mockgaiaA".to_string(), 1);
let chain_id_b = ChainId::new("mockgaiaB".to_string(), 1);
let chain_id_a = ChainId::new("mockgaiaA", 1);
let chain_id_b = ChainId::new("mockgaiaB", 1);

// Create two mock contexts, one for each chain.
let mut ctx_a =
Expand Down Expand Up @@ -145,7 +145,7 @@ mod tests {

assert!(
client_msg_b_res.is_ok(),
"create_client_update failed for context destination {ctx_b:?}, error: {client_msg_b_res:?}",
"create_client_update failed for context destination {ctx_b:?}, error: {client_msg_b_res:?}",
);

let client_msg_b = client_msg_b_res.unwrap();
Expand Down