imp: CheckSubstituteAndUpdateState for 08-wasm client (#4974)

cosmos · Oct 30, 2023 · 56fb2af · 56fb2af
1 parent ddb514d
commit 56fb2af
Show file tree

Hide file tree

Showing 7 changed files with 24 additions and 228 deletions.
diff --git a/modules/light-clients/08-wasm/testing/mock_engine.go b/modules/light-clients/08-wasm/testing/mock_engine.go
@@ -21,7 +21,7 @@ var (
 	queryTypes = [...]any{types.StatusMsg{}, types.ExportMetadataMsg{}, types.TimestampAtHeightMsg{}, types.VerifyClientMessageMsg{}, types.CheckForMisbehaviourMsg{}}
 
 	// sudoTypes contains all the possible sudo message types.
-	sudoTypes = [...]any{types.UpdateStateMsg{}, types.UpdateStateOnMisbehaviourMsg{}, types.VerifyUpgradeAndUpdateStateMsg{}, types.CheckSubstituteAndUpdateStateMsg{}, types.VerifyMembershipMsg{}, types.VerifyNonMembershipMsg{}}
+	sudoTypes = [...]any{types.UpdateStateMsg{}, types.UpdateStateOnMisbehaviourMsg{}, types.VerifyUpgradeAndUpdateStateMsg{}, types.VerifyMembershipMsg{}, types.VerifyNonMembershipMsg{}}
 )
 
 type (
@@ -200,10 +200,6 @@ func getSudoMsgPayloadTypeName(sudoMsgBz []byte) string {
 		payloadField = *payload.VerifyUpgradeAndUpdateState
 	}
 
-	if payload.CheckSubstituteAndUpdateState != nil {
-		payloadField = *payload.CheckSubstituteAndUpdateState
-	}
-
 	if payload.VerifyMembership != nil {
 		payloadField = *payload.VerifyMembership
 	}

diff --git a/modules/light-clients/08-wasm/types/client_state_test.go b/modules/light-clients/08-wasm/types/client_state_test.go
@@ -729,7 +729,6 @@ func (suite *TypesTestSuite) TestVerifyMembership() {
 					suite.Require().NoError(err)
 
 					suite.Require().NotNil(payload.VerifyMembership)
-					suite.Require().Nil(payload.CheckSubstituteAndUpdateState)
 					suite.Require().Nil(payload.UpdateState)
 					suite.Require().Nil(payload.UpdateStateOnMisbehaviour)
 					suite.Require().Nil(payload.VerifyNonMembership)
@@ -758,7 +757,6 @@ func (suite *TypesTestSuite) TestVerifyMembership() {
 					suite.Require().NoError(err)
 
 					suite.Require().NotNil(payload.VerifyMembership)
-					suite.Require().Nil(payload.CheckSubstituteAndUpdateState)
 					suite.Require().Nil(payload.UpdateState)
 					suite.Require().Nil(payload.UpdateStateOnMisbehaviour)
 					suite.Require().Nil(payload.VerifyNonMembership)
@@ -1071,7 +1069,6 @@ func (suite *TypesTestSuite) TestVerifyNonMembership() {
 					suite.Require().NoError(err)
 
 					suite.Require().NotNil(payload.VerifyNonMembership)
-					suite.Require().Nil(payload.CheckSubstituteAndUpdateState)
 					suite.Require().Nil(payload.UpdateState)
 					suite.Require().Nil(payload.UpdateStateOnMisbehaviour)
 					suite.Require().Nil(payload.VerifyMembership)
@@ -1099,7 +1096,6 @@ func (suite *TypesTestSuite) TestVerifyNonMembership() {
 					suite.Require().NoError(err)
 
 					suite.Require().NotNil(payload.VerifyNonMembership)
-					suite.Require().Nil(payload.CheckSubstituteAndUpdateState)
 					suite.Require().Nil(payload.UpdateState)
 					suite.Require().Nil(payload.UpdateStateOnMisbehaviour)
 					suite.Require().Nil(payload.VerifyMembership)

diff --git a/modules/light-clients/08-wasm/types/contract_api.go b/modules/light-clients/08-wasm/types/contract_api.go
@@ -49,12 +49,12 @@ type CheckForMisbehaviourMsg struct {
 // this is required in order to be compatible with Rust's enum matching as used in the contract.
 // Only one field should be set at a time.
 type SudoMsg struct {
-	UpdateState                   *UpdateStateMsg                   `json:"update_state,omitempty"`
-	UpdateStateOnMisbehaviour     *UpdateStateOnMisbehaviourMsg     `json:"update_state_on_misbehaviour,omitempty"`
-	VerifyUpgradeAndUpdateState   *VerifyUpgradeAndUpdateStateMsg   `json:"verify_upgrade_and_update_state,omitempty"`
-	CheckSubstituteAndUpdateState *CheckSubstituteAndUpdateStateMsg `json:"check_substitute_and_update_state,omitempty"`
-	VerifyMembership              *VerifyMembershipMsg              `json:"verify_membership,omitempty"`
-	VerifyNonMembership           *VerifyNonMembershipMsg           `json:"verify_non_membership,omitempty"`
+	UpdateState                 *UpdateStateMsg                 `json:"update_state,omitempty"`
+	UpdateStateOnMisbehaviour   *UpdateStateOnMisbehaviourMsg   `json:"update_state_on_misbehaviour,omitempty"`
+	VerifyUpgradeAndUpdateState *VerifyUpgradeAndUpdateStateMsg `json:"verify_upgrade_and_update_state,omitempty"`
+	VerifyMembership            *VerifyMembershipMsg            `json:"verify_membership,omitempty"`
+	VerifyNonMembership         *VerifyNonMembershipMsg         `json:"verify_non_membership,omitempty"`
+	MigrateClientStore          *MigrateClientStoreMsg          `json:"migrate_client_store,omitempty"`
 }
 
 // UpdateStateMsg is a sudoMsg sent to the contract to update the client state.
@@ -94,8 +94,8 @@ type VerifyUpgradeAndUpdateStateMsg struct {
 	ProofUpgradeConsensusState []byte         `json:"proof_upgrade_consensus_state"`
 }
 
-// CheckSubstituteAndUpdateStateMsg is a sudoMsg sent to the contract to check a given substitute client and update to its state.
-type CheckSubstituteAndUpdateStateMsg struct{}
+// MigrateClientStore is a sudoMsg sent to the contract to verify a given substitute client and update to its state.
+type MigrateClientStoreMsg struct{}
 
 // ContractResult is a type constraint that defines the expected results that can be returned by a contract call/query.
 type ContractResult interface {

diff --git a/modules/light-clients/08-wasm/types/proposal_handle.go b/modules/light-clients/08-wasm/types/proposal_handle.go
@@ -1,6 +1,9 @@
 package types
 
 import (
+	"bytes"
+	"encoding/hex"
+
 	errorsmod "cosmossdk.io/errors"
 	storetypes "cosmossdk.io/store/types"
 
@@ -11,31 +14,32 @@ import (
 	"github.com/cosmos/ibc-go/v8/modules/core/exported"
 )
 
-// CheckSubstituteAndUpdateState will try to update the client with the state of the
-// substitute.
-func (cs ClientState) CheckSubstituteAndUpdateState(
-	ctx sdk.Context,
-	_ codec.BinaryCodec,
-	subjectClientStore, substituteClientStore storetypes.KVStore,
-	substituteClient exported.ClientState,
-) error {
+// CheckSubstituteAndUpdateState will verify that a substitute client state is valid and update the subject client state.
+// Note that this method is used only for recovery and will not allow changes to the code hash.
+func (cs ClientState) CheckSubstituteAndUpdateState(ctx sdk.Context, _ codec.BinaryCodec, subjectClientStore, substituteClientStore storetypes.KVStore, substituteClient exported.ClientState) error {
 	var (
 		subjectPrefix    = []byte("subject/")
 		substitutePrefix = []byte("substitute/")
 	)
 
-	_, ok := substituteClient.(*ClientState)
+	substituteClientState, ok := substituteClient.(*ClientState)
 	if !ok {
 		return errorsmod.Wrapf(
 			clienttypes.ErrInvalidClient,
-			"invalid substitute client state. expected type %T, got %T", &ClientState{}, substituteClient,
+			"invalid substitute client state: expected type %T, got %T", &ClientState{}, substituteClient,
 		)
 	}
 
+	// check that code hashes of subject client state and substitute client state match
+	// changing the code hash is only allowed through the migrate contract RPC endpoint
+	if !bytes.Equal(cs.CodeHash, substituteClientState.CodeHash) {
+		return errorsmod.Wrapf(clienttypes.ErrInvalidClient, "expected code hashes to be equal: expected %s, got %s", hex.EncodeToString(cs.CodeHash), hex.EncodeToString(substituteClientState.CodeHash))
+	}
+
 	store := newUpdateProposalWrappedStore(subjectClientStore, substituteClientStore, subjectPrefix, substitutePrefix)
 
 	payload := SudoMsg{
-		CheckSubstituteAndUpdateState: &CheckSubstituteAndUpdateStateMsg{},
+		MigrateClientStore: &MigrateClientStoreMsg{},
 	}
 
 	_, err := wasmSudo[EmptyResult](ctx, store, &cs, payload)

diff --git a/modules/light-clients/08-wasm/types/proposal_handle_test.go b/modules/light-clients/08-wasm/types/proposal_handle_test.go
@@ -78,203 +78,6 @@ func (suite *TypesTestSuite) TestCheckSubstituteAndUpdateStateGrandpa() {
 	}
 }
 
-// func (suite *TypesTestSuite) TestCheckSubstituteAndUpdateStateBasicTendermint() {
-// 	var (
-// 		substituteClientState exported.ClientState
-// 		substitutePath        *ibctesting.Path
-// 	)
-// 	testCases := []struct {
-// 		name     string
-// 		malleate func()
-// 	}{
-// 		{
-// 			"solo machine used for substitute", func() {
-// 				substituteClientState = ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "solo machine", "", 1).ClientState()
-// 			},
-// 		},
-// 		{
-// 			"non-matching substitute", func() {
-// 				suite.coordinator.SetupClients(substitutePath)
-// 				substituteWasmClientState := suite.chainA.GetClientState(substitutePath.EndpointA.ClientID).(*types.ClientState)
-
-// 				var clientStateData exported.ClientState
-// 				err := suite.chainA.Codec.UnmarshalInterface(substituteWasmClientState.Data, &clientStateData)
-// 				suite.Require().NoError(err)
-// 				tmClientState := clientStateData.(*ibctm.ClientState)
-
-// 				// change unbonding period so that test should fail
-// 				tmClientState.UnbondingPeriod = time.Hour * 24 * 7
-
-// 				tmClientStateBz, err := clienttypes.MarshalClientState(suite.chainA.App.AppCodec(), tmClientState)
-// 				suite.Require().NoError(err)
-
-// 				substituteWasmClientState.Data = tmClientStateBz
-
-// 				substituteClientState = substituteWasmClientState
-// 				substitutePath.EndpointA.SetClientState(substituteClientState)
-// 			},
-// 		},
-// 	}
-
-// 	for _, tc := range testCases {
-// 		suite.Run(tc.name, func() {
-// 			suite.SetupWasmTendermint() // reset
-// 			subjectPath := ibctesting.NewPath(suite.chainA, suite.chainB)
-// 			substitutePath = ibctesting.NewPath(suite.chainA, suite.chainB)
-
-// 			suite.coordinator.SetupClients(subjectPath)
-// 			subjectClientState := suite.chainA.GetClientState(subjectPath.EndpointA.ClientID).(*types.ClientState)
-
-// 			var clientStateData exported.ClientState
-// 			err := suite.chainA.Codec.UnmarshalInterface(subjectClientState.Data, &clientStateData)
-// 			suite.Require().NoError(err)
-// 			tmClientState := clientStateData.(*ibctm.ClientState)
-
-// 			// expire subject client
-// 			suite.coordinator.IncrementTimeBy(tmClientState.TrustingPeriod)
-// 			suite.coordinator.CommitBlock(suite.chainA, suite.chainB)
-
-// 			tc.malleate()
-
-// 			subjectClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID)
-// 			substituteClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), substitutePath.EndpointA.ClientID)
-
-// 			err = subjectClientState.CheckSubstituteAndUpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), subjectClientStore, substituteClientStore, substituteClientState)
-// 			suite.Require().Error(err)
-// 		})
-// 	}
-// }
-
-// func (suite *TypesTestSuite) TestCheckSubstituteAndUpdateStateTendermint() {
-// 	testCases := []struct {
-// 		name         string
-// 		FreezeClient bool
-// 		expPass      bool
-// 	}{
-// 		{
-// 			name:         "PASS: update checks are deprecated, client is not frozen",
-// 			FreezeClient: false,
-// 			expPass:      true,
-// 		},
-// 		{
-// 			name:         "PASS: update checks are deprecated, client is frozen",
-// 			FreezeClient: true,
-// 			expPass:      true,
-// 		},
-// 	}
-
-// 	for _, tc := range testCases {
-// 		suite.Run(tc.name, func() {
-// 			suite.SetupWasmTendermint() // reset
-
-// 			// construct subject using test case parameters
-// 			subjectPath := ibctesting.NewPath(suite.chainA, suite.chainB)
-// 			suite.coordinator.SetupClients(subjectPath)
-// 			subjectWasmClientState := suite.chainA.GetClientState(subjectPath.EndpointA.ClientID).(*types.ClientState)
-
-// 			var subjectWasmClientStateData exported.ClientState
-// 			err := suite.chainA.Codec.UnmarshalInterface(subjectWasmClientState.Data, &subjectWasmClientStateData)
-// 			suite.Require().NoError(err)
-// 			subjectTmClientState := subjectWasmClientStateData.(*ibctm.ClientState)
-
-// 			if tc.FreezeClient {
-// 				subjectTmClientState.FrozenHeight = frozenHeight
-// 			}
-
-// 			subjectTmClientStateBz, err := clienttypes.MarshalClientState(suite.chainA.App.AppCodec(), subjectTmClientState)
-// 			suite.Require().NoError(err)
-// 			subjectWasmClientState.Data = subjectTmClientStateBz
-// 			subjectPath.EndpointA.SetClientState(subjectWasmClientState)
-
-// 			// construct the substitute to match the subject client
-
-// 			substitutePath := ibctesting.NewPath(suite.chainA, suite.chainB)
-// 			suite.coordinator.SetupClients(substitutePath)
-// 			substituteWasmClientState := suite.chainA.GetClientState(substitutePath.EndpointA.ClientID).(*types.ClientState)
-
-// 			var substituteWasmClientStateData exported.ClientState
-// 			err = suite.chainA.Codec.UnmarshalInterface(substituteWasmClientState.Data, &substituteWasmClientStateData)
-// 			suite.Require().NoError(err)
-// 			substituteTmClientState := substituteWasmClientStateData.(*ibctm.ClientState)
-
-// 			// update trusting period of substitute client state
-// 			substituteTmClientState.TrustingPeriod = time.Hour * 24 * 7
-
-// 			substituteTmClientStateBz, err := clienttypes.MarshalClientState(suite.chainA.App.AppCodec(), substituteTmClientState)
-// 			suite.Require().NoError(err)
-// 			substituteWasmClientState.Data = substituteTmClientStateBz
-// 			substitutePath.EndpointA.SetClientState(substituteWasmClientState)
-
-// 			// update substitute a few times
-// 			for i := 0; i < 3; i++ {
-// 				err := substitutePath.EndpointA.UpdateClient()
-// 				suite.Require().NoError(err)
-// 				// skip a block
-// 				suite.coordinator.CommitBlock(suite.chainA, suite.chainB)
-// 			}
-
-// 			// get updated substitute
-// 			substituteWasmClientState = suite.chainA.GetClientState(substitutePath.EndpointA.ClientID).(*types.ClientState)
-// 			err = suite.chainA.Codec.UnmarshalInterface(substituteWasmClientState.Data, &substituteWasmClientStateData)
-// 			suite.Require().NoError(err)
-// 			substituteTmClientState = substituteWasmClientStateData.(*ibctm.ClientState)
-
-// 			// test that subject gets updated chain-id
-// 			newChainID := "new-chain-id"
-// 			substituteTmClientState.ChainId = newChainID
-
-// 			substituteTmClientStateBz, err = clienttypes.MarshalClientState(suite.chainA.App.AppCodec(), substituteTmClientState)
-// 			suite.Require().NoError(err)
-// 			substituteWasmClientState.Data = substituteTmClientStateBz
-// 			substitutePath.EndpointA.SetClientState(substituteWasmClientState)
-
-// 			subjectClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID)
-// 			substituteClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), substitutePath.EndpointA.ClientID)
-
-// 			expectedConsState := substitutePath.EndpointA.GetConsensusState(substituteWasmClientState.GetLatestHeight())
-// 			expectedProcessedTime, found := ibctm.GetProcessedTime(substituteClientStore, substituteWasmClientState.GetLatestHeight())
-// 			suite.Require().True(found)
-// 			expectedProcessedHeight, found := GetProcessedHeight(substituteClientStore, substituteWasmClientState.GetLatestHeight())
-// 			suite.Require().True(found)
-// 			expectedIterationKey := ibctm.GetIterationKey(substituteClientStore, substituteWasmClientState.GetLatestHeight())
-
-// 			err = subjectWasmClientState.CheckSubstituteAndUpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), subjectClientStore, substituteClientStore, substituteWasmClientState)
-
-// 			if tc.expPass {
-// 				suite.Require().NoError(err)
-
-// 				updatedWasmClient := subjectPath.EndpointA.GetClientState().(*types.ClientState)
-// 				var clientStateData exported.ClientState
-// 				err := suite.chainA.Codec.UnmarshalInterface(updatedWasmClient.Data, &clientStateData)
-// 				suite.Require().NoError(err)
-// 				updatedTmClientState := clientStateData.(*ibctm.ClientState)
-// 				suite.Require().Equal(clienttypes.ZeroHeight(), updatedTmClientState.FrozenHeight)
-
-// 				subjectClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), subjectPath.EndpointA.ClientID)
-
-// 				// check that the correct consensus state was copied over
-// 				suite.Require().Equal(substituteWasmClientState.GetLatestHeight(), updatedWasmClient.GetLatestHeight())
-// 				subjectConsState := subjectPath.EndpointA.GetConsensusState(updatedWasmClient.GetLatestHeight())
-// 				subjectProcessedTime, found := ibctm.GetProcessedTime(subjectClientStore, updatedWasmClient.GetLatestHeight())
-// 				suite.Require().True(found)
-// 				subjectProcessedHeight, found := GetProcessedHeight(subjectClientStore, updatedWasmClient.GetLatestHeight())
-// 				suite.Require().True(found)
-// 				subjectIterationKey := ibctm.GetIterationKey(subjectClientStore, updatedWasmClient.GetLatestHeight())
-
-// 				suite.Require().Equal(expectedConsState, subjectConsState)
-// 				suite.Require().Equal(expectedProcessedTime, subjectProcessedTime)
-// 				suite.Require().Equal(expectedProcessedHeight, subjectProcessedHeight)
-// 				suite.Require().Equal(expectedIterationKey, subjectIterationKey)
-
-// 				suite.Require().Equal(newChainID, updatedTmClientState.ChainId)
-// 				suite.Require().Equal(time.Hour*24*7, updatedTmClientState.TrustingPeriod)
-// 			} else {
-// 				suite.Require().Error(err)
-// 			}
-// 		})
-// 	}
-// }
-
 func GetProcessedHeight(clientStore storetypes.KVStore, height exported.Height) (uint64, bool) {
 	key := ibctm.ProcessedHeightKey(height)
 	bz := clientStore.Get(key)

diff --git a/modules/light-clients/08-wasm/types/update_test.go b/modules/light-clients/08-wasm/types/update_test.go
@@ -494,7 +494,6 @@ func (suite *TypesTestSuite) TestUpdateState() {
 					suite.Require().Nil(msg.VerifyNonMembership)
 					suite.Require().Nil(msg.UpdateStateOnMisbehaviour)
 					suite.Require().Nil(msg.VerifyUpgradeAndUpdateState)
-					suite.Require().Nil(msg.CheckSubstituteAndUpdateState)
 
 					suite.Require().Equal(env.Contract.Address, defaultWasmClientID)
 
@@ -637,7 +636,6 @@ func (suite *TypesTestSuite) TestUpdateStateOnMisbehaviour() {
 					suite.Require().Nil(msg.VerifyMembership)
 					suite.Require().Nil(msg.VerifyNonMembership)
 					suite.Require().Nil(msg.VerifyUpgradeAndUpdateState)
-					suite.Require().Nil(msg.CheckSubstituteAndUpdateState)
 
 					resp, err := json.Marshal(types.EmptyResult{})
 					if err != nil {

diff --git a/modules/light-clients/08-wasm/types/upgrade_test.go b/modules/light-clients/08-wasm/types/upgrade_test.go
@@ -134,7 +134,6 @@ func (suite *TypesTestSuite) TestVerifyUpgradeAndUpdateState() {
 					suite.Require().Equal(proofUpgradedConsState, payload.VerifyUpgradeAndUpdateState.ProofUpgradeConsensusState)
 
 					// verify other Sudo fields are nil
-					suite.Require().Nil(payload.CheckSubstituteAndUpdateState)
 					suite.Require().Nil(payload.UpdateState)
 					suite.Require().Nil(payload.UpdateStateOnMisbehaviour)
 					suite.Require().Nil(payload.VerifyMembership)