feat: add standalone relayer commands #709

ilgooz · 2021-01-31T17:20:56Z

To test:

Please scaffold a new chain on Gitpod.
Serve the chain. ~ wait for chain to be started.
Open another terminal window and run starport relayer configure. No need to enter any value to prompted questions, just hit enter to all. ~ there is an auto faucet discovery.
Finally run starport relayer connect.

ilgooz · 2021-01-31T18:04:51Z

There is something alien in this PR... :12345 somehow serves SDK's Swagger RPC page. Got to fix it before merging.

_
edit: fixed by 450aff7.

fadeev · 2021-01-31T18:33:56Z

This is soooo cool! 🔥🔥🔥 Bob got his tokens on SPN now:

https://rest.alpha.starport.network/cosmos/bank/v1beta1/balances/cosmos1qrsrf6x54szmug5ht747tl9hm73hauxhgma228

It's exactly the functionality that I think we need so very much! Right now connect configures relayer, creates accounts, etc., starport relayer start creates light clients. I was thinking light client creation can happen within "connect" and start will be a long-running process that relays packets (before I realized that rly already has a start command). Maybe we need to rename connect to configure and start to connect. I'll think more about UX of this and we discuss it more on the devx call.

lumtis

Great job👌
Code looks great, I do some tests now

starport/interface/cli/starport/cmd/relayer_connect.go

starport/interface/cli/starport/cmd/relayer_start.go

starport/pkg/chaincmd/runner/account.go

starport/pkg/clispinner/clispinner.go

starport/pkg/xrelayer/chain.go

starport/pkg/xstrings/xstrings.go

fadeev · 2021-02-01T11:43:17Z

Since the standalone relayer command replaces the previous workflow, we may need to disable the relayer info/secret.yml behavior. Right now after connect, when you add relayer's address to config.yml, Starport complains that this address cannot be added to genesis, because it has already been added.

The "happy path" for users for connecting to new chains:

serve both chains
connect them, skip faucet
add relayer's addresses to both chains' config.yml
start the relayer
send tokens

lumtis

Works well for me. Approving it unless Denis' suggestions are for this PR

starport/templates/app/stargate/config.yml

without getting their logs mixed with each other. undo replace directive in go.mod after cosmos/relayer#402 has merged.

Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com>

* rename `relayer connect` to `relayer configure`. * rename `relayer start` to `relayer start`. * auto relay packets between chains via `relayer start` command. * improve logs/docs show account balances, port and channel ids. * add gracefull command cancelation. * only allow to print important relayer logs. * link and start paths in parallel.

integration tests sometimes fails, this might be due to high cpu usage. let's see if this will fix it.

fadeev

Honestly, this is the coolest thing I've seen in a while! @ilgooz, you've outdone yourself! 🔥🔥🔥

fadeev · 2021-02-09T05:56:37Z

I just want to clarify a couple of details.

gitpod /workspace/starport/docs $ starport relayer connect
⛓  4 chains already connected.

---------------------------------------------
Chains by paths
---------------------------------------------

world-bar:
    world > (port: transfer) (channel: connection-1)
    bar   > (port: transfer) (channel: connection-0)

world-spn:
    world > (port: transfer) (channel: connection-0)
    spn   > (port: transfer) (channel: connection-4)

---------------------------------------------
Listening and relaying txs between chains...
---------------------------------------------

I don't think we have 4 chains connected (3 chains, 2 paths). Also, which is a bit more important, channel specifies a connection name (channel: connection-0).

ilgooz · 2021-02-09T08:15:37Z

@fadeev solved by cb1a556.

lumtis · 2021-02-09T09:22:49Z

Very huge 💪🔥 This is very intuitive and easy to use

Some points but honestly none of them are critical for this PR:

starport connect sometimes crashes with the error:

error: RPC error -32603 - Internal error: height 65065 must be less than or equal to the current blockchain height 65064

I restart the command and then it's ok. For me, it seems to happen soon after the first launch of the command, and then it no longer occurs.

Since the connection phase between two chains takes some time, we could show the steps in real-time of the handshake (open, init, ack…)
Once a chain is stopped and I try connect:

could not link chains for "toto-spn" path: please ensure provided on-chain client (07-tendermint-0) exists on the chain (toto): 07-tendermint-0: light client not found [cosmos/cosmos-sdk@v0.41.0/x/ibc/core/02-client/client/utils/utils.go:50]

(a bit weird but when I retry the command the error is slightly different)

could not link chains for "toto-spn" path: ! Connection failed: [toto]client{07-tendermint-0}conn{connection-0} -> [spn]client{07-tendermint-5}conn{connection-5}

I think it could be great to be able to forget a chain in the configuration or ignore that chain that you cannot connect two since if starport relayer can be used for chain development, the developer could have no longer used blockchains in its configuration

It happens only once but the packet has timed out one time and the output was:

I[2021-02-09|09:03:16.970] ✔ [spn]@{65482} - msg(0:update_client,1:timeout) hash(90CCC0493BCE768D9EF7CE6F7084459C566EBAAAA3A4A6333A6EC618FE997C75) 
I[2021-02-09|09:03:16.970] ★ Relayed 1 packets: [toto]port{transfer}->[spn]port{transfer}

Maybe we could have a more explicit message that highlights the fact that the packet failed to be delivered

rly This application relays data between configured IBC enabled chains, I would instead precise that this is the command for the low-level github.com/cosmos/relayer

Available Commands:
  configure   Configure source and target chains for relaying
  connect     Link chains associated with paths and start relaying tx packets in between
  rly         Low-level commands from the relayer binary (github.com/cosmos/relayer)

lumtis · 2021-02-09T09:23:40Z

integration/cmd_serve_test.go

@@ -11,8 +11,6 @@ import (
 )

 func TestServeLaunchpadAppWithWasm(t *testing.T) {
-	t.Parallel()


Pure curiosity: why?

It was related to reduce the cpu usage on CI. Because parallel tests create a huge stress on it and our tests tends to timeout.

ilgooz · 2021-02-09T13:21:15Z

@ltacker thank you! ❤️ These errors will occour when using relayer directly as well, I think handling them gracefully would be better handled there internally.

If you would like to open an issue concerning your thoughts on UX maybe we can start a discussion there?

About the low level starport relayer rly, we're importing the root command directly from relayer, we should be able to customize the description, I'll check on that. 👍

* feat: add standalone relayer commands * fix linter errs + cosmetics * rm good old relayer * fix tests * rm leftovers from the old relayer * support Gitpod for faucet discovery * scaffold: enable faucet by default * fix Gitpod faucet discovery * silence relayer so `relayer start` can link multiple chains in parallel without getting their logs mixed with each other. undo replace directive in go.mod after cosmos/relayer#402 has merged. * link chains in parallel in the relayer start cmd * docs Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com> * rm faucet.coins_max * use bob for the faucet * back to cosmos/relayer * add cancelation for start cmd * refactor & add features: relayer commands * rename `relayer connect` to `relayer configure`. * rename `relayer start` to `relayer start`. * auto relay packets between chains via `relayer start` command. * improve logs/docs show account balances, port and channel ids. * add gracefull command cancelation. * only allow to print important relayer logs. * link and start paths in parallel. * fix test + lint * attach original rly to 'starport relayer rly' depends on cosmos/relayer#411. * cmd docs * upgrade relayer * ensure secure rpc addr always has :443 attached * rm path from rpc addr * fix xurl.CleanPath * fix config * simplify errors * inc timeout for integration tests * fix conflicting statik * rm parallelism from tests that serves an app integration tests sometimes fails, this might be due to high cpu usage. let's see if this will fix it. * fix install script * docs * docs+cosmetics * add conf alias to config command * fix docs Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com>

feat: add standalone relayer commands

1210b6d

ilgooz linked an issue Jan 31, 2021 that may be closed by this pull request

feat: starport relayer #691

Closed

fix linter errs + cosmetics

080aa78

This was referenced Jan 31, 2021

refactor: use cosmosfaucet tendermint/faucet#6

Merged

feat: upgrade SDK version to v0.40.1 tendermint/spn#111

Merged

ilgooz marked this pull request as ready for review January 31, 2021 17:44

ilgooz requested review from fadeev and lumtis as code owners January 31, 2021 17:44

lumtis reviewed Feb 1, 2021

View reviewed changes

lumtis previously approved these changes Feb 1, 2021

View reviewed changes

rm good old relayer

c149b1a

ilgooz dismissed lumtis’s stale review via c149b1a February 2, 2021 14:02

ilgooz added 5 commits February 2, 2021 06:07

fix tests

38edd10

rm leftovers from the old relayer

eaa46fe

support Gitpod for faucet discovery

146d5c5

scaffold: enable faucet by default

93c7fd7

fix Gitpod faucet discovery

f70f1d2

fadeev reviewed Feb 2, 2021

View reviewed changes

starport/templates/app/stargate/config.yml Show resolved Hide resolved

ilgooz and others added 9 commits February 3, 2021 01:29

silence relayer so relayer start can link multiple chains in parallel

3c25c69

without getting their logs mixed with each other. undo replace directive in go.mod after cosmos/relayer#402 has merged.

link chains in parallel in the relayer start cmd

0bf5809

docs

84e0a7e

Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com>

rm faucet.coins_max

0a4ec76

use bob for the faucet

ba94eda

back to cosmos/relayer

157f443

add cancelation for start cmd

fd7878b

Merge branch 'develop' into feat/relayer

a4659bc

ilgooz added 11 commits February 5, 2021 03:06

ensure secure rpc addr always has :443 attached

0703262

rm path from rpc addr

83aacdb

fix xurl.CleanPath

445bf03

fix config

40c7e94

simplify errors

3425223

inc timeout for integration tests

c5ef332

fix conflicting statik

450aff7

rm parallelism from tests that serves an app

83d5934

integration tests sometimes fails, this might be due to high cpu usage. let's see if this will fix it.

fix install script

a62f43d

docs

07ca95e

docs+cosmetics

f77f9bf

ilgooz requested review from fadeev and lumtis February 5, 2021 15:05

add conf alias to config command

9a5e6fb

fadeev previously approved these changes Feb 9, 2021

View reviewed changes

fix docs

cb1a556

ilgooz dismissed fadeev’s stale review via cb1a556 February 9, 2021 08:16

ilgooz requested a review from fadeev February 9, 2021 08:16

Merge branch 'develop' into feat/relayer

5894767

lumtis reviewed Feb 9, 2021

View reviewed changes

lumtis approved these changes Feb 9, 2021

View reviewed changes

fadeev approved these changes Feb 9, 2021

View reviewed changes

ilgooz merged commit 80941cf into develop Feb 9, 2021

ilgooz deleted the feat/relayer branch February 9, 2021 11:13

ilgooz mentioned this pull request Feb 9, 2021

docs(cmd/relayer): document low-level relayer command #737

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add standalone relayer commands #709

feat: add standalone relayer commands #709

ilgooz commented Jan 31, 2021 •

edited

Loading

ilgooz commented Jan 31, 2021 •

edited

Loading

fadeev commented Jan 31, 2021 •

edited

Loading

lumtis left a comment

fadeev commented Feb 1, 2021

lumtis left a comment

fadeev left a comment

fadeev commented Feb 9, 2021

ilgooz commented Feb 9, 2021

lumtis commented Feb 9, 2021

lumtis Feb 9, 2021

ilgooz Feb 9, 2021

ilgooz commented Feb 9, 2021

feat: add standalone relayer commands #709

feat: add standalone relayer commands #709

Conversation

ilgooz commented Jan 31, 2021 • edited Loading

ilgooz commented Jan 31, 2021 • edited Loading

fadeev commented Jan 31, 2021 • edited Loading

lumtis left a comment

Choose a reason for hiding this comment

fadeev commented Feb 1, 2021

lumtis left a comment

Choose a reason for hiding this comment

fadeev left a comment

Choose a reason for hiding this comment

fadeev commented Feb 9, 2021

ilgooz commented Feb 9, 2021

lumtis commented Feb 9, 2021

lumtis Feb 9, 2021

Choose a reason for hiding this comment

ilgooz Feb 9, 2021

Choose a reason for hiding this comment

ilgooz commented Feb 9, 2021

ilgooz commented Jan 31, 2021 •

edited

Loading

ilgooz commented Jan 31, 2021 •

edited

Loading

fadeev commented Jan 31, 2021 •

edited

Loading