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.

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

fix: uniformity in ownership and borrowing patterns for *_update_meta args #1072

Merged
Merged
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
21 changes: 18 additions & 3 deletions ibc-clients/ics07-tendermint/src/client_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,12 @@
),
tm_consensus_state.into(),
)?;
ctx.store_update_meta(client_id, self.latest_height(), host_timestamp, host_height)?;
ctx.store_update_meta(
client_id.clone(),
self.latest_height(),
host_timestamp,
host_height,
)?;

Ok(())
}
Expand Down Expand Up @@ -386,7 +391,12 @@
ClientStatePath::new(client_id),
ClientState::from(new_client_state).into(),
)?;
ctx.store_update_meta(client_id, header_height, host_timestamp, host_height)?;
ctx.store_update_meta(
client_id.clone(),
header_height,
host_timestamp,
host_height,
)?;
}

Ok(vec![header_height])
Expand Down Expand Up @@ -481,7 +491,12 @@
),
TmConsensusState::from(new_consensus_state).into(),
)?;
ctx.store_update_meta(client_id, latest_height, host_timestamp, host_height)?;
ctx.store_update_meta(
client_id.clone(),
latest_height,
host_timestamp,
host_height,
)?;

Check warning on line 499 in ibc-clients/ics07-tendermint/src/client_state.rs

View check run for this annotation

Codecov / codecov/patch

ibc-clients/ics07-tendermint/src/client_state.rs#L494-L499

Added lines #L494 - L499 were not covered by tests

Ok(latest_height)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ impl ClientState {
break;
}
ctx.delete_consensus_state(client_consensus_state_path)?;
ctx.delete_update_meta(client_id, height)?;
ctx.delete_update_meta(client_id.clone(), height)?;
}

Ok(())
Expand Down
6 changes: 3 additions & 3 deletions ibc-core/ics02-client/context/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub trait ClientValidationContext {
fn update_meta(
&self,
client_id: &ClientId,
height: Height,
height: &Height,
) -> Result<(Timestamp, Height), ContextError>;
}

Expand Down Expand Up @@ -60,7 +60,7 @@ pub trait ClientExecutionContext: Sized {
/// and height as the time at which this update (or header) was processed.
fn store_update_meta(
&mut self,
client_id: &ClientId,
client_id: ClientId,
height: Height,
host_timestamp: Timestamp,
host_height: Height,
Expand All @@ -75,7 +75,7 @@ pub trait ClientExecutionContext: Sized {
/// Note that this timestamp is determined by the host.
fn delete_update_meta(
&mut self,
client_id: &ClientId,
client_id: ClientId,
height: Height,
) -> Result<(), ContextError>;
}
2 changes: 1 addition & 1 deletion ibc-core/ics03-connection/src/delay.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ where
let client_id = connection_end.client_id();
let last_client_update = ctx
.get_client_validation_context()
.update_meta(client_id, packet_proof_height)?;
.update_meta(client_id, &packet_proof_height)?;

// Fetch the connection delay time and height periods.
let conn_delay_time_period = connection_end.delay_period();
Expand Down
9 changes: 7 additions & 2 deletions ibc-testkit/src/testapp/ibc/clients/mock/client_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ where
)?;
ctx.store_client_state(ClientStatePath::new(client_id), new_client_state.into())?;
ctx.store_update_meta(
client_id,
client_id.clone(),
header_height,
ctx.host_timestamp()?,
ctx.host_height()?,
Expand Down Expand Up @@ -399,7 +399,12 @@ where
let host_timestamp = ctx.host_timestamp()?;
let host_height = ctx.host_height()?;

ctx.store_update_meta(client_id, latest_height, host_timestamp, host_height)?;
ctx.store_update_meta(
client_id.clone(),
latest_height,
host_timestamp,
host_height,
)?;

Ok(latest_height)
}
Expand Down
12 changes: 6 additions & 6 deletions ibc-testkit/src/testapp/ibc/core/client_ctx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,9 @@ impl ClientValidationContext for MockContext {
fn update_meta(
&self,
client_id: &ClientId,
height: Height,
height: &Height,
) -> Result<(Timestamp, Height), ContextError> {
let key = (client_id.clone(), height);
let key = (client_id.clone(), *height);
(|| {
let ibc_store = self.ibc_store.lock();
let time = ibc_store.client_processed_times.get(&key)?;
Expand All @@ -170,7 +170,7 @@ impl ClientValidationContext for MockContext {
})()
.ok_or(ClientError::UpdateMetaDataNotFound {
client_id: key.0,
height,
height: key.1,
})
.map_err(ContextError::from)
}
Expand Down Expand Up @@ -256,7 +256,7 @@ impl ClientExecutionContext for MockContext {

fn delete_update_meta(
&mut self,
client_id: &ClientId,
client_id: ClientId,
height: Height,
) -> Result<(), ContextError> {
let key = (client_id.clone(), height);
Expand All @@ -268,7 +268,7 @@ impl ClientExecutionContext for MockContext {

fn store_update_meta(
&mut self,
client_id: &ClientId,
client_id: ClientId,
height: Height,
host_timestamp: Timestamp,
host_height: Height,
Expand All @@ -279,7 +279,7 @@ impl ClientExecutionContext for MockContext {
.insert((client_id.clone(), height), host_timestamp);
ibc_store
.client_processed_heights
.insert((client_id.clone(), height), host_height);
.insert((client_id, height), host_height);
Ok(())
}
}
4 changes: 2 additions & 2 deletions ibc-testkit/tests/core/ics02_client/update_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ fn test_consensus_state_pruning() {
expired_height.revision_number(),
expired_height.revision_height(),
);
assert!(ctx.update_meta(&client_id, expired_height).is_err());
assert!(ctx.update_meta(&client_id, &expired_height).is_err());
assert!(ctx.consensus_state(&client_cons_state_path).is_err());

// Check that latest valid consensus state exists.
Expand All @@ -158,7 +158,7 @@ fn test_consensus_state_pruning() {
earliest_valid_height.revision_height(),
);

assert!(ctx.update_meta(&client_id, earliest_valid_height).is_ok());
assert!(ctx.update_meta(&client_id, &earliest_valid_height).is_ok());
assert!(ctx.consensus_state(&client_cons_state_path).is_ok());

let end_host_timestamp = ctx.host_timestamp().unwrap();
Expand Down
2 changes: 1 addition & 1 deletion ibc-testkit/tests/core/ics04_channel/acknowledgement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ fn ack_success_happy_path(fixture: Fixture) {
);
ctx.get_client_execution_context()
.store_update_meta(
&ClientId::default(),
ClientId::default(),
client_height,
Timestamp::from_nanoseconds(1000).unwrap(),
Height::new(0, 4).unwrap(),
Expand Down
2 changes: 1 addition & 1 deletion ibc-testkit/tests/core/ics04_channel/recv_packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ fn recv_packet_validate_happy_path(fixture: Fixture) {
context
.get_client_execution_context()
.store_update_meta(
&ClientId::default(),
ClientId::default(),
client_height,
Timestamp::from_nanoseconds(1000).unwrap(),
Height::new(0, 5).unwrap(),
Expand Down
6 changes: 3 additions & 3 deletions ibc-testkit/tests/core/ics04_channel/timeout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ fn timeout_fail_proof_timeout_not_reached(fixture: Fixture) {
);

ctx.store_update_meta(
&ClientId::default(),
ClientId::default(),
client_height,
Timestamp::from_nanoseconds(5).unwrap(),
Height::new(0, 4).unwrap(),
Expand Down Expand Up @@ -286,7 +286,7 @@ fn timeout_unordered_channel_validate(fixture: Fixture) {

ctx.get_client_execution_context()
.store_update_meta(
&ClientId::default(),
ClientId::default(),
client_height,
Timestamp::from_nanoseconds(1000).unwrap(),
Height::new(0, 5).unwrap(),
Expand Down Expand Up @@ -335,7 +335,7 @@ fn timeout_ordered_channel_validate(fixture: Fixture) {
);

ctx.store_update_meta(
&ClientId::default(),
ClientId::default(),
client_height,
Timestamp::from_nanoseconds(1000).unwrap(),
Height::new(0, 4).unwrap(),
Expand Down
2 changes: 1 addition & 1 deletion ibc-testkit/tests/core/ics04_channel/timeout_on_close.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ fn timeout_on_close_success_happy_path(fixture: Fixture) {
context
.get_client_execution_context()
.store_update_meta(
&ClientId::default(),
ClientId::default(),
Height::new(0, 2).unwrap(),
Timestamp::from_nanoseconds(5000).unwrap(),
Height::new(0, 5).unwrap(),
Expand Down
Loading