From ca1fa2271b0515cea49183c84f109d1d6c85c05a Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Fri, 14 Jul 2023 16:35:13 -0600 Subject: [PATCH] ibc client authority (backport #198) (#204) Co-authored-by: Andrew Gouin --- .github/workflows/e2e-tests.yaml | 4 +- Makefile | 4 +- app/app.go | 11 + cmd/root.go | 8 + go.mod | 2 +- go.sum | 4 +- interchaintest/go.mod | 2 +- interchaintest/go.sum | 4 +- .../ibc_client_expire_substitute_test.go | 243 ++++++++++++++++++ 9 files changed, 273 insertions(+), 9 deletions(-) create mode 100644 interchaintest/ibc_client_expire_substitute_test.go diff --git a/.github/workflows/e2e-tests.yaml b/.github/workflows/e2e-tests.yaml index 1b7b768e..73864248 100644 --- a/.github/workflows/e2e-tests.yaml +++ b/.github/workflows/e2e-tests.yaml @@ -17,7 +17,7 @@ jobs: - name: Build Docker image uses: strangelove-ventures/heighliner-build-action@v0.0.3 with: - registry: # empty registry, image only shared for e2e testing + registry: "" # empty registry, image only shared for e2e testing tag: local # emulate local environment for consistency in interchaintest cases tar-export-path: ${{ env.TAR_PATH }} # export a tarball that can be uploaded as an artifact for the e2e jobs platform: linux/amd64 # test runner architecture only @@ -42,7 +42,7 @@ jobs: strategy: matrix: # names of `make` commands to run tests - test: ["ictest-tkn-factory", "ictest-packet-forward", "ictest-paramauthority", "ictest-chain-upgrade-noble-1", "ictest-chain-upgrade-grand-1", "ictest-globalFee", "ictest-ics20-bps-fees"] + test: ["ictest-tkn-factory", "ictest-packet-forward", "ictest-paramauthority", "ictest-chain-upgrade-noble-1", "ictest-chain-upgrade-grand-1", "ictest-globalFee", "ictest-ics20-bps-fees", "ictest-client-substitution"] fail-fast: false steps: diff --git a/Makefile b/Makefile index 90b2bf01..4ccef598 100644 --- a/Makefile +++ b/Makefile @@ -122,7 +122,9 @@ ictest-globalFee: ictest-ics20-bps-fees: cd interchaintest && go test -race -v -run ^TestICS20BPSFees$$ . - + +ictest-client-substitution: + cd interchaintest && go test -race -v -run ^TestClientSubstitution$$ . ############################################################################### ### Build Image ### diff --git a/app/app.go b/app/app.go index c4957873..861c11c0 100644 --- a/app/app.go +++ b/app/app.go @@ -74,6 +74,8 @@ import ( packetforward "github.com/strangelove-ventures/packet-forward-middleware/v3/router" packetforwardkeeper "github.com/strangelove-ventures/packet-forward-middleware/v3/router/keeper" packetforwardtypes "github.com/strangelove-ventures/packet-forward-middleware/v3/router/types" + paramauthorityibc "github.com/strangelove-ventures/paramauthority/x/ibc" + paramauthorityibctypes "github.com/strangelove-ventures/paramauthority/x/ibc/types" paramauthority "github.com/strangelove-ventures/paramauthority/x/params" paramauthoritykeeper "github.com/strangelove-ventures/paramauthority/x/params/keeper" paramauthorityupgrade "github.com/strangelove-ventures/paramauthority/x/upgrade" @@ -603,12 +605,21 @@ func New( // Uncomment if you want to set a custom migration order here. // app.mm.SetOrderMigrations(custom order) + // Register interfaces for paramauthority ibc proposal shim + paramauthorityibctypes.RegisterInterfaces(interfaceRegistry) + app.mm.RegisterInvariants(&app.CrisisKeeper) app.mm.RegisterRoutes(app.Router(), app.QueryRouter(), encodingConfig.Amino) app.configurator = module.NewConfigurator(app.appCodec, app.MsgServiceRouter(), app.GRPCQueryRouter()) app.mm.RegisterServices(app.configurator) + // Register authoritative IBC client update and IBC upgrade msg handlers + paramauthorityibctypes.RegisterMsgServer( + app.configurator.MsgServer(), + paramauthorityibc.NewMsgServer(app.UpgradeKeeper, app.IBCKeeper.ClientKeeper), + ) + // create the simulation manager and define the order of the modules for deterministic simulations app.sm = module.NewSimulationManager( auth.NewAppModule(appCodec, app.AccountKeeper, authsims.RandomGenesisAccounts), diff --git a/cmd/root.go b/cmd/root.go index 7c1ab4af..7874ea89 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -28,6 +28,7 @@ import ( "github.com/spf13/cast" "github.com/spf13/cobra" "github.com/spf13/pflag" + paramauthorityibccli "github.com/strangelove-ventures/paramauthority/x/ibc/client/cli" paramauthorityparamscli "github.com/strangelove-ventures/paramauthority/x/params/client/cli" paramauthorityupgradecli "github.com/strangelove-ventures/paramauthority/x/upgrade/client/cli" tmcli "github.com/tendermint/tendermint/libs/cli" @@ -284,6 +285,12 @@ func txCommand(moduleBasics module.BasicManager) *cobra.Command { RunE: client.ValidateCmd, } + upgradeCmd := paramauthorityupgradecli.GetTxCmd() + upgradeCmd.AddCommand( + paramauthorityibccli.NewCmdSubmitUpdateClientProposal(), + paramauthorityibccli.NewCmdSubmitUpgradeProposal(), + ) + cmd.AddCommand( authcmd.GetSignCommand(), authcmd.GetSignBatchCommand(), @@ -293,6 +300,7 @@ func txCommand(moduleBasics module.BasicManager) *cobra.Command { authcmd.GetBroadcastCommand(), authcmd.GetEncodeCommand(), authcmd.GetDecodeCommand(), + upgradeCmd, paramauthorityupgradecli.GetTxCmd(), paramauthorityparamscli.GetTxCmd(), ) diff --git a/go.mod b/go.mod index b4749968..c153f9a3 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/spf13/cobra v1.6.1 github.com/spf13/pflag v1.0.5 github.com/strangelove-ventures/packet-forward-middleware/v3 v3.1.5 - github.com/strangelove-ventures/paramauthority v0.1.2 + github.com/strangelove-ventures/paramauthority v0.2.0 github.com/stretchr/testify v1.8.1 github.com/tendermint/tendermint v0.34.27 github.com/tendermint/tm-db v0.6.7 diff --git a/go.sum b/go.sum index 1bf81634..686ad4f2 100644 --- a/go.sum +++ b/go.sum @@ -938,8 +938,8 @@ github.com/spf13/viper v1.14.0/go.mod h1:WT//axPky3FdvXHzGw33dNdXXXfFQqmEalje+eg github.com/status-im/keycard-go v0.0.0-20190316090335-8537d3370df4/go.mod h1:RZLeN1LMWmRsyYjvAu+I6Dm9QmlDaIIt+Y+4Kd7Tp+Q= github.com/strangelove-ventures/packet-forward-middleware/v3 v3.1.5 h1:iXXjziCSAebzuRUPFSnqD7epSDB8LEPgkh9zhbj7ha4= github.com/strangelove-ventures/packet-forward-middleware/v3 v3.1.5/go.mod h1:ncgsf5rykh36HkM16BNcKKx1XzVRdWXt+4pph1syDHE= -github.com/strangelove-ventures/paramauthority v0.1.2 h1:PnbztiGi4ZhFqgcYXC86tQX2bLML17gsqC0EKZEhxg0= -github.com/strangelove-ventures/paramauthority v0.1.2/go.mod h1:PHq/b8q6YfBDJpo34RlnWw+cL5m5Bids2prBUWzsoEM= +github.com/strangelove-ventures/paramauthority v0.2.0 h1:h/ApdnvwV0gAjgQAFJ0Z2U6xuARvBnpmzhkvJRdkJZU= +github.com/strangelove-ventures/paramauthority v0.2.0/go.mod h1:31HVpoItQMa4Wj2BimVhQWbIYeb+kdUDJ8MzBEbGj28= github.com/streadway/amqp v0.0.0-20190404075320-75d898a42a94/go.mod h1:AZpEONHx3DKn8O/DFsRAY58/XVQiIPMTMB1SddzLXVw= github.com/streadway/amqp v0.0.0-20190827072141-edfb9018d271/go.mod h1:AZpEONHx3DKn8O/DFsRAY58/XVQiIPMTMB1SddzLXVw= github.com/streadway/handy v0.0.0-20190108123426-d5acb3125c2a/go.mod h1:qNTQ5P5JnDBl6z3cMAg/SywNDC5ABu5ApDIw6lUbRmI= diff --git a/interchaintest/go.mod b/interchaintest/go.mod index e1164465..2f73658b 100644 --- a/interchaintest/go.mod +++ b/interchaintest/go.mod @@ -8,7 +8,7 @@ require ( github.com/icza/dyno v0.0.0-20220812133438-f0b6f8a18845 github.com/strangelove-ventures/interchaintest/v3 v3.0.0-20230622221919-28c608364e27 github.com/strangelove-ventures/noble v0.0.0-00010101000000-000000000000 - github.com/strangelove-ventures/paramauthority v0.1.2 + github.com/strangelove-ventures/paramauthority v0.2.0 github.com/stretchr/testify v1.8.4 go.uber.org/zap v1.24.0 ) diff --git a/interchaintest/go.sum b/interchaintest/go.sum index 757bce72..ad7fb4c0 100644 --- a/interchaintest/go.sum +++ b/interchaintest/go.sum @@ -987,8 +987,8 @@ github.com/spf13/viper v1.15.0/go.mod h1:fFcTBJxvhhzSJiZy8n+PeW6t8l+KeT/uTARa0jH github.com/status-im/keycard-go v0.0.0-20190316090335-8537d3370df4/go.mod h1:RZLeN1LMWmRsyYjvAu+I6Dm9QmlDaIIt+Y+4Kd7Tp+Q= github.com/strangelove-ventures/interchaintest/v3 v3.0.0-20230622221919-28c608364e27 h1:oJ9yZIFp3yRTlH8BOLnACrsjR1fPIpHyOoLFjM5VRBc= github.com/strangelove-ventures/interchaintest/v3 v3.0.0-20230622221919-28c608364e27/go.mod h1:dWv7E8XtgidmA/A5Gy9x76qMIygES+SxPTnlWjYUb7g= -github.com/strangelove-ventures/paramauthority v0.1.2 h1:PnbztiGi4ZhFqgcYXC86tQX2bLML17gsqC0EKZEhxg0= -github.com/strangelove-ventures/paramauthority v0.1.2/go.mod h1:PHq/b8q6YfBDJpo34RlnWw+cL5m5Bids2prBUWzsoEM= +github.com/strangelove-ventures/paramauthority v0.2.0 h1:h/ApdnvwV0gAjgQAFJ0Z2U6xuARvBnpmzhkvJRdkJZU= +github.com/strangelove-ventures/paramauthority v0.2.0/go.mod h1:31HVpoItQMa4Wj2BimVhQWbIYeb+kdUDJ8MzBEbGj28= github.com/streadway/amqp v0.0.0-20190404075320-75d898a42a94/go.mod h1:AZpEONHx3DKn8O/DFsRAY58/XVQiIPMTMB1SddzLXVw= github.com/streadway/amqp v0.0.0-20190827072141-edfb9018d271/go.mod h1:AZpEONHx3DKn8O/DFsRAY58/XVQiIPMTMB1SddzLXVw= github.com/streadway/handy v0.0.0-20190108123426-d5acb3125c2a/go.mod h1:qNTQ5P5JnDBl6z3cMAg/SywNDC5ABu5ApDIw6lUbRmI= diff --git a/interchaintest/ibc_client_expire_substitute_test.go b/interchaintest/ibc_client_expire_substitute_test.go new file mode 100644 index 00000000..8f3c3b07 --- /dev/null +++ b/interchaintest/ibc_client_expire_substitute_test.go @@ -0,0 +1,243 @@ +package interchaintest_test + +import ( + "context" + "encoding/json" + "fmt" + "testing" + + "github.com/strangelove-ventures/interchaintest/v3" + "github.com/strangelove-ventures/interchaintest/v3/chain/cosmos" + "github.com/strangelove-ventures/interchaintest/v3/ibc" + "github.com/strangelove-ventures/interchaintest/v3/testreporter" + "github.com/strangelove-ventures/interchaintest/v3/testutil" + "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" +) + +// run `make local-image`to rebuild updated binary before running test +func TestClientSubstitution(t *testing.T) { + if testing.Short() { + t.Skip() + } + + t.Parallel() + + ctx := context.Background() + + rep := testreporter.NewNopReporter() + eRep := rep.RelayerExecReporter(t) + + client, network := interchaintest.DockerSetup(t) + + var ( + noble *cosmos.CosmosChain + roles NobleRoles + roles2 NobleRoles + paramauthorityWallet ibc.Wallet + ) + + chainCfg := ibc.ChainConfig{ + Type: "cosmos", + Name: "noble", + ChainID: "noble-1", + Bin: "nobled", + Denom: "token", + Bech32Prefix: "noble", + CoinType: "118", + GasPrices: "0.0token", + GasAdjustment: 1.1, + TrustingPeriod: "504h", + NoHostMount: false, + Images: nobleImageInfo, + EncodingConfig: NobleEncoding(), + PreGenesis: func(cc ibc.ChainConfig) (err error) { + val := noble.Validators[0] + err = createTokenfactoryRoles(ctx, &roles, denomMetadataRupee, val, true) + if err != nil { + return err + } + err = createTokenfactoryRoles(ctx, &roles2, denomMetadataDrachma, val, true) + if err != nil { + return err + } + paramauthorityWallet, err = createParamAuthAtGenesis(ctx, val) + return err + }, + ModifyGenesis: func(cc ibc.ChainConfig, b []byte) ([]byte, error) { + g := make(map[string]interface{}) + if err := json.Unmarshal(b, &g); err != nil { + return nil, fmt.Errorf("failed to unmarshal genesis file: %w", err) + } + if err := modifyGenesisTokenfactory(g, "tokenfactory", denomMetadataRupee, &roles, true); err != nil { + return nil, err + } + if err := modifyGenesisTokenfactory(g, "fiat-tokenfactory", denomMetadataDrachma, &roles2, true); err != nil { + return nil, err + } + if err := modifyGenesisParamAuthority(g, paramauthorityWallet.FormattedAddress()); err != nil { + return nil, err + } + if err := modifyGenesisTariffDefaults(g, paramauthorityWallet.FormattedAddress()); err != nil { + return nil, err + } + out, err := json.Marshal(&g) + if err != nil { + return nil, fmt.Errorf("failed to marshal genesis bytes to json: %w", err) + } + return out, nil + }, + } + + nv := 1 + nf := 0 + + cf := interchaintest.NewBuiltinChainFactory(zaptest.NewLogger(t), []*interchaintest.ChainSpec{ + { + ChainConfig: chainCfg, + NumValidators: &nv, + NumFullNodes: &nf, + }, + { + Name: "gaia", + Version: "v10.0.2", + NumValidators: &nv, + NumFullNodes: &nf, + }, + }) + + chains, err := cf.Chains(t.Name()) + require.NoError(t, err) + + noble = chains[0].(*cosmos.CosmosChain) + gaia := chains[1].(*cosmos.CosmosChain) + + r := interchaintest.NewBuiltinRelayerFactory( + ibc.CosmosRly, + zaptest.NewLogger(t), + relayerImage, + ).Build(t, client, network) + + pathName := "noble-gaia" + + ic := interchaintest.NewInterchain(). + AddChain(noble). + AddChain(gaia). + AddRelayer(r, "r"). + AddLink(interchaintest.InterchainLink{ + Chain1: noble, + Chain2: gaia, + Relayer: r, + Path: pathName, + }) + + require.NoError(t, ic.Build(ctx, eRep, interchaintest.InterchainBuildOptions{ + TestName: t.Name(), + Client: client, + NetworkID: network, + + SkipPathCreation: true, + })) + t.Cleanup(func() { + _ = ic.Close() + }) + + nobleChainID := noble.Config().ChainID + gaiaChainID := gaia.Config().ChainID + + err = r.GeneratePath(ctx, eRep, nobleChainID, gaiaChainID, pathName) + require.NoError(t, err) + + // create client on noble with short trusting period which will expire. + res := r.Exec(ctx, eRep, []string{"rly", "tx", "client", nobleChainID, gaiaChainID, pathName, "--client-tp", "20s", "--home", "/home/relayer"}, nil) + require.NoError(t, res.Err) + + // create client on gaia with longer trusting period so it won't expire for this test. + res = r.Exec(ctx, eRep, []string{"rly", "tx", "client", gaiaChainID, nobleChainID, pathName, "--home", "/home/relayer"}, nil) + require.NoError(t, res.Err) + + err = testutil.WaitForBlocks(ctx, 2, noble, gaia) + require.NoError(t, err) + + err = r.CreateConnections(ctx, eRep, pathName) + require.NoError(t, err) + + err = testutil.WaitForBlocks(ctx, 2, noble, gaia) + require.NoError(t, err) + + err = r.CreateChannel(ctx, eRep, pathName, ibc.CreateChannelOptions{ + SourcePortName: "transfer", + DestPortName: "transfer", + Order: ibc.Unordered, + Version: "ics20-1", + }) + require.NoError(t, err) + + const userFunds = int64(10_000_000_000) + users := interchaintest.GetAndFundTestUsers(t, ctx, t.Name(), userFunds, noble, gaia) + + nobleClients, err := r.GetClients(ctx, eRep, nobleChainID) + require.NoError(t, err) + require.Len(t, nobleClients, 1) + + nobleClient := nobleClients[0] + + nobleChannels, err := r.GetChannels(ctx, eRep, nobleChainID) + require.NoError(t, err) + require.Len(t, nobleChannels, 1) + nobleChannel := nobleChannels[0] + + err = testutil.WaitForBlocks(ctx, 20, noble) + require.NoError(t, err) + + // client should now be expired, no relayer was running to update the clients during the 20s trusting period. + + _, err = noble.SendIBCTransfer(ctx, nobleChannel.ChannelID, users[0].KeyName(), ibc.WalletAmount{ + Address: users[1].FormattedAddress(), + Amount: 1000000, + Denom: noble.Config().Denom, + }, ibc.TransferOptions{}) + + require.Error(t, err) + require.ErrorContains(t, err, "status Expired: client is not active") + + // create new client on noble + res = r.Exec(ctx, eRep, []string{"rly", "tx", "client", nobleChainID, gaiaChainID, pathName, "--override", "--home", "/home/relayer"}, nil) + require.NoError(t, res.Err) + + nobleClients, err = r.GetClients(ctx, eRep, nobleChainID) + require.NoError(t, err) + require.Len(t, nobleClients, 2) + + newNobleClient := nobleClients[1] + + // substitute new client state into old client + _, err = noble.Validators[0].ExecTx(ctx, paramauthorityWallet.KeyName(), "upgrade", "update-client", nobleClient.ClientID, newNobleClient.ClientID) + require.NoError(t, err) + + // update config to old client ID + res = r.Exec(ctx, eRep, []string{"rly", "paths", "update", pathName, "--src-client-id", nobleClient.ClientID, "--home", "/home/relayer"}, nil) + require.NoError(t, res.Err) + + // start up relayer and test a transfer + err = r.StartRelayer(ctx, eRep, pathName) + require.NoError(t, err) + + t.Cleanup(func() { + _ = r.StopRelayer(ctx, eRep) + }) + + nobleHeight, err := noble.Height(ctx) + require.NoError(t, err) + + // send a packet on the same channel with new client, should succeed. + tx, err := noble.SendIBCTransfer(ctx, nobleChannel.ChannelID, users[0].KeyName(), ibc.WalletAmount{ + Address: users[1].FormattedAddress(), + Amount: 1000000, + Denom: noble.Config().Denom, + }, ibc.TransferOptions{}) + require.NoError(t, err) + + _, err = testutil.PollForAck(ctx, noble, nobleHeight, nobleHeight+10, tx.Packet) + require.NoError(t, err) +}