[BP: v7 <- #965] Fixes and wires up CreateClient (#969)

* Fixes and wires up `CreateClient` (#965)

* createClient WIP

* use chain id instead of chain name

* imp hermes

(cherry picked from commit 23a8755)

# Conflicts:
#	relayer/rly/cosmos_relayer.go

* clean up merge

* add create client options

* hermes

* remove comment

* fix comment

* clean up hyperspace

* simd version in test

* mod tidy

---------

Co-authored-by: Dan Kanefsky <[email protected]>
Co-authored-by: Dan Kanefsky <[email protected]>

examples/ibc/client_creation_test.go

@@ -0,0 +1,143 @@
+package ibc_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/strangelove-ventures/interchaintest/v7"
+	"github.com/strangelove-ventures/interchaintest/v7/chain/cosmos"
+	"github.com/strangelove-ventures/interchaintest/v7/ibc"
+	"github.com/strangelove-ventures/interchaintest/v7/testreporter"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap/zaptest"
+)
+
+var (
+	numVals      = 1
+	numFullNodes = 0
+)
+
+type relayerImp struct {
+	name       string
+	relayerImp ibc.RelayerImplementation
+}
+
+func TestCreatClient(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping in short mode")
+	}
+
+	t.Parallel()
+
+	var tests = []relayerImp{
+		{
+			name:       "Cosmos Relayer",
+			relayerImp: ibc.CosmosRly,
+		},
+		{
+			name:       "Hermes",
+			relayerImp: ibc.Hermes,
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		testname := tt.name
+		t.Run(testname, func(t *testing.T) {
+			t.Parallel()
+
+			ctx := context.Background()
+
+			chainSpec := []*interchaintest.ChainSpec{
+				{
+					Name:      "ibc-go-simd",
+					ChainName: "chain1",
+					Version:   "v7.2.0",
+
+					NumValidators: &numVals,
+					NumFullNodes:  &numFullNodes,
+				},
+				{
+					Name:      "ibc-go-simd",
+					ChainName: "chain2",
+					Version:   "v7.2.0",
+
+					NumValidators: &numVals,
+					NumFullNodes:  &numFullNodes,
+				},
+			}
+
+			chains := interchaintest.CreateChainsWithChainSpecs(t, chainSpec)
+
+			client, network := interchaintest.DockerSetup(t)
+
+			chain, counterpartyChain := chains[0].(*cosmos.CosmosChain), chains[1].(*cosmos.CosmosChain)
+
+			rf := interchaintest.NewBuiltinRelayerFactory(
+				tt.relayerImp,
+				zaptest.NewLogger(t),
+			)
+
+			r := rf.Build(t, client, network)
+
+			pathName := "ibc-path"
+
+			ic := interchaintest.NewInterchain().
+				AddChain(chain).
+				AddChain(counterpartyChain).
+				AddRelayer(r, "relayer").
+				AddLink(interchaintest.InterchainLink{
+					Chain1:  chain,
+					Chain2:  counterpartyChain,
+					Relayer: r,
+					Path:    pathName,
+				})
+
+			rep := testreporter.NewNopReporter()
+
+			require.NoError(t, ic.Build(ctx, rep.RelayerExecReporter(t), interchaintest.InterchainBuildOptions{
+				TestName:         t.Name(),
+				Client:           client,
+				NetworkID:        network,
+				SkipPathCreation: true,
+			}))
+			t.Cleanup(func() {
+				_ = ic.Close()
+			})
+
+			eRep := rep.RelayerExecReporter(t)
+
+			// Get clients for each chain
+			srcClientInfoBefore, err := r.GetClients(ctx, eRep, chain.Config().ChainID)
+			require.NoError(t, err)
+			destClientInfoBefore, err := r.GetClients(ctx, eRep, counterpartyChain.Config().ChainID)
+			require.NoError(t, err)
+
+			require.NoError(t,
+				r.GeneratePath(ctx, eRep, chain.Config().ChainID, counterpartyChain.Config().ChainID, pathName))
+
+			// create single client
+			require.NoError(t,
+				r.CreateClient(ctx, eRep,
+					chain.Config().ChainID,
+					counterpartyChain.Config().ChainID,
+					pathName, ibc.CreateClientOptions{},
+				),
+			)
+
+			srcClientInfoAfter, err := r.GetClients(ctx, eRep, chain.Config().ChainID)
+			require.NoError(t, err)
+			destClientInfoAfter, err := r.GetClients(ctx, eRep, counterpartyChain.Config().ChainID)
+			require.NoError(t, err)
+
+			// After creating the single client on the source chain, there should be one more client than before
+			require.Equal(t, len(srcClientInfoBefore), (len(srcClientInfoAfter) - 1), "there is not exactly 1 more client on the source chain after running createClient")
+
+			// createClients should only create a client on source, NOT destination chain
+			require.Equal(t, len(destClientInfoBefore), len(destClientInfoAfter), "a client was created on the destination chain")
+
+		})
+
+	}
+
+}

go.mod

@@ -52,7 +52,7 @@ require (
 	cloud.google.com/go/compute/metadata v0.2.3 // indirect
 	cloud.google.com/go/iam v1.1.1 // indirect
 	cloud.google.com/go/storage v1.30.1 // indirect
-	cosmossdk.io/api v0.3.1 // indirect
+	cosmossdk.io/api v0.3.1
 	cosmossdk.io/core v0.6.1 // indirect
 	cosmossdk.io/depinject v1.0.0-alpha.4 // indirect
 	cosmossdk.io/errors v1.0.0 // indirect

ibc/relayer.go

@@ -73,9 +73,10 @@ type Relayer interface {
 	// Flush flushes any outstanding packets and then returns.
 	Flush(ctx context.Context, rep RelayerExecReporter, pathName string, channelID string) error
 
-	// CreateClients performs the client handshake steps necessary for creating a light client
+	// CreateClient performs the client handshake steps necessary for creating a light client
 	// on src that tracks the state of dst.
-	CreateClient(ctx context.Context, rep RelayerExecReporter, srcChainID string, dstChainID string, pathName string, opts CreateClientOptions) error
+	// Unlike CreateClients, this only creates the client on the source chain
+	CreateClient(ctx context.Context, rep RelayerExecReporter, srcChainID, dstChainID, pathName string, opts CreateClientOptions) error
 
 	// CreateClients performs the client handshake steps necessary for creating a light client
 	// on src that tracks the state of dst, and a light client on dst that tracks the state of src.
@@ -272,23 +273,36 @@ func (o Order) Validate() error {
 }
 
 // CreateClientOptions contains the configuration for creating a client.
+
+// a zero value is the same as not specifying the flag and will use the relayer defauls
 type CreateClientOptions struct {
-	TrustingPeriod string
+	TrustingPeriod           string
+	TrustingPeriodPercentage int64 // only available for Go Relayer
+	MaxClockDrift            string
 }
 
 // DefaultClientOpts returns the default settings for creating clients.
-// These default options are usually determined by the relayer
+
+// empty values will use the relayer defaults
 func DefaultClientOpts() CreateClientOptions {
-	return CreateClientOptions{
-		TrustingPeriod: "0",
-	}
+	return CreateClientOptions{}
 }
 
 func (opts CreateClientOptions) Validate() error {
-	_, err := time.ParseDuration(opts.TrustingPeriod)
-	if err != nil {
-		return err
+	if opts.TrustingPeriod != "" {
+		_, err := time.ParseDuration(opts.TrustingPeriod)
+		if err != nil {
+			return err
+		}
 	}
+
+	if opts.MaxClockDrift != "" {
+		_, err := time.ParseDuration(opts.MaxClockDrift)
+		if err != nil {
+			return err
+		}
+	}
+
 	return nil
 }
 

ibc/relayer_test.go

@@ -34,3 +34,31 @@ func TestChannelOptsConfigured(t *testing.T) {
 	}
 	require.Error(t, opts.Validate())
 }
+
+func TestClientOptsConfigured(t *testing.T) {
+	// Test the default client opts
+	opts := DefaultClientOpts()
+	require.NoError(t, opts.Validate())
+
+	// Test empty struct client opts
+	opts = CreateClientOptions{}
+	require.NoError(t, opts.Validate())
+
+	// Test partial client opts
+	opts = CreateClientOptions{
+		MaxClockDrift: "5m",
+	}
+	require.NoError(t, opts.Validate())
+
+	// Test invalid MaxClockDrift
+	opts = CreateClientOptions{
+		MaxClockDrift: "invalid duration",
+	}
+	require.Error(t, opts.Validate())
+
+	// Test invalid TrustingPeriod
+	opts = CreateClientOptions{
+		TrustingPeriod: "invalid duration",
+	}
+	require.Error(t, opts.Validate())
+}

relayer/docker.go

@@ -222,14 +222,14 @@ func (r *DockerRelayer) CreateChannel(ctx context.Context, rep ibc.RelayerExecRe
 	return res.Err
 }
 
-func (r *DockerRelayer) CreateClient(ctx context.Context, rep ibc.RelayerExecReporter, srcChainID string, dstChainID string, pathName string, opts ibc.CreateClientOptions) error {
-	cmd := r.c.CreateClient(srcChainID, dstChainID, pathName, opts, r.HomeDir())
+func (r *DockerRelayer) CreateClients(ctx context.Context, rep ibc.RelayerExecReporter, pathName string, opts ibc.CreateClientOptions) error {
+	cmd := r.c.CreateClients(pathName, opts, r.HomeDir())
 	res := r.Exec(ctx, rep, cmd, nil)
 	return res.Err
 }
 
-func (r *DockerRelayer) CreateClients(ctx context.Context, rep ibc.RelayerExecReporter, pathName string, opts ibc.CreateClientOptions) error {
-	cmd := r.c.CreateClients(pathName, opts, r.HomeDir())
+func (r *DockerRelayer) CreateClient(ctx context.Context, rep ibc.RelayerExecReporter, srcChainID, dstChainID, pathName string, opts ibc.CreateClientOptions) error {
+	cmd := r.c.CreateClient(srcChainID, dstChainID, pathName, opts, r.HomeDir())
 	res := r.Exec(ctx, rep, cmd, nil)
 	return res.Err
 }

relayer/hermes/hermes_relayer.go

@@ -194,9 +194,12 @@ func (r *Relayer) UpdateClients(ctx context.Context, rep ibc.RelayerExecReporter
 func (r *Relayer) CreateClients(ctx context.Context, rep ibc.RelayerExecReporter, pathName string, opts ibc.CreateClientOptions) error {
 	pathConfig := r.paths[pathName]
 	chainACreateClientCmd := []string{hermes, "--json", "create", "client", "--host-chain", pathConfig.chainA.chainID, "--reference-chain", pathConfig.chainB.chainID}
-	if opts.TrustingPeriod != "0" {
+	if opts.TrustingPeriod != "" {
 		chainACreateClientCmd = append(chainACreateClientCmd, "--trusting-period", opts.TrustingPeriod)
 	}
+	if opts.MaxClockDrift != "" {
+		chainACreateClientCmd = append(chainACreateClientCmd, "--clock-drift", opts.MaxClockDrift)
+	}
 	res := r.Exec(ctx, rep, chainACreateClientCmd, nil)
 	if res.Err != nil {
 		return res.Err
@@ -209,9 +212,12 @@ func (r *Relayer) CreateClients(ctx context.Context, rep ibc.RelayerExecReporter
 	pathConfig.chainA.clientID = chainAClientId
 
 	chainBCreateClientCmd := []string{hermes, "--json", "create", "client", "--host-chain", pathConfig.chainB.chainID, "--reference-chain", pathConfig.chainA.chainID}
-	if opts.TrustingPeriod != "0" {
+	if opts.TrustingPeriod != "" {
 		chainBCreateClientCmd = append(chainBCreateClientCmd, "--trusting-period", opts.TrustingPeriod)
 	}
+	if opts.MaxClockDrift != "" {
+		chainBCreateClientCmd = append(chainBCreateClientCmd, "--clock-drift", opts.MaxClockDrift)
+	}
 	res = r.Exec(ctx, rep, chainBCreateClientCmd, nil)
 	if res.Err != nil {
 		return res.Err
@@ -226,6 +232,37 @@ func (r *Relayer) CreateClients(ctx context.Context, rep ibc.RelayerExecReporter
 	return res.Err
 }
 
+func (r *Relayer) CreateClient(ctx context.Context, rep ibc.RelayerExecReporter, srcChainID, dstChainID, pathName string, opts ibc.CreateClientOptions) error {
+	pathConfig := r.paths[pathName]
+
+	createClientCmd := []string{hermes, "--json", "create", "client", "--host-chain", srcChainID, "--reference-chain", dstChainID}
+	if opts.TrustingPeriod != "" {
+		createClientCmd = append(createClientCmd, "--trusting-period", opts.TrustingPeriod)
+	}
+	if opts.MaxClockDrift != "" {
+		createClientCmd = append(createClientCmd, "--clock-drift", opts.MaxClockDrift)
+	}
+	res := r.Exec(ctx, rep, createClientCmd, nil)
+	if res.Err != nil {
+		return res.Err
+	}
+
+	clientId, err := GetClientIdFromStdout(res.Stdout)
+	if err != nil {
+		return err
+	}
+
+	if pathConfig.chainA.chainID == srcChainID {
+		pathConfig.chainA.chainID = clientId
+	} else if pathConfig.chainB.chainID == srcChainID {
+		pathConfig.chainB.chainID = clientId
+	} else {
+		return fmt.Errorf("%s not found in path config", srcChainID)
+	}
+
+	return res.Err
+}
+
 // RestoreKey restores a key from a mnemonic. In hermes, you must provide a file containing the mnemonic. We need
 // to copy the contents of the mnemonic into a file on disk and then reference the newly created file.
 func (r *Relayer) RestoreKey(ctx context.Context, rep ibc.RelayerExecReporter, cfg ibc.ChainConfig, keyName, mnemonic string) error {

relayer/hyperspace/hyperspace_commander.go

@@ -81,10 +81,6 @@ func (c *hyperspaceCommander) CreateChannel(pathName string, opts ibc.CreateChan
 	}
 }
 
-func (c *hyperspaceCommander) CreateClient(srcChainID, dstChainID, pathName string, opts ibc.CreateClientOptions, homeDir string) []string {
-	panic("[CreateClient] do not call me")
-}
-
 func (c *hyperspaceCommander) CreateClients(pathName string, opts ibc.CreateClientOptions, homeDir string) []string {
 	fmt.Println("[hyperspace] CreateClients", pathName, opts, homeDir)
 	_, ok := c.paths[pathName]
@@ -109,6 +105,11 @@ func (c *hyperspaceCommander) CreateClients(pathName string, opts ibc.CreateClie
 	}
 }
 
+// TODO: Implement if available in hyperspace relayer
+func (hyperspaceCommander) CreateClient(srcChainID, dstChainID, pathName string, opts ibc.CreateClientOptions, homeDir string) []string {
+	panic("[CreateClient] Not Implemented")
+}
+
 func (c *hyperspaceCommander) CreateConnections(pathName, homeDir string) []string {
 	fmt.Println("[hyperspace] CreateConnections", pathName, homeDir)
 	_, ok := c.paths[pathName]