fix: ibc clients creation stuck #639
Conversation
…ng using tx send commands
…tion to match new optimistic ibc.
| @@ -69,64 +112,49 @@ func (r *Relayer) CreateIBCChannel(override bool, logFileOption utils.CommandOpt | |||
| }, nil | |||
| } | |||
|
|
|||
| // waitForValidRollappHeight waits for the rollapp height to be greater than 2 otherwise | |||
There was a problem hiding this comment.
Waiting for the rollapp height on the hub to be greater than initial height not needed anymore?
There was a problem hiding this comment.
no. because we open a connection optimistically so only the rollapp height is important.
relayer/create_ibc_channel.go
Outdated
| @@ -34,15 +44,48 @@ func (r *Relayer) CreateIBCChannel(override bool, logFileOption utils.CommandOpt | |||
| return ConnectionChannels{}, err | |||
| } | |||
|
|
|||
| // We always pass override otherwise this commands hangs if there are too many clients | |||
There was a problem hiding this comment.
what the difference with previous version?
it worked without hanging, and without override=true
There was a problem hiding this comment.
it did only if you run it against local hub without thousands of ibc clients. if you run it against froopyland it hangs. the difference i the data on the hub which makes this command hangs as it searches first to see if an ibc client already exists on the hub and when it needs to iterates thousands of clients and consensus states it takes forever.
| time.Sleep(10 * time.Second) | ||
|
|
||
| connectionID, _ := r.GetActiveConnection() | ||
| if connectionID == "" || override { |
There was a problem hiding this comment.
if u set override=true in the createClients, this check is wrong.
if creating new client, u will must to create new connection as well
relayer/create_ibc_channel.go
Outdated
| if !override { | ||
| // Check if clients exist | ||
| clientsExist, err = r.CheckClientsExist() | ||
| if err != nil { |
There was a problem hiding this comment.
ignore err as u don't won't to return here
…e may cause channel creation to hang as it doesn't find a connection.
relayer/clients.go
Outdated
| } | ||
| clientIDRollapp_raw, err := roller_utils.GetNestedValue(rlyCfg, []string{"paths", consts.DefaultRelayerPath, "dst", "client-id"}) | ||
| if err != nil { | ||
| return false, nil |
There was a problem hiding this comment.
better to return the err, and ignore it by the caller
keep same standard as in GetActiveConnection
…'re using now optimistic creation so not necessary.
This PR improves substantially the channel creation.
Channel creation is now done < 3 mins.
It removes unnecessary code which used the old assumption of the forked ibc and changed the chain block production to use send fund commands from sequencer to itself vs update-client which would introduce problems such as account sequence mismatch and header heights coordination between received ibc packet header and already existing ibc header sent using the update command.
PR Standards
Opening a pull request should be able to meet the following requirements
For Author:
godoccommentsFor Reviewer:
After reviewer approval: