refactor: remove legacy client update proposal (#4581)

* refactor: remove legacy client update proposal

* e2e: swap from ClientUpdateProposal e2e to RecoverClient

* refactor: remove unused events

.github/compatibility-test-matrices/main/client-chain-a.json

@@ -18,7 +18,7 @@
     "TestClientTestSuite"
   ],
   "test": [
-    "TestClientUpdateProposal_Succeeds",
+    "TestRecoverClient_Succeeds",
     "TestClient_Update_Misbehaviour",
     "TestAllowedClientsParam"
   ],

.github/compatibility-test-matrices/main/client-chain-b.json

@@ -18,7 +18,7 @@
     "TestClientTestSuite"
   ],
   "test": [
-    "TestClientUpdateProposal_Succeeds",
+    "TestRecoverClient_Succeeds",
     "TestClient_Update_Misbehaviour"
   ],
   "relayer-type": [

.github/compatibility-test-matrices/release-v6.1.x/client-chain-a.json

@@ -17,7 +17,7 @@
     "TestClientTestSuite"
   ],
   "test": [
-    "TestClientUpdateProposal_Succeeds",
+    "TestRecoverClient_Succeeds",
     "TestClient_Update_Misbehaviour",
     "TestAllowedClientsParam"
   ],

.github/compatibility-test-matrices/release-v6.1.x/client-chain-b.json

@@ -17,7 +17,7 @@
     "TestClientTestSuite"
   ],
   "test": [
-    "TestClientUpdateProposal_Succeeds",
+    "TestRecoverClient_Succeeds",
     "TestClient_Update_Misbehaviour"
   ],
   "relayer-type": [

.github/compatibility-test-matrices/release-v6.2.x/client-chain-a.json

@@ -17,7 +17,7 @@
     "TestClientTestSuite"
   ],
   "test": [
-    "TestClientUpdateProposal_Succeeds",
+    "TestRecoverClient_Succeeds",
     "TestClient_Update_Misbehaviour",
     "TestAllowedClientsParam"
   ],

.github/compatibility-test-matrices/release-v6.2.x/client-chain-b.json

@@ -17,7 +17,7 @@
     "TestClientTestSuite"
   ],
   "test": [
-    "TestClientUpdateProposal_Succeeds",
+    "TestRecoverClient_Succeeds",
     "TestClient_Update_Misbehaviour"
   ],
   "relayer-type": [

.github/compatibility-test-matrices/release-v7.2.x/client-chain-a.json

@@ -17,7 +17,7 @@
     "TestClientTestSuite"
   ],
   "test": [
-    "TestClientUpdateProposal_Succeeds",
+    "TestRecoverClient_Succeeds",
     "TestClient_Update_Misbehaviour",
     "TestAllowedClientsParam"
   ],

.github/compatibility-test-matrices/release-v7.2.x/client-chain-b.json

@@ -17,7 +17,7 @@
     "TestClientTestSuite"
   ],
   "test": [
-    "TestClientUpdateProposal_Succeeds",
+    "TestRecoverClient_Succeeds",
     "TestClient_Update_Misbehaviour"
   ],
   "relayer-type": [

.github/compatibility-test-matrices/release-v7.3.x/client-chain-a.json

@@ -17,7 +17,7 @@
     "TestClientTestSuite"
   ],
   "test": [
-    "TestClientUpdateProposal_Succeeds",
+    "TestRecoverClient_Succeeds",
     "TestClient_Update_Misbehaviour",
     "TestAllowedClientsParam"
   ],

.github/compatibility-test-matrices/release-v7.3.x/client-chain-b.json

@@ -17,7 +17,7 @@
     "TestClientTestSuite"
   ],
   "test": [
-    "TestClientUpdateProposal_Succeeds",
+    "TestRecoverClient_Succeeds",
     "TestClient_Update_Misbehaviour"
   ],
   "relayer-type": [

.github/compatibility-test-matrices/unreleased/client.json

@@ -21,7 +21,7 @@
     "TestClientTestSuite"
   ],
   "test": [
-    "TestClientUpdateProposal_Succeeds",
+    "TestRecoverClient_Succeeds",
     "TestClient_Update_Misbehaviour",
     "TestAllowedClientsParam"
   ],

e2e/tests/core/02-client/client_test.go

@@ -71,88 +71,6 @@ func (s *ClientTestSuite) QueryAllowedClients(ctx context.Context, chain ibc.Cha
 	return res.Params.AllowedClients
 }
 
-func (s *ClientTestSuite) TestClientUpdateProposal_Succeeds() {
-	t := s.T()
-	ctx := context.TODO()
-
-	var (
-		pathName           string
-		relayer            ibc.Relayer
-		subjectClientID    string
-		substituteClientID string
-		// set the trusting period to a value which will still be valid upon client creation, but invalid before the first update
-		badTrustingPeriod = time.Second * 10
-	)
-
-	t.Run("create substitute client with correct trusting period", func(t *testing.T) {
-		relayer, _ = s.SetupChainsRelayerAndChannel(ctx)
-
-		// TODO: update when client identifier created is accessible
-		// currently assumes first client is 07-tendermint-0
-		substituteClientID = clienttypes.FormatClientIdentifier(ibcexported.Tendermint, 0)
-
-		// TODO: replace with better handling of path names
-		pathName = fmt.Sprintf("%s-path-%d", s.T().Name(), 0)
-		pathName = strings.ReplaceAll(pathName, "/", "-")
-	})
-
-	chainA, chainB := s.GetChains()
-	chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount)
-
-	t.Run("create subject client with bad trusting period", func(t *testing.T) {
-		createClientOptions := ibc.CreateClientOptions{
-			TrustingPeriod: badTrustingPeriod.String(),
-		}
-
-		s.SetupClients(ctx, relayer, createClientOptions)
-
-		// TODO: update when client identifier created is accessible
-		// currently assumes second client is 07-tendermint-1
-		subjectClientID = clienttypes.FormatClientIdentifier(ibcexported.Tendermint, 1)
-	})
-
-	time.Sleep(badTrustingPeriod)
-
-	t.Run("update substitute client", func(t *testing.T) {
-		s.UpdateClients(ctx, relayer, pathName)
-	})
-
-	s.Require().NoError(test.WaitForBlocks(ctx, 1, chainA, chainB), "failed to wait for blocks")
-
-	t.Run("check status of each client", func(t *testing.T) {
-		t.Run("substitute should be active", func(t *testing.T) {
-			status, err := s.Status(ctx, chainA, substituteClientID)
-			s.Require().NoError(err)
-			s.Require().Equal(ibcexported.Active.String(), status)
-		})
-
-		t.Run("subject should be expired", func(t *testing.T) {
-			status, err := s.Status(ctx, chainA, subjectClientID)
-			s.Require().NoError(err)
-			s.Require().Equal(ibcexported.Expired.String(), status)
-		})
-	})
-
-	t.Run("pass client update proposal", func(t *testing.T) {
-		proposal := clienttypes.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subjectClientID, substituteClientID)
-		s.ExecuteGovProposal(ctx, chainA, chainAWallet, proposal)
-	})
-
-	t.Run("check status of each client", func(t *testing.T) {
-		t.Run("substitute should be active", func(t *testing.T) {
-			status, err := s.Status(ctx, chainA, substituteClientID)
-			s.Require().NoError(err)
-			s.Require().Equal(ibcexported.Active.String(), status)
-		})
-
-		t.Run("subject should be active", func(t *testing.T) {
-			status, err := s.Status(ctx, chainA, subjectClientID)
-			s.Require().NoError(err)
-			s.Require().Equal(ibcexported.Active.String(), status)
-		})
-	})
-}
-
 // TestRecoverClient_Succeeds tests that a governance proposal to recover a client using a MsgRecoverClient is successful.
 func (s *ClientTestSuite) TestRecoverClient_Succeeds() {
 	t := s.T()

modules/core/02-client/client/cli/tx.go

@@ -303,63 +303,6 @@ func newSubmitRecoverClientProposalCmd() *cobra.Command {
 	return cmd
 }
 
-// NewCmdSubmitUpdateClientProposal implements a command handler for submitting an update IBC client proposal transaction.
-func NewCmdSubmitUpdateClientProposal() *cobra.Command {
-	cmd := &cobra.Command{
-		Use:   "update-client [subject-client-id] [substitute-client-id]",
-		Args:  cobra.ExactArgs(2),
-		Short: "Submit an update IBC client proposal",
-		Long: "Submit an update IBC client proposal along with an initial deposit.\n" +
-			"Please specify a subject client identifier you want to update.\n" +
-			"Please specify the substitute client the subject client will be updated to.",
-		RunE: func(cmd *cobra.Command, args []string) error {
-			clientCtx, err := client.GetClientTxContext(cmd)
-			if err != nil {
-				return err
-			}
-
-			title, err := cmd.Flags().GetString(govcli.FlagTitle) //nolint:staticcheck // need this till full govv1 conversion.
-			if err != nil {
-				return err
-			}
-
-			description, err := cmd.Flags().GetString(govcli.FlagDescription) //nolint:staticcheck // need this till full govv1 conversion.
-			if err != nil {
-				return err
-			}
-
-			subjectClientID := args[0]
-			substituteClientID := args[1]
-
-			content := types.NewClientUpdateProposal(title, description, subjectClientID, substituteClientID)
-
-			from := clientCtx.GetFromAddress()
-
-			depositStr, err := cmd.Flags().GetString(govcli.FlagDeposit)
-			if err != nil {
-				return err
-			}
-			deposit, err := sdk.ParseCoinsNormalized(depositStr)
-			if err != nil {
-				return err
-			}
-
-			msg, err := govv1beta1.NewMsgSubmitProposal(content, deposit, from)
-			if err != nil {
-				return err
-			}
-
-			return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
-		},
-	}
-
-	cmd.Flags().String(govcli.FlagTitle, "", "title of proposal")             //nolint:staticcheck // need this till full govv1 conversion.
-	cmd.Flags().String(govcli.FlagDescription, "", "description of proposal") //nolint:staticcheck // need this till full govv1 conversion.
-	cmd.Flags().String(govcli.FlagDeposit, "", "deposit of proposal")
-
-	return cmd
-}
-
 // NewCmdSubmitUpgradeProposal implements a command handler for submitting an upgrade IBC client proposal transaction.
 func NewCmdSubmitUpgradeProposal() *cobra.Command {
 	cmd := &cobra.Command{

modules/core/02-client/client/proposal_handler.go

@@ -6,7 +6,4 @@ import (
 	"github.com/cosmos/ibc-go/v7/modules/core/02-client/client/cli"
 )
 
-var (
-	UpdateClientProposalHandler = govclient.NewProposalHandler(cli.NewCmdSubmitUpdateClientProposal)
-	UpgradeProposalHandler      = govclient.NewProposalHandler(cli.NewCmdSubmitUpgradeProposal)
-)
+var UpgradeProposalHandler = govclient.NewProposalHandler(cli.NewCmdSubmitUpgradeProposal)

modules/core/02-client/keeper/events.go

@@ -82,21 +82,6 @@ func emitUpgradeClientEvent(ctx sdk.Context, clientID string, clientState export
 	})
 }
 
-// emitUpdateClientProposalEvent emits an update client proposal event
-func emitUpdateClientProposalEvent(ctx sdk.Context, clientID, clientType string) {
-	ctx.EventManager().EmitEvents(sdk.Events{
-		sdk.NewEvent(
-			types.EventTypeUpdateClientProposal,
-			sdk.NewAttribute(types.AttributeKeySubjectClientID, clientID),
-			sdk.NewAttribute(types.AttributeKeyClientType, clientType),
-		),
-		sdk.NewEvent(
-			sdk.EventTypeMessage,
-			sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
-		),
-	})
-}
-
 // emitRecoverClientEvent emits a recover client event
 func emitRecoverClientEvent(ctx sdk.Context, clientID, clientType string) {
 	ctx.EventManager().EmitEvents(sdk.Events{

modules/core/02-client/keeper/proposal.go

@@ -1,73 +1,13 @@
 package keeper
 
 import (
-	metrics "github.com/hashicorp/go-metrics"
-
 	errorsmod "cosmossdk.io/errors"
 
-	"github.com/cosmos/cosmos-sdk/telemetry"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
 	"github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
-	"github.com/cosmos/ibc-go/v7/modules/core/exported"
 )
 
-// ClientUpdateProposal will retrieve the subject and substitute client.
-// A callback will occur to the subject client state with the client
-// prefixed store being provided for both the subject and the substitute client.
-// The IBC client implementations are responsible for validating the parameters of the
-// subtitute (enusring they match the subject's parameters) as well as copying
-// the necessary consensus states from the subtitute to the subject client
-// store. The substitute must be Active and the subject must not be Active.
-func (k Keeper) ClientUpdateProposal(ctx sdk.Context, p *types.ClientUpdateProposal) error {
-	subjectClientState, found := k.GetClientState(ctx, p.SubjectClientId)
-	if !found {
-		return errorsmod.Wrapf(types.ErrClientNotFound, "subject client with ID %s", p.SubjectClientId)
-	}
-
-	subjectClientStore := k.ClientStore(ctx, p.SubjectClientId)
-
-	if status := k.GetClientStatus(ctx, subjectClientState, p.SubjectClientId); status == exported.Active {
-		return errorsmod.Wrap(types.ErrInvalidRecoveryClient, "cannot update Active subject client")
-	}
-
-	substituteClientState, found := k.GetClientState(ctx, p.SubstituteClientId)
-	if !found {
-		return errorsmod.Wrapf(types.ErrClientNotFound, "substitute client with ID %s", p.SubstituteClientId)
-	}
-
-	if subjectClientState.GetLatestHeight().GTE(substituteClientState.GetLatestHeight()) {
-		return errorsmod.Wrapf(types.ErrInvalidHeight, "subject client state latest height is greater or equal to substitute client state latest height (%s >= %s)", subjectClientState.GetLatestHeight(), substituteClientState.GetLatestHeight())
-	}
-
-	substituteClientStore := k.ClientStore(ctx, p.SubstituteClientId)
-
-	if status := k.GetClientStatus(ctx, substituteClientState, p.SubstituteClientId); status != exported.Active {
-		return errorsmod.Wrapf(types.ErrClientNotActive, "substitute client is not Active, status is %s", status)
-	}
-
-	if err := subjectClientState.CheckSubstituteAndUpdateState(ctx, k.cdc, subjectClientStore, substituteClientStore, substituteClientState); err != nil {
-		return err
-	}
-
-	k.Logger(ctx).Info("client updated after governance proposal passed", "client-id", p.SubjectClientId)
-
-	defer telemetry.IncrCounterWithLabels(
-		[]string{"ibc", "client", "update"},
-		1,
-		[]metrics.Label{
-			telemetry.NewLabel(types.LabelClientType, substituteClientState.ClientType()),
-			telemetry.NewLabel(types.LabelClientID, p.SubjectClientId),
-			telemetry.NewLabel(types.LabelUpdateType, "proposal"),
-		},
-	)
-
-	// emitting events in the keeper for proposal updates to clients
-	emitUpdateClientProposalEvent(ctx, p.SubjectClientId, substituteClientState.ClientType())
-
-	return nil
-}
-
 // HandleUpgradeProposal sets the upgraded client state in the upgrade store. It clears
 // an IBC client state and consensus state if a previous plan was set. Then  it
 // will schedule an upgrade and finally set the upgraded client state in upgrade