Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: remove legacy client update proposal #4581

Merged
merged 5 commits into from
Sep 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour",
"TestAllowedClientsParam"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour"
],
"relayer-type": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour",
"TestAllowedClientsParam"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour"
],
"relayer-type": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour",
"TestAllowedClientsParam"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour"
],
"relayer-type": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour",
"TestAllowedClientsParam"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour"
],
"relayer-type": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour",
"TestAllowedClientsParam"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour"
],
"relayer-type": [
Expand Down
2 changes: 1 addition & 1 deletion .github/compatibility-test-matrices/unreleased/client.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"TestClientTestSuite"
],
"test": [
"TestClientUpdateProposal_Succeeds",
"TestRecoverClient_Succeeds",
"TestClient_Update_Misbehaviour",
"TestAllowedClientsParam"
],
Expand Down
82 changes: 0 additions & 82 deletions e2e/tests/core/02-client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
57 changes: 0 additions & 57 deletions modules/core/02-client/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
5 changes: 1 addition & 4 deletions modules/core/02-client/client/proposal_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
15 changes: 0 additions & 15 deletions modules/core/02-client/keeper/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
60 changes: 0 additions & 60 deletions modules/core/02-client/keeper/proposal.go
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Loading
Loading