test: tendermint client update when client expires or validator set has changed #921

rnbguy · 2023-10-13T16:08:21Z

Closes: #538

Description

This PR adds and modifies a few test scenarios.

Client expiry while client state update.
Client state update with validator set changes (happy and sad).

PR author checklist:

Added changelog entry, using unclog.
Create a new issue for parameterized mock Header, Client, and Context configuration.
Added tests.
Linked to GitHub issue.
Updated code comments and documentation (e.g., docs/).
Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

Reviewed Files changed in the GitHub PR explorer.
Manually tested (in case integration/unit/mock tests are absent).

codecov · 2023-10-14T13:51:29Z

Codecov Report

Attention: 29 lines in your changes are missing coverage. Please review.

Comparison is base (245d2fc) 67.51% compared to head (57180f9) 67.56%.
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #921      +/-   ##
==========================================
+ Coverage   67.51%   67.56%   +0.04%     
==========================================
  Files         131      131              
  Lines       16091    16283     +192     
==========================================
+ Hits        10864    11001     +137     
- Misses       5227     5282      +55

Files	Coverage Δ
crates/ibc/src/clients/ics07_tendermint/header.rs	`72.81% <100.00%> (+0.13%)`	⬆️
...ibc/src/core/ics02_client/handler/update_client.rs	`95.37% <100.00%> (+0.13%)`	⬆️
crates/ibc/src/mock/host.rs	`92.10% <97.36%> (+0.50%)`	⬆️
...s/ibc/src/clients/ics07_tendermint/client_state.rs	`74.23% <90.47%> (+1.40%)`	⬆️
crates/ibc/src/mock/context.rs	`72.81% <83.54%> (-2.41%)`	⬇️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

rnbguy · 2023-10-14T13:55:00Z

I am not sure what is going on. The earlier commits failed on gh action, although they were successful on my machine (cargo test --all-features --no-run and act). I added 0f4503c as a fix - even though with it, the build fails on my machine - but it makes gh actions successful!

@Farhad-Shabani can you please take a look later?

rnbguy · 2023-10-16T12:10:04Z

Did I hit a rare issue with Git?

This is from this PR

ibc-rs/crates/ibc/src/mock/context.rs

Line 240 in 0f4503c

max_history_size: usize,

This is from main branch

ibc-rs/crates/ibc/src/mock/context.rs

Line 203 in 245d2fc

max_history_size: u64,

Although it's shown when two commits are compared.

rnbguy · 2023-10-16T12:27:36Z

~~Please ignore my previous comments.~~ I just realized the main branch progressed from what I had.

The new changes did not conflict with my PR syntactically but they conflicted semantically. 😅

crates/ibc/src/core/ics02_client/handler/update_client.rs

crates/ibc/src/mock/context.rs

crates/ibc/src/mock/host.rs

crates/ibc/src/core/ics02_client/handler/update_client.rs

crates/ibc/Cargo.toml

crates/ibc/src/mock/context.rs

crates/ibc/Cargo.toml

rnbguy · 2023-10-18T14:49:51Z

@Farhad-Shabani I have made the changes. I also added a new test to check max_clock_drift. I will create the issue about the TODO comments after this PR gets merged.

Do I need to add a changelog entry?

Farhad-Shabani · 2023-10-18T14:53:28Z

@Farhad-Shabani I have made the changes. I also added a new test to check max_clock_drift. I will create the issue about the TODO comments after this PR gets merged.

Thanks a lot @rnbguy.

Do I need to add a changelog entry?

Yes please!

Farhad-Shabani

All set 🙏🏻

…as changed (#921) * add client expiry test * tm hostblock supports trusted next validator set * fix validator change update test * fix incorrect validator updates at each block * add sad client update for validator change * cargo fmt * catch up with main branch changes * update MockContextConfig with validator set history * refactor tests with updated MockContextConfig * rm duplicate def of `on` * add todo for max_history_size and validator_set_history * consistent variable naming in tests * bump typed-builder version * rm redundant builder arguments * replace todo with panic * mv Tendermint ClientStateConfig under ics07 * use ctx_a with ctx_b instead of only ctx * use client_id consistently * use mocks feature directly in dev-deps * include trusting_period and max_clock_drift in mock light client config * revert advance chain height with timestamp * update client expiry test * add test to check max_clock_drift * rm TODO comments in favor of gh issue * revert ctx_a renaming * add changelog entry

rnbguy requested a review from Farhad-Shabani October 13, 2023 16:08

rnbguy self-assigned this Oct 13, 2023

rnbguy added 6 commits October 16, 2023 14:10

add client expiry test

1d78333

tm hostblock supports trusted next validator set

90b006d

fix validator change update test

6039366

fix incorrect validator updates at each block

a2163f1

add sad client update for validator change

7dfc285

cargo fmt

78f5eda

catch up with main branch changes

89ffdbf

rnbguy force-pushed the rano/test-client-update branch from 0f4503c to 89ffdbf Compare October 16, 2023 12:47

rnbguy added 4 commits October 16, 2023 16:06

update MockContextConfig with validator set history

2d90996

refactor tests with updated MockContextConfig

ef774cc

rm duplicate def of on

2750e90

add todo for max_history_size and validator_set_history

25b9b4d

rnbguy marked this pull request as ready for review October 16, 2023 15:03

Farhad-Shabani requested changes Oct 17, 2023

View reviewed changes

rnbguy added 7 commits October 17, 2023 14:31

consistent variable naming in tests

f05537a

bump typed-builder version

e827067

rm redundant builder arguments

bf55a97

replace todo with panic

87196d3

mv Tendermint ClientStateConfig under ics07

122a570

use ctx_a with ctx_b instead of only ctx

7f7f79f

use client_id consistently

eb8798c

rnbguy commented Oct 17, 2023

View reviewed changes

crates/ibc/Cargo.toml Outdated Show resolved Hide resolved

rnbguy added 2 commits October 17, 2023 23:08

use mocks feature directly in dev-deps

6b47dc7

include trusting_period and max_clock_drift in mock light client config

f7cc801

rnbguy added 3 commits October 18, 2023 16:33

revert advance chain height with timestamp

0d2d642

update client expiry test

3020a41

add test to check max_clock_drift

1d3bba8

rnbguy force-pushed the rano/test-client-update branch from d36e9be to 1d3bba8 Compare October 18, 2023 14:33

rm TODO comments in favor of gh issue

554ac48

rnbguy added 2 commits October 18, 2023 16:55

revert ctx_a renaming

0da962b

add changelog entry

57180f9

Farhad-Shabani approved these changes Oct 18, 2023

View reviewed changes

Farhad-Shabani merged commit 476937b into main Oct 18, 2023
13 checks passed

Farhad-Shabani deleted the rano/test-client-update branch October 18, 2023 15:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: tendermint client update when client expires or validator set has changed #921

test: tendermint client update when client expires or validator set has changed #921

rnbguy commented Oct 13, 2023 •

edited

Loading

codecov bot commented Oct 14, 2023 •

edited

Loading

rnbguy commented Oct 14, 2023

rnbguy commented Oct 16, 2023

rnbguy commented Oct 16, 2023 •

edited

Loading

rnbguy commented Oct 18, 2023

Farhad-Shabani commented Oct 18, 2023

Farhad-Shabani left a comment

test: tendermint client update when client expires or validator set has changed #921

test: tendermint client update when client expires or validator set has changed #921

Conversation

rnbguy commented Oct 13, 2023 • edited Loading

Description

PR author checklist:

Reviewer checklist:

codecov bot commented Oct 14, 2023 • edited Loading

Codecov Report

rnbguy commented Oct 14, 2023

rnbguy commented Oct 16, 2023

rnbguy commented Oct 16, 2023 • edited Loading

rnbguy commented Oct 18, 2023

Farhad-Shabani commented Oct 18, 2023

Farhad-Shabani left a comment

Choose a reason for hiding this comment

rnbguy commented Oct 13, 2023 •

edited

Loading

codecov bot commented Oct 14, 2023 •

edited

Loading

rnbguy commented Oct 16, 2023 •

edited

Loading