From 81eee38c1f39e49d7b8e590bbc2796f6a8faa050 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 30 Mar 2022 11:21:02 +0200 Subject: [PATCH 1/3] adding storage ops to 07-tendermint UpdateState, updating tests --- .../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 | 40 +++++++++++++------ .../07-tendermint/types/upgrade.go | 2 +- 5 files changed, 35 insertions(+), 17 deletions(-) diff --git a/modules/light-clients/07-tendermint/types/proposal_handle.go b/modules/light-clients/07-tendermint/types/proposal_handle.go index bfa7f242e92..35c181fd1f5 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 0862447a4b3..c162c85eafe 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 a6ae04c0897..eadb5f755b9 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" @@ -397,11 +398,10 @@ 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 ) testCases := []struct { @@ -415,7 +415,10 @@ 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 + bz := clientStore.Get(host.ClientStateKey()) + updatedClientState := clienttypes.MustUnmarshalClientState(suite.chainA.App.AppCodec(), bz) + + suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().EQ(updatedClientState.GetLatestHeight())) // new update, updated client state should have changed }, true, }, { @@ -429,6 +432,9 @@ func (suite *TendermintTestSuite) TestUpdateState() { suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().GT(clientMessage.GetHeight())) }, func() { + bz := clientStore.Get(host.ClientStateKey()) + updatedClientState := clienttypes.MustUnmarshalClientState(suite.chainA.App.AppCodec(), bz) + suite.Require().Equal(path.EndpointA.GetClientState(), updatedClientState) // fill in height, no change to client state }, true, }, @@ -444,6 +450,12 @@ func (suite *TendermintTestSuite) TestUpdateState() { suite.Require().Equal(path.EndpointA.GetClientState().GetLatestHeight(), clientMessage.GetHeight()) }, func() { + bz := clientStore.Get(host.ClientStateKey()) + updatedClientState := clienttypes.MustUnmarshalClientState(suite.chainA.App.AppCodec(), bz) + + bz = clientStore.Get(host.ConsensusStateKey(clientMessage.GetHeight())) + updatedConsensusState := clienttypes.MustUnmarshalConsensusState(suite.chainA.App.AppCodec(), bz) + suite.Require().Equal(path.EndpointA.GetClientState(), updatedClientState) suite.Require().Equal(path.EndpointA.GetConsensusState(clientMessage.GetHeight()), updatedConsensusState) }, true, @@ -474,7 +486,10 @@ 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 + bz := clientStore.Get(host.ClientStateKey()) + updatedClientState := clienttypes.MustUnmarshalClientState(suite.chainA.App.AppCodec(), bz) + + suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().EQ(updatedClientState.GetLatestHeight())) // new update, updated client state should have changed // ensure consensus state was pruned _, found := path.EndpointA.Chain.GetConsensusState(path.EndpointA.ClientID, pruneHeight) @@ -511,8 +526,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) @@ -523,13 +538,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 From f98617fb7c9bc211da4021bc1906c4b481e109c4 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 30 Mar 2022 14:09:48 +0200 Subject: [PATCH 2/3] use clientMessage.GetHeight() as per review suggestions --- .../light-clients/07-tendermint/types/update_test.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/modules/light-clients/07-tendermint/types/update_test.go b/modules/light-clients/07-tendermint/types/update_test.go index eadb5f755b9..205a92a825b 100644 --- a/modules/light-clients/07-tendermint/types/update_test.go +++ b/modules/light-clients/07-tendermint/types/update_test.go @@ -415,10 +415,7 @@ func (suite *TendermintTestSuite) TestUpdateState() { suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().LT(clientMessage.GetHeight())) }, func() { - bz := clientStore.Get(host.ClientStateKey()) - updatedClientState := clienttypes.MustUnmarshalClientState(suite.chainA.App.AppCodec(), bz) - - suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().EQ(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, }, { @@ -486,10 +483,7 @@ func (suite *TendermintTestSuite) TestUpdateState() { suite.Require().NoError(err) }, func() { - bz := clientStore.Get(host.ClientStateKey()) - updatedClientState := clienttypes.MustUnmarshalClientState(suite.chainA.App.AppCodec(), bz) - - suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().EQ(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) From 45bf66a373f72941da9a72716e651bb6e254a476 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 30 Mar 2022 14:39:38 +0200 Subject: [PATCH 3/3] updating tests to obtain previous client/consensus state in malleate --- .../07-tendermint/types/update_test.go | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/modules/light-clients/07-tendermint/types/update_test.go b/modules/light-clients/07-tendermint/types/update_test.go index 205a92a825b..978ff7a1870 100644 --- a/modules/light-clients/07-tendermint/types/update_test.go +++ b/modules/light-clients/07-tendermint/types/update_test.go @@ -398,10 +398,12 @@ func (suite *TendermintTestSuite) TestVerifyHeader() { func (suite *TendermintTestSuite) TestUpdateState() { var ( - path *ibctesting.Path - clientMessage exported.ClientMessage - clientStore sdk.KVStore - pruneHeight clienttypes.Height + path *ibctesting.Path + clientMessage exported.ClientMessage + clientStore sdk.KVStore + pruneHeight clienttypes.Height + prevClientState exported.ClientState + prevConsensusState exported.ConsensusState ) testCases := []struct { @@ -427,12 +429,11 @@ func (suite *TendermintTestSuite) TestUpdateState() { suite.Require().NoError(err) suite.Require().True(path.EndpointA.GetClientState().GetLatestHeight().GT(clientMessage.GetHeight())) + + prevClientState = path.EndpointA.GetClientState() }, func() { - bz := clientStore.Get(host.ClientStateKey()) - updatedClientState := clienttypes.MustUnmarshalClientState(suite.chainA.App.AppCodec(), bz) - - 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, }, { @@ -445,16 +446,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() { - bz := clientStore.Get(host.ClientStateKey()) - updatedClientState := clienttypes.MustUnmarshalClientState(suite.chainA.App.AppCodec(), bz) - - bz = clientStore.Get(host.ConsensusStateKey(clientMessage.GetHeight())) - updatedConsensusState := clienttypes.MustUnmarshalConsensusState(suite.chainA.App.AppCodec(), bz) - - 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, }, {