From 98230b3988cb40f9d1b5c7095b89df90d61ffb8c Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Fri, 9 Feb 2024 14:46:47 +0100 Subject: [PATCH 01/19] wip: implementation and tests for verify upgrade and update state function in light client modules --- modules/core/02-client/keeper/client.go | 36 ++-- modules/core/02-client/keeper/events.go | 8 +- modules/core/keeper/msg_server.go | 18 +- .../06-solomachine/light_client_module.go | 20 +- .../07-tendermint/light_client_module.go | 29 ++- .../07-tendermint/light_client_module_test.go | 184 ++++++++++++++++++ .../08-wasm/light_client_module.go | 70 ++++++- 7 files changed, 302 insertions(+), 63 deletions(-) create mode 100644 modules/light-clients/07-tendermint/light_client_module_test.go diff --git a/modules/core/02-client/keeper/client.go b/modules/core/02-client/keeper/client.go index a21f4646377..b70cf58d456 100644 --- a/modules/core/02-client/keeper/client.go +++ b/modules/core/02-client/keeper/client.go @@ -9,7 +9,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" - ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" "github.com/cosmos/ibc-go/v8/modules/core/exported" ) @@ -122,46 +121,41 @@ func (k Keeper) UpdateClient(ctx sdk.Context, clientID string, clientMsg exporte // UpgradeClient upgrades the client to a new client state if this new client was committed to // by the old client at the specified upgrade height -func (k Keeper) UpgradeClient(ctx sdk.Context, clientID string, upgradedClient exported.ClientState, upgradedConsState exported.ConsensusState, - upgradeClientProof, upgradeConsensusStateProof []byte, +func (k Keeper) UpgradeClient( + ctx sdk.Context, + clientID string, + upgradedClient, upgradedConsState, upgradeClientProof, upgradeConsensusStateProof []byte, ) error { - clientState, found := k.GetClientState(ctx, clientID) - if !found { - return errorsmod.Wrapf(types.ErrClientNotFound, "cannot update client with ID %s", clientID) - } - - clientStore := k.ClientStore(ctx, clientID) - if status := k.GetClientStatus(ctx, clientID); status != exported.Active { return errorsmod.Wrapf(types.ErrClientNotActive, "cannot upgrade client (%s) with status %s", clientID, status) } - // last height of current counterparty chain must be client's latest height - lastHeight := clientState.GetLatestHeight() + clientType, _, err := types.ParseClientIdentifier(clientID) + if err != nil { + return errorsmod.Wrapf(types.ErrClientNotFound, "clientID (%s)", clientID) + } - if !upgradedClient.GetLatestHeight().GT(lastHeight) { - return errorsmod.Wrapf(ibcerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s", - upgradedClient.GetLatestHeight(), lastHeight) + lightClientModule, found := k.router.GetRoute(clientID) + if !found { + return errorsmod.Wrap(types.ErrRouteNotFound, clientID) } - if err := clientState.VerifyUpgradeAndUpdateState(ctx, k.cdc, clientStore, - upgradedClient, upgradedConsState, upgradeClientProof, upgradeConsensusStateProof, - ); err != nil { + if err := lightClientModule.VerifyUpgradeAndUpdateState(ctx, clientID, upgradedClient, upgradedConsState, upgradeClientProof, upgradeConsensusStateProof); err != nil { return errorsmod.Wrapf(err, "cannot upgrade client with ID %s", clientID) } - k.Logger(ctx).Info("client state upgraded", "client-id", clientID, "height", upgradedClient.GetLatestHeight().String()) + // k.Logger(ctx).Info("client state upgraded", "client-id", clientID, "height", upgradedClient.GetLatestHeight().String()) defer telemetry.IncrCounterWithLabels( []string{"ibc", "client", "upgrade"}, 1, []metrics.Label{ - telemetry.NewLabel(types.LabelClientType, upgradedClient.ClientType()), + telemetry.NewLabel(types.LabelClientType, clientType), telemetry.NewLabel(types.LabelClientID, clientID), }, ) - emitUpgradeClientEvent(ctx, clientID, upgradedClient) + emitUpgradeClientEvent(ctx, clientID, clientType) return nil } diff --git a/modules/core/02-client/keeper/events.go b/modules/core/02-client/keeper/events.go index 3c57db7ef2e..581b19c0eab 100644 --- a/modules/core/02-client/keeper/events.go +++ b/modules/core/02-client/keeper/events.go @@ -21,7 +21,7 @@ func emitCreateClientEvent(ctx sdk.Context, clientID, clientType string) { types.EventTypeCreateClient, sdk.NewAttribute(types.AttributeKeyClientID, clientID), sdk.NewAttribute(types.AttributeKeyClientType, clientType), - // sdk.NewAttribute(types.AttributeKeyConsensusHeight, clientState.GetLatestHeight().String()), + // sdk.NewAttribute(types.AttributeKeyConsensusHeight, clientState.GetLatestHeight().String()), ), sdk.NewEvent( sdk.EventTypeMessage, @@ -60,13 +60,13 @@ func emitUpdateClientEvent(ctx sdk.Context, clientID string, clientType string, } // emitUpgradeClientEvent emits an upgrade client event -func emitUpgradeClientEvent(ctx sdk.Context, clientID string, clientState exported.ClientState) { +func emitUpgradeClientEvent(ctx sdk.Context, clientID string, clientType string) { ctx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( types.EventTypeUpgradeClient, sdk.NewAttribute(types.AttributeKeyClientID, clientID), - sdk.NewAttribute(types.AttributeKeyClientType, clientState.ClientType()), - sdk.NewAttribute(types.AttributeKeyConsensusHeight, clientState.GetLatestHeight().String()), + sdk.NewAttribute(types.AttributeKeyClientType, clientType), + // sdk.NewAttribute(types.AttributeKeyConsensusHeight, clientState.GetLatestHeight().String()), ), sdk.NewEvent( sdk.EventTypeMessage, diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index b373ac73025..a43b921ed16 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -69,17 +69,13 @@ func (k Keeper) UpdateClient(goCtx context.Context, msg *clienttypes.MsgUpdateCl func (k Keeper) UpgradeClient(goCtx context.Context, msg *clienttypes.MsgUpgradeClient) (*clienttypes.MsgUpgradeClientResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) - upgradedClient, err := clienttypes.UnpackClientState(msg.ClientState) - if err != nil { - return nil, err - } - upgradedConsState, err := clienttypes.UnpackConsensusState(msg.ConsensusState) - if err != nil { - return nil, err - } - - if err = k.ClientKeeper.UpgradeClient(ctx, msg.ClientId, upgradedClient, upgradedConsState, - msg.ProofUpgradeClient, msg.ProofUpgradeConsensusState); err != nil { + if err := k.ClientKeeper.UpgradeClient( + ctx, msg.ClientId, + msg.ClientState.Value, + msg.ConsensusState.Value, + msg.ProofUpgradeClient, + msg.ProofUpgradeConsensusState, + ); err != nil { return nil, err } diff --git a/modules/light-clients/06-solomachine/light_client_module.go b/modules/light-clients/06-solomachine/light_client_module.go index 68941c1e478..94a2db9539f 100644 --- a/modules/light-clients/06-solomachine/light_client_module.go +++ b/modules/light-clients/06-solomachine/light_client_module.go @@ -222,14 +222,14 @@ func (LightClientModule) RecoverClient(ctx sdk.Context, clientID, substituteClie return nil } -// // Upgrade functions -// // NOTE: proof heights are not included as upgrade to a new revision is expected to pass only on the last -// // height committed by the current revision. Clients are responsible for ensuring that the planned last -// // height of the current revision is somehow encoded in the proof verification process. -// // This is to ensure that no premature upgrades occur, since upgrade plans committed to by the counterparty -// // may be cancelled or modified before the last planned height. -// // If the upgrade is verified, the upgraded client and consensus states must be set in the client store. -// // Deprecated: will be removed as performs internal functionality -func (LightClientModule) VerifyUpgradeAndUpdateState(ctx sdk.Context, clientID string, newClient []byte, newConsState []byte, upgradeClientProof, upgradeConsensusStateProof []byte) error { - return nil +// VerifyUpgradeAndUpdateState returns an error since solomachine client does not support upgrades +func (lcm LightClientModule) VerifyUpgradeAndUpdateState( + ctx sdk.Context, + clientID string, + newClient []byte, + newConsState []byte, + upgradeClientProof, + upgradeConsensusStateProof []byte, +) error { + return errorsmod.Wrap(clienttypes.ErrInvalidUpgradeClient, "cannot upgrade solomachine client") } diff --git a/modules/light-clients/07-tendermint/light_client_module.go b/modules/light-clients/07-tendermint/light_client_module.go index 98625ef3ab3..3d2423271bb 100644 --- a/modules/light-clients/07-tendermint/light_client_module.go +++ b/modules/light-clients/07-tendermint/light_client_module.go @@ -7,6 +7,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" + ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" "github.com/cosmos/ibc-go/v8/modules/core/exported" "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint/internal/keeper" ) @@ -278,6 +279,13 @@ func (lcm LightClientModule) RecoverClient(ctx sdk.Context, clientID, substitute return clientState.CheckSubstituteAndUpdateState(ctx, cdc, clientStore, substituteClientStore, substituteClient) } +// Upgrade functions +// NOTE: proof heights are not included as upgrade to a new revision is expected to pass only on the last +// height committed by the current revision. Clients are responsible for ensuring that the planned last +// height of the current revision is somehow encoded in the proof verification process. +// This is to ensure that no premature upgrades occur, since upgrade plans committed to by the counterparty +// may be cancelled or modified before the last planned height. +// If the upgrade is verified, the upgraded client and consensus states must be set in the client store. func (lcm LightClientModule) VerifyUpgradeAndUpdateState( ctx sdk.Context, clientID string, @@ -295,17 +303,20 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( return errorsmod.Wrapf(clienttypes.ErrInvalidClientType, "expected: %s, got: %s", exported.Tendermint, clientType) } - var newClientState ClientState - if err := lcm.keeper.Codec().Unmarshal(newClient, &newClientState); err != nil { + var ( + cdc = lcm.keeper.Codec() + newClientState ClientState + newConsensusState ConsensusState + ) + + if err := cdc.Unmarshal(newClient, &newClientState); err != nil { return err } - if err := newClientState.Validate(); err != nil { return err } - var newConsensusState ConsensusState - if err := lcm.keeper.Codec().Unmarshal(newConsState, &newConsensusState); err != nil { + if err := cdc.Unmarshal(newConsState, &newConsensusState); err != nil { return err } if err := newConsensusState.ValidateBasic(); err != nil { @@ -313,12 +324,16 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( } clientStore := lcm.storeProvider.ClientStore(ctx, clientID) - cdc := lcm.keeper.Codec() - clientState, found := getClientState(clientStore, cdc) if !found { return errorsmod.Wrap(clienttypes.ErrClientNotFound, clientID) } + // last height of current counterparty chain must be client's latest height + lastHeight := clientState.GetLatestHeight() + if !newClientState.GetLatestHeight().GT(lastHeight) { + return errorsmod.Wrapf(ibcerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s", newClientState.GetLatestHeight(), lastHeight) + } + return clientState.VerifyUpgradeAndUpdateState(ctx, cdc, clientStore, &newClientState, &newConsensusState, upgradeClientProof, upgradeConsensusStateProof) } diff --git a/modules/light-clients/07-tendermint/light_client_module_test.go b/modules/light-clients/07-tendermint/light_client_module_test.go new file mode 100644 index 00000000000..495f4163582 --- /dev/null +++ b/modules/light-clients/07-tendermint/light_client_module_test.go @@ -0,0 +1,184 @@ +package tendermint_test + +import ( + "crypto/sha256" + "time" + + upgradetypes "cosmossdk.io/x/upgrade/types" + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" + commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types" + host "github.com/cosmos/ibc-go/v8/modules/core/24-host" + "github.com/cosmos/ibc-go/v8/modules/core/exported" + ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" + ibctesting "github.com/cosmos/ibc-go/v8/testing" +) + +const ( + tmClientID = "07-tendermint-100" + wasmClientID = "08-wasm-0" +) + +func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { + var ( + clientID string + path *ibctesting.Path + upgradedClientState exported.ClientState + upgradedClientStateBz, upgradedConsStateBz []byte + upgradedClientStateProof, upgradedConsensusStateProof []byte + ) + + testCases := []struct { + name string + malleate func() + expErr error + }{ + { + "success", + func() { + // upgrade height is at next block + lastHeight := clienttypes.NewHeight(0, uint64(suite.chainB.GetContext().BlockHeight()+1)) + + // zero custom fields and store in upgrade store + zeroedUpgradedClient := upgradedClientState.ZeroCustomFields() + zeroedUpgradedClientBz := suite.chainA.App.AppCodec().MustMarshal(zeroedUpgradedClient) + suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), zeroedUpgradedClientBz) //nolint:errcheck // ignore error for test + suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsStateBz) //nolint:errcheck // ignore error for test + + // commit upgrade store changes and update clients + suite.coordinator.CommitBlock(suite.chainB) + err := path.EndpointA.UpdateClient() + suite.Require().NoError(err) + + cs, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(suite.chainA.GetContext(), clientID) + suite.Require().True(found) + + upgradedClientStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) + upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) + }, + nil, + }, + { + "cannot parse malformed client ID", + func() { + clientID = ibctesting.InvalidID + }, + host.ErrInvalidID, + }, + { + "client type is not 07-tendermint", + func() { + clientID = wasmClientID + }, + clienttypes.ErrInvalidClientType, + }, + { + "cannot find client state", + func() { + clientID = tmClientID + }, + clienttypes.ErrClientNotFound, + }, + { + "cannot unmarshal upgraded client state", + func() { + upgradedClientStateBz = []byte{} + }, + clienttypes.ErrClientNotFound, // TODO: use right error here + }, + { + "cannot unmarshal upgraded consensus state", + func() { + upgradedConsStateBz = []byte{} + }, + clienttypes.ErrClientNotFound, // TODO: use right error here + }, + // { + // "upgraded client state height is not greater than current height", + // func() { + // // last Height is at next block + // lastHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) + + // // zero custom fields and store in upgrade store + // err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedClientBz) + // suite.Require().NoError(err) + // err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsStateBz) + // suite.Require().NoError(err) + + // // change upgradedClient height to be lower than current client state height + // tmClient := upgradedClient.(*ibctm.ClientState) + // tmClient.LatestHeight = clienttypes.NewHeight(0, 1) + // upgradedClient = tmClient + + // suite.coordinator.CommitBlock(suite.chainB) + // err = path.EndpointA.UpdateClient() + // suite.Require().NoError(err) + + // cs, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID) + // suite.Require().True(found) + + // upgradedClientProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) + // upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) + // }, + // ibcerrors.ErrInvalidHeight + // }, + } + + for _, tc := range testCases { + tc := tc + suite.Run(tc.name, func() { + suite.SetupTest() // reset + cdc := suite.chainA.App.AppCodec() + ctx := suite.chainA.GetContext() + + path = ibctesting.NewPath(suite.chainA, suite.chainB) + path.SetupClients() + clientID = path.EndpointA.ClientID + clientState := path.EndpointA.GetClientState().(*ibctm.ClientState) + revisionNumber := clienttypes.ParseChainID(clientState.ChainId) + + var err error + newChainID, err := clienttypes.SetRevisionNumber(clientState.ChainId, revisionNumber+1) + suite.Require().NoError(err) + + upgradedClientState = ibctm.NewClientState(newChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, clienttypes.NewHeight(revisionNumber+1, clientState.GetLatestHeight().GetRevisionHeight()+1), commitmenttypes.GetSDKSpecs(), upgradePath) + upgradedClientStateBz = cdc.MustMarshal(upgradedClientState) + + nextValsHash := sha256.Sum256([]byte("nextValsHash")) + upgradedConsState := ibctm.NewConsensusState(time.Now(), commitmenttypes.NewMerkleRoot([]byte("merkle-root")), nextValsHash[:]) + upgradedConsStateBz = cdc.MustMarshal(upgradedConsState) + + lightClientModule, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetRouter().GetRoute(clientID) + suite.Require().True(found) + + tc.malleate() + + err = lightClientModule.VerifyUpgradeAndUpdateState( + ctx, + clientID, + upgradedClientStateBz, + upgradedConsStateBz, + upgradedClientStateProof, + upgradedConsensusStateProof, + ) + + expPass := tc.expErr == nil + if expPass { + suite.Require().NoError(err) + + clientState := suite.chainA.GetClientState(clientID) + suite.Require().NotNil(clientState) + // TODO: check client state bytes matches upgraded client state bytes + + consensusState, found := suite.chainA.GetConsensusState(clientID, clientState.GetLatestHeight()) + suite.Require().True(found) + suite.Require().NotNil(consensusState) + tmConsensusState, ok := consensusState.(*ibctm.ConsensusState) + suite.Require().True(ok) + suite.Require().Equal(upgradedConsState.NextValidatorsHash, tmConsensusState.NextValidatorsHash) + } else { + suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) + } + }) + } +} diff --git a/modules/light-clients/08-wasm/light_client_module.go b/modules/light-clients/08-wasm/light_client_module.go index 141d78bfd38..d189bfeea80 100644 --- a/modules/light-clients/08-wasm/light_client_module.go +++ b/modules/light-clients/08-wasm/light_client_module.go @@ -7,7 +7,9 @@ import ( wasmkeeper "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/keeper" "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" + wasmtypes "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" + ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" "github.com/cosmos/ibc-go/v8/modules/core/exported" ) @@ -241,14 +243,62 @@ func (LightClientModule) RecoverClient(ctx sdk.Context, clientID, substituteClie return nil } -// // Upgrade functions -// // NOTE: proof heights are not included as upgrade to a new revision is expected to pass only on the last -// // height committed by the current revision. Clients are responsible for ensuring that the planned last -// // height of the current revision is somehow encoded in the proof verification process. -// // This is to ensure that no premature upgrades occur, since upgrade plans committed to by the counterparty -// // may be cancelled or modified before the last planned height. -// // If the upgrade is verified, the upgraded client and consensus states must be set in the client store. -// // DEPRECATED: will be removed as performs internal functionality -func (LightClientModule) VerifyUpgradeAndUpdateState(ctx sdk.Context, clientID string, newClient []byte, newConsState []byte, upgradeClientProof, upgradeConsensusStateProof []byte) error { - return nil +// Upgrade functions +// NOTE: proof heights are not included as upgrade to a new revision is expected to pass only on the last +// height committed by the current revision. Clients are responsible for ensuring that the planned last +// height of the current revision is somehow encoded in the proof verification process. +// This is to ensure that no premature upgrades occur, since upgrade plans committed to by the counterparty +// may be cancelled or modified before the last planned height. +// If the upgrade is verified, the upgraded client and consensus states must be set in the client store. +func (lcm LightClientModule) VerifyUpgradeAndUpdateState( + ctx sdk.Context, + clientID string, + newClient []byte, + newConsState []byte, + upgradeClientProof, + upgradeConsensusStateProof []byte, +) error { + clientType, _, err := clienttypes.ParseClientIdentifier(clientID) + if err != nil { + return err + } + + if clientType != wasmtypes.Wasm { + return errorsmod.Wrapf(clienttypes.ErrInvalidClientType, "expected: %s, got: %s", wasmtypes.Wasm, clientType) + } + + var ( + cdc = lcm.keeper.Codec() + newClientState wasmtypes.ClientState + newConsensusState wasmtypes.ConsensusState + ) + + if err := cdc.Unmarshal(newClient, &newClientState); err != nil { + return err + } + if err := newClientState.Validate(); err != nil { + return err + } + + if err := cdc.Unmarshal(newConsState, &newConsensusState); err != nil { + return err + } + if err := newConsensusState.ValidateBasic(); err != nil { + return err + } + + clientStore := lcm.storeProvider.ClientStore(ctx, clientID) + clientState, found := wasmtypes.GetClientState(clientStore, cdc) + if !found { + return errorsmod.Wrap(clienttypes.ErrClientNotFound, clientID) + } + + // last height of current counterparty chain must be client's latest height + lastHeight := clientState.GetLatestHeight() + if !newClientState.GetLatestHeight().GT(lastHeight) { + return errorsmod.Wrapf(ibcerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s", newClientState.GetLatestHeight(), lastHeight) + } + + return clientState.VerifyUpgradeAndUpdateState(ctx, cdc, clientStore, &newClientState, &newConsensusState, upgradeClientProof, upgradeConsensusStateProof) + } From 83ce8cc6537eeb8901699f348b713667b66adca1 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Fri, 9 Feb 2024 15:01:15 +0100 Subject: [PATCH 02/19] add test case --- .../07-tendermint/light_client_module_test.go | 79 ++++++++++--------- 1 file changed, 43 insertions(+), 36 deletions(-) diff --git a/modules/light-clients/07-tendermint/light_client_module_test.go b/modules/light-clients/07-tendermint/light_client_module_test.go index 495f4163582..8c6efc62928 100644 --- a/modules/light-clients/07-tendermint/light_client_module_test.go +++ b/modules/light-clients/07-tendermint/light_client_module_test.go @@ -8,6 +8,7 @@ import ( clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types" host "github.com/cosmos/ibc-go/v8/modules/core/24-host" + ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" "github.com/cosmos/ibc-go/v8/modules/core/exported" ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" ibctesting "github.com/cosmos/ibc-go/v8/testing" @@ -23,7 +24,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { clientID string path *ibctesting.Path upgradedClientState exported.ClientState - upgradedClientStateBz, upgradedConsStateBz []byte + upgradedClientStateBz, upgradedConsensusStateBz []byte upgradedClientStateProof, upgradedConsensusStateProof []byte ) @@ -41,12 +42,14 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { // zero custom fields and store in upgrade store zeroedUpgradedClient := upgradedClientState.ZeroCustomFields() zeroedUpgradedClientBz := suite.chainA.App.AppCodec().MustMarshal(zeroedUpgradedClient) - suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), zeroedUpgradedClientBz) //nolint:errcheck // ignore error for test - suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsStateBz) //nolint:errcheck // ignore error for test + err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), zeroedUpgradedClientBz) + suite.Require().NoError(err) + err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsensusStateBz) + suite.Require().NoError(err) // commit upgrade store changes and update clients suite.coordinator.CommitBlock(suite.chainB) - err := path.EndpointA.UpdateClient() + err = path.EndpointA.UpdateClient() suite.Require().NoError(err) cs, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(suite.chainA.GetContext(), clientID) @@ -88,39 +91,43 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { { "cannot unmarshal upgraded consensus state", func() { - upgradedConsStateBz = []byte{} + upgradedConsensusStateBz = []byte{} }, clienttypes.ErrClientNotFound, // TODO: use right error here }, - // { - // "upgraded client state height is not greater than current height", - // func() { - // // last Height is at next block - // lastHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) - - // // zero custom fields and store in upgrade store - // err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedClientBz) - // suite.Require().NoError(err) - // err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsStateBz) - // suite.Require().NoError(err) - - // // change upgradedClient height to be lower than current client state height - // tmClient := upgradedClient.(*ibctm.ClientState) - // tmClient.LatestHeight = clienttypes.NewHeight(0, 1) - // upgradedClient = tmClient - - // suite.coordinator.CommitBlock(suite.chainB) - // err = path.EndpointA.UpdateClient() - // suite.Require().NoError(err) - - // cs, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID) - // suite.Require().True(found) - - // upgradedClientProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) - // upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) - // }, - // ibcerrors.ErrInvalidHeight - // }, + { + "upgraded client state height is not greater than current height", + func() { + // last Height is at next block + lastHeight := clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) + + // zero custom fields and store in upgrade store + zeroedUpgradedClient := upgradedClientState.ZeroCustomFields() + zeroedUpgradedClientBz := suite.chainA.App.AppCodec().MustMarshal(zeroedUpgradedClient) + err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), zeroedUpgradedClientBz) + suite.Require().NoError(err) + err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsensusStateBz) + suite.Require().NoError(err) + + // change upgraded client state height to be lower than current client state height + clientState := path.EndpointA.GetClientState().(*ibctm.ClientState) + tmClient := upgradedClientState.(*ibctm.ClientState) + tmClient.ChainId = clientState.ChainId + tmClient.LatestHeight = clienttypes.NewHeight(1, 1) + upgradedClientStateBz = suite.chainA.App.AppCodec().MustMarshal(tmClient) + + suite.coordinator.CommitBlock(suite.chainB) + err = path.EndpointA.UpdateClient() + suite.Require().NoError(err) + + cs, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID) + suite.Require().True(found) + + zeroedUpgradedClientBz, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) + upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) + }, + ibcerrors.ErrInvalidHeight, + }, } for _, tc := range testCases { @@ -145,7 +152,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { nextValsHash := sha256.Sum256([]byte("nextValsHash")) upgradedConsState := ibctm.NewConsensusState(time.Now(), commitmenttypes.NewMerkleRoot([]byte("merkle-root")), nextValsHash[:]) - upgradedConsStateBz = cdc.MustMarshal(upgradedConsState) + upgradedConsensusStateBz = cdc.MustMarshal(upgradedConsState) lightClientModule, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetRouter().GetRoute(clientID) suite.Require().True(found) @@ -156,7 +163,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { ctx, clientID, upgradedClientStateBz, - upgradedConsStateBz, + upgradedConsensusStateBz, upgradedClientStateProof, upgradedConsensusStateProof, ) From fb7700518ebefc322979624e80e50292c1632aa8 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Fri, 9 Feb 2024 20:44:40 +0100 Subject: [PATCH 03/19] fix un-marshaling in tendermint light client module --- .../07-tendermint/light_client_module.go | 18 ++++++++++----- .../07-tendermint/light_client_module_test.go | 22 +++++++++---------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/modules/light-clients/07-tendermint/light_client_module.go b/modules/light-clients/07-tendermint/light_client_module.go index 3d2423271bb..ba4b1d7a651 100644 --- a/modules/light-clients/07-tendermint/light_client_module.go +++ b/modules/light-clients/07-tendermint/light_client_module.go @@ -305,20 +305,28 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( var ( cdc = lcm.keeper.Codec() - newClientState ClientState - newConsensusState ConsensusState + newClientState exported.ClientState + newConsensusState exported.ConsensusState ) - if err := cdc.Unmarshal(newClient, &newClientState); err != nil { + if err := cdc.UnmarshalInterface(newClient, &newClientState); err != nil { return err } + newTmClientState, ok := newClientState.(*ClientState) + if !ok { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "expected client state type %T, got %T", (*ClientState)(nil), newClientState) + } if err := newClientState.Validate(); err != nil { return err } - if err := cdc.Unmarshal(newConsState, &newConsensusState); err != nil { + if err := cdc.UnmarshalInterface(newConsState, &newConsensusState); err != nil { return err } + newTmConsensusState, ok := newConsensusState.(*ConsensusState) + if !ok { + return errorsmod.Wrapf(clienttypes.ErrInvalidConsensus, "expected consensus state type %T, got %T", (*ConsensusState)(nil), newConsensusState) + } if err := newConsensusState.ValidateBasic(); err != nil { return err } @@ -335,5 +343,5 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( return errorsmod.Wrapf(ibcerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s", newClientState.GetLatestHeight(), lastHeight) } - return clientState.VerifyUpgradeAndUpdateState(ctx, cdc, clientStore, &newClientState, &newConsensusState, upgradeClientProof, upgradeConsensusStateProof) + return clientState.VerifyUpgradeAndUpdateState(ctx, cdc, clientStore, newTmClientState, newTmConsensusState, upgradeClientProof, upgradeConsensusStateProof) } diff --git a/modules/light-clients/07-tendermint/light_client_module_test.go b/modules/light-clients/07-tendermint/light_client_module_test.go index 8c6efc62928..b3676f3e830 100644 --- a/modules/light-clients/07-tendermint/light_client_module_test.go +++ b/modules/light-clients/07-tendermint/light_client_module_test.go @@ -41,7 +41,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { // zero custom fields and store in upgrade store zeroedUpgradedClient := upgradedClientState.ZeroCustomFields() - zeroedUpgradedClientBz := suite.chainA.App.AppCodec().MustMarshal(zeroedUpgradedClient) + zeroedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), zeroedUpgradedClient) err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), zeroedUpgradedClientBz) suite.Require().NoError(err) err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsensusStateBz) @@ -82,18 +82,18 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { clienttypes.ErrClientNotFound, }, { - "cannot unmarshal upgraded client state", + "upgraded client state is not for 07-tendermint", func() { upgradedClientStateBz = []byte{} }, - clienttypes.ErrClientNotFound, // TODO: use right error here + clienttypes.ErrInvalidClient, }, { - "cannot unmarshal upgraded consensus state", + "upgraded consensus state is not for 07-tendermint", func() { upgradedConsensusStateBz = []byte{} }, - clienttypes.ErrClientNotFound, // TODO: use right error here + clienttypes.ErrInvalidConsensus, }, { "upgraded client state height is not greater than current height", @@ -103,7 +103,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { // zero custom fields and store in upgrade store zeroedUpgradedClient := upgradedClientState.ZeroCustomFields() - zeroedUpgradedClientBz := suite.chainA.App.AppCodec().MustMarshal(zeroedUpgradedClient) + zeroedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), zeroedUpgradedClient) err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), zeroedUpgradedClientBz) suite.Require().NoError(err) err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsensusStateBz) @@ -114,7 +114,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { tmClient := upgradedClientState.(*ibctm.ClientState) tmClient.ChainId = clientState.ChainId tmClient.LatestHeight = clienttypes.NewHeight(1, 1) - upgradedClientStateBz = suite.chainA.App.AppCodec().MustMarshal(tmClient) + upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), tmClient) suite.coordinator.CommitBlock(suite.chainB) err = path.EndpointA.UpdateClient() @@ -148,11 +148,11 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { suite.Require().NoError(err) upgradedClientState = ibctm.NewClientState(newChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, clienttypes.NewHeight(revisionNumber+1, clientState.GetLatestHeight().GetRevisionHeight()+1), commitmenttypes.GetSDKSpecs(), upgradePath) - upgradedClientStateBz = cdc.MustMarshal(upgradedClientState) + upgradedClientStateBz = clienttypes.MustMarshalClientState(cdc, upgradedClientState) nextValsHash := sha256.Sum256([]byte("nextValsHash")) - upgradedConsState := ibctm.NewConsensusState(time.Now(), commitmenttypes.NewMerkleRoot([]byte("merkle-root")), nextValsHash[:]) - upgradedConsensusStateBz = cdc.MustMarshal(upgradedConsState) + upgradedConsensusState := ibctm.NewConsensusState(time.Now(), commitmenttypes.NewMerkleRoot([]byte("merkle-root")), nextValsHash[:]) + upgradedConsensusStateBz = clienttypes.MustMarshalConsensusState(cdc, upgradedConsensusState) lightClientModule, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetRouter().GetRoute(clientID) suite.Require().True(found) @@ -181,7 +181,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { suite.Require().NotNil(consensusState) tmConsensusState, ok := consensusState.(*ibctm.ConsensusState) suite.Require().True(ok) - suite.Require().Equal(upgradedConsState.NextValidatorsHash, tmConsensusState.NextValidatorsHash) + suite.Require().Equal(upgradedConsensusState.NextValidatorsHash, tmConsensusState.NextValidatorsHash) } else { suite.Require().Error(err) suite.Require().ErrorIs(err, tc.expErr) From a4e8693c5cc4dd83d87b768dbb9c581582ca6f4e Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Fri, 9 Feb 2024 21:28:02 +0100 Subject: [PATCH 04/19] add tests for verify upgrade and update state of 08-wasm light client module --- .../07-tendermint/light_client_module_test.go | 8 +- .../08-wasm/light_client_module.go | 19 +- .../08-wasm/light_client_module_test.go | 192 ++++++++++++++++++ modules/light-clients/08-wasm/wasm_test.go | 126 ++++++++++++ 4 files changed, 335 insertions(+), 10 deletions(-) create mode 100644 modules/light-clients/08-wasm/light_client_module_test.go create mode 100644 modules/light-clients/08-wasm/wasm_test.go diff --git a/modules/light-clients/07-tendermint/light_client_module_test.go b/modules/light-clients/07-tendermint/light_client_module_test.go index b3676f3e830..ad157b971f6 100644 --- a/modules/light-clients/07-tendermint/light_client_module_test.go +++ b/modules/light-clients/07-tendermint/light_client_module_test.go @@ -82,14 +82,14 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { clienttypes.ErrClientNotFound, }, { - "upgraded client state is not for 07-tendermint", + "upgraded client state is not for tendermint client state", func() { upgradedClientStateBz = []byte{} }, clienttypes.ErrInvalidClient, }, { - "upgraded consensus state is not for 07-tendermint", + "upgraded consensus state is not tendermint consensus state", func() { upgradedConsensusStateBz = []byte{} }, @@ -150,8 +150,8 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { upgradedClientState = ibctm.NewClientState(newChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, clienttypes.NewHeight(revisionNumber+1, clientState.GetLatestHeight().GetRevisionHeight()+1), commitmenttypes.GetSDKSpecs(), upgradePath) upgradedClientStateBz = clienttypes.MustMarshalClientState(cdc, upgradedClientState) - nextValsHash := sha256.Sum256([]byte("nextValsHash")) - upgradedConsensusState := ibctm.NewConsensusState(time.Now(), commitmenttypes.NewMerkleRoot([]byte("merkle-root")), nextValsHash[:]) + nextValsHash := sha256.Sum256([]byte("new-nextValsHash")) + upgradedConsensusState := ibctm.NewConsensusState(time.Now(), commitmenttypes.NewMerkleRoot([]byte("new-hash")), nextValsHash[:]) upgradedConsensusStateBz = clienttypes.MustMarshalConsensusState(cdc, upgradedConsensusState) lightClientModule, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetRouter().GetRoute(clientID) diff --git a/modules/light-clients/08-wasm/light_client_module.go b/modules/light-clients/08-wasm/light_client_module.go index d189bfeea80..5c9ce302fb2 100644 --- a/modules/light-clients/08-wasm/light_client_module.go +++ b/modules/light-clients/08-wasm/light_client_module.go @@ -269,20 +269,28 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( var ( cdc = lcm.keeper.Codec() - newClientState wasmtypes.ClientState - newConsensusState wasmtypes.ConsensusState + newClientState exported.ClientState + newConsensusState exported.ConsensusState ) - if err := cdc.Unmarshal(newClient, &newClientState); err != nil { + if err := cdc.UnmarshalInterface(newClient, &newClientState); err != nil { return err } + newWasmClientState, ok := newClientState.(*wasmtypes.ClientState) + if !ok { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "expected client state type %T, got %T", (*wasmtypes.ClientState)(nil), newClientState) + } if err := newClientState.Validate(); err != nil { return err } - if err := cdc.Unmarshal(newConsState, &newConsensusState); err != nil { + if err := cdc.UnmarshalInterface(newConsState, &newConsensusState); err != nil { return err } + newWasmConsensusState, ok := newConsensusState.(*wasmtypes.ConsensusState) + if !ok { + return errorsmod.Wrapf(clienttypes.ErrInvalidConsensus, "expected consensus state type %T, got %T", (*wasmtypes.ConsensusState)(nil), newConsensusState) + } if err := newConsensusState.ValidateBasic(); err != nil { return err } @@ -299,6 +307,5 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( return errorsmod.Wrapf(ibcerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s", newClientState.GetLatestHeight(), lastHeight) } - return clientState.VerifyUpgradeAndUpdateState(ctx, cdc, clientStore, &newClientState, &newConsensusState, upgradeClientProof, upgradeConsensusStateProof) - + return clientState.VerifyUpgradeAndUpdateState(ctx, cdc, clientStore, newWasmClientState, newWasmConsensusState, upgradeClientProof, upgradeConsensusStateProof) } diff --git a/modules/light-clients/08-wasm/light_client_module_test.go b/modules/light-clients/08-wasm/light_client_module_test.go new file mode 100644 index 00000000000..30d1670496e --- /dev/null +++ b/modules/light-clients/08-wasm/light_client_module_test.go @@ -0,0 +1,192 @@ +package wasm_test + +import ( + "encoding/json" + "time" + + wasmvm "github.com/CosmWasm/wasmvm" + wasmvmtypes "github.com/CosmWasm/wasmvm/types" + wasmtesting "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/testing" + "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" + wasmtypes "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" + commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types" + host "github.com/cosmos/ibc-go/v8/modules/core/24-host" + ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" + "github.com/cosmos/ibc-go/v8/modules/core/exported" + ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" + ibctesting "github.com/cosmos/ibc-go/v8/testing" +) + +const ( + tmClientID = "07-tendermint-0" + wasmClientID = "08-wasm-100" +) + +func (suite *WasmTestSuite) TestVerifyUpgradeAndUpdateState() { + var ( + clientID string + clientState *wasmtypes.ClientState + upgradedClientState exported.ClientState + upgradedConsensusState exported.ConsensusState + upgradedClientStateBz, upgradedConsensusStateBz []byte + upgradedClientStateProof, upgradedConsensusStateProof []byte + ) + + testCases := []struct { + name string + malleate func() + expErr error + }{ + { + "success", + func() { + suite.mockVM.RegisterSudoCallback(types.VerifyUpgradeAndUpdateStateMsg{}, func(_ wasmvm.Checksum, _ wasmvmtypes.Env, sudoMsg []byte, store wasmvm.KVStore, _ wasmvm.GoAPI, _ wasmvm.Querier, _ wasmvm.GasMeter, _ uint64, _ wasmvmtypes.UFraction) (*wasmvmtypes.Response, uint64, error) { + var payload types.SudoMsg + + err := json.Unmarshal(sudoMsg, &payload) + suite.Require().NoError(err) + + expectedUpgradedClient, ok := upgradedClientState.(*types.ClientState) + suite.Require().True(ok) + expectedUpgradedConsensus, ok := upgradedConsensusState.(*types.ConsensusState) + suite.Require().True(ok) + + // verify payload values + suite.Require().Equal(expectedUpgradedClient.Data, payload.VerifyUpgradeAndUpdateState.UpgradeClientState) + suite.Require().Equal(expectedUpgradedConsensus.Data, payload.VerifyUpgradeAndUpdateState.UpgradeConsensusState) + suite.Require().Equal(upgradedClientStateProof, payload.VerifyUpgradeAndUpdateState.ProofUpgradeClient) + suite.Require().Equal(upgradedConsensusStateProof, payload.VerifyUpgradeAndUpdateState.ProofUpgradeConsensusState) + + // verify other Sudo fields are nil + suite.Require().Nil(payload.UpdateState) + suite.Require().Nil(payload.UpdateStateOnMisbehaviour) + suite.Require().Nil(payload.VerifyMembership) + suite.Require().Nil(payload.VerifyNonMembership) + + data, err := json.Marshal(types.EmptyResult{}) + suite.Require().NoError(err) + + // set new client state and consensus state + wrappedUpgradedClient := clienttypes.MustUnmarshalClientState(suite.chainA.App.AppCodec(), expectedUpgradedClient.Data) + store.Set(host.ClientStateKey(), upgradedClientStateBz) + store.Set(host.ConsensusStateKey(wrappedUpgradedClient.GetLatestHeight()), upgradedConsensusStateBz) + + return &wasmvmtypes.Response{Data: data}, wasmtesting.DefaultGasUsed, nil + }) + }, + nil, + }, + { + "cannot parse malformed client ID", + func() { + clientID = ibctesting.InvalidID + }, + host.ErrInvalidID, + }, + { + "client type is not 08-wasm", + func() { + clientID = tmClientID + }, + clienttypes.ErrInvalidClientType, + }, + { + "cannot find client state", + func() { + clientID = wasmClientID + }, + clienttypes.ErrClientNotFound, + }, + { + "upgraded client state is not wasm client state", + func() { + upgradedClientStateBz = []byte{} + }, + clienttypes.ErrInvalidClient, + }, + { + "upgraded consensus state is not wasm consensus sate", + func() { + upgradedConsensusStateBz = []byte{} + }, + clienttypes.ErrInvalidConsensus, + }, + { + "upgraded client state height is not greater than current height", + func() { + latestHeight := clientState.GetLatestHeight() + newLatestHeight := clienttypes.NewHeight(latestHeight.GetRevisionNumber(), latestHeight.GetRevisionHeight()-1) + + wrappedUpgradedClient := wasmtesting.CreateMockTendermintClientState(newLatestHeight) + wrappedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), wrappedUpgradedClient) + upgradedClientState = types.NewClientState(wrappedUpgradedClientBz, clientState.Checksum, newLatestHeight) + upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), upgradedClientState) + }, + ibcerrors.ErrInvalidHeight, + }, + } + + for _, tc := range testCases { + tc := tc + suite.Run(tc.name, func() { + suite.SetupWasmWithMockVM() // reset suite + cdc := suite.chainA.App.AppCodec() + ctx := suite.chainA.GetContext() + + endpoint := wasmtesting.NewWasmEndpoint(suite.chainA) + err := endpoint.CreateClient() + suite.Require().NoError(err) + clientID = endpoint.ClientID + + clientState = endpoint.GetClientState().(*wasmtypes.ClientState) + latestHeight := clientState.GetLatestHeight() + + newLatestHeight := clienttypes.NewHeight(latestHeight.GetRevisionNumber(), latestHeight.GetRevisionHeight()+1) + wrappedUpgradedClient := wasmtesting.CreateMockTendermintClientState(newLatestHeight) + wrappedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), wrappedUpgradedClient) + upgradedClientState = types.NewClientState(wrappedUpgradedClientBz, clientState.Checksum, newLatestHeight) + upgradedClientStateBz = clienttypes.MustMarshalClientState(cdc, upgradedClientState) + + wrappedUpgradedConsensus := ibctm.NewConsensusState(time.Now(), commitmenttypes.NewMerkleRoot([]byte("new-hash")), []byte("new-nextValsHash")) + wrappedUpgradedConsensusBz := clienttypes.MustMarshalConsensusState(cdc, wrappedUpgradedConsensus) + upgradedConsensusState = types.NewConsensusState(wrappedUpgradedConsensusBz) + upgradedConsensusStateBz = clienttypes.MustMarshalConsensusState(cdc, upgradedConsensusState) + + lightClientModule, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetRouter().GetRoute(clientID) + suite.Require().True(found) + + tc.malleate() + + clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), clientID) + + upgradedClientStateProof = wasmtesting.MockUpgradedClientStateProofBz + upgradedConsensusStateProof = wasmtesting.MockUpgradedConsensusStateProofBz + + err = lightClientModule.VerifyUpgradeAndUpdateState( + ctx, + clientID, + upgradedClientStateBz, + upgradedConsensusStateBz, + upgradedClientStateProof, + upgradedConsensusStateProof, + ) + expPass := tc.expErr == nil + if expPass { + suite.Require().NoError(err) + + // verify new client state and consensus state + clientStateBz := clientStore.Get(host.ClientStateKey()) + suite.Require().NotEmpty(clientStateBz) + suite.Require().Equal(upgradedClientStateBz, clientStateBz) + + consensusStateBz := clientStore.Get(host.ConsensusStateKey(upgradedClientState.GetLatestHeight())) + suite.Require().NotEmpty(consensusStateBz) + suite.Require().NotEmpty(upgradedConsensusStateBz, consensusStateBz) + } else { + suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expErr) + } + }) + } +} diff --git a/modules/light-clients/08-wasm/wasm_test.go b/modules/light-clients/08-wasm/wasm_test.go new file mode 100644 index 00000000000..79702fa860c --- /dev/null +++ b/modules/light-clients/08-wasm/wasm_test.go @@ -0,0 +1,126 @@ +package wasm_test + +import ( + "encoding/json" + "errors" + "testing" + + wasmvm "github.com/CosmWasm/wasmvm" + wasmvmtypes "github.com/CosmWasm/wasmvm/types" + dbm "github.com/cosmos/cosmos-db" + testifysuite "github.com/stretchr/testify/suite" + + "cosmossdk.io/log" + storetypes "cosmossdk.io/store/types" + + simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" + + wasmtesting "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/testing" + simapp "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/testing/simapp" + "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" + host "github.com/cosmos/ibc-go/v8/modules/core/24-host" + "github.com/cosmos/ibc-go/v8/modules/core/exported" + ibctesting "github.com/cosmos/ibc-go/v8/testing" +) + +type WasmTestSuite struct { + testifysuite.Suite + coordinator *ibctesting.Coordinator + chainA *ibctesting.TestChain + mockVM *wasmtesting.MockWasmEngine + + checksum types.Checksum +} + +func TestWasmTestSuite(t *testing.T) { + testifysuite.Run(t, new(WasmTestSuite)) +} + +func (suite *WasmTestSuite) SetupTest() { + ibctesting.DefaultTestingAppInit = setupTestingApp + + suite.coordinator = ibctesting.NewCoordinator(suite.T(), 1) + suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1)) +} + +func init() { + ibctesting.DefaultTestingAppInit = setupTestingApp +} + +// GetSimApp returns the duplicated SimApp from within the 08-wasm directory. +// This must be used instead of chain.GetSimApp() for tests within this directory. +func GetSimApp(chain *ibctesting.TestChain) *simapp.SimApp { + app, ok := chain.App.(*simapp.SimApp) + if !ok { + panic(errors.New("chain is not a simapp.SimApp")) + } + return app +} + +// setupTestingApp provides the duplicated simapp which is specific to the 08-wasm module on chain creation. +func setupTestingApp() (ibctesting.TestingApp, map[string]json.RawMessage) { + db := dbm.NewMemDB() + app := simapp.NewSimApp(log.NewNopLogger(), db, nil, true, simtestutil.EmptyAppOptions{}, nil) + return app, app.DefaultGenesis() +} + +// SetupWasmWithMockVM sets up mock cometbft chain with a mock vm. +func (suite *WasmTestSuite) SetupWasmWithMockVM() { + ibctesting.DefaultTestingAppInit = suite.setupWasmWithMockVM + + suite.coordinator = ibctesting.NewCoordinator(suite.T(), 1) + suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1)) + suite.checksum = storeWasmCode(suite, wasmtesting.Code) +} + +func (suite *WasmTestSuite) setupWasmWithMockVM() (ibctesting.TestingApp, map[string]json.RawMessage) { + suite.mockVM = wasmtesting.NewMockWasmEngine() + + suite.mockVM.InstantiateFn = func(checksum wasmvm.Checksum, env wasmvmtypes.Env, info wasmvmtypes.MessageInfo, initMsg []byte, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) (*wasmvmtypes.Response, uint64, error) { + var payload types.InstantiateMessage + err := json.Unmarshal(initMsg, &payload) + suite.Require().NoError(err) + + wrappedClientState := clienttypes.MustUnmarshalClientState(suite.chainA.App.AppCodec(), payload.ClientState) + + clientState := types.NewClientState(payload.ClientState, payload.Checksum, wrappedClientState.GetLatestHeight().(clienttypes.Height)) + clientStateBz := clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), clientState) + store.Set(host.ClientStateKey(), clientStateBz) + + consensusState := types.NewConsensusState(payload.ConsensusState) + consensusStateBz := clienttypes.MustMarshalConsensusState(suite.chainA.App.AppCodec(), consensusState) + store.Set(host.ConsensusStateKey(clientState.GetLatestHeight()), consensusStateBz) + + resp, err := json.Marshal(types.EmptyResult{}) + suite.Require().NoError(err) + + return &wasmvmtypes.Response{Data: resp}, 0, nil + } + + suite.mockVM.RegisterQueryCallback(types.StatusMsg{}, func(checksum wasmvm.Checksum, env wasmvmtypes.Env, queryMsg []byte, store wasmvm.KVStore, goapi wasmvm.GoAPI, querier wasmvm.Querier, gasMeter wasmvm.GasMeter, gasLimit uint64, deserCost wasmvmtypes.UFraction) ([]byte, uint64, error) { + resp, err := json.Marshal(types.StatusResult{Status: exported.Active.String()}) + suite.Require().NoError(err) + return resp, wasmtesting.DefaultGasUsed, nil + }) + + db := dbm.NewMemDB() + app := simapp.NewSimApp(log.NewNopLogger(), db, nil, true, simtestutil.EmptyAppOptions{}, suite.mockVM) + + // reset DefaultTestingAppInit to its original value + ibctesting.DefaultTestingAppInit = setupTestingApp + return app, app.DefaultGenesis() +} + +// storeWasmCode stores the wasm code on chain and returns the checksum. +func storeWasmCode(suite *WasmTestSuite, wasmCode []byte) types.Checksum { + ctx := suite.chainA.GetContext().WithBlockGasMeter(storetypes.NewInfiniteGasMeter()) + + msg := types.NewMsgStoreCode(authtypes.NewModuleAddress(govtypes.ModuleName).String(), wasmCode) + response, err := GetSimApp(suite.chainA).WasmClientKeeper.StoreCode(ctx, msg) + suite.Require().NoError(err) + suite.Require().NotNil(response.Checksum) + return response.Checksum +} From 79fb56f1cc846f0b2ff9a155f4d932fb055e2d9f Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Fri, 9 Feb 2024 22:21:24 +0100 Subject: [PATCH 05/19] fix linter warnings --- modules/core/02-client/keeper/client_test.go | 2 +- .../06-solomachine/light_client_module.go | 2 +- .../07-tendermint/light_client_module_test.go | 3 ++- .../light-clients/08-wasm/light_client_module.go | 15 +++++++-------- .../08-wasm/light_client_module_test.go | 6 +++--- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/modules/core/02-client/keeper/client_test.go b/modules/core/02-client/keeper/client_test.go index 1985fecce01..43ce1ba78e7 100644 --- a/modules/core/02-client/keeper/client_test.go +++ b/modules/core/02-client/keeper/client_test.go @@ -502,7 +502,7 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { tc.setup() - err = suite.chainA.App.GetIBCKeeper().ClientKeeper.UpgradeClient(suite.chainA.GetContext(), path.EndpointA.ClientID, upgradedClient, upgradedConsState, upgradedClientProof, upgradedConsensusStateProof) + err = suite.chainA.App.GetIBCKeeper().ClientKeeper.UpgradeClient(suite.chainA.GetContext(), path.EndpointA.ClientID, upgradedClientBz, upgradedConsStateBz, upgradedClientProof, upgradedConsensusStateProof) if tc.expPass { suite.Require().NoError(err, "verify upgrade failed on valid case: %s", tc.name) diff --git a/modules/light-clients/06-solomachine/light_client_module.go b/modules/light-clients/06-solomachine/light_client_module.go index 94a2db9539f..d83b661795e 100644 --- a/modules/light-clients/06-solomachine/light_client_module.go +++ b/modules/light-clients/06-solomachine/light_client_module.go @@ -223,7 +223,7 @@ func (LightClientModule) RecoverClient(ctx sdk.Context, clientID, substituteClie } // VerifyUpgradeAndUpdateState returns an error since solomachine client does not support upgrades -func (lcm LightClientModule) VerifyUpgradeAndUpdateState( +func (LightClientModule) VerifyUpgradeAndUpdateState( ctx sdk.Context, clientID string, newClient []byte, diff --git a/modules/light-clients/07-tendermint/light_client_module_test.go b/modules/light-clients/07-tendermint/light_client_module_test.go index ad157b971f6..2a1a9939ba1 100644 --- a/modules/light-clients/07-tendermint/light_client_module_test.go +++ b/modules/light-clients/07-tendermint/light_client_module_test.go @@ -5,6 +5,7 @@ import ( "time" upgradetypes "cosmossdk.io/x/upgrade/types" + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types" host "github.com/cosmos/ibc-go/v8/modules/core/24-host" @@ -123,7 +124,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { cs, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID) suite.Require().True(found) - zeroedUpgradedClientBz, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) + upgradedClientStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) }, ibcerrors.ErrInvalidHeight, diff --git a/modules/light-clients/08-wasm/light_client_module.go b/modules/light-clients/08-wasm/light_client_module.go index 5c9ce302fb2..fc449333d3f 100644 --- a/modules/light-clients/08-wasm/light_client_module.go +++ b/modules/light-clients/08-wasm/light_client_module.go @@ -7,7 +7,6 @@ import ( wasmkeeper "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/keeper" "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" - wasmtypes "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" "github.com/cosmos/ibc-go/v8/modules/core/exported" @@ -263,8 +262,8 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( return err } - if clientType != wasmtypes.Wasm { - return errorsmod.Wrapf(clienttypes.ErrInvalidClientType, "expected: %s, got: %s", wasmtypes.Wasm, clientType) + if clientType != types.Wasm { + return errorsmod.Wrapf(clienttypes.ErrInvalidClientType, "expected: %s, got: %s", types.Wasm, clientType) } var ( @@ -276,9 +275,9 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( if err := cdc.UnmarshalInterface(newClient, &newClientState); err != nil { return err } - newWasmClientState, ok := newClientState.(*wasmtypes.ClientState) + newWasmClientState, ok := newClientState.(*types.ClientState) if !ok { - return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "expected client state type %T, got %T", (*wasmtypes.ClientState)(nil), newClientState) + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "expected client state type %T, got %T", (*types.ClientState)(nil), newClientState) } if err := newClientState.Validate(); err != nil { return err @@ -287,16 +286,16 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( if err := cdc.UnmarshalInterface(newConsState, &newConsensusState); err != nil { return err } - newWasmConsensusState, ok := newConsensusState.(*wasmtypes.ConsensusState) + newWasmConsensusState, ok := newConsensusState.(*types.ConsensusState) if !ok { - return errorsmod.Wrapf(clienttypes.ErrInvalidConsensus, "expected consensus state type %T, got %T", (*wasmtypes.ConsensusState)(nil), newConsensusState) + return errorsmod.Wrapf(clienttypes.ErrInvalidConsensus, "expected consensus state type %T, got %T", (*types.ConsensusState)(nil), newConsensusState) } if err := newConsensusState.ValidateBasic(); err != nil { return err } clientStore := lcm.storeProvider.ClientStore(ctx, clientID) - clientState, found := wasmtypes.GetClientState(clientStore, cdc) + clientState, found := types.GetClientState(clientStore, cdc) if !found { return errorsmod.Wrap(clienttypes.ErrClientNotFound, clientID) } diff --git a/modules/light-clients/08-wasm/light_client_module_test.go b/modules/light-clients/08-wasm/light_client_module_test.go index 30d1670496e..0937164c163 100644 --- a/modules/light-clients/08-wasm/light_client_module_test.go +++ b/modules/light-clients/08-wasm/light_client_module_test.go @@ -6,9 +6,9 @@ import ( wasmvm "github.com/CosmWasm/wasmvm" wasmvmtypes "github.com/CosmWasm/wasmvm/types" + wasmtesting "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/testing" "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" - wasmtypes "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types" host "github.com/cosmos/ibc-go/v8/modules/core/24-host" @@ -26,7 +26,7 @@ const ( func (suite *WasmTestSuite) TestVerifyUpgradeAndUpdateState() { var ( clientID string - clientState *wasmtypes.ClientState + clientState *types.ClientState upgradedClientState exported.ClientState upgradedConsensusState exported.ConsensusState upgradedClientStateBz, upgradedConsensusStateBz []byte @@ -139,7 +139,7 @@ func (suite *WasmTestSuite) TestVerifyUpgradeAndUpdateState() { suite.Require().NoError(err) clientID = endpoint.ClientID - clientState = endpoint.GetClientState().(*wasmtypes.ClientState) + clientState = endpoint.GetClientState().(*types.ClientState) latestHeight := clientState.GetLatestHeight() newLatestHeight := clienttypes.NewHeight(latestHeight.GetRevisionNumber(), latestHeight.GetRevisionHeight()+1) From 623d8c6b085fc5442a06cbd945ee3f66ea22433e Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Tue, 13 Feb 2024 00:18:12 +0100 Subject: [PATCH 06/19] check client state post upgrade --- .../07-tendermint/light_client_module_test.go | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/modules/light-clients/07-tendermint/light_client_module_test.go b/modules/light-clients/07-tendermint/light_client_module_test.go index 2a1a9939ba1..7a1cb453003 100644 --- a/modules/light-clients/07-tendermint/light_client_module_test.go +++ b/modules/light-clients/07-tendermint/light_client_module_test.go @@ -15,9 +15,9 @@ import ( ibctesting "github.com/cosmos/ibc-go/v8/testing" ) -const ( - tmClientID = "07-tendermint-100" - wasmClientID = "08-wasm-0" +var ( + tmClientID = clienttypes.FormatClientIdentifier(exported.Tendermint, 100) + solomachineClientID = clienttypes.FormatClientIdentifier(exported.Solomachine, 0) ) func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { @@ -71,7 +71,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { { "client type is not 07-tendermint", func() { - clientID = wasmClientID + clientID = solomachineClientID }, clienttypes.ErrInvalidClientType, }, @@ -113,8 +113,9 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { // change upgraded client state height to be lower than current client state height clientState := path.EndpointA.GetClientState().(*ibctm.ClientState) tmClient := upgradedClientState.(*ibctm.ClientState) - tmClient.ChainId = clientState.ChainId - tmClient.LatestHeight = clienttypes.NewHeight(1, 1) + newLatestheight, ok := clientState.GetLatestHeight().Decrement() + suite.Require().True(ok) + tmClient.LatestHeight = newLatestheight.(clienttypes.Height) upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), tmClient) suite.coordinator.CommitBlock(suite.chainB) @@ -144,11 +145,8 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { clientState := path.EndpointA.GetClientState().(*ibctm.ClientState) revisionNumber := clienttypes.ParseChainID(clientState.ChainId) - var err error - newChainID, err := clienttypes.SetRevisionNumber(clientState.ChainId, revisionNumber+1) - suite.Require().NoError(err) - - upgradedClientState = ibctm.NewClientState(newChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, clienttypes.NewHeight(revisionNumber+1, clientState.GetLatestHeight().GetRevisionHeight()+1), commitmenttypes.GetSDKSpecs(), upgradePath) + newUnbondindPeriod := ubdPeriod + trustingPeriod + upgradedClientState = ibctm.NewClientState(clientState.ChainId, ibctm.DefaultTrustLevel, trustingPeriod, newUnbondindPeriod, maxClockDrift, clienttypes.NewHeight(revisionNumber+1, clientState.GetLatestHeight().GetRevisionHeight()+1), commitmenttypes.GetSDKSpecs(), upgradePath) upgradedClientStateBz = clienttypes.MustMarshalClientState(cdc, upgradedClientState) nextValsHash := sha256.Sum256([]byte("new-nextValsHash")) @@ -175,7 +173,8 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { clientState := suite.chainA.GetClientState(clientID) suite.Require().NotNil(clientState) - // TODO: check client state bytes matches upgraded client state bytes + clientStateBz := clienttypes.MustMarshalClientState(cdc, upgradedClientState) + suite.Require().Equal(upgradedClientStateBz, clientStateBz) consensusState, found := suite.chainA.GetConsensusState(clientID, clientState.GetLatestHeight()) suite.Require().True(found) From 3e311639ef002c2272ed02e11cbd5aee1bc543d3 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Fri, 16 Feb 2024 12:59:50 +0100 Subject: [PATCH 07/19] fix: encoding issue: unmarshal to concrete types and not interfaces in 07-tendermint --- modules/core/keeper/msg_server_test.go | 2 +- .../07-tendermint/light_client_module.go | 24 +++++++------------ 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index 689a115549e..eba21963b07 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -888,7 +888,7 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { tc.setup() ctx := suite.chainA.GetContext() - _, err = keeper.Keeper.UpgradeClient(*suite.chainA.App.GetIBCKeeper(), ctx, msg) + _, err = suite.chainA.GetSimApp().GetIBCKeeper().UpgradeClient(ctx, msg) if tc.expPass { suite.Require().NoError(err, "upgrade handler failed on valid case: %s", tc.name) diff --git a/modules/light-clients/07-tendermint/light_client_module.go b/modules/light-clients/07-tendermint/light_client_module.go index ba4b1d7a651..aeaa0acb853 100644 --- a/modules/light-clients/07-tendermint/light_client_module.go +++ b/modules/light-clients/07-tendermint/light_client_module.go @@ -303,30 +303,22 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( return errorsmod.Wrapf(clienttypes.ErrInvalidClientType, "expected: %s, got: %s", exported.Tendermint, clientType) } - var ( - cdc = lcm.keeper.Codec() - newClientState exported.ClientState - newConsensusState exported.ConsensusState - ) + cdc := lcm.keeper.Codec() - if err := cdc.UnmarshalInterface(newClient, &newClientState); err != nil { + var newClientState ClientState + if err := cdc.Unmarshal(newClient, &newClientState); err != nil { return err } - newTmClientState, ok := newClientState.(*ClientState) - if !ok { - return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "expected client state type %T, got %T", (*ClientState)(nil), newClientState) - } + if err := newClientState.Validate(); err != nil { return err } - if err := cdc.UnmarshalInterface(newConsState, &newConsensusState); err != nil { + var newConsensusState ConsensusState + if err := cdc.Unmarshal(newConsState, &newConsensusState); err != nil { return err } - newTmConsensusState, ok := newConsensusState.(*ConsensusState) - if !ok { - return errorsmod.Wrapf(clienttypes.ErrInvalidConsensus, "expected consensus state type %T, got %T", (*ConsensusState)(nil), newConsensusState) - } + if err := newConsensusState.ValidateBasic(); err != nil { return err } @@ -343,5 +335,5 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( return errorsmod.Wrapf(ibcerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s", newClientState.GetLatestHeight(), lastHeight) } - return clientState.VerifyUpgradeAndUpdateState(ctx, cdc, clientStore, newTmClientState, newTmConsensusState, upgradeClientProof, upgradeConsensusStateProof) + return clientState.VerifyUpgradeAndUpdateState(ctx, cdc, clientStore, &newClientState, &newConsensusState, upgradeClientProof, upgradeConsensusStateProof) } From e92393b87811d99aa5c31fab3c79789c5ad02fb2 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Sun, 18 Feb 2024 23:19:58 +0100 Subject: [PATCH 08/19] fix import --- modules/light-clients/08-wasm/light_client_module_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/light-clients/08-wasm/light_client_module_test.go b/modules/light-clients/08-wasm/light_client_module_test.go index 68b7e89265a..415a3486cb3 100644 --- a/modules/light-clients/08-wasm/light_client_module_test.go +++ b/modules/light-clients/08-wasm/light_client_module_test.go @@ -87,7 +87,7 @@ func (suite *WasmTestSuite) TestRecoverClient() { { "subject and substitute have equal latest height", func() { - wasmClientState, ok := subjectClientState.(*wasmtypes.ClientState) + wasmClientState, ok := subjectClientState.(*types.ClientState) suite.Require().True(ok) wasmClientState.LatestHeight = substituteClientState.GetLatestHeight().(clienttypes.Height) suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), subjectClientID, wasmClientState) @@ -97,7 +97,7 @@ func (suite *WasmTestSuite) TestRecoverClient() { { "subject height is greater than substitute height", func() { - wasmClientState, ok := subjectClientState.(*wasmtypes.ClientState) + wasmClientState, ok := subjectClientState.(*types.ClientState) suite.Require().True(ok) wasmClientState.LatestHeight = substituteClientState.GetLatestHeight().Increment().(clienttypes.Height) suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), subjectClientID, wasmClientState) From 4dfcf27f2a98b643e4a430dd7865b715e3bbfd2d Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Sun, 18 Feb 2024 23:34:26 +0100 Subject: [PATCH 09/19] cast type before calling zero custom fields --- .../light-clients/07-tendermint/light_client_module_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/light-clients/07-tendermint/light_client_module_test.go b/modules/light-clients/07-tendermint/light_client_module_test.go index 2f77ceb149f..6f237a8c12b 100644 --- a/modules/light-clients/07-tendermint/light_client_module_test.go +++ b/modules/light-clients/07-tendermint/light_client_module_test.go @@ -166,7 +166,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { lastHeight := clienttypes.NewHeight(0, uint64(suite.chainB.GetContext().BlockHeight()+1)) // zero custom fields and store in upgrade store - zeroedUpgradedClient := upgradedClientState.ZeroCustomFields() + zeroedUpgradedClient := upgradedClientState.(*ibctm.ClientState).ZeroCustomFields() zeroedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), zeroedUpgradedClient) err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), zeroedUpgradedClientBz) suite.Require().NoError(err) @@ -228,7 +228,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { lastHeight := clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) // zero custom fields and store in upgrade store - zeroedUpgradedClient := upgradedClientState.ZeroCustomFields() + zeroedUpgradedClient := upgradedClientState.(*ibctm.ClientState).ZeroCustomFields() zeroedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), zeroedUpgradedClient) err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), zeroedUpgradedClientBz) suite.Require().NoError(err) From 70a1182da5caa8be679d32e2c41dc5224e4a93dd Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Mon, 19 Feb 2024 15:43:03 +0100 Subject: [PATCH 10/19] test: marshal concrete types using chain codec for upgrade tests which bypass msg server entrypoint --- modules/core/02-client/keeper/client_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/core/02-client/keeper/client_test.go b/modules/core/02-client/keeper/client_test.go index cd01a4e2c32..d0e2cf0e688 100644 --- a/modules/core/02-client/keeper/client_test.go +++ b/modules/core/02-client/keeper/client_test.go @@ -487,13 +487,13 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { upgradedClient = ibctm.NewClientState(newChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, clienttypes.NewHeight(revisionNumber+1, clientState.GetLatestHeight().GetRevisionHeight()+1), commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath) upgradedClient = upgradedClient.ZeroCustomFields() - upgradedClientBz, err = clienttypes.MarshalClientState(suite.chainA.App.AppCodec(), upgradedClient) + upgradedClientBz, err = suite.chainA.Codec.Marshal(upgradedClient) suite.Require().NoError(err) upgradedConsState = &ibctm.ConsensusState{ NextValidatorsHash: []byte("nextValsHash"), } - upgradedConsStateBz, err = clienttypes.MarshalConsensusState(suite.chainA.App.AppCodec(), upgradedConsState) + upgradedConsStateBz, err = suite.chainA.Codec.Marshal(upgradedConsState) suite.Require().NoError(err) tc.setup() From 1bad03b52e68645c2090dedf9bfff0e724b01381 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Mon, 19 Feb 2024 16:01:17 +0100 Subject: [PATCH 11/19] chore: remove basic validation of lc types in tm VerifyUpgradeAndUpdateState --- .../light-clients/07-tendermint/light_client_module.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/modules/light-clients/07-tendermint/light_client_module.go b/modules/light-clients/07-tendermint/light_client_module.go index c66071f879e..1921cc6c6ed 100644 --- a/modules/light-clients/07-tendermint/light_client_module.go +++ b/modules/light-clients/07-tendermint/light_client_module.go @@ -323,19 +323,11 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( return err } - if err := newClientState.Validate(); err != nil { - return err - } - var newConsensusState ConsensusState if err := cdc.Unmarshal(newConsState, &newConsensusState); err != nil { return err } - if err := newConsensusState.ValidateBasic(); err != nil { - return err - } - clientStore := lcm.storeProvider.ClientStore(ctx, clientID) clientState, found := getClientState(clientStore, cdc) if !found { From fbe8b921e27cff49071317a3ebbec14f503abaf5 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Mon, 26 Feb 2024 09:46:02 +0100 Subject: [PATCH 12/19] marshal to concrete type in tests --- modules/core/02-client/keeper/client_test.go | 48 +++++++++-------- modules/core/exported/client.go | 18 ------- modules/core/keeper/msg_server_test.go | 1 - .../06-solomachine/client_state.go | 8 --- .../light_client_module_test.go | 12 +++++ .../07-tendermint/light_client_module.go | 27 ++++------ .../07-tendermint/light_client_module_test.go | 52 ++++++++----------- .../07-tendermint/upgrade_test.go | 2 +- .../08-wasm/light_client_module.go | 36 +++---------- .../08-wasm/light_client_module_test.go | 30 ++++------- .../09-localhost/client_state.go | 13 ----- .../09-localhost/client_state_test.go | 6 --- .../09-localhost/light_client_module_test.go | 10 ++++ 13 files changed, 97 insertions(+), 166 deletions(-) diff --git a/modules/core/02-client/keeper/client_test.go b/modules/core/02-client/keeper/client_test.go index d0e2cf0e688..9f3239490c3 100644 --- a/modules/core/02-client/keeper/client_test.go +++ b/modules/core/02-client/keeper/client_test.go @@ -320,7 +320,7 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { var ( path *ibctesting.Path upgradedClient *ibctm.ClientState - upgradedConsState exported.ConsensusState + upgradedConsState *ibctm.ConsensusState lastHeight exported.Height upgradedClientProof, upgradedConsensusStateProof []byte upgradedClientBz, upgradedConsStateBz []byte @@ -476,35 +476,37 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { for _, tc := range testCases { tc := tc - path = ibctesting.NewPath(suite.chainA, suite.chainB) - path.SetupClients() + suite.Run(tc.name, func() { + path = ibctesting.NewPath(suite.chainA, suite.chainB) + path.SetupClients() - clientState := path.EndpointA.GetClientState().(*ibctm.ClientState) - revisionNumber := clienttypes.ParseChainID(clientState.ChainId) + clientState := path.EndpointA.GetClientState().(*ibctm.ClientState) + revisionNumber := clienttypes.ParseChainID(clientState.ChainId) - newChainID, err := clienttypes.SetRevisionNumber(clientState.ChainId, revisionNumber+1) - suite.Require().NoError(err) + newChainID, err := clienttypes.SetRevisionNumber(clientState.ChainId, revisionNumber+1) + suite.Require().NoError(err) - upgradedClient = ibctm.NewClientState(newChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, clienttypes.NewHeight(revisionNumber+1, clientState.GetLatestHeight().GetRevisionHeight()+1), commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath) - upgradedClient = upgradedClient.ZeroCustomFields() - upgradedClientBz, err = suite.chainA.Codec.Marshal(upgradedClient) - suite.Require().NoError(err) + upgradedClient = ibctm.NewClientState(newChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, clienttypes.NewHeight(revisionNumber+1, clientState.GetLatestHeight().GetRevisionHeight()+1), commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath) + upgradedClient = upgradedClient.ZeroCustomFields() + upgradedClientBz, err = suite.chainA.Codec.Marshal(upgradedClient) + suite.Require().NoError(err) - upgradedConsState = &ibctm.ConsensusState{ - NextValidatorsHash: []byte("nextValsHash"), - } - upgradedConsStateBz, err = suite.chainA.Codec.Marshal(upgradedConsState) - suite.Require().NoError(err) + upgradedConsState = &ibctm.ConsensusState{ + NextValidatorsHash: []byte("nextValsHash"), + } + upgradedConsStateBz, err = suite.chainA.Codec.Marshal(upgradedConsState) + suite.Require().NoError(err) - tc.setup() + tc.setup() - err = suite.chainA.App.GetIBCKeeper().ClientKeeper.UpgradeClient(suite.chainA.GetContext(), path.EndpointA.ClientID, upgradedClientBz, upgradedConsStateBz, upgradedClientProof, upgradedConsensusStateProof) + err = suite.chainA.App.GetIBCKeeper().ClientKeeper.UpgradeClient(suite.chainA.GetContext(), path.EndpointA.ClientID, upgradedClientBz, upgradedConsStateBz, upgradedClientProof, upgradedConsensusStateProof) - if tc.expPass { - suite.Require().NoError(err, "verify upgrade failed on valid case: %s", tc.name) - } else { - suite.Require().Error(err, "verify upgrade passed on invalid case: %s", tc.name) - } + if tc.expPass { + suite.Require().NoError(err, "verify upgrade failed on valid case: %s", tc.name) + } else { + suite.Require().Error(err, "verify upgrade passed on invalid case: %s", tc.name) + } + }) } } diff --git a/modules/core/exported/client.go b/modules/core/exported/client.go index 93a7f039625..0c975591587 100644 --- a/modules/core/exported/client.go +++ b/modules/core/exported/client.go @@ -5,7 +5,6 @@ import ( storetypes "cosmossdk.io/store/types" - "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -139,23 +138,6 @@ type ClientState interface { ClientType() string GetLatestHeight() Height Validate() error - - // Upgrade functions - // NOTE: proof heights are not included as upgrade to a new revision is expected to pass only on the last - // height committed by the current revision. Clients are responsible for ensuring that the planned last - // height of the current revision is somehow encoded in the proof verification process. - // This is to ensure that no premature upgrades occur, since upgrade plans committed to by the counterparty - // may be cancelled or modified before the last planned height. - // If the upgrade is verified, the upgraded client and consensus states must be set in the client store. - VerifyUpgradeAndUpdateState( - ctx sdk.Context, - cdc codec.BinaryCodec, - store storetypes.KVStore, - newClient ClientState, - newConsState ConsensusState, - upgradeClientProof, - upgradeConsensusStateProof []byte, - ) error } // ConsensusState is the state of the consensus process diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index 8560fa7bc35..ee8c86daf0d 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -903,7 +903,6 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { expectedEvents = sdk.MarkEventsToIndex(expectedEvents, map[string]struct{}{}) ibctesting.AssertEvents(&suite.Suite, expectedEvents, ctx.EventManager().Events().ToABCIEvents()) - } else { suite.Require().Error(err, "upgrade handler passed on invalid case: %s", tc.name) } diff --git a/modules/light-clients/06-solomachine/client_state.go b/modules/light-clients/06-solomachine/client_state.go index 88f41ae07a0..d2509e7b3f2 100644 --- a/modules/light-clients/06-solomachine/client_state.go +++ b/modules/light-clients/06-solomachine/client_state.go @@ -87,14 +87,6 @@ func (cs ClientState) Initialize(_ sdk.Context, cdc codec.BinaryCodec, clientSto return nil } -// VerifyUpgradeAndUpdateState returns an error since solomachine client does not support upgrades -func (ClientState) VerifyUpgradeAndUpdateState( - _ sdk.Context, _ codec.BinaryCodec, _ storetypes.KVStore, - _ exported.ClientState, _ exported.ConsensusState, _, _ []byte, -) error { - return errorsmod.Wrap(clienttypes.ErrInvalidUpgradeClient, "cannot upgrade solomachine client") -} - // VerifyMembership is a generic proof verification method which verifies a proof of the existence of a value at a given CommitmentPath at the latest sequence. // The caller is expected to construct the full CommitmentPath from a CommitmentPrefix and a standardized path (as defined in ICS 24). func (cs *ClientState) VerifyMembership( diff --git a/modules/light-clients/06-solomachine/light_client_module_test.go b/modules/light-clients/06-solomachine/light_client_module_test.go index ce9cca27e8b..fa526935c8f 100644 --- a/modules/light-clients/06-solomachine/light_client_module_test.go +++ b/modules/light-clients/06-solomachine/light_client_module_test.go @@ -146,3 +146,15 @@ func (suite *SoloMachineTestSuite) TestRecoverClient() { }) } } + +func (suite *SoloMachineTestSuite) TestVerifyUpgradeAndUpdateState() { + suite.SetupTest() + + clientID := suite.chainA.App.GetIBCKeeper().ClientKeeper.GenerateClientIdentifier(suite.chainA.GetContext(), exported.Solomachine) + + lightClientModule, found := suite.chainA.GetSimApp().IBCKeeper.ClientKeeper.GetRouter().GetRoute(clientID) + suite.Require().True(found) + + err := lightClientModule.VerifyUpgradeAndUpdateState(suite.chainA.GetContext(), clientID, nil, nil, nil, nil) + suite.Require().Error(err) +} diff --git a/modules/light-clients/07-tendermint/light_client_module.go b/modules/light-clients/07-tendermint/light_client_module.go index 1921cc6c6ed..f06b9336410 100644 --- a/modules/light-clients/07-tendermint/light_client_module.go +++ b/modules/light-clients/07-tendermint/light_client_module.go @@ -292,13 +292,17 @@ func (lcm LightClientModule) RecoverClient(ctx sdk.Context, clientID, substitute return clientState.CheckSubstituteAndUpdateState(ctx, cdc, clientStore, substituteClientStore, substituteClient) } -// Upgrade functions -// NOTE: proof heights are not included as upgrade to a new revision is expected to pass only on the last -// height committed by the current revision. Clients are responsible for ensuring that the planned last -// height of the current revision is somehow encoded in the proof verification process. -// This is to ensure that no premature upgrades occur, since upgrade plans committed to by the counterparty -// may be cancelled or modified before the last planned height. -// If the upgrade is verified, the upgraded client and consensus states must be set in the client store. +// VerifyUpgradeAndUpdateState checks if the upgraded client has been committed by the current client +// It will zero out all client-specific fields (e.g. TrustingPeriod) and verify all data +// in client state that must be the same across all valid Tendermint clients for the new chain. +// VerifyUpgrade will return an error if: +// - the upgradedClient is not a Tendermint ClientState +// - the latest height of the client state does not have the same revision number or has a greater +// height than the committed client. +// - the height of upgraded client is not greater than that of current client +// - the latest height of the new client does not match or is greater than the height in committed client +// - any Tendermint chain specified parameter in upgraded client such as ChainID, UnbondingPeriod, +// and ProofSpecs do not match parameters set by committed client func (lcm LightClientModule) VerifyUpgradeAndUpdateState( ctx sdk.Context, clientID string, @@ -307,15 +311,6 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( upgradeClientProof, upgradeConsensusStateProof []byte, ) error { - clientType, _, err := clienttypes.ParseClientIdentifier(clientID) - if err != nil { - return err - } - - if clientType != exported.Tendermint { - return errorsmod.Wrapf(clienttypes.ErrInvalidClientType, "expected: %s, got: %s", exported.Tendermint, clientType) - } - cdc := lcm.keeper.Codec() var newClientState ClientState diff --git a/modules/light-clients/07-tendermint/light_client_module_test.go b/modules/light-clients/07-tendermint/light_client_module_test.go index 6f237a8c12b..f5645bd795c 100644 --- a/modules/light-clients/07-tendermint/light_client_module_test.go +++ b/modules/light-clients/07-tendermint/light_client_module_test.go @@ -11,6 +11,7 @@ import ( host "github.com/cosmos/ibc-go/v8/modules/core/24-host" ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" "github.com/cosmos/ibc-go/v8/modules/core/exported" + solomachine "github.com/cosmos/ibc-go/v8/modules/light-clients/06-solomachine" ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" ibctesting "github.com/cosmos/ibc-go/v8/testing" ) @@ -149,7 +150,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { var ( clientID string path *ibctesting.Path - upgradedClientState exported.ClientState + upgradedClientState *ibctm.ClientState upgradedClientStateBz, upgradedConsensusStateBz []byte upgradedClientStateProof, upgradedConsensusStateProof []byte ) @@ -166,7 +167,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { lastHeight := clienttypes.NewHeight(0, uint64(suite.chainB.GetContext().BlockHeight()+1)) // zero custom fields and store in upgrade store - zeroedUpgradedClient := upgradedClientState.(*ibctm.ClientState).ZeroCustomFields() + zeroedUpgradedClient := upgradedClientState.ZeroCustomFields() zeroedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), zeroedUpgradedClient) err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), zeroedUpgradedClientBz) suite.Require().NoError(err) @@ -186,20 +187,6 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { }, nil, }, - { - "cannot parse malformed client ID", - func() { - clientID = ibctesting.InvalidID - }, - host.ErrInvalidID, - }, - { - "client type is not 07-tendermint", - func() { - clientID = solomachineClientID - }, - clienttypes.ErrInvalidClientType, - }, { "cannot find client state", func() { @@ -210,14 +197,18 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { { "upgraded client state is not for tendermint client state", func() { - upgradedClientStateBz = []byte{} + var err error + upgradedClientStateBz, err = suite.chainA.Codec.Marshal(solomachine.NewClientState(0, &solomachine.ConsensusState{})) + suite.Require().NoError(err) }, clienttypes.ErrInvalidClient, }, { "upgraded consensus state is not tendermint consensus state", func() { - upgradedConsensusStateBz = []byte{} + var err error + upgradedConsensusStateBz, err = suite.chainA.Codec.Marshal(&solomachine.ConsensusState{}) + suite.Require().NoError(err) }, clienttypes.ErrInvalidConsensus, }, @@ -228,7 +219,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { lastHeight := clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) // zero custom fields and store in upgrade store - zeroedUpgradedClient := upgradedClientState.(*ibctm.ClientState).ZeroCustomFields() + zeroedUpgradedClient := upgradedClientState.ZeroCustomFields() zeroedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), zeroedUpgradedClient) err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), zeroedUpgradedClientBz) suite.Require().NoError(err) @@ -237,12 +228,11 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { // change upgraded client state height to be lower than current client state height clientState := path.EndpointA.GetClientState().(*ibctm.ClientState) - tmClient := upgradedClientState.(*ibctm.ClientState) newLatestheight, ok := clientState.GetLatestHeight().Decrement() suite.Require().True(ok) - tmClient.LatestHeight = newLatestheight.(clienttypes.Height) - upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), tmClient) - + upgradedClientState.LatestHeight = newLatestheight.(clienttypes.Height) + upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), upgradedClientState) + suite.Require().NoError(err) suite.coordinator.CommitBlock(suite.chainB) err = path.EndpointA.UpdateClient() suite.Require().NoError(err) @@ -261,8 +251,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { tc := tc suite.Run(tc.name, func() { suite.SetupTest() // reset - cdc := suite.chainA.App.AppCodec() - ctx := suite.chainA.GetContext() + var err error path = ibctesting.NewPath(suite.chainA, suite.chainB) path.SetupClients() @@ -272,19 +261,21 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { newUnbondindPeriod := ubdPeriod + trustingPeriod upgradedClientState = ibctm.NewClientState(clientState.ChainId, ibctm.DefaultTrustLevel, trustingPeriod, newUnbondindPeriod, maxClockDrift, clienttypes.NewHeight(revisionNumber+1, clientState.GetLatestHeight().GetRevisionHeight()+1), commitmenttypes.GetSDKSpecs(), upgradePath) - upgradedClientStateBz = clienttypes.MustMarshalClientState(cdc, upgradedClientState) + upgradedClientStateBz, err = suite.chainA.Codec.Marshal(upgradedClientState) + suite.Require().NoError(err) nextValsHash := sha256.Sum256([]byte("new-nextValsHash")) upgradedConsensusState := ibctm.NewConsensusState(time.Now(), commitmenttypes.NewMerkleRoot([]byte("new-hash")), nextValsHash[:]) - upgradedConsensusStateBz = clienttypes.MustMarshalConsensusState(cdc, upgradedConsensusState) + upgradedConsensusStateBz, err = suite.chainA.Codec.Marshal(upgradedConsensusState) + suite.Require().NoError(err) lightClientModule, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetRouter().GetRoute(clientID) suite.Require().True(found) tc.malleate() - err := lightClientModule.VerifyUpgradeAndUpdateState( - ctx, + err = lightClientModule.VerifyUpgradeAndUpdateState( + suite.chainA.GetContext(), clientID, upgradedClientStateBz, upgradedConsensusStateBz, @@ -298,7 +289,8 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { clientState := suite.chainA.GetClientState(clientID) suite.Require().NotNil(clientState) - clientStateBz := clienttypes.MustMarshalClientState(cdc, upgradedClientState) + clientStateBz, err := suite.chainA.Codec.Marshal(upgradedClientState) + suite.Require().NoError(err) suite.Require().Equal(upgradedClientStateBz, clientStateBz) consensusState, found := suite.chainA.GetConsensusState(clientID, clientState.GetLatestHeight()) diff --git a/modules/light-clients/07-tendermint/upgrade_test.go b/modules/light-clients/07-tendermint/upgrade_test.go index 290fcae730a..3177245f3a9 100644 --- a/modules/light-clients/07-tendermint/upgrade_test.go +++ b/modules/light-clients/07-tendermint/upgrade_test.go @@ -455,7 +455,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgrade() { tc.setup() - cs := suite.chainA.GetClientState(path.EndpointA.ClientID) + cs := suite.chainA.GetClientState(path.EndpointA.ClientID).(*ibctm.ClientState) clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) // Call ZeroCustomFields on upgraded clients to clear any client-chosen parameters in test-case upgradedClient diff --git a/modules/light-clients/08-wasm/light_client_module.go b/modules/light-clients/08-wasm/light_client_module.go index 5fb1ad07f1d..af17767c07d 100644 --- a/modules/light-clients/08-wasm/light_client_module.go +++ b/modules/light-clients/08-wasm/light_client_module.go @@ -277,13 +277,8 @@ func (lcm LightClientModule) RecoverClient(ctx sdk.Context, clientID, substitute return clientState.CheckSubstituteAndUpdateState(ctx, cdc, clientStore, substituteClientStore, substituteClient) } -// Upgrade functions -// NOTE: proof heights are not included as upgrade to a new revision is expected to pass only on the last -// height committed by the current revision. Clients are responsible for ensuring that the planned last -// height of the current revision is somehow encoded in the proof verification process. -// This is to ensure that no premature upgrades occur, since upgrade plans committed to by the counterparty -// may be cancelled or modified before the last planned height. -// If the upgrade is verified, the upgraded client and consensus states must be set in the client store. +// VerifyUpgradeAndUpdateState, on a successful verification expects the contract to update +// the new client state, consensus state, and any other client metadata. func (lcm LightClientModule) VerifyUpgradeAndUpdateState( ctx sdk.Context, clientID string, @@ -292,39 +287,22 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( upgradeClientProof, upgradeConsensusStateProof []byte, ) error { - clientType, _, err := clienttypes.ParseClientIdentifier(clientID) - if err != nil { - return err - } - - if clientType != types.Wasm { - return errorsmod.Wrapf(clienttypes.ErrInvalidClientType, "expected: %s, got: %s", types.Wasm, clientType) - } - var ( cdc = lcm.keeper.Codec() - newClientState exported.ClientState - newConsensusState exported.ConsensusState + newClientState types.ClientState + newConsensusState types.ConsensusState ) - if err := cdc.UnmarshalInterface(newClient, &newClientState); err != nil { + if err := cdc.Unmarshal(newClient, &newClientState); err != nil { return err } - newWasmClientState, ok := newClientState.(*types.ClientState) - if !ok { - return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "expected client state type %T, got %T", (*types.ClientState)(nil), newClientState) - } if err := newClientState.Validate(); err != nil { return err } - if err := cdc.UnmarshalInterface(newConsState, &newConsensusState); err != nil { + if err := cdc.Unmarshal(newConsState, &newConsensusState); err != nil { return err } - newWasmConsensusState, ok := newConsensusState.(*types.ConsensusState) - if !ok { - return errorsmod.Wrapf(clienttypes.ErrInvalidConsensus, "expected consensus state type %T, got %T", (*types.ConsensusState)(nil), newConsensusState) - } if err := newConsensusState.ValidateBasic(); err != nil { return err } @@ -341,5 +319,5 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( return errorsmod.Wrapf(ibcerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s", newClientState.GetLatestHeight(), lastHeight) } - return clientState.VerifyUpgradeAndUpdateState(ctx, cdc, clientStore, newWasmClientState, newWasmConsensusState, upgradeClientProof, upgradeConsensusStateProof) + return clientState.VerifyUpgradeAndUpdateState(ctx, cdc, clientStore, &newClientState, &newConsensusState, upgradeClientProof, upgradeConsensusStateProof) } diff --git a/modules/light-clients/08-wasm/light_client_module_test.go b/modules/light-clients/08-wasm/light_client_module_test.go index 415a3486cb3..a2f52999c8a 100644 --- a/modules/light-clients/08-wasm/light_client_module_test.go +++ b/modules/light-clients/08-wasm/light_client_module_test.go @@ -201,20 +201,6 @@ func (suite *WasmTestSuite) TestVerifyUpgradeAndUpdateState() { }, nil, }, - { - "cannot parse malformed client ID", - func() { - clientID = ibctesting.InvalidID - }, - host.ErrInvalidID, - }, - { - "client type is not 08-wasm", - func() { - clientID = tmClientID - }, - clienttypes.ErrInvalidClientType, - }, { "cannot find client state", func() { @@ -239,13 +225,15 @@ func (suite *WasmTestSuite) TestVerifyUpgradeAndUpdateState() { { "upgraded client state height is not greater than current height", func() { + var err error latestHeight := clientState.GetLatestHeight() newLatestHeight := clienttypes.NewHeight(latestHeight.GetRevisionNumber(), latestHeight.GetRevisionHeight()-1) wrappedUpgradedClient := wasmtesting.CreateMockTendermintClientState(newLatestHeight) wrappedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), wrappedUpgradedClient) upgradedClientState = types.NewClientState(wrappedUpgradedClientBz, clientState.Checksum, newLatestHeight) - upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), upgradedClientState) + upgradedClientStateBz, err = suite.chainA.Codec.Marshal(upgradedClientState) + suite.Require().NoError(err) }, ibcerrors.ErrInvalidHeight, }, @@ -255,8 +243,6 @@ func (suite *WasmTestSuite) TestVerifyUpgradeAndUpdateState() { tc := tc suite.Run(tc.name, func() { suite.SetupWasmWithMockVM() // reset suite - cdc := suite.chainA.App.AppCodec() - ctx := suite.chainA.GetContext() endpoint := wasmtesting.NewWasmEndpoint(suite.chainA) err := endpoint.CreateClient() @@ -270,12 +256,14 @@ func (suite *WasmTestSuite) TestVerifyUpgradeAndUpdateState() { wrappedUpgradedClient := wasmtesting.CreateMockTendermintClientState(newLatestHeight) wrappedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), wrappedUpgradedClient) upgradedClientState = types.NewClientState(wrappedUpgradedClientBz, clientState.Checksum, newLatestHeight) - upgradedClientStateBz = clienttypes.MustMarshalClientState(cdc, upgradedClientState) + upgradedClientStateBz, err = suite.chainA.Codec.Marshal(upgradedClientState) + suite.Require().NoError(err) wrappedUpgradedConsensus := ibctm.NewConsensusState(time.Now(), commitmenttypes.NewMerkleRoot([]byte("new-hash")), []byte("new-nextValsHash")) - wrappedUpgradedConsensusBz := clienttypes.MustMarshalConsensusState(cdc, wrappedUpgradedConsensus) + wrappedUpgradedConsensusBz := clienttypes.MustMarshalConsensusState(suite.chainA.App.AppCodec(), wrappedUpgradedConsensus) upgradedConsensusState = types.NewConsensusState(wrappedUpgradedConsensusBz) - upgradedConsensusStateBz = clienttypes.MustMarshalConsensusState(cdc, upgradedConsensusState) + upgradedConsensusStateBz, err = suite.chainA.Codec.Marshal(upgradedConsensusState) + suite.Require().NoError(err) lightClientModule, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetRouter().GetRoute(clientID) suite.Require().True(found) @@ -288,7 +276,7 @@ func (suite *WasmTestSuite) TestVerifyUpgradeAndUpdateState() { upgradedConsensusStateProof = wasmtesting.MockUpgradedConsensusStateProofBz err = lightClientModule.VerifyUpgradeAndUpdateState( - ctx, + suite.chainA.GetContext(), clientID, upgradedClientStateBz, upgradedConsensusStateBz, diff --git a/modules/light-clients/09-localhost/client_state.go b/modules/light-clients/09-localhost/client_state.go index 93d15fe9803..e5ed588ea09 100644 --- a/modules/light-clients/09-localhost/client_state.go +++ b/modules/light-clients/09-localhost/client_state.go @@ -171,16 +171,3 @@ func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, client func (ClientState) CheckSubstituteAndUpdateState(_ sdk.Context, _ codec.BinaryCodec, _, _ storetypes.KVStore, _ exported.ClientState) error { return errorsmod.Wrap(clienttypes.ErrUpdateClientFailed, "cannot update localhost client with a proposal") } - -// VerifyUpgradeAndUpdateState returns an error since localhost cannot be upgraded -func (ClientState) VerifyUpgradeAndUpdateState( - _ sdk.Context, - _ codec.BinaryCodec, - _ storetypes.KVStore, - _ exported.ClientState, - _ exported.ConsensusState, - _, - _ []byte, -) error { - return errorsmod.Wrap(clienttypes.ErrInvalidUpgradeClient, "cannot upgrade localhost client") -} diff --git a/modules/light-clients/09-localhost/client_state_test.go b/modules/light-clients/09-localhost/client_state_test.go index 23a36c39334..9770f4ab8ca 100644 --- a/modules/light-clients/09-localhost/client_state_test.go +++ b/modules/light-clients/09-localhost/client_state_test.go @@ -125,9 +125,3 @@ func (suite *LocalhostTestSuite) TestCheckSubstituteAndUpdateState() { err := clientState.CheckSubstituteAndUpdateState(suite.chain.GetContext(), suite.chain.Codec, nil, nil, nil) suite.Require().Error(err) } - -func (suite *LocalhostTestSuite) TestVerifyUpgradeAndUpdateState() { - clientState := localhost.NewClientState(clienttypes.NewHeight(1, 10)) - err := clientState.VerifyUpgradeAndUpdateState(suite.chain.GetContext(), suite.chain.Codec, nil, nil, nil, nil, nil) - suite.Require().Error(err) -} diff --git a/modules/light-clients/09-localhost/light_client_module_test.go b/modules/light-clients/09-localhost/light_client_module_test.go index 97a5df1306e..ada90831a9d 100644 --- a/modules/light-clients/09-localhost/light_client_module_test.go +++ b/modules/light-clients/09-localhost/light_client_module_test.go @@ -302,3 +302,13 @@ func (suite *LocalhostTestSuite) TestVerifyNonMembership() { }) } } + +func (suite *LocalhostTestSuite) TestVerifyUpgradeAndUpdateState() { + suite.SetupTest() + + lightClientModule, found := suite.chain.GetSimApp().IBCKeeper.ClientKeeper.GetRouter().GetRoute(exported.LocalhostClientID) + suite.Require().True(found) + + err := lightClientModule.VerifyUpgradeAndUpdateState(suite.chain.GetContext(), exported.LocalhostClientID, nil, nil, nil, nil) + suite.Require().Error(err) +} From a9d611322fdf6b38d191ce324de425203111877d Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Tue, 27 Feb 2024 22:07:44 +0100 Subject: [PATCH 13/19] keep client/consensus state as interface marshalled --- modules/core/02-client/keeper/client.go | 6 +- modules/core/02-client/keeper/client_test.go | 103 ++++-------------- modules/core/02-client/keeper/events.go | 8 +- modules/core/02-client/keeper/events_test.go | 2 +- modules/core/keeper/msg_server.go | 14 ++- .../07-tendermint/light_client_module.go | 22 +++- .../07-tendermint/light_client_module_test.go | 44 ++++---- .../08-wasm/light_client_module.go | 22 ++-- .../08-wasm/light_client_module_test.go | 15 +-- 9 files changed, 102 insertions(+), 134 deletions(-) diff --git a/modules/core/02-client/keeper/client.go b/modules/core/02-client/keeper/client.go index becce0ce59b..9a5bec673ae 100644 --- a/modules/core/02-client/keeper/client.go +++ b/modules/core/02-client/keeper/client.go @@ -53,7 +53,8 @@ func (k Keeper) CreateClient( []metrics.Label{telemetry.NewLabel(types.LabelClientType, clientType)}, ) - emitCreateClientEvent(ctx, clientID, clientType) + latestHeight := types.ZeroHeight() // TODO: replace with correct height when LatestHeight is added to light client module + emitCreateClientEvent(ctx, clientID, clientType, latestHeight) return clientID, nil } @@ -155,7 +156,8 @@ func (k Keeper) UpgradeClient( }, ) - emitUpgradeClientEvent(ctx, clientID, clientType) + latestHeight := types.ZeroHeight() // TODO: replace with correct height when LatestHeight is added to light client module + emitUpgradeClientEvent(ctx, clientID, clientType, latestHeight) return nil } diff --git a/modules/core/02-client/keeper/client_test.go b/modules/core/02-client/keeper/client_test.go index 9f3239490c3..6bc4d38c316 100644 --- a/modules/core/02-client/keeper/client_test.go +++ b/modules/core/02-client/keeper/client_test.go @@ -320,8 +320,8 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { var ( path *ibctesting.Path upgradedClient *ibctm.ClientState - upgradedConsState *ibctm.ConsensusState - lastHeight exported.Height + upgradedConsState exported.ConsensusState + upgradeHeight exported.Height upgradedClientProof, upgradedConsensusStateProof []byte upgradedClientBz, upgradedConsStateBz []byte ) @@ -334,13 +334,13 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { { name: "successful upgrade", setup: func() { - // last Height is at next block - lastHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) + // upgrade Height is at next block + upgradeHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) // zero custom fields and store in upgrade store - err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedClientBz) + err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedClientBz) suite.Require().NoError(err) - err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsStateBz) + err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedConsStateBz) suite.Require().NoError(err) // commit upgrade store changes and update clients @@ -351,55 +351,26 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { cs, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID) suite.Require().True(found) - upgradedClientProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) - upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) + upgradedClientProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(upgradeHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) + upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(upgradeHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) }, expPass: true, }, - { - name: "client state not found", - setup: func() { - // last Height is at next block - lastHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) - - // zero custom fields and store in upgrade store - err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedClientBz) - suite.Require().NoError(err) - err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsStateBz) - suite.Require().NoError(err) - - // commit upgrade store changes and update clients - - suite.coordinator.CommitBlock(suite.chainB) - err = path.EndpointA.UpdateClient() - suite.Require().NoError(err) - - cs, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID) - suite.Require().True(found) - - upgradedClientProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) - upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) - - path.EndpointA.ClientID = "wrongclientid" - }, - expPass: false, - }, { name: "client state is not active", setup: func() { // client is frozen // last Height is at next block - lastHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) + upgradeHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) // zero custom fields and store in upgrade store - err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedClientBz) + err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedClientBz) suite.Require().NoError(err) - err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsStateBz) + err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedConsStateBz) suite.Require().NoError(err) // commit upgrade store changes and update clients - suite.coordinator.CommitBlock(suite.chainB) err = path.EndpointA.UpdateClient() suite.Require().NoError(err) @@ -407,8 +378,8 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { cs, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID) suite.Require().True(found) - upgradedClientProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) - upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) + upgradedClientProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(upgradeHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) + upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(upgradeHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) // set frozen client in store tmClient, ok := cs.(*ibctm.ClientState) @@ -419,19 +390,20 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { expPass: false, }, { - name: "tendermint client VerifyUpgrade fails", + name: "light client module VerifyUpgradeAndUpdateState fails", setup: func() { // last Height is at next block - lastHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) + upgradeHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) // zero custom fields and store in upgrade store - err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedClientBz) + err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedClientBz) suite.Require().NoError(err) - err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsStateBz) + err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedConsStateBz) suite.Require().NoError(err) // change upgradedClient client-specified parameters upgradedClient.ChainId = "wrongchainID" + upgradedClientBz = clienttypes.MustMarshalClientState(suite.chainA.Codec, upgradedClient) suite.coordinator.CommitBlock(suite.chainB) err = path.EndpointA.UpdateClient() @@ -440,35 +412,8 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { cs, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID) suite.Require().True(found) - upgradedClientProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) - upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) - }, - expPass: false, - }, - { - name: "unsuccessful upgrade: upgraded height is not greater than current height", - setup: func() { - // last Height is at next block - lastHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) - - // zero custom fields and store in upgrade store - err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedClientBz) - suite.Require().NoError(err) - err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsStateBz) - suite.Require().NoError(err) - - // change upgradedClient height to be lower than current client state height - upgradedClient.LatestHeight = clienttypes.NewHeight(0, 1) - - suite.coordinator.CommitBlock(suite.chainB) - err = path.EndpointA.UpdateClient() - suite.Require().NoError(err) - - cs, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID) - suite.Require().True(found) - - upgradedClientProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) - upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) + upgradedClientProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(upgradeHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) + upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(upgradeHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) }, expPass: false, }, @@ -488,14 +433,12 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { upgradedClient = ibctm.NewClientState(newChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, clienttypes.NewHeight(revisionNumber+1, clientState.GetLatestHeight().GetRevisionHeight()+1), commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath) upgradedClient = upgradedClient.ZeroCustomFields() - upgradedClientBz, err = suite.chainA.Codec.Marshal(upgradedClient) - suite.Require().NoError(err) - upgradedConsState = &ibctm.ConsensusState{ NextValidatorsHash: []byte("nextValsHash"), } - upgradedConsStateBz, err = suite.chainA.Codec.Marshal(upgradedConsState) - suite.Require().NoError(err) + + upgradedClientBz = clienttypes.MustMarshalClientState(suite.chainA.Codec, upgradedClient) + upgradedConsStateBz = clienttypes.MustMarshalConsensusState(suite.chainA.Codec, upgradedConsState) tc.setup() diff --git a/modules/core/02-client/keeper/events.go b/modules/core/02-client/keeper/events.go index 581b19c0eab..adc918d7fd6 100644 --- a/modules/core/02-client/keeper/events.go +++ b/modules/core/02-client/keeper/events.go @@ -15,13 +15,13 @@ import ( ) // emitCreateClientEvent emits a create client event -func emitCreateClientEvent(ctx sdk.Context, clientID, clientType string) { +func emitCreateClientEvent(ctx sdk.Context, clientID, clientType string, latestHeight exported.Height) { ctx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( types.EventTypeCreateClient, sdk.NewAttribute(types.AttributeKeyClientID, clientID), sdk.NewAttribute(types.AttributeKeyClientType, clientType), - // sdk.NewAttribute(types.AttributeKeyConsensusHeight, clientState.GetLatestHeight().String()), + sdk.NewAttribute(types.AttributeKeyConsensusHeight, latestHeight.String()), ), sdk.NewEvent( sdk.EventTypeMessage, @@ -60,13 +60,13 @@ func emitUpdateClientEvent(ctx sdk.Context, clientID string, clientType string, } // emitUpgradeClientEvent emits an upgrade client event -func emitUpgradeClientEvent(ctx sdk.Context, clientID string, clientType string) { +func emitUpgradeClientEvent(ctx sdk.Context, clientID string, clientType string, latestHeight exported.Height) { ctx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( types.EventTypeUpgradeClient, sdk.NewAttribute(types.AttributeKeyClientID, clientID), sdk.NewAttribute(types.AttributeKeyClientType, clientType), - // sdk.NewAttribute(types.AttributeKeyConsensusHeight, clientState.GetLatestHeight().String()), + sdk.NewAttribute(types.AttributeKeyConsensusHeight, latestHeight.String()), ), sdk.NewEvent( sdk.EventTypeMessage, diff --git a/modules/core/02-client/keeper/events_test.go b/modules/core/02-client/keeper/events_test.go index a1dcbe9ffa9..94b8cc60a54 100644 --- a/modules/core/02-client/keeper/events_test.go +++ b/modules/core/02-client/keeper/events_test.go @@ -39,7 +39,7 @@ func (suite *KeeperTestSuite) TestMsgCreateClientEvents() { clienttypes.EventTypeCreateClient, sdk.NewAttribute(clienttypes.AttributeKeyClientID, ibctesting.FirstClientID), sdk.NewAttribute(clienttypes.AttributeKeyClientType, clientState.ClientType()), - // sdk.NewAttribute(clienttypes.AttributeKeyConsensusHeight, clientState.GetLatestHeight().String()), + sdk.NewAttribute(clienttypes.AttributeKeyConsensusHeight, clientState.GetLatestHeight().String()), ), }.ToABCIEvents() diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index a43b921ed16..c39e74c7acd 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -69,10 +69,20 @@ func (k Keeper) UpdateClient(goCtx context.Context, msg *clienttypes.MsgUpdateCl func (k Keeper) UpgradeClient(goCtx context.Context, msg *clienttypes.MsgUpgradeClient) (*clienttypes.MsgUpgradeClientResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) + upgradedClientState, err := k.cdc.Marshal(msg.ClientState) + if err != nil { + return nil, err + } + + upgradedConsensusState, err := k.cdc.Marshal(msg.ConsensusState) + if err != nil { + return nil, err + } + if err := k.ClientKeeper.UpgradeClient( ctx, msg.ClientId, - msg.ClientState.Value, - msg.ConsensusState.Value, + upgradedClientState, + upgradedConsensusState, msg.ProofUpgradeClient, msg.ProofUpgradeConsensusState, ); err != nil { diff --git a/modules/light-clients/07-tendermint/light_client_module.go b/modules/light-clients/07-tendermint/light_client_module.go index f06b9336410..ee68612ee46 100644 --- a/modules/light-clients/07-tendermint/light_client_module.go +++ b/modules/light-clients/07-tendermint/light_client_module.go @@ -311,17 +311,27 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( upgradeClientProof, upgradeConsensusStateProof []byte, ) error { - cdc := lcm.keeper.Codec() + var ( + cdc = lcm.keeper.Codec() + newClientState exported.ClientState + newConsensusState exported.ConsensusState + ) - var newClientState ClientState - if err := cdc.Unmarshal(newClient, &newClientState); err != nil { + if err := cdc.UnmarshalInterface(newClient, &newClientState); err != nil { return err } + newTmClientState, ok := newClientState.(*ClientState) + if !ok { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "expected client state type %T, got %T", (*ClientState)(nil), newClientState) + } - var newConsensusState ConsensusState - if err := cdc.Unmarshal(newConsState, &newConsensusState); err != nil { + if err := cdc.UnmarshalInterface(newConsState, &newConsensusState); err != nil { return err } + newTmConsensusState, ok := newConsensusState.(*ConsensusState) + if !ok { + return errorsmod.Wrapf(clienttypes.ErrInvalidConsensus, "expected consensus state type %T, got %T", (*ConsensusState)(nil), newConsensusState) + } clientStore := lcm.storeProvider.ClientStore(ctx, clientID) clientState, found := getClientState(clientStore, cdc) @@ -335,5 +345,5 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( return errorsmod.Wrapf(ibcerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s", newClientState.GetLatestHeight(), lastHeight) } - return clientState.VerifyUpgradeAndUpdateState(ctx, cdc, clientStore, &newClientState, &newConsensusState, upgradeClientProof, upgradeConsensusStateProof) + return clientState.VerifyUpgradeAndUpdateState(ctx, cdc, clientStore, newTmClientState, newTmConsensusState, upgradeClientProof, upgradeConsensusStateProof) } diff --git a/modules/light-clients/07-tendermint/light_client_module_test.go b/modules/light-clients/07-tendermint/light_client_module_test.go index f5645bd795c..ebfb4f923c9 100644 --- a/modules/light-clients/07-tendermint/light_client_module_test.go +++ b/modules/light-clients/07-tendermint/light_client_module_test.go @@ -150,7 +150,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { var ( clientID string path *ibctesting.Path - upgradedClientState *ibctm.ClientState + upgradedClientState exported.ClientState upgradedClientStateBz, upgradedConsensusStateBz []byte upgradedClientStateProof, upgradedConsensusStateProof []byte ) @@ -164,14 +164,14 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { "success", func() { // upgrade height is at next block - lastHeight := clienttypes.NewHeight(0, uint64(suite.chainB.GetContext().BlockHeight()+1)) + upgradeHeight := clienttypes.NewHeight(0, uint64(suite.chainB.GetContext().BlockHeight()+1)) // zero custom fields and store in upgrade store - zeroedUpgradedClient := upgradedClientState.ZeroCustomFields() - zeroedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), zeroedUpgradedClient) - err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), zeroedUpgradedClientBz) + zeroedUpgradedClient := upgradedClientState.(*ibctm.ClientState).ZeroCustomFields() + zeroedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.Codec, zeroedUpgradedClient) + err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), zeroedUpgradedClientBz) suite.Require().NoError(err) - err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsensusStateBz) + err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedConsensusStateBz) suite.Require().NoError(err) // commit upgrade store changes and update clients @@ -182,8 +182,8 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { cs, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(suite.chainA.GetContext(), clientID) suite.Require().True(found) - upgradedClientStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) - upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(lastHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) + upgradedClientStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(upgradeHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) + upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(upgradeHeight.GetRevisionHeight())), cs.GetLatestHeight().GetRevisionHeight()) }, nil, }, @@ -197,18 +197,14 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { { "upgraded client state is not for tendermint client state", func() { - var err error - upgradedClientStateBz, err = suite.chainA.Codec.Marshal(solomachine.NewClientState(0, &solomachine.ConsensusState{})) - suite.Require().NoError(err) + upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.Codec, solomachine.NewClientState(0, &solomachine.ConsensusState{})) }, clienttypes.ErrInvalidClient, }, { "upgraded consensus state is not tendermint consensus state", func() { - var err error - upgradedConsensusStateBz, err = suite.chainA.Codec.Marshal(&solomachine.ConsensusState{}) - suite.Require().NoError(err) + upgradedConsensusStateBz = clienttypes.MustMarshalConsensusState(suite.chainA.Codec, &solomachine.ConsensusState{}) }, clienttypes.ErrInvalidConsensus, }, @@ -219,8 +215,8 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { lastHeight := clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) // zero custom fields and store in upgrade store - zeroedUpgradedClient := upgradedClientState.ZeroCustomFields() - zeroedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), zeroedUpgradedClient) + zeroedUpgradedClient := upgradedClientState.(*ibctm.ClientState).ZeroCustomFields() + zeroedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.Codec, zeroedUpgradedClient) err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), zeroedUpgradedClientBz) suite.Require().NoError(err) err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsensusStateBz) @@ -228,10 +224,11 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { // change upgraded client state height to be lower than current client state height clientState := path.EndpointA.GetClientState().(*ibctm.ClientState) + tmClient := upgradedClientState.(*ibctm.ClientState) newLatestheight, ok := clientState.GetLatestHeight().Decrement() suite.Require().True(ok) - upgradedClientState.LatestHeight = newLatestheight.(clienttypes.Height) - upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), upgradedClientState) + tmClient.LatestHeight = newLatestheight.(clienttypes.Height) + upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.Codec, tmClient) suite.Require().NoError(err) suite.coordinator.CommitBlock(suite.chainB) err = path.EndpointA.UpdateClient() @@ -260,13 +257,15 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { revisionNumber := clienttypes.ParseChainID(clientState.ChainId) newUnbondindPeriod := ubdPeriod + trustingPeriod - upgradedClientState = ibctm.NewClientState(clientState.ChainId, ibctm.DefaultTrustLevel, trustingPeriod, newUnbondindPeriod, maxClockDrift, clienttypes.NewHeight(revisionNumber+1, clientState.GetLatestHeight().GetRevisionHeight()+1), commitmenttypes.GetSDKSpecs(), upgradePath) - upgradedClientStateBz, err = suite.chainA.Codec.Marshal(upgradedClientState) + newChainID, err := clienttypes.SetRevisionNumber(clientState.ChainId, revisionNumber+1) suite.Require().NoError(err) + upgradedClientState = ibctm.NewClientState(newChainID, ibctm.DefaultTrustLevel, trustingPeriod, newUnbondindPeriod, maxClockDrift, clienttypes.NewHeight(revisionNumber+1, clientState.GetLatestHeight().GetRevisionHeight()+1), commitmenttypes.GetSDKSpecs(), upgradePath) + upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.Codec, upgradedClientState) + nextValsHash := sha256.Sum256([]byte("new-nextValsHash")) upgradedConsensusState := ibctm.NewConsensusState(time.Now(), commitmenttypes.NewMerkleRoot([]byte("new-hash")), nextValsHash[:]) - upgradedConsensusStateBz, err = suite.chainA.Codec.Marshal(upgradedConsensusState) + upgradedConsensusStateBz = clienttypes.MustMarshalConsensusState(suite.chainA.Codec, upgradedConsensusState) suite.Require().NoError(err) lightClientModule, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetRouter().GetRoute(clientID) @@ -289,8 +288,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { clientState := suite.chainA.GetClientState(clientID) suite.Require().NotNil(clientState) - clientStateBz, err := suite.chainA.Codec.Marshal(upgradedClientState) - suite.Require().NoError(err) + clientStateBz := clienttypes.MustMarshalClientState(suite.chainA.Codec, upgradedClientState) suite.Require().Equal(upgradedClientStateBz, clientStateBz) consensusState, found := suite.chainA.GetConsensusState(clientID, clientState.GetLatestHeight()) diff --git a/modules/light-clients/08-wasm/light_client_module.go b/modules/light-clients/08-wasm/light_client_module.go index af17767c07d..21fcc7cb6ec 100644 --- a/modules/light-clients/08-wasm/light_client_module.go +++ b/modules/light-clients/08-wasm/light_client_module.go @@ -289,21 +289,29 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( ) error { var ( cdc = lcm.keeper.Codec() - newClientState types.ClientState - newConsensusState types.ConsensusState + newClientState exported.ClientState + newConsensusState exported.ConsensusState ) - if err := cdc.Unmarshal(newClient, &newClientState); err != nil { + if err := cdc.UnmarshalInterface(newClient, &newClientState); err != nil { return err } - if err := newClientState.Validate(); err != nil { + newWasmClientState, ok := newClientState.(*types.ClientState) + if !ok { + return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "expected client state type %T, got %T", (*types.ClientState)(nil), newClientState) + } + if err := newWasmClientState.Validate(); err != nil { return err } - if err := cdc.Unmarshal(newConsState, &newConsensusState); err != nil { + if err := cdc.UnmarshalInterface(newConsState, &newConsensusState); err != nil { return err } - if err := newConsensusState.ValidateBasic(); err != nil { + newWasmConsensusState, ok := newConsensusState.(*types.ConsensusState) + if !ok { + return errorsmod.Wrapf(clienttypes.ErrInvalidConsensus, "expected consensus state type %T, got %T", (*types.ConsensusState)(nil), newConsensusState) + } + if err := newWasmConsensusState.ValidateBasic(); err != nil { return err } @@ -319,5 +327,5 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( return errorsmod.Wrapf(ibcerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s", newClientState.GetLatestHeight(), lastHeight) } - return clientState.VerifyUpgradeAndUpdateState(ctx, cdc, clientStore, &newClientState, &newConsensusState, upgradeClientProof, upgradeConsensusStateProof) + return clientState.VerifyUpgradeAndUpdateState(ctx, cdc, clientStore, newWasmClientState, newWasmConsensusState, upgradeClientProof, upgradeConsensusStateProof) } diff --git a/modules/light-clients/08-wasm/light_client_module_test.go b/modules/light-clients/08-wasm/light_client_module_test.go index a2f52999c8a..7d1fc1105e5 100644 --- a/modules/light-clients/08-wasm/light_client_module_test.go +++ b/modules/light-clients/08-wasm/light_client_module_test.go @@ -14,6 +14,7 @@ import ( host "github.com/cosmos/ibc-go/v8/modules/core/24-host" ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" "github.com/cosmos/ibc-go/v8/modules/core/exported" + solomachine "github.com/cosmos/ibc-go/v8/modules/light-clients/06-solomachine" ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" ibctesting "github.com/cosmos/ibc-go/v8/testing" ) @@ -211,29 +212,27 @@ func (suite *WasmTestSuite) TestVerifyUpgradeAndUpdateState() { { "upgraded client state is not wasm client state", func() { - upgradedClientStateBz = []byte{} + upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.Codec, solomachine.NewClientState(0, &solomachine.ConsensusState{})) }, clienttypes.ErrInvalidClient, }, { "upgraded consensus state is not wasm consensus sate", func() { - upgradedConsensusStateBz = []byte{} + upgradedConsensusStateBz = clienttypes.MustMarshalConsensusState(suite.chainA.Codec, &solomachine.ConsensusState{}) }, clienttypes.ErrInvalidConsensus, }, { "upgraded client state height is not greater than current height", func() { - var err error latestHeight := clientState.GetLatestHeight() newLatestHeight := clienttypes.NewHeight(latestHeight.GetRevisionNumber(), latestHeight.GetRevisionHeight()-1) wrappedUpgradedClient := wasmtesting.CreateMockTendermintClientState(newLatestHeight) wrappedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), wrappedUpgradedClient) upgradedClientState = types.NewClientState(wrappedUpgradedClientBz, clientState.Checksum, newLatestHeight) - upgradedClientStateBz, err = suite.chainA.Codec.Marshal(upgradedClientState) - suite.Require().NoError(err) + upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.Codec, upgradedClientState) }, ibcerrors.ErrInvalidHeight, }, @@ -256,14 +255,12 @@ func (suite *WasmTestSuite) TestVerifyUpgradeAndUpdateState() { wrappedUpgradedClient := wasmtesting.CreateMockTendermintClientState(newLatestHeight) wrappedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), wrappedUpgradedClient) upgradedClientState = types.NewClientState(wrappedUpgradedClientBz, clientState.Checksum, newLatestHeight) - upgradedClientStateBz, err = suite.chainA.Codec.Marshal(upgradedClientState) - suite.Require().NoError(err) + upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.Codec, upgradedClientState) wrappedUpgradedConsensus := ibctm.NewConsensusState(time.Now(), commitmenttypes.NewMerkleRoot([]byte("new-hash")), []byte("new-nextValsHash")) wrappedUpgradedConsensusBz := clienttypes.MustMarshalConsensusState(suite.chainA.App.AppCodec(), wrappedUpgradedConsensus) upgradedConsensusState = types.NewConsensusState(wrappedUpgradedConsensusBz) - upgradedConsensusStateBz, err = suite.chainA.Codec.Marshal(upgradedConsensusState) - suite.Require().NoError(err) + upgradedConsensusStateBz = clienttypes.MustMarshalConsensusState(suite.chainA.Codec, upgradedConsensusState) lightClientModule, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetRouter().GetRoute(clientID) suite.Require().True(found) From 14081525ba33e45df1c2b85a16ca26146d2a9ddc Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Mon, 11 Mar 2024 10:18:48 +0100 Subject: [PATCH 14/19] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> --- modules/core/keeper/msg_server.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index c39e74c7acd..22e89344ff7 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -69,11 +69,14 @@ func (k Keeper) UpdateClient(goCtx context.Context, msg *clienttypes.MsgUpdateCl func (k Keeper) UpgradeClient(goCtx context.Context, msg *clienttypes.MsgUpgradeClient) (*clienttypes.MsgUpgradeClientResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) + // Backwards Compatibility: the light client module is expecting + // to unmarshal an interface so we marshal the client state Any upgradedClientState, err := k.cdc.Marshal(msg.ClientState) if err != nil { return nil, err } - + // Backwards Compatibility: the light client module is expecting + // to unmarshal an interface so we marshal the consensus state Any upgradedConsensusState, err := k.cdc.Marshal(msg.ConsensusState) if err != nil { return nil, err From cfa5eec2678bf9d45292158b12f60b3bb26ab24c Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Mon, 11 Mar 2024 17:51:50 +0100 Subject: [PATCH 15/19] chore: make lint-fix --- modules/core/keeper/msg_server.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index 6fa2593120d..28db336eb97 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -70,14 +70,14 @@ func (k Keeper) UpdateClient(goCtx context.Context, msg *clienttypes.MsgUpdateCl func (k Keeper) UpgradeClient(goCtx context.Context, msg *clienttypes.MsgUpgradeClient) (*clienttypes.MsgUpgradeClientResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) - // Backwards Compatibility: the light client module is expecting - // to unmarshal an interface so we marshal the client state Any + // Backwards Compatibility: the light client module is expecting + // to unmarshal an interface so we marshal the client state Any upgradedClientState, err := k.cdc.Marshal(msg.ClientState) if err != nil { return nil, err } - // Backwards Compatibility: the light client module is expecting - // to unmarshal an interface so we marshal the consensus state Any + // Backwards Compatibility: the light client module is expecting + // to unmarshal an interface so we marshal the consensus state Any upgradedConsensusState, err := k.cdc.Marshal(msg.ConsensusState) if err != nil { return nil, err From a65ab8bf32d80a541c3697759c5d4b65d330f253 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Mon, 11 Mar 2024 18:01:07 +0100 Subject: [PATCH 16/19] chore: rm redundant test, tested in lightclient module --- modules/core/02-client/keeper/client_test.go | 30 -------------------- 1 file changed, 30 deletions(-) diff --git a/modules/core/02-client/keeper/client_test.go b/modules/core/02-client/keeper/client_test.go index 58ba76c0595..f080a06c2ae 100644 --- a/modules/core/02-client/keeper/client_test.go +++ b/modules/core/02-client/keeper/client_test.go @@ -453,36 +453,6 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { }, expPass: false, }, - // TODO: Needs to move to light client module? - // { - // name: "unsuccessful upgrade: upgraded height is not greater than current height", - // setup: func() { - // // last Height is at next block - // upgradeHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) - - // // zero custom fields and store in upgrade store - // err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedClientBz) - // suite.Require().NoError(err) - // err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedConsStateBz) - // suite.Require().NoError(err) - - // // change upgradedClient height to be lower than current client state height - // upgradedClient.LatestHeight = clienttypes.NewHeight(0, 1) - - // suite.coordinator.CommitBlock(suite.chainB) - // err = path.EndpointA.UpdateClient() - // suite.Require().NoError(err) - - // cs, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetClientState(suite.chainA.GetContext(), path.EndpointA.ClientID) - // suite.Require().True(found) - // tmCs, ok := cs.(*ibctm.ClientState) - // suite.Require().True(ok) - - // upgradedClientProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(upgradeHeight.GetRevisionHeight())), tmCs.LatestHeight.GetRevisionHeight()) - // upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(upgradeHeight.GetRevisionHeight())), tmCs.LatestHeight.GetRevisionHeight()) - // }, - // expPass: false, - // }, } for _, tc := range testCases { From 290a23768399e6a80a6b9313a2e59bd89e436940 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 12 Mar 2024 13:35:48 +0100 Subject: [PATCH 17/19] fix: remove marshalling of anys to bytes for verify upgrade (#5967) * fix: remove marshalling of anys to bytes for verify upgrade * test: refactor 02-client tests for verify upgrade * test: fix tests in 07-tenderint client module * chore: rm backwards compatability notice and marshalling of anys in core msg server * test: cleanup happy path assertions in 07-tendermint verify upgrade test * nit: update inline code commentin test * doc: update godoc of UpgradeClient handler in core ibc * lint: make lint-fix --- modules/core/02-client/keeper/client_test.go | 31 ++++---- modules/core/keeper/msg_server.go | 22 ++---- .../07-tendermint/light_client_module.go | 30 +++----- .../07-tendermint/light_client_module_test.go | 73 +++++++++++-------- .../light-clients/07-tendermint/upgrade.go | 1 + 5 files changed, 79 insertions(+), 78 deletions(-) diff --git a/modules/core/02-client/keeper/client_test.go b/modules/core/02-client/keeper/client_test.go index f080a06c2ae..fed49bbb28d 100644 --- a/modules/core/02-client/keeper/client_test.go +++ b/modules/core/02-client/keeper/client_test.go @@ -6,6 +6,7 @@ import ( upgradetypes "cosmossdk.io/x/upgrade/types" + codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" @@ -322,8 +323,8 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { upgradedClient *ibctm.ClientState upgradedConsState exported.ConsensusState upgradeHeight exported.Height + upgradedClientAny, upgradedConsStateAny *codectypes.Any upgradedClientProof, upgradedConsensusStateProof []byte - upgradedClientBz, upgradedConsStateBz []byte ) testCases := []struct { @@ -338,9 +339,9 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { upgradeHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) // zero custom fields and store in upgrade store - err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedClientBz) + err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), suite.chainB.Codec.MustMarshal(upgradedClientAny)) suite.Require().NoError(err) - err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedConsStateBz) + err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), suite.chainB.Codec.MustMarshal(upgradedConsStateAny)) suite.Require().NoError(err) // commit upgrade store changes and update clients @@ -365,9 +366,9 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { upgradeHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) // zero custom fields and store in upgrade store - err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedClientBz) + err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), suite.chainB.Codec.MustMarshal(upgradedClientAny)) suite.Require().NoError(err) - err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedConsStateBz) + err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), suite.chainB.Codec.MustMarshal(upgradedConsStateAny)) suite.Require().NoError(err) // commit upgrade store changes and update clients @@ -397,9 +398,9 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { upgradeHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) // zero custom fields and store in upgrade store - err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedClientBz) + err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), suite.chainB.Codec.MustMarshal(upgradedClientAny)) suite.Require().NoError(err) - err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedConsStateBz) + err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), suite.chainB.Codec.MustMarshal(upgradedConsStateAny)) suite.Require().NoError(err) // commit upgrade store changes and update clients @@ -430,14 +431,15 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { upgradeHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) // zero custom fields and store in upgrade store - err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedClientBz) + err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), suite.chainB.Codec.MustMarshal(upgradedClientAny)) suite.Require().NoError(err) - err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedConsStateBz) + err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), suite.chainB.Codec.MustMarshal(upgradedConsStateAny)) suite.Require().NoError(err) // change upgradedClient client-specified parameters upgradedClient.ChainId = "wrongchainID" - upgradedClientBz = clienttypes.MustMarshalClientState(suite.chainA.Codec, upgradedClient) + upgradedClientAny, err = codectypes.NewAnyWithValue(upgradedClient) + suite.Require().NoError(err) suite.coordinator.CommitBlock(suite.chainB) err = path.EndpointA.UpdateClient() @@ -469,15 +471,18 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { upgradedClient = ibctm.NewClientState(newChainID, ibctm.DefaultTrustLevel, trustingPeriod, ubdPeriod+trustingPeriod, maxClockDrift, clienttypes.NewHeight(revisionNumber+1, clientState.LatestHeight.GetRevisionHeight()+1), commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath) upgradedClient = upgradedClient.ZeroCustomFields() - upgradedClientBz, err = clienttypes.MarshalClientState(suite.chainA.App.AppCodec(), upgradedClient) + + upgradedClientAny, err = codectypes.NewAnyWithValue(upgradedClient) suite.Require().NoError(err) upgradedConsState = &ibctm.ConsensusState{NextValidatorsHash: []byte("nextValsHash")} - upgradedConsStateBz = clienttypes.MustMarshalConsensusState(suite.chainA.Codec, upgradedConsState) + + upgradedConsStateAny, err = codectypes.NewAnyWithValue(upgradedConsState) + suite.Require().NoError(err) tc.setup() - err = suite.chainA.App.GetIBCKeeper().ClientKeeper.UpgradeClient(suite.chainA.GetContext(), path.EndpointA.ClientID, upgradedClientBz, upgradedConsStateBz, upgradedClientProof, upgradedConsensusStateProof) + err = suite.chainA.App.GetIBCKeeper().ClientKeeper.UpgradeClient(suite.chainA.GetContext(), path.EndpointA.ClientID, upgradedClientAny.Value, upgradedConsStateAny.Value, upgradedClientProof, upgradedConsensusStateProof) if tc.expPass { suite.Require().NoError(err, "verify upgrade failed on valid case: %s", tc.name) diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index 28db336eb97..75119959f54 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -67,26 +67,18 @@ func (k Keeper) UpdateClient(goCtx context.Context, msg *clienttypes.MsgUpdateCl } // UpgradeClient defines a rpc handler method for MsgUpgradeClient. +// NOTE: The raw bytes of the conretes types encoded into protobuf.Any is passed to the client keeper. +// The 02-client handler will route to the appropriate light client module based on client identifier and it is the responsibility +// of the light client module to unmarshal and interpret the proto encoded bytes. +// Backwards compatibility with older versions of ibc-go is maintained through the light client module reconstructing and encoding +// the expected concrete type to the protobuf.Any for proof verification. func (k Keeper) UpgradeClient(goCtx context.Context, msg *clienttypes.MsgUpgradeClient) (*clienttypes.MsgUpgradeClientResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) - // Backwards Compatibility: the light client module is expecting - // to unmarshal an interface so we marshal the client state Any - upgradedClientState, err := k.cdc.Marshal(msg.ClientState) - if err != nil { - return nil, err - } - // Backwards Compatibility: the light client module is expecting - // to unmarshal an interface so we marshal the consensus state Any - upgradedConsensusState, err := k.cdc.Marshal(msg.ConsensusState) - if err != nil { - return nil, err - } - if err := k.ClientKeeper.UpgradeClient( ctx, msg.ClientId, - upgradedClientState, - upgradedConsensusState, + msg.ClientState.Value, + msg.ConsensusState.Value, msg.ProofUpgradeClient, msg.ProofUpgradeConsensusState, ); err != nil { diff --git a/modules/light-clients/07-tendermint/light_client_module.go b/modules/light-clients/07-tendermint/light_client_module.go index 9ac67b40c82..669876cbafd 100644 --- a/modules/light-clients/07-tendermint/light_client_module.go +++ b/modules/light-clients/07-tendermint/light_client_module.go @@ -277,26 +277,16 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( upgradeClientProof, upgradeConsensusStateProof []byte, ) error { - var ( - cdc = lcm.keeper.Codec() - newClientState exported.ClientState - newConsensusState exported.ConsensusState - ) + cdc := lcm.keeper.Codec() - if err := cdc.UnmarshalInterface(newClient, &newClientState); err != nil { - return err - } - newTmClientState, ok := newClientState.(*ClientState) - if !ok { - return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "expected client state type %T, got %T", (*ClientState)(nil), newClientState) + var newClientState ClientState + if err := cdc.Unmarshal(newClient, &newClientState); err != nil { + return errorsmod.Wrap(clienttypes.ErrInvalidClient, err.Error()) } - if err := cdc.UnmarshalInterface(newConsState, &newConsensusState); err != nil { - return err - } - newTmConsensusState, ok := newConsensusState.(*ConsensusState) - if !ok { - return errorsmod.Wrapf(clienttypes.ErrInvalidConsensus, "expected consensus state type %T, got %T", (*ConsensusState)(nil), newConsensusState) + var newConsensusState ConsensusState + if err := cdc.Unmarshal(newConsState, &newConsensusState); err != nil { + return errorsmod.Wrap(clienttypes.ErrInvalidConsensus, err.Error()) } clientStore := lcm.storeProvider.ClientStore(ctx, clientID) @@ -307,9 +297,9 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( // last height of current counterparty chain must be client's latest height lastHeight := clientState.LatestHeight - if !newTmClientState.LatestHeight.GT(lastHeight) { - return errorsmod.Wrapf(ibcerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s", newTmClientState.LatestHeight, lastHeight) + if !newClientState.LatestHeight.GT(lastHeight) { + return errorsmod.Wrapf(ibcerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s", newClientState.LatestHeight, lastHeight) } - return clientState.VerifyUpgradeAndUpdateState(ctx, cdc, clientStore, newTmClientState, newTmConsensusState, upgradeClientProof, upgradeConsensusStateProof) + return clientState.VerifyUpgradeAndUpdateState(ctx, cdc, clientStore, &newClientState, &newConsensusState, upgradeClientProof, upgradeConsensusStateProof) } diff --git a/modules/light-clients/07-tendermint/light_client_module_test.go b/modules/light-clients/07-tendermint/light_client_module_test.go index 52aea5ec445..4dd646eae3e 100644 --- a/modules/light-clients/07-tendermint/light_client_module_test.go +++ b/modules/light-clients/07-tendermint/light_client_module_test.go @@ -6,12 +6,13 @@ import ( upgradetypes "cosmossdk.io/x/upgrade/types" + codectypes "github.com/cosmos/cosmos-sdk/codec/types" + clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types" host "github.com/cosmos/ibc-go/v8/modules/core/24-host" ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" "github.com/cosmos/ibc-go/v8/modules/core/exported" - solomachine "github.com/cosmos/ibc-go/v8/modules/light-clients/06-solomachine" ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" ibctesting "github.com/cosmos/ibc-go/v8/testing" ) @@ -116,7 +117,7 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { clientID string path *ibctesting.Path upgradedClientState exported.ClientState - upgradedClientStateBz, upgradedConsensusStateBz []byte + upgradedClientStateAny, upgradedConsensusStateAny *codectypes.Any upgradedClientStateProof, upgradedConsensusStateProof []byte ) @@ -133,10 +134,13 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { // zero custom fields and store in upgrade store zeroedUpgradedClient := upgradedClientState.(*ibctm.ClientState).ZeroCustomFields() - zeroedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.Codec, zeroedUpgradedClient) - err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), zeroedUpgradedClientBz) + zeroedUpgradedClientAny, err := codectypes.NewAnyWithValue(zeroedUpgradedClient) suite.Require().NoError(err) - err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), upgradedConsensusStateBz) + + err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), suite.chainB.Codec.MustMarshal(zeroedUpgradedClientAny)) + suite.Require().NoError(err) + + err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), suite.chainB.Codec.MustMarshal(upgradedConsensusStateAny)) suite.Require().NoError(err) // commit upgrade store changes and update clients @@ -159,29 +163,36 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { { "upgraded client state is not for tendermint client state", func() { - upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.Codec, solomachine.NewClientState(0, &solomachine.ConsensusState{})) + upgradedClientStateAny = &codectypes.Any{ + Value: []byte("invalid client state bytes"), + } }, clienttypes.ErrInvalidClient, }, { "upgraded consensus state is not tendermint consensus state", func() { - upgradedConsensusStateBz = clienttypes.MustMarshalConsensusState(suite.chainA.Codec, &solomachine.ConsensusState{}) + upgradedConsensusStateAny = &codectypes.Any{ + Value: []byte("invalid consensus state bytes"), + } }, clienttypes.ErrInvalidConsensus, }, { "upgraded client state height is not greater than current height", func() { - // last Height is at next block - lastHeight := clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) + // upgrade height is at next block + upgradeHeight := clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) // zero custom fields and store in upgrade store zeroedUpgradedClient := upgradedClientState.(*ibctm.ClientState).ZeroCustomFields() - zeroedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.Codec, zeroedUpgradedClient) - err := suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), zeroedUpgradedClientBz) + zeroedUpgradedClientAny, err := codectypes.NewAnyWithValue(zeroedUpgradedClient) suite.Require().NoError(err) - err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(lastHeight.GetRevisionHeight()), upgradedConsensusStateBz) + + err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedClient(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), suite.chainB.Codec.MustMarshal(zeroedUpgradedClientAny)) + suite.Require().NoError(err) + + err = suite.chainB.GetSimApp().UpgradeKeeper.SetUpgradedConsensusState(suite.chainB.GetContext(), int64(upgradeHeight.GetRevisionHeight()), suite.chainB.Codec.MustMarshal(upgradedConsensusStateAny)) suite.Require().NoError(err) // change upgraded client state height to be lower than current client state height @@ -191,15 +202,15 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { suite.Require().True(ok) tmClient.LatestHeight = newLatestheight.(clienttypes.Height) - upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.Codec, tmClient) + upgradedClientStateAny, err = codectypes.NewAnyWithValue(tmClient) suite.Require().NoError(err) suite.coordinator.CommitBlock(suite.chainB) err = path.EndpointA.UpdateClient() suite.Require().NoError(err) - upgradedClientStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(lastHeight.GetRevisionHeight())), path.EndpointA.GetClientLatestHeight().GetRevisionHeight()) - upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(lastHeight.GetRevisionHeight())), path.EndpointA.GetClientLatestHeight().GetRevisionHeight()) + upgradedClientStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedClientKey(int64(upgradeHeight.GetRevisionHeight())), path.EndpointA.GetClientLatestHeight().GetRevisionHeight()) + upgradedConsensusStateProof, _ = suite.chainB.QueryUpgradeProof(upgradetypes.UpgradedConsStateKey(int64(upgradeHeight.GetRevisionHeight())), path.EndpointA.GetClientLatestHeight().GetRevisionHeight()) }, ibcerrors.ErrInvalidHeight, }, @@ -209,10 +220,10 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { tc := tc suite.Run(tc.name, func() { suite.SetupTest() // reset - var err error path = ibctesting.NewPath(suite.chainA, suite.chainB) path.SetupClients() + clientID = path.EndpointA.ClientID clientState := path.EndpointA.GetClientState().(*ibctm.ClientState) revisionNumber := clienttypes.ParseChainID(clientState.ChainId) @@ -222,23 +233,25 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { suite.Require().NoError(err) upgradedClientState = ibctm.NewClientState(newChainID, ibctm.DefaultTrustLevel, trustingPeriod, newUnbondindPeriod, maxClockDrift, clienttypes.NewHeight(revisionNumber+1, clientState.LatestHeight.GetRevisionHeight()+1), commitmenttypes.GetSDKSpecs(), upgradePath) - upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.Codec, upgradedClientState) + upgradedClientStateAny, err = codectypes.NewAnyWithValue(upgradedClientState) + suite.Require().NoError(err) nextValsHash := sha256.Sum256([]byte("new-nextValsHash")) upgradedConsensusState := ibctm.NewConsensusState(time.Now(), commitmenttypes.NewMerkleRoot([]byte("new-hash")), nextValsHash[:]) - upgradedConsensusStateBz = clienttypes.MustMarshalConsensusState(suite.chainA.Codec, upgradedConsensusState) + + upgradedConsensusStateAny, err = codectypes.NewAnyWithValue(upgradedConsensusState) suite.Require().NoError(err) + tc.malleate() + lightClientModule, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetRouter().GetRoute(clientID) suite.Require().True(found) - tc.malleate() - err = lightClientModule.VerifyUpgradeAndUpdateState( suite.chainA.GetContext(), clientID, - upgradedClientStateBz, - upgradedConsensusStateBz, + upgradedClientStateAny.Value, + upgradedConsensusStateAny.Value, upgradedClientStateProof, upgradedConsensusStateProof, ) @@ -247,15 +260,15 @@ func (suite *TendermintTestSuite) TestVerifyUpgradeAndUpdateState() { if expPass { suite.Require().NoError(err) - clientStateBz := clienttypes.MustMarshalClientState(suite.chainA.Codec, upgradedClientState) - suite.Require().Equal(upgradedClientStateBz, clientStateBz) + expClientState := path.EndpointA.GetClientState() + expClientStateBz := suite.chainA.Codec.MustMarshal(expClientState) + suite.Require().Equal(upgradedClientStateAny.Value, expClientStateBz) - consensusState, found := suite.chainA.GetConsensusState(clientID, path.EndpointA.GetClientLatestHeight()) - suite.Require().True(found) - suite.Require().NotNil(consensusState) - tmConsensusState, ok := consensusState.(*ibctm.ConsensusState) - suite.Require().True(ok) - suite.Require().Equal(upgradedConsensusState.NextValidatorsHash, tmConsensusState.NextValidatorsHash) + expConsensusState := ibctm.NewConsensusState(upgradedConsensusState.Timestamp, commitmenttypes.NewMerkleRoot([]byte(ibctm.SentinelRoot)), upgradedConsensusState.NextValidatorsHash) + expConsensusStateBz := suite.chainA.Codec.MustMarshal(expConsensusState) + + consensusStateBz := suite.chainA.Codec.MustMarshal(path.EndpointA.GetConsensusState(path.EndpointA.GetClientLatestHeight())) + suite.Require().Equal(expConsensusStateBz, consensusStateBz) } else { suite.Require().Error(err) suite.Require().ErrorIs(err, tc.expErr) diff --git a/modules/light-clients/07-tendermint/upgrade.go b/modules/light-clients/07-tendermint/upgrade.go index be19cffda9a..ef62e707795 100644 --- a/modules/light-clients/07-tendermint/upgrade.go +++ b/modules/light-clients/07-tendermint/upgrade.go @@ -43,6 +43,7 @@ func (cs ClientState) VerifyUpgradeAndUpdateState( return errorsmod.Wrapf(clienttypes.ErrInvalidClientType, "upgraded client must be Tendermint client. expected: %T got: %T", &ClientState{}, upgradedClient) } + tmUpgradeConsState, ok := upgradedConsState.(*ConsensusState) if !ok { return errorsmod.Wrapf(clienttypes.ErrInvalidConsensus, "upgraded consensus state must be Tendermint consensus state. expected %T, got: %T", From f7b21925411a558d0724658fd2fbcd6a85f64022 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 12 Mar 2024 13:44:21 +0100 Subject: [PATCH 18/19] nit: inline comments in tests last Height -> upgrade height --- modules/core/02-client/keeper/client_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/core/02-client/keeper/client_test.go b/modules/core/02-client/keeper/client_test.go index fed49bbb28d..446fe195f03 100644 --- a/modules/core/02-client/keeper/client_test.go +++ b/modules/core/02-client/keeper/client_test.go @@ -362,7 +362,7 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { { name: "client state not found", setup: func() { - // last Height is at next block + // upgrade height is at next block upgradeHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) // zero custom fields and store in upgrade store @@ -394,7 +394,7 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { setup: func() { // client is frozen - // last Height is at next block + // upgrade height is at next block upgradeHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) // zero custom fields and store in upgrade store @@ -427,7 +427,7 @@ func (suite *KeeperTestSuite) TestUpgradeClient() { { name: "light client module VerifyUpgradeAndUpdateState fails", setup: func() { - // last Height is at next block + // upgrade height is at next block upgradeHeight = clienttypes.NewHeight(1, uint64(suite.chainB.GetContext().BlockHeight()+1)) // zero custom fields and store in upgrade store From 3b3256a8c8c794d723c5a5d3c7dc2a5e4a350088 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 12 Mar 2024 14:59:30 +0100 Subject: [PATCH 19/19] fix: address 08-wasm upgrade client and tests --- .../08-wasm/light_client_module.go | 36 ++++--------- .../08-wasm/light_client_module_test.go | 50 +++++++++++++------ 2 files changed, 46 insertions(+), 40 deletions(-) diff --git a/modules/light-clients/08-wasm/light_client_module.go b/modules/light-clients/08-wasm/light_client_module.go index 85ba77c9758..589215ae472 100644 --- a/modules/light-clients/08-wasm/light_client_module.go +++ b/modules/light-clients/08-wasm/light_client_module.go @@ -266,32 +266,16 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( upgradeClientProof, upgradeConsensusStateProof []byte, ) error { - var ( - cdc = lcm.keeper.Codec() - newClientState exported.ClientState - newConsensusState exported.ConsensusState - ) + cdc := lcm.keeper.Codec() - if err := cdc.UnmarshalInterface(newClient, &newClientState); err != nil { - return err - } - newWasmClientState, ok := newClientState.(*types.ClientState) - if !ok { - return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "expected client state type %T, got %T", (*types.ClientState)(nil), newClientState) - } - if err := newWasmClientState.Validate(); err != nil { - return err + var newClientState types.ClientState + if err := cdc.Unmarshal(newClient, &newClientState); err != nil { + return errorsmod.Wrap(clienttypes.ErrInvalidClient, err.Error()) } - if err := cdc.UnmarshalInterface(newConsState, &newConsensusState); err != nil { - return err - } - newWasmConsensusState, ok := newConsensusState.(*types.ConsensusState) - if !ok { - return errorsmod.Wrapf(clienttypes.ErrInvalidConsensus, "expected consensus state type %T, got %T", (*types.ConsensusState)(nil), newConsensusState) - } - if err := newWasmConsensusState.ValidateBasic(); err != nil { - return err + var newConsensusState types.ConsensusState + if err := cdc.Unmarshal(newConsState, &newConsensusState); err != nil { + return errorsmod.Wrap(clienttypes.ErrInvalidConsensus, err.Error()) } clientStore := lcm.storeProvider.ClientStore(ctx, clientID) @@ -302,9 +286,9 @@ func (lcm LightClientModule) VerifyUpgradeAndUpdateState( // last height of current counterparty chain must be client's latest height lastHeight := clientState.LatestHeight - if !newWasmClientState.LatestHeight.GT(lastHeight) { - return errorsmod.Wrapf(ibcerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s", newWasmClientState.LatestHeight, lastHeight) + if !newClientState.LatestHeight.GT(lastHeight) { + return errorsmod.Wrapf(ibcerrors.ErrInvalidHeight, "upgraded client height %s must be at greater than current client height %s", newClientState.LatestHeight, lastHeight) } - return clientState.VerifyUpgradeAndUpdateState(ctx, cdc, clientStore, newWasmClientState, newWasmConsensusState, upgradeClientProof, upgradeConsensusStateProof) + return clientState.VerifyUpgradeAndUpdateState(ctx, cdc, clientStore, &newClientState, &newConsensusState, upgradeClientProof, upgradeConsensusStateProof) } diff --git a/modules/light-clients/08-wasm/light_client_module_test.go b/modules/light-clients/08-wasm/light_client_module_test.go index 41432193769..e1c969fc5aa 100644 --- a/modules/light-clients/08-wasm/light_client_module_test.go +++ b/modules/light-clients/08-wasm/light_client_module_test.go @@ -7,6 +7,8 @@ import ( wasmvm "github.com/CosmWasm/wasmvm" wasmvmtypes "github.com/CosmWasm/wasmvm/types" + codectypes "github.com/cosmos/cosmos-sdk/codec/types" + wasmtesting "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/testing" "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" @@ -14,7 +16,6 @@ import ( host "github.com/cosmos/ibc-go/v8/modules/core/24-host" ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" "github.com/cosmos/ibc-go/v8/modules/core/exported" - solomachine "github.com/cosmos/ibc-go/v8/modules/light-clients/06-solomachine" ibctm "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" ibctesting "github.com/cosmos/ibc-go/v8/testing" ) @@ -116,7 +117,7 @@ func (suite *WasmTestSuite) TestVerifyUpgradeAndUpdateState() { clientState *types.ClientState upgradedClientState exported.ClientState upgradedConsensusState exported.ConsensusState - upgradedClientStateBz, upgradedConsensusStateBz []byte + upgradedClientStateAny, upgradedConsensusStateAny *codectypes.Any upgradedClientStateProof, upgradedConsensusStateProof []byte ) @@ -155,9 +156,15 @@ func (suite *WasmTestSuite) TestVerifyUpgradeAndUpdateState() { suite.Require().NoError(err) // set new client state and consensus state - // wrappedUpgradedClient := clienttypes.MustUnmarshalClientState(suite.chainA.App.AppCodec(), expectedUpgradedClient.Data) - store.Set(host.ClientStateKey(), upgradedClientStateBz) - store.Set(host.ConsensusStateKey(expectedUpgradedClient.LatestHeight), upgradedConsensusStateBz) + bz, err := suite.chainA.Codec.MarshalInterface(upgradedClientState) + suite.Require().NoError(err) + + store.Set(host.ClientStateKey(), bz) + + bz, err = suite.chainA.Codec.MarshalInterface(upgradedConsensusState) + suite.Require().NoError(err) + + store.Set(host.ConsensusStateKey(expectedUpgradedClient.LatestHeight), bz) return &wasmvmtypes.Response{Data: data}, wasmtesting.DefaultGasUsed, nil }) @@ -174,27 +181,33 @@ func (suite *WasmTestSuite) TestVerifyUpgradeAndUpdateState() { { "upgraded client state is not wasm client state", func() { - upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.Codec, solomachine.NewClientState(0, &solomachine.ConsensusState{})) + upgradedClientStateAny = &codectypes.Any{ + Value: []byte("invalid client state bytes"), + } }, clienttypes.ErrInvalidClient, }, { "upgraded consensus state is not wasm consensus sate", func() { - upgradedConsensusStateBz = clienttypes.MustMarshalConsensusState(suite.chainA.Codec, &solomachine.ConsensusState{}) + upgradedConsensusStateAny = &codectypes.Any{ + Value: []byte("invalid consensus state bytes"), + } }, clienttypes.ErrInvalidConsensus, }, { "upgraded client state height is not greater than current height", func() { + var err error latestHeight := clientState.LatestHeight newLatestHeight := clienttypes.NewHeight(latestHeight.GetRevisionNumber(), latestHeight.GetRevisionHeight()-1) wrappedUpgradedClient := wasmtesting.CreateMockTendermintClientState(newLatestHeight) wrappedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), wrappedUpgradedClient) upgradedClientState = types.NewClientState(wrappedUpgradedClientBz, clientState.Checksum, newLatestHeight) - upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.Codec, upgradedClientState) + upgradedClientStateAny, err = codectypes.NewAnyWithValue(upgradedClientState) + suite.Require().NoError(err) }, ibcerrors.ErrInvalidHeight, }, @@ -217,12 +230,14 @@ func (suite *WasmTestSuite) TestVerifyUpgradeAndUpdateState() { wrappedUpgradedClient := wasmtesting.CreateMockTendermintClientState(newLatestHeight) wrappedUpgradedClientBz := clienttypes.MustMarshalClientState(suite.chainA.App.AppCodec(), wrappedUpgradedClient) upgradedClientState = types.NewClientState(wrappedUpgradedClientBz, clientState.Checksum, newLatestHeight) - upgradedClientStateBz = clienttypes.MustMarshalClientState(suite.chainA.Codec, upgradedClientState) + upgradedClientStateAny, err = codectypes.NewAnyWithValue(upgradedClientState) + suite.Require().NoError(err) wrappedUpgradedConsensus := ibctm.NewConsensusState(time.Now(), commitmenttypes.NewMerkleRoot([]byte("new-hash")), []byte("new-nextValsHash")) wrappedUpgradedConsensusBz := clienttypes.MustMarshalConsensusState(suite.chainA.App.AppCodec(), wrappedUpgradedConsensus) upgradedConsensusState = types.NewConsensusState(wrappedUpgradedConsensusBz) - upgradedConsensusStateBz = clienttypes.MustMarshalConsensusState(suite.chainA.Codec, upgradedConsensusState) + upgradedConsensusStateAny, err = codectypes.NewAnyWithValue(upgradedConsensusState) + suite.Require().NoError(err) lightClientModule, found := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetRouter().GetRoute(clientID) suite.Require().True(found) @@ -237,11 +252,12 @@ func (suite *WasmTestSuite) TestVerifyUpgradeAndUpdateState() { err = lightClientModule.VerifyUpgradeAndUpdateState( suite.chainA.GetContext(), clientID, - upgradedClientStateBz, - upgradedConsensusStateBz, + upgradedClientStateAny.Value, + upgradedConsensusStateAny.Value, upgradedClientStateProof, upgradedConsensusStateProof, ) + expPass := tc.expErr == nil if expPass { suite.Require().NoError(err) @@ -249,11 +265,17 @@ func (suite *WasmTestSuite) TestVerifyUpgradeAndUpdateState() { // verify new client state and consensus state clientStateBz := clientStore.Get(host.ClientStateKey()) suite.Require().NotEmpty(clientStateBz) - suite.Require().Equal(upgradedClientStateBz, clientStateBz) + + expClientStateBz, err := suite.chainA.Codec.MarshalInterface(upgradedClientState) + suite.Require().NoError(err) + suite.Require().Equal(expClientStateBz, clientStateBz) consensusStateBz := clientStore.Get(host.ConsensusStateKey(endpoint.GetClientLatestHeight())) suite.Require().NotEmpty(consensusStateBz) - suite.Require().NotEmpty(upgradedConsensusStateBz, consensusStateBz) + + expConsensusStateBz, err := suite.chainA.Codec.MarshalInterface(upgradedConsensusState) + suite.Require().NoError(err) + suite.Require().Equal(expConsensusStateBz, consensusStateBz) } else { suite.Require().Error(err) suite.Require().ErrorIs(err, tc.expErr)