Skip to content

Commit

Permalink
Merge PR cosmos#869: Add ability to provide a custom light client tru…
Browse files Browse the repository at this point in the history
…sting period

* add custom client trusting period

* remove extra print statment

* fix tests

* make duration conversion more readable

* fix tests

* update flag string

* use time.duration flag

* remove extra print

* remove comment

* handle feedback
  • Loading branch information
boojamya authored Jul 27, 2022
1 parent 4354f48 commit 73b6364
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 28 deletions.
16 changes: 8 additions & 8 deletions _test/relayer_chain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func chainTest(t *testing.T, tcs []testChain) {
require.NoError(t, eg.Wait())

t.Log("Creating clients")
_, err = src.CreateClients(ctx, dst, true, true, false, "")
_, err = src.CreateClients(ctx, dst, true, true, false, 0, "")
require.NoError(t, err)
testClientPair(ctx, t, src, dst)

Expand Down Expand Up @@ -164,7 +164,7 @@ func TestGaiaReuseIdentifiers(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

_, err = src.CreateClients(ctx, dst, true, true, false, "")
_, err = src.CreateClients(ctx, dst, true, true, false, 0, "")
require.NoError(t, err)
testClientPair(ctx, t, src, dst)

Expand Down Expand Up @@ -194,7 +194,7 @@ func TestGaiaReuseIdentifiers(t *testing.T) {
dst.PathEnd.ClientID = ""
dst.PathEnd.ConnectionID = ""

_, err = src.CreateClients(ctx, dst, true, true, false, "")
_, err = src.CreateClients(ctx, dst, true, true, false, 0, "")
require.NoError(t, err)
testClientPair(ctx, t, src, dst)

Expand All @@ -216,7 +216,7 @@ func TestGaiaReuseIdentifiers(t *testing.T) {
src.PathEnd.ClientID = ""
dst.PathEnd.ClientID = ""

_, err = src.CreateClients(ctx, dst, true, true, true, "")
_, err = src.CreateClients(ctx, dst, true, true, true, 0, "")
require.NoError(t, err)
testClientPair(ctx, t, src, dst)

Expand Down Expand Up @@ -245,7 +245,7 @@ func TestGaiaMisbehaviourMonitoring(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

_, err = src.CreateClients(ctx, dst, true, true, false, "")
_, err = src.CreateClients(ctx, dst, true, true, false, 0, "")
require.NoError(t, err)
testClientPair(ctx, t, src, dst)

Expand Down Expand Up @@ -385,7 +385,7 @@ func TestRelayAllChannelsOnConnection(t *testing.T) {
require.NoError(t, eg.Wait())

t.Log("Creating clients")
_, err = src.CreateClients(ctx, dst, true, true, false, "")
_, err = src.CreateClients(ctx, dst, true, true, false, 0, "")
require.NoError(t, err)
testClientPair(ctx, t, src, dst)

Expand Down Expand Up @@ -582,7 +582,7 @@ func TestUnorderedChannelBlockHeightTimeout(t *testing.T) {
require.NoError(t, eg.Wait())

// create path
_, err = src.CreateClients(ctx, dst, true, true, false, "")
_, err = src.CreateClients(ctx, dst, true, true, false, 0, "")
require.NoError(t, err)
testClientPair(ctx, t, src, dst)

Expand Down Expand Up @@ -680,7 +680,7 @@ func TestUnorderedChannelTimestampTimeout(t *testing.T) {
require.NoError(t, eg.Wait())

// create path
_, err = src.CreateClients(ctx, dst, true, true, false, "")
_, err = src.CreateClients(ctx, dst, true, true, false, 0, "")
require.NoError(t, err)
testClientPair(ctx, t, src, dst)

Expand Down
5 changes: 5 additions & 0 deletions cmd/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const (
flagThresholdTime = "time-threshold"
flagUpdateAfterExpiry = "update-after-expiry"
flagUpdateAfterMisbehaviour = "update-after-misbehaviour"
flagClientTrustingPeriod = "client-tp"
flagOverride = "override"
flagSrcPort = "src-port"
flagDstPort = "dst-port"
Expand Down Expand Up @@ -231,12 +232,16 @@ func clientParameterFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command {
"allow governance to update the client if expiry occurs")
cmd.Flags().BoolP(flagUpdateAfterMisbehaviour, "m", true,
"allow governance to update the client if misbehaviour freezing occurs")
cmd.Flags().Duration(flagClientTrustingPeriod, 0, "custom light client trusting period ex. 24h (default: 85% of chains reported unbonding time)")
if err := v.BindPFlag(flagUpdateAfterExpiry, cmd.Flags().Lookup(flagUpdateAfterExpiry)); err != nil {
panic(err)
}
if err := v.BindPFlag(flagUpdateAfterMisbehaviour, cmd.Flags().Lookup(flagUpdateAfterMisbehaviour)); err != nil {
panic(err)
}
if err := v.BindPFlag(flagClientTrustingPeriod, cmd.Flags().Lookup(flagClientTrustingPeriod)); err != nil {
panic(err)
}
return cmd
}

Expand Down
28 changes: 24 additions & 4 deletions cmd/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,11 @@ func createClientsCmd(a *appState) *cobra.Command {
return err
}

customClientTrustingPeriod, err := cmd.Flags().GetDuration(flagClientTrustingPeriod)
if err != nil {
return err
}

override, err := cmd.Flags().GetBool(flagOverride)
if err != nil {
return err
Expand All @@ -138,7 +143,7 @@ func createClientsCmd(a *appState) *cobra.Command {
return fmt.Errorf("key %s not found on dst chain %s", c[dst].ChainProvider.Key(), c[dst].ChainID())
}

modified, err := c[src].CreateClients(cmd.Context(), c[dst], allowUpdateAfterExpiry, allowUpdateAfterMisbehaviour, override, a.Config.memo(cmd))
modified, err := c[src].CreateClients(cmd.Context(), c[dst], allowUpdateAfterExpiry, allowUpdateAfterMisbehaviour, override, customClientTrustingPeriod, a.Config.memo(cmd))
if err != nil {
return err
}
Expand Down Expand Up @@ -177,6 +182,11 @@ func createClientCmd(a *appState) *cobra.Command {
return err
}

customClientTrustingPeriod, err := cmd.Flags().GetDuration(flagClientTrustingPeriod)
if err != nil {
return err
}

override, err := cmd.Flags().GetBool(flagOverride)
if err != nil {
return err
Expand Down Expand Up @@ -244,7 +254,7 @@ func createClientCmd(a *appState) *cobra.Command {
return err
}

modified, err := relayer.CreateClient(cmd.Context(), src, dst, srcUpdateHeader, dstUpdateHeader, allowUpdateAfterExpiry, allowUpdateAfterMisbehaviour, override, a.Config.memo(cmd))
modified, err := relayer.CreateClient(cmd.Context(), src, dst, srcUpdateHeader, dstUpdateHeader, allowUpdateAfterExpiry, allowUpdateAfterMisbehaviour, override, customClientTrustingPeriod, a.Config.memo(cmd))
if err != nil {
return err
}
Expand Down Expand Up @@ -361,6 +371,11 @@ $ %s tx conn demo-path --timeout 5s`,
return err
}

customClientTrustingPeriod, err := cmd.Flags().GetDuration(flagClientTrustingPeriod)
if err != nil {
return err
}

c, src, dst, err := a.Config.ChainsFromPath(args[0])
if err != nil {
return err
Expand Down Expand Up @@ -397,7 +412,7 @@ $ %s tx conn demo-path --timeout 5s`,
}

// ensure that the clients exist
modified, err := c[src].CreateClients(cmd.Context(), c[dst], allowUpdateAfterExpiry, allowUpdateAfterMisbehaviour, override, memo)
modified, err := c[src].CreateClients(cmd.Context(), c[dst], allowUpdateAfterExpiry, allowUpdateAfterMisbehaviour, override, customClientTrustingPeriod, memo)
if err != nil {
return err
}
Expand Down Expand Up @@ -591,6 +606,11 @@ $ %s tx connect demo-path --src-port transfer --dst-port transfer --order unorde
return err
}

customClientTrustingPeriod, err := cmd.Flags().GetDuration(flagClientTrustingPeriod)
if err != nil {
return err
}

pth, err := a.Config.Paths.Get(args[0])
if err != nil {
return err
Expand Down Expand Up @@ -656,7 +676,7 @@ $ %s tx connect demo-path --src-port transfer --dst-port transfer --order unorde
}

// create clients if they aren't already created
modified, err := c[src].CreateClients(cmd.Context(), c[dst], allowUpdateAfterExpiry, allowUpdateAfterMisbehaviour, override, memo)
modified, err := c[src].CreateClients(cmd.Context(), c[dst], allowUpdateAfterExpiry, allowUpdateAfterMisbehaviour, override, customClientTrustingPeriod, memo)
if err != nil {
return fmt.Errorf("error creating clients: %w", err)
}
Expand Down
35 changes: 19 additions & 16 deletions relayer/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
)

// CreateClients creates clients for src on dst and dst on src if the client ids are unspecified.
func (c *Chain) CreateClients(ctx context.Context, dst *Chain, allowUpdateAfterExpiry, allowUpdateAfterMisbehaviour, override bool, memo string) (bool, error) {
func (c *Chain) CreateClients(ctx context.Context, dst *Chain, allowUpdateAfterExpiry, allowUpdateAfterMisbehaviour, override bool, customClientTrustingPeriod time.Duration, memo string) (bool, error) {
// Query the latest heights on src and dst and retry if the query fails
var srch, dsth int64
if err := retry.Do(func() error {
Expand Down Expand Up @@ -58,7 +58,7 @@ func (c *Chain) CreateClients(ctx context.Context, dst *Chain, allowUpdateAfterE
eg.Go(func() error {
var err error
// Create client on src for dst if the client id is unspecified
modifiedSrc, err = CreateClient(egCtx, c, dst, srcUpdateHeader, dstUpdateHeader, allowUpdateAfterExpiry, allowUpdateAfterMisbehaviour, override, memo)
modifiedSrc, err = CreateClient(egCtx, c, dst, srcUpdateHeader, dstUpdateHeader, allowUpdateAfterExpiry, allowUpdateAfterMisbehaviour, override, customClientTrustingPeriod, memo)
if err != nil {
return fmt.Errorf("failed to create client on src chain{%s}: %w", c.ChainID(), err)
}
Expand All @@ -68,7 +68,7 @@ func (c *Chain) CreateClients(ctx context.Context, dst *Chain, allowUpdateAfterE
eg.Go(func() error {
var err error
// Create client on dst for src if the client id is unspecified
modifiedDst, err = CreateClient(egCtx, dst, c, dstUpdateHeader, srcUpdateHeader, allowUpdateAfterExpiry, allowUpdateAfterMisbehaviour, override, memo)
modifiedDst, err = CreateClient(egCtx, dst, c, dstUpdateHeader, srcUpdateHeader, allowUpdateAfterExpiry, allowUpdateAfterMisbehaviour, override, customClientTrustingPeriod, memo)
if err != nil {
return fmt.Errorf("failed to create client on dst chain{%s}: %w", dst.ChainID(), err)
}
Expand All @@ -91,7 +91,7 @@ func (c *Chain) CreateClients(ctx context.Context, dst *Chain, allowUpdateAfterE
return modifiedSrc || modifiedDst, nil
}

func CreateClient(ctx context.Context, src, dst *Chain, srcUpdateHeader, dstUpdateHeader ibcexported.Header, allowUpdateAfterExpiry, allowUpdateAfterMisbehaviour, override bool, memo string) (bool, error) {
func CreateClient(ctx context.Context, src, dst *Chain, srcUpdateHeader, dstUpdateHeader ibcexported.Header, allowUpdateAfterExpiry, allowUpdateAfterMisbehaviour, override bool, customClientTrustingPeriod time.Duration, memo string) (bool, error) {
// If a client ID was specified in the path, ensure it exists.
if src.PathEnd.ClientID != "" {
// TODO: check client is not expired
Expand All @@ -107,19 +107,22 @@ func CreateClient(ctx context.Context, src, dst *Chain, srcUpdateHeader, dstUpda
// Otherwise, create client for the destination chain on the source chain.

// Query the trusting period for dst and retry if the query fails
var tp time.Duration
if err := retry.Do(func() error {
var err error
tp, err = dst.GetTrustingPeriod(ctx)
if err != nil {
return fmt.Errorf("failed to get trusting period for chain{%s}: %w", dst.ChainID(), err)
}
if tp == 0 {
return fmt.Errorf("chain %s reported invalid zero trusting period", dst.ChainID())
// var tp time.Duration
tp := customClientTrustingPeriod
if tp == 0 {
if err := retry.Do(func() error {
var err error
tp, err = dst.GetTrustingPeriod(ctx)
if err != nil {
return fmt.Errorf("failed to get trusting period for chain{%s}: %w", dst.ChainID(), err)
}
if tp == 0 {
return retry.Unrecoverable(fmt.Errorf("chain %s reported invalid zero trusting period", dst.ChainID()))
}
return nil
}, retry.Context(ctx), RtyAtt, RtyDel, RtyErr); err != nil {
return false, err
}
return nil
}, retry.Context(ctx), RtyAtt, RtyDel, RtyErr); err != nil {
return false, err
}

src.log.Debug(
Expand Down

0 comments on commit 73b6364

Please sign in to comment.