From 2e2bfab6a714144f02a986fbfe5b6ced1c02f737 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 30 Mar 2022 15:21:54 +0200 Subject: [PATCH] chore: 07-tendermint set client/consensus states in UpdateState (#1199) * adding storage ops to 07-tendermint UpdateState, updating tests * use clientMessage.GetHeight() as per review suggestions * updating tests to obtain previous client/consensus state in malleate --- .../07-tendermint/types/proposal_handle.go | 2 +- .../07-tendermint/types/store.go | 4 +- .../07-tendermint/types/update.go | 4 +- .../07-tendermint/types/update_test.go | 38 +++++++++++-------- .../07-tendermint/types/upgrade.go | 2 +- 5 files changed, 30 insertions(+), 20 deletions(-) diff --git a/modules/light-clients/07-tendermint/types/proposal_handle.go b/modules/light-clients/07-tendermint/types/proposal_handle.go index a87f9576567..c01ec6fd8b7 100644 --- a/modules/light-clients/07-tendermint/types/proposal_handle.go +++ b/modules/light-clients/07-tendermint/types/proposal_handle.go @@ -67,7 +67,7 @@ func (cs ClientState) CheckSubstituteAndUpdateState( return nil, sdkerrors.Wrap(err, "unable to retrieve latest consensus state for substitute client") } - SetConsensusState(subjectClientStore, cdc, consensusState, height) + setConsensusState(subjectClientStore, cdc, consensusState, height) // set metadata stored for the substitute consensus state processedHeight, found := GetProcessedHeight(substituteClientStore, height) diff --git a/modules/light-clients/07-tendermint/types/store.go b/modules/light-clients/07-tendermint/types/store.go index 33c2386bb78..f2c954cc029 100644 --- a/modules/light-clients/07-tendermint/types/store.go +++ b/modules/light-clients/07-tendermint/types/store.go @@ -50,8 +50,8 @@ func setClientState(clientStore sdk.KVStore, cdc codec.BinaryCodec, clientState clientStore.Set(key, val) } -// SetConsensusState stores the consensus state at the given height. -func SetConsensusState(clientStore sdk.KVStore, cdc codec.BinaryCodec, consensusState *ConsensusState, height exported.Height) { +// setConsensusState stores the consensus state at the given height. +func setConsensusState(clientStore sdk.KVStore, cdc codec.BinaryCodec, consensusState *ConsensusState, height exported.Height) { key := host.ConsensusStateKey(height) val := clienttypes.MustMarshalConsensusState(cdc, consensusState) clientStore.Set(key, val) diff --git a/modules/light-clients/07-tendermint/types/update.go b/modules/light-clients/07-tendermint/types/update.go index 615ed14a381..0a25cbc2518 100644 --- a/modules/light-clients/07-tendermint/types/update.go +++ b/modules/light-clients/07-tendermint/types/update.go @@ -262,7 +262,9 @@ func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, client NextValidatorsHash: header.Header.NextValidatorsHash, } - // set metadata for this consensus state + // set client state, consensus state and asssociated metadata + setClientState(clientStore, cdc, &cs) + setConsensusState(clientStore, cdc, consensusState, header.GetHeight()) setConsensusMetadata(ctx, clientStore, header.GetHeight()) return &cs, consensusState, nil diff --git a/modules/light-clients/07-tendermint/types/update_test.go b/modules/light-clients/07-tendermint/types/update_test.go index 3bc96b0fb93..4f9648bd74a 100644 --- a/modules/light-clients/07-tendermint/types/update_test.go +++ b/modules/light-clients/07-tendermint/types/update_test.go @@ -4,6 +4,7 @@ import ( "fmt" "time" + sdk "github.com/cosmos/cosmos-sdk/types" clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" commitmenttypes "github.com/cosmos/ibc-go/v3/modules/core/23-commitment/types" host "github.com/cosmos/ibc-go/v3/modules/core/24-host" @@ -394,11 +395,12 @@ func (suite *TendermintTestSuite) TestVerifyHeader() { func (suite *TendermintTestSuite) TestUpdateState() { var ( - path *ibctesting.Path - clientMessage exported.ClientMessage - pruneHeight clienttypes.Height - updatedClientState *types.ClientState // TODO: retrieve from state after 'UpdateState' call - updatedConsensusState *types.ConsensusState // TODO: retrieve from state after 'UpdateState' call + path *ibctesting.Path + clientMessage exported.ClientMessage + clientStore sdk.KVStore + pruneHeight clienttypes.Height + prevClientState exported.ClientState + prevConsensusState exported.ConsensusState ) testCases := []struct { @@ -412,7 +414,7 @@ func (suite *TendermintTestSuite) TestUpdateState() { suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().LT(clientMessage.GetHeight())) }, func() { - suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().LT(updatedClientState.GetLatestHeight())) // new update, updated client state should have changed + suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().EQ(clientMessage.GetHeight())) // new update, updated client state should have changed }, true, }, { @@ -424,9 +426,11 @@ func (suite *TendermintTestSuite) TestUpdateState() { suite.Require().NoError(err) suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().GT(clientMessage.GetHeight())) + + prevClientState = path.EndpointA.GetClientState() }, func() { - suite.Require().Equal(path.EndpointA.GetClientState(), updatedClientState) // fill in height, no change to client state + suite.Require().Equal(path.EndpointA.GetClientState(), prevClientState) // fill in height, no change to client state }, true, }, { @@ -439,10 +443,13 @@ func (suite *TendermintTestSuite) TestUpdateState() { clientMessage, err = path.EndpointA.Chain.ConstructUpdateTMClientHeader(path.EndpointA.Counterparty.Chain, path.EndpointA.ClientID) suite.Require().NoError(err) suite.Require().Equal(path.EndpointA.GetClientState().GetLatestHeight(), clientMessage.GetHeight()) + + prevClientState = path.EndpointA.GetClientState() + prevConsensusState = path.EndpointA.GetConsensusState(clientMessage.GetHeight()) }, func() { - suite.Require().Equal(path.EndpointA.GetClientState(), updatedClientState) - suite.Require().Equal(path.EndpointA.GetConsensusState(clientMessage.GetHeight()), updatedConsensusState) + suite.Require().Equal(path.EndpointA.GetClientState(), prevClientState) + suite.Require().Equal(path.EndpointA.GetConsensusState(clientMessage.GetHeight()), prevConsensusState) }, true, }, { @@ -471,7 +478,7 @@ func (suite *TendermintTestSuite) TestUpdateState() { suite.Require().NoError(err) }, func() { - suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().LT(updatedClientState.GetLatestHeight())) // new update, updated client state should have changed + suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().EQ(clientMessage.GetHeight())) // new update, updated client state should have changed // ensure consensus state was pruned _, found := path.EndpointA.Chain.GetConsensusState(path.EndpointA.ClientID, pruneHeight) @@ -508,8 +515,8 @@ func (suite *TendermintTestSuite) TestUpdateState() { tmClientState, ok := clientState.(*types.ClientState) suite.Require().True(ok) - clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) - updatedClientState, updatedConsensusState, err = tmClientState.UpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore, clientMessage) + clientStore = suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID) + _, _, err = tmClientState.UpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), clientStore, clientMessage) if tc.expPass { suite.Require().NoError(err) @@ -520,13 +527,14 @@ func (suite *TendermintTestSuite) TestUpdateState() { Root: commitmenttypes.NewMerkleRoot(header.Header.GetAppHash()), NextValidatorsHash: header.Header.NextValidatorsHash, } + + bz := clientStore.Get(host.ConsensusStateKey(header.GetHeight())) + updatedConsensusState := clienttypes.MustUnmarshalConsensusState(suite.chainA.App.AppCodec(), bz) + suite.Require().Equal(expConsensusState, updatedConsensusState) } else { suite.Require().Error(err) - suite.Require().Nil(updatedClientState) - suite.Require().Nil(updatedConsensusState) - } // perform custom checks diff --git a/modules/light-clients/07-tendermint/types/upgrade.go b/modules/light-clients/07-tendermint/types/upgrade.go index 471769f0610..eab2e3681df 100644 --- a/modules/light-clients/07-tendermint/types/upgrade.go +++ b/modules/light-clients/07-tendermint/types/upgrade.go @@ -120,7 +120,7 @@ func (cs ClientState) VerifyUpgradeAndUpdateState( ) setClientState(clientStore, cdc, newClientState) - SetConsensusState(clientStore, cdc, newConsState, newClientState.LatestHeight) + setConsensusState(clientStore, cdc, newConsState, newClientState.LatestHeight) setConsensusMetadata(ctx, clientStore, tmUpgradeClient.LatestHeight) return nil