Update to latest tendermint-rs and adapt relayer to new light client #255

romac · 2020-09-25T09:50:37Z

Close: #243
Close: #90

Depends on informalsystems/tendermint-rs#583

Description

For contributor use:

Unit tests written
Added test to CI if applicable
Updated CHANGELOG_PENDING.md
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Updated relevant documentation (docs/) and code comments
Re-reviewed Files changed in the Github PR explorer

andynog · 2020-09-25T12:57:38Z

Hi @romac, I've checked out your branch romac/light-client-cleanup and when I try cargo build I'm getting an error it can't find the tendermint-rs/tendermint/Cargo.toml file.

~/dev/github.com/informalsystems/ibc-rs$ cargo build
error: failed to read `/home/andy/dev/github.com/informalsystems/tendermint-rs/tendermint/Cargo.toml`

Caused by:
  No such file or directory (os error 2)

On this machine I don't have tendermint-rs in the location it's trying to find it. I believe this is because in Cargo.toml the path is specified like this tendermint = { version = "0.16.0", path = "../../tendermint-rs/tendermint" } but should we have a repo dependency like this ? It assumes users would need to git clone tendermint-rs.

Should we just have the version ?

romac · 2020-09-25T14:29:41Z

My bad, sorry about that! Will update it to use the tendermint-rs version from the branch in informalsystems/tendermint-rs#583.

romac · 2020-09-25T14:35:49Z

@andynog 4fecf88

romac · 2020-09-25T14:36:40Z

Just to clarify, the git dependency is needed because this PR depends on these unreleased changes in tendermint-rs: informalsystems/tendermint-rs#583

andynog · 2020-09-25T15:30:41Z

Hi @romac, it's building fine now. Thanks !

codecov-commenter · 2020-09-29T13:28:47Z

Codecov Report

Merging #255 into master will increase coverage by 23.5%.
The diff coverage is 61.8%.

@@            Coverage Diff            @@
##           master    #255      +/-   ##
=========================================
+ Coverage    13.6%   37.2%   +23.5%     
=========================================
  Files          69     120      +51     
  Lines        3752    7632    +3880     
  Branches     1374    2706    +1332     
=========================================
+ Hits          513    2843    +2330     
- Misses       2618    4370    +1752     
+ Partials      621     419     -202

Impacted Files	Coverage Δ
modules/src/events.rs	`0.0% <0.0%> (ø)`
modules/src/ics02_client/events.rs	`0.0% <ø> (ø)`
modules/src/ics02_client/raw.rs	`0.0% <0.0%> (ø)`
modules/src/ics03_connection/context.rs	`0.0% <0.0%> (ø)`
modules/src/ics03_connection/error.rs	`16.6% <0.0%> (-16.7%)`	⬇️
modules/src/ics04_channel/error.rs	`75.0% <ø> (+50.0%)`	⬆️
modules/src/ics04_channel/packet.rs	`0.0% <0.0%> (ø)`
modules/src/ics07_tendermint/client_def.rs	`0.0% <0.0%> (ø)`
modules/src/ics07_tendermint/error.rs	`75.0% <ø> (+75.0%)`	⬆️
modules/src/ics18_relayer/error.rs	`0.0% <0.0%> (ø)`
... and 217 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e5f0b2...4e840c8. Read the comment docs.

ancazamfir

Looks good to merge for now. The listen mode does not work but we should look into this later.

andynog

I've run the tests again and I could configure both light clients for chain_A and chain_B, but when starting the relayer I'm getting a parse error. Not sure if it's related to the config TOML (the line it complains doesn't match with something in the config I think so maybe somewhere else). Here's the message:

(base) andy@ubuntu:~/dev/github.com/informalsystems/ibc-rs$ cargo run --bin relayer -- -c ./relayer/tests/config/fixtures/relayer_conf_example.toml start --reset
    Finished dev [unoptimized + debuginfo] target(s) in 0.20s
     Running `target/debug/relayer -c ./relayer/tests/config/fixtures/relayer_conf_example.toml start --reset`
Sep 30 10:58:40.686  INFO relayer_cli::commands::start: spawning light client chain.id=chain_A
     Relayer spawning light client for chain chain_A
Sep 30 10:58:40.828  INFO relayer_cli::commands::start: resetting client to trust options state chain.id=chain_A
error: relayer-cli fatal error: I/O error: Parse error. Invalid JSON: invalid type: string "1", expected u64 at line 17 column 24 (code: -32700)

romac · 2020-09-30T15:47:25Z

The relayer start command currently fails with a relayer-cli fatal error: hash mismatch.
This is due to a bug in tendermint-rs that we just surfaced today.
As such, I don't think it should block the merge of this PR.

romac · 2020-09-30T15:47:49Z

We still need to wait for informalsystems/tendermint-rs#583 to be merged first.

andynog · 2020-09-30T18:40:03Z

So I've updated the chains (chain_A and chain_B) to run with the latest stargate-3 version. Now there's another error.

(base) andy@ubuntu:~/dev/github.com/informalsystems/ibc-rs$  cargo run --bin relayer -- -c ./relayer/tests/config/fixtures/relayer_conf_example.toml start --reset
    Finished dev [unoptimized + debuginfo] target(s) in 0.15s
     Running `target/debug/relayer -c ./relayer/tests/config/fixtures/relayer_conf_example.toml start --reset`
Sep 30 14:30:01.285  INFO relayer_cli::commands::start: spawning light client chain.id=chain_A
     Relayer spawning light client for chain chain_A
Sep 30 14:30:01.416  INFO relayer_cli::commands::start: resetting client to trust options state chain.id=chain_A
error: relayer-cli fatal error: I/O error: Parse error. Invalid JSON: missing field `app` at line 9 column 9 (code: -32700)

Not sure which JSON it's referring too, maybe it's a genesis.json for chain_A and somewhere there's a validation for that. Maybe this error is coming from other dependencies.

But gaiad says the genesis are valid

root@ubuntu:/home/andy/dev/github.com/informalsystems/ibc-rs/ci# ./gaia/build/gaiad validate-genesis ./gaia/build/chain_a/node0/gaiad/config/genesis.json 
validating genesis file at ./gaia/build/chain_a/node0/gaiad/config/genesis.json
File at ./gaia/build/chain_a/node0/gaiad/config/genesis.json is a valid genesis file
root@ubuntu:/home/andy/dev/github.com/informalsystems/ibc-rs/ci# ./gaia/build/gaiad validate-genesis ./gaia/build/chain_b/node0/gaiad/config/genesis.json 
validating genesis file at ./gaia/build/chain_b/node0/gaiad/config/genesis.json
File at ./gaia/build/chain_b/node0/gaiad/config/genesis.json is a valid genesis file

andynog

I think this is OK to merge, it's not failing on tests and it builds fine. I think the tests we have with chain are not in particular having issues related to this implementation. So I believe this is good to go.

…nformalsystems#255) * Update to tendermint-rs v0.16, disable client module. * Update to latest tendermint-rpc with WebSocketClient * WIP: Define new tendermint light client * Add minimal interface for light client * Fix collect_events * Cleanup config * Change light init command to store light client config into a TOML file * More cleanup in event monitor * Remove light client from Chain object * Rename hash to header_hash * Remove store module * Proper error handling in light init command * Adapt relayer start command to use the new light client * Specify git dependencies on tendermint-rs instead of local * Fix signed_header test fixtures for tendermint-rs 0.16 * Formatting * Show latest trusted state height in relayer task * Formatting

romac added 13 commits September 25, 2020 12:03

Update to tendermint-rs v0.16, disable client module.

df8ac36

Update to latest tendermint-rpc with WebSocketClient

3ee621a

WIP: Define new tendermint light client

5901f4a

Add minimal interface for light client

5a2fd18

Fix collect_events

891e425

Cleanup config

816a617

Change light init command to store light client config into a TOML file

2ba3a1d

More cleanup in event monitor

5bb77d6

Remove light client from Chain object

0cd7f31

Rename hash to header_hash

16111fe

Remove store module

4937d66

Proper error handling in light init command

9a9d717

Adapt relayer start command to use the new light client

23c3b3f

romac force-pushed the romac/light-client-cleanup branch from b57a246 to 23c3b3f Compare September 25, 2020 10:06

ancazamfir assigned romac Sep 25, 2020

Specify git dependencies on tendermint-rs instead of local

4fecf88

Fix signed_header test fixtures for tendermint-rs 0.16

1ec5375

Merge branch 'master' into romac/light-client-cleanup

74a8210

romac requested review from ancazamfir and greg-szabo September 29, 2020 11:37

romac marked this pull request as ready for review September 29, 2020 11:37

Formatting

2b7f3c9

romac requested a review from andynog September 29, 2020 13:18

Show latest trusted state height in relayer task

1afe240

ancazamfir approved these changes Sep 30, 2020

View reviewed changes

andynog requested changes Sep 30, 2020

View reviewed changes

romac added 2 commits September 30, 2020 17:07

Merge branch 'master' into romac/light-client-cleanup

2b90b9a

Formatting

4e840c8

ancazamfir requested a review from andynog September 30, 2020 19:58

andynog approved these changes Sep 30, 2020

View reviewed changes

ancazamfir merged commit 3cd50d0 into master Sep 30, 2020

ancazamfir deleted the romac/light-client-cleanup branch September 30, 2020 20:05

ancazamfir mentioned this pull request Oct 1, 2020

fix tendermint dependencies #280

Merged

ancazamfir restored the romac/light-client-cleanup branch October 1, 2020 12:36

ancazamfir deleted the romac/light-client-cleanup branch October 1, 2020 12:50

ancazamfir mentioned this pull request Oct 21, 2020

Split tasks spawned by CLI commands into their own modules #331

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to latest tendermint-rs and adapt relayer to new light client #255

Update to latest tendermint-rs and adapt relayer to new light client #255

romac commented Sep 25, 2020 •

edited

Loading

andynog commented Sep 25, 2020

romac commented Sep 25, 2020

romac commented Sep 25, 2020

romac commented Sep 25, 2020

andynog commented Sep 25, 2020

codecov-commenter commented Sep 29, 2020 •

edited

Loading

ancazamfir left a comment

andynog left a comment

romac commented Sep 30, 2020

romac commented Sep 30, 2020

andynog commented Sep 30, 2020 •

edited

Loading

andynog left a comment

Update to latest tendermint-rs and adapt relayer to new light client #255

Update to latest tendermint-rs and adapt relayer to new light client #255

Conversation

romac commented Sep 25, 2020 • edited Loading

Description

andynog commented Sep 25, 2020

romac commented Sep 25, 2020

romac commented Sep 25, 2020

romac commented Sep 25, 2020

andynog commented Sep 25, 2020

codecov-commenter commented Sep 29, 2020 • edited Loading

Codecov Report

ancazamfir left a comment

Choose a reason for hiding this comment

andynog left a comment

Choose a reason for hiding this comment

romac commented Sep 30, 2020

romac commented Sep 30, 2020

andynog commented Sep 30, 2020 • edited Loading

andynog left a comment

Choose a reason for hiding this comment

romac commented Sep 25, 2020 •

edited

Loading

codecov-commenter commented Sep 29, 2020 •

edited

Loading

andynog commented Sep 30, 2020 •

edited

Loading