From 86281cd20456899c693653995e2d8763c6e753e6 Mon Sep 17 00:00:00 2001 From: shakeshack Date: Wed, 9 Feb 2022 00:52:47 -0500 Subject: [PATCH 1/7] feat: add GetTimestampAtHeight to client state --- .../core/02-client/legacy/v100/solomachine.go | 7 +++ modules/core/exported/client.go | 6 ++ .../06-solomachine/types/client_state.go | 14 +++++ .../06-solomachine/types/client_state_test.go | 57 ++++++++++++++++++- .../06-solomachine/types/store.go | 38 +++++++++++++ .../07-tendermint/types/client_state.go | 18 ++++++ .../07-tendermint/types/client_state_test.go | 47 +++++++++++++++ .../09-localhost/types/client_state.go | 10 ++++ .../09-localhost/types/client_state_test.go | 41 +++++++++++++ 9 files changed, 237 insertions(+), 1 deletion(-) create mode 100644 modules/light-clients/06-solomachine/types/store.go diff --git a/modules/core/02-client/legacy/v100/solomachine.go b/modules/core/02-client/legacy/v100/solomachine.go index c9814439902..5d53152d599 100644 --- a/modules/core/02-client/legacy/v100/solomachine.go +++ b/modules/core/02-client/legacy/v100/solomachine.go @@ -187,6 +187,13 @@ func (cs ClientState) VerifyNextSequenceRecv( panic("legacy solo machine is deprecated!") } +// GetTimestampAtHeight panics! +func (cs ClientState) GetTimestampAtHeight( + sdk.Context, sdk.KVStore, codec.BinaryCodec, exported.Height, +) (uint64, error) { + panic("legacy solo machine is deprecated!") +} + // ClientType panics! func (ConsensusState) ClientType() string { panic("legacy solo machine is deprecated!") diff --git a/modules/core/exported/client.go b/modules/core/exported/client.go index 39095aff28f..2a5946d1977 100644 --- a/modules/core/exported/client.go +++ b/modules/core/exported/client.go @@ -177,6 +177,12 @@ type ClientState interface { channelID string, nextSequenceRecv uint64, ) error + GetTimestampAtHeight( + ctx sdk.Context, + clientStore sdk.KVStore, + cdc codec.BinaryCodec, + height Height, + ) (uint64, error) } // ConsensusState is the state of the consensus process diff --git a/modules/light-clients/06-solomachine/types/client_state.go b/modules/light-clients/06-solomachine/types/client_state.go index ef3088b314f..fd45e6f975f 100644 --- a/modules/light-clients/06-solomachine/types/client_state.go +++ b/modules/light-clients/06-solomachine/types/client_state.go @@ -39,6 +39,20 @@ func (cs ClientState) GetLatestHeight() exported.Height { return clienttypes.NewHeight(0, cs.Sequence) } +// GetTimestampAtHeight returns 0. +func (cs ClientState) GetTimestampAtHeight( + _ sdk.Context, + clientStore sdk.KVStore, + cdc codec.BinaryCodec, + height exported.Height, +) (uint64, error) { + consensusState, err := GetConsensusState(clientStore, cdc, height) + if err != nil { + return 0, err + } + return consensusState.GetTimestamp(), nil +} + // Status returns the status of the solo machine client. // The client may be: // - Active: if frozen sequence is 0 diff --git a/modules/light-clients/06-solomachine/types/client_state_test.go b/modules/light-clients/06-solomachine/types/client_state_test.go index 09ea9693119..bdca5e37bb8 100644 --- a/modules/light-clients/06-solomachine/types/client_state_test.go +++ b/modules/light-clients/06-solomachine/types/client_state_test.go @@ -5,6 +5,7 @@ import ( connectiontypes "github.com/cosmos/ibc-go/v3/modules/core/03-connection/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" commitmenttypes "github.com/cosmos/ibc-go/v3/modules/core/23-commitment/types" + host "github.com/cosmos/ibc-go/v3/modules/core/24-host" "github.com/cosmos/ibc-go/v3/modules/core/exported" "github.com/cosmos/ibc-go/v3/modules/light-clients/06-solomachine/types" ibctmtypes "github.com/cosmos/ibc-go/v3/modules/light-clients/07-tendermint/types" @@ -27,7 +28,7 @@ var ( func (suite *SoloMachineTestSuite) TestStatus() { clientState := suite.solomachine.ClientState() - // solo machine discards arguements + // solo machine discards arguments status := clientState.Status(suite.chainA.GetContext(), nil, nil) suite.Require().Equal(exported.Active, status) @@ -845,3 +846,57 @@ func (suite *SoloMachineTestSuite) TestVerifyNextSeqRecv() { } } } + +func (suite *SoloMachineTestSuite) TestGetTimestampAtHeight() { + tmPath := ibctesting.NewPath(suite.chainA, suite.chainB) + suite.coordinator.SetupClients(tmPath) + + testCases := []struct { + name string + clientState *types.ClientState + // Write directly to store since solomachine isn't hooked up to the test app. + malleate func() + expValue uint64 + expPass bool + }{ + { + name: "get timestamp at height exists", + clientState: suite.solomachine.ClientState(), + malleate: func() { + value, err := clienttypes.MarshalConsensusState(suite.chainA.Codec, suite.solomachine.ClientState().ConsensusState) + suite.Require().NoError(err) + suite.store.Set(host.ConsensusStateKey(consensusHeight), value) + }, + expValue: suite.solomachine.ClientState().ConsensusState.Timestamp, + expPass: true, + }, + { + name: "get timestamp at height not exists", + clientState: suite.solomachine.ClientState(), + malleate: func() {}, + }, + } + + for i, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.SetupTest() + tc.malleate() + + ctx := suite.chainA.GetContext() + + ts, err := tc.clientState.GetTimestampAtHeight( + ctx, suite.store, suite.chainA.Codec, consensusHeight, + ) + + suite.Require().Equal(tc.expValue, ts) + + if tc.expPass { + suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.name) + } else { + suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.name) + } + }) + } +} diff --git a/modules/light-clients/06-solomachine/types/store.go b/modules/light-clients/06-solomachine/types/store.go new file mode 100644 index 00000000000..82bd5227e32 --- /dev/null +++ b/modules/light-clients/06-solomachine/types/store.go @@ -0,0 +1,38 @@ +package types + +import ( + "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + + clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" + host "github.com/cosmos/ibc-go/v3/modules/core/24-host" + "github.com/cosmos/ibc-go/v3/modules/core/exported" +) + +// GetConsensusState retrieves the consensus state from the client prefixed +// store. An error is returned if the consensus state does not exist. +func GetConsensusState(store sdk.KVStore, cdc codec.BinaryCodec, height exported.Height) (*ConsensusState, error) { + bz := store.Get(host.ConsensusStateKey(height)) + if bz == nil { + return nil, sdkerrors.Wrapf( + clienttypes.ErrConsensusStateNotFound, + "consensus state does not exist for height %s", height, + ) + } + + consensusStateI, err := clienttypes.UnmarshalConsensusState(cdc, bz) + if err != nil { + return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidConsensus, "unmarshal error: %v", err) + } + + consensusState, ok := consensusStateI.(*ConsensusState) + if !ok { + return nil, sdkerrors.Wrapf( + clienttypes.ErrInvalidConsensus, + "invalid consensus type %T, expected %T", consensusState, &ConsensusState{}, + ) + } + + return consensusState, nil +} diff --git a/modules/light-clients/07-tendermint/types/client_state.go b/modules/light-clients/07-tendermint/types/client_state.go index 51f826979fd..928f55dae3a 100644 --- a/modules/light-clients/07-tendermint/types/client_state.go +++ b/modules/light-clients/07-tendermint/types/client_state.go @@ -58,6 +58,24 @@ func (cs ClientState) GetLatestHeight() exported.Height { return cs.LatestHeight } +// GetTimestampAtHeight returns the timestamp in nanoseconds of the consensus state at the given height. +func (cs ClientState) GetTimestampAtHeight( + ctx sdk.Context, + clientStore sdk.KVStore, + cdc codec.BinaryCodec, + height exported.Height, +) (uint64, error) { + // get latest consensus state from clientStore to check for expiry + consState, err := GetConsensusState(clientStore, cdc, height) + if err != nil { + return 0, sdkerrors.Wrapf( + err, + "height (%s)", height, + ) + } + return consState.GetTimestamp(), nil +} + // Status returns the status of the tendermint client. // The client may be: // - Active: FrozenHeight is zero and client is not expired diff --git a/modules/light-clients/07-tendermint/types/client_state_test.go b/modules/light-clients/07-tendermint/types/client_state_test.go index cf52d2996b5..b2538b18655 100644 --- a/modules/light-clients/07-tendermint/types/client_state_test.go +++ b/modules/light-clients/07-tendermint/types/client_state_test.go @@ -881,3 +881,50 @@ func (suite *TendermintTestSuite) TestVerifyNextSeqRecv() { }) } } + +func (suite *TendermintTestSuite) TestGetTimestampAtHeight() { + suite.SetupTest() + + path := ibctesting.NewPath(suite.chainA, suite.chainB) + path.SetChannelOrdered() + suite.coordinator.Setup(path) + + ctx := suite.chainA.GetContext() + clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(ctx, path.EndpointA.ClientID) + clientState := path.EndpointA.GetClientState() + + testCases := []struct { + name string + height exported.Height + expValue uint64 + expPass bool + }{ + { + name: "get timestamp at height exists", + height: clientState.GetLatestHeight(), + expValue: path.EndpointA.GetConsensusState(clientState.GetLatestHeight()).GetTimestamp(), + expPass: true, + }, + { + name: "get timestamp at height not exists", + height: clientState.GetLatestHeight().Increment(), + }, + } + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + ts, err := clientState.GetTimestampAtHeight( + ctx, clientStore, suite.chainA.Codec, tc.height, + ) + + suite.Require().Equal(tc.expValue, ts) + + if tc.expPass { + suite.Require().NoError(err) + } else { + suite.Require().Error(err) + } + }) + } +} diff --git a/modules/light-clients/09-localhost/types/client_state.go b/modules/light-clients/09-localhost/types/client_state.go index 728e4ec5f12..50d0279c5a8 100644 --- a/modules/light-clients/09-localhost/types/client_state.go +++ b/modules/light-clients/09-localhost/types/client_state.go @@ -42,6 +42,16 @@ func (cs ClientState) GetLatestHeight() exported.Height { return cs.Height } +// GetTimestampAtHeight returns 0. Localhost client has no consensus state. +func (cs ClientState) GetTimestampAtHeight( + _ sdk.Context, + _ sdk.KVStore, + _ codec.BinaryCodec, + _ exported.Height, +) (uint64, error) { + return 0, nil +} + // Status always returns Active. The localhost status cannot be changed. func (cs ClientState) Status(_ sdk.Context, _ sdk.KVStore, _ codec.BinaryCodec, ) exported.Status { diff --git a/modules/light-clients/09-localhost/types/client_state_test.go b/modules/light-clients/09-localhost/types/client_state_test.go index a54cc8efe9a..a59578f2743 100644 --- a/modules/light-clients/09-localhost/types/client_state_test.go +++ b/modules/light-clients/09-localhost/types/client_state_test.go @@ -527,3 +527,44 @@ func (suite *LocalhostTestSuite) TestVerifyNextSeqRecv() { }) } } + +func (suite *LocalhostTestSuite) TestGetTimestampAtHeight() { + testCases := []struct { + name string + clientState *types.ClientState + malleate func() + checkHeight exported.Height + expValue int // this is always 0 for localhost client + }{ + { + name: "get timestamp at height returns 0", + clientState: types.NewClientState("chainID", clientHeight), + checkHeight: clientHeight, + }, + { + name: "get timestamp at client height + 1 returns 0", + clientState: types.NewClientState("chainID", clientHeight), + checkHeight: clientHeight + 1, + }, + { + name: "get timestamp at client height + 2returns 0", + clientState: types.NewClientState("chainID", clientHeight), + checkHeight: clientHeight + 2, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.SetupTest() + tc.malleate() + + ts, err := tc.clientState.GetTimestampAtHeight( + suite.ctx, suite.store, suite.cdc, clientHeight, + ) + + suite.Require().Equal(tc.expValue, ts) + }) + } +} From 111b137fd56a9427256b08eb19a91ffa603590b9 Mon Sep 17 00:00:00 2001 From: shakeshack Date: Wed, 9 Feb 2022 01:02:07 -0500 Subject: [PATCH 2/7] chore: add space --- modules/light-clients/09-localhost/types/client_state_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/light-clients/09-localhost/types/client_state_test.go b/modules/light-clients/09-localhost/types/client_state_test.go index a59578f2743..c857d7c4392 100644 --- a/modules/light-clients/09-localhost/types/client_state_test.go +++ b/modules/light-clients/09-localhost/types/client_state_test.go @@ -547,7 +547,7 @@ func (suite *LocalhostTestSuite) TestGetTimestampAtHeight() { checkHeight: clientHeight + 1, }, { - name: "get timestamp at client height + 2returns 0", + name: "get timestamp at client height + 2 returns 0", clientState: types.NewClientState("chainID", clientHeight), checkHeight: clientHeight + 2, }, From 5c4c95ba7e71ffc5e17a61d7a2706267fc53e6f6 Mon Sep 17 00:00:00 2001 From: shakeshack Date: Wed, 9 Feb 2022 15:19:29 -0500 Subject: [PATCH 3/7] chore: address pr feedback --- modules/light-clients/06-solomachine/types/client_state.go | 4 ++-- modules/light-clients/07-tendermint/types/client_state.go | 5 +---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/modules/light-clients/06-solomachine/types/client_state.go b/modules/light-clients/06-solomachine/types/client_state.go index fd45e6f975f..d48f59fbc20 100644 --- a/modules/light-clients/06-solomachine/types/client_state.go +++ b/modules/light-clients/06-solomachine/types/client_state.go @@ -39,7 +39,7 @@ func (cs ClientState) GetLatestHeight() exported.Height { return clienttypes.NewHeight(0, cs.Sequence) } -// GetTimestampAtHeight returns 0. +// GetTimestampAtHeight returns the timestamp in nanoseconds of the consensus state at the given height. func (cs ClientState) GetTimestampAtHeight( _ sdk.Context, clientStore sdk.KVStore, @@ -48,7 +48,7 @@ func (cs ClientState) GetTimestampAtHeight( ) (uint64, error) { consensusState, err := GetConsensusState(clientStore, cdc, height) if err != nil { - return 0, err + return 0, sdkerrors.Wrapf(err, "height (%s)", height) } return consensusState.GetTimestamp(), nil } diff --git a/modules/light-clients/07-tendermint/types/client_state.go b/modules/light-clients/07-tendermint/types/client_state.go index 928f55dae3a..40164c27c25 100644 --- a/modules/light-clients/07-tendermint/types/client_state.go +++ b/modules/light-clients/07-tendermint/types/client_state.go @@ -68,10 +68,7 @@ func (cs ClientState) GetTimestampAtHeight( // get latest consensus state from clientStore to check for expiry consState, err := GetConsensusState(clientStore, cdc, height) if err != nil { - return 0, sdkerrors.Wrapf( - err, - "height (%s)", height, - ) + return 0, sdkerrors.Wrapf(err, "height (%s)", height) } return consState.GetTimestamp(), nil } From 176340cecf4a9485f21355b1792fcf91bab935f8 Mon Sep 17 00:00:00 2001 From: shakeshack Date: Wed, 9 Feb 2022 19:49:41 -0500 Subject: [PATCH 4/7] fix: increment height + check err --- .../light-clients/09-localhost/types/client_state_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/light-clients/09-localhost/types/client_state_test.go b/modules/light-clients/09-localhost/types/client_state_test.go index c857d7c4392..f9d53cb3640 100644 --- a/modules/light-clients/09-localhost/types/client_state_test.go +++ b/modules/light-clients/09-localhost/types/client_state_test.go @@ -544,12 +544,12 @@ func (suite *LocalhostTestSuite) TestGetTimestampAtHeight() { { name: "get timestamp at client height + 1 returns 0", clientState: types.NewClientState("chainID", clientHeight), - checkHeight: clientHeight + 1, + checkHeight: clientHeight.Increment(), }, { name: "get timestamp at client height + 2 returns 0", clientState: types.NewClientState("chainID", clientHeight), - checkHeight: clientHeight + 2, + checkHeight: clientHeight.Increment().Increment(), }, } @@ -564,6 +564,7 @@ func (suite *LocalhostTestSuite) TestGetTimestampAtHeight() { suite.ctx, suite.store, suite.cdc, clientHeight, ) + suite.Require().NoError(err) suite.Require().Equal(tc.expValue, ts) }) } From 397342c4b9e4ced99c7b84ab3660ea5e9914f04d Mon Sep 17 00:00:00 2001 From: Bo Du Date: Thu, 10 Feb 2022 20:56:25 -0500 Subject: [PATCH 5/7] chore: address PR feedback --- .../06-solomachine/types/client_state.go | 7 ++-- .../06-solomachine/types/client_state_test.go | 27 +++++-------- .../06-solomachine/types/store.go | 38 ------------------- .../07-tendermint/types/client_state.go | 2 +- .../09-localhost/types/client_state.go | 2 +- .../09-localhost/types/client_state_test.go | 13 +++---- 6 files changed, 20 insertions(+), 69 deletions(-) delete mode 100644 modules/light-clients/06-solomachine/types/store.go diff --git a/modules/light-clients/06-solomachine/types/client_state.go b/modules/light-clients/06-solomachine/types/client_state.go index d48f59fbc20..99914a34731 100644 --- a/modules/light-clients/06-solomachine/types/client_state.go +++ b/modules/light-clients/06-solomachine/types/client_state.go @@ -46,11 +46,10 @@ func (cs ClientState) GetTimestampAtHeight( cdc codec.BinaryCodec, height exported.Height, ) (uint64, error) { - consensusState, err := GetConsensusState(clientStore, cdc, height) - if err != nil { - return 0, sdkerrors.Wrapf(err, "height (%s)", height) + if !cs.GetLatestHeight().EQ(height) { + return 0, sdkerrors.Wrapf(ErrInvalidSequence, "not latest height (%s)", height) } - return consensusState.GetTimestamp(), nil + return cs.ConsensusState.Timestamp, nil } // Status returns the status of the solo machine client. diff --git a/modules/light-clients/06-solomachine/types/client_state_test.go b/modules/light-clients/06-solomachine/types/client_state_test.go index bdca5e37bb8..3e0c4904b7b 100644 --- a/modules/light-clients/06-solomachine/types/client_state_test.go +++ b/modules/light-clients/06-solomachine/types/client_state_test.go @@ -5,7 +5,6 @@ import ( connectiontypes "github.com/cosmos/ibc-go/v3/modules/core/03-connection/types" channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types" commitmenttypes "github.com/cosmos/ibc-go/v3/modules/core/23-commitment/types" - host "github.com/cosmos/ibc-go/v3/modules/core/24-host" "github.com/cosmos/ibc-go/v3/modules/core/exported" "github.com/cosmos/ibc-go/v3/modules/light-clients/06-solomachine/types" ibctmtypes "github.com/cosmos/ibc-go/v3/modules/light-clients/07-tendermint/types" @@ -850,30 +849,27 @@ func (suite *SoloMachineTestSuite) TestVerifyNextSeqRecv() { func (suite *SoloMachineTestSuite) TestGetTimestampAtHeight() { tmPath := ibctesting.NewPath(suite.chainA, suite.chainB) suite.coordinator.SetupClients(tmPath) + // Single setup for all test cases. + suite.SetupTest() testCases := []struct { name string clientState *types.ClientState - // Write directly to store since solomachine isn't hooked up to the test app. - malleate func() - expValue uint64 - expPass bool + height exported.Height + expValue uint64 + expPass bool }{ { name: "get timestamp at height exists", clientState: suite.solomachine.ClientState(), - malleate: func() { - value, err := clienttypes.MarshalConsensusState(suite.chainA.Codec, suite.solomachine.ClientState().ConsensusState) - suite.Require().NoError(err) - suite.store.Set(host.ConsensusStateKey(consensusHeight), value) - }, - expValue: suite.solomachine.ClientState().ConsensusState.Timestamp, - expPass: true, + height: suite.solomachine.ClientState().GetLatestHeight(), + expValue: suite.solomachine.ClientState().ConsensusState.Timestamp, + expPass: true, }, { name: "get timestamp at height not exists", clientState: suite.solomachine.ClientState(), - malleate: func() {}, + height: suite.solomachine.ClientState().GetLatestHeight().Increment(), }, } @@ -881,13 +877,10 @@ func (suite *SoloMachineTestSuite) TestGetTimestampAtHeight() { tc := tc suite.Run(tc.name, func() { - suite.SetupTest() - tc.malleate() - ctx := suite.chainA.GetContext() ts, err := tc.clientState.GetTimestampAtHeight( - ctx, suite.store, suite.chainA.Codec, consensusHeight, + ctx, suite.store, suite.chainA.Codec, tc.height, ) suite.Require().Equal(tc.expValue, ts) diff --git a/modules/light-clients/06-solomachine/types/store.go b/modules/light-clients/06-solomachine/types/store.go deleted file mode 100644 index 82bd5227e32..00000000000 --- a/modules/light-clients/06-solomachine/types/store.go +++ /dev/null @@ -1,38 +0,0 @@ -package types - -import ( - "github.com/cosmos/cosmos-sdk/codec" - sdk "github.com/cosmos/cosmos-sdk/types" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - - clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" - host "github.com/cosmos/ibc-go/v3/modules/core/24-host" - "github.com/cosmos/ibc-go/v3/modules/core/exported" -) - -// GetConsensusState retrieves the consensus state from the client prefixed -// store. An error is returned if the consensus state does not exist. -func GetConsensusState(store sdk.KVStore, cdc codec.BinaryCodec, height exported.Height) (*ConsensusState, error) { - bz := store.Get(host.ConsensusStateKey(height)) - if bz == nil { - return nil, sdkerrors.Wrapf( - clienttypes.ErrConsensusStateNotFound, - "consensus state does not exist for height %s", height, - ) - } - - consensusStateI, err := clienttypes.UnmarshalConsensusState(cdc, bz) - if err != nil { - return nil, sdkerrors.Wrapf(clienttypes.ErrInvalidConsensus, "unmarshal error: %v", err) - } - - consensusState, ok := consensusStateI.(*ConsensusState) - if !ok { - return nil, sdkerrors.Wrapf( - clienttypes.ErrInvalidConsensus, - "invalid consensus type %T, expected %T", consensusState, &ConsensusState{}, - ) - } - - return consensusState, nil -} diff --git a/modules/light-clients/07-tendermint/types/client_state.go b/modules/light-clients/07-tendermint/types/client_state.go index 40164c27c25..0a4b3d2a1a9 100644 --- a/modules/light-clients/07-tendermint/types/client_state.go +++ b/modules/light-clients/07-tendermint/types/client_state.go @@ -65,7 +65,7 @@ func (cs ClientState) GetTimestampAtHeight( cdc codec.BinaryCodec, height exported.Height, ) (uint64, error) { - // get latest consensus state from clientStore to check for expiry + // get consensus state at height from clientStore to check for expiry consState, err := GetConsensusState(clientStore, cdc, height) if err != nil { return 0, sdkerrors.Wrapf(err, "height (%s)", height) diff --git a/modules/light-clients/09-localhost/types/client_state.go b/modules/light-clients/09-localhost/types/client_state.go index 50d0279c5a8..cf35878f218 100644 --- a/modules/light-clients/09-localhost/types/client_state.go +++ b/modules/light-clients/09-localhost/types/client_state.go @@ -49,7 +49,7 @@ func (cs ClientState) GetTimestampAtHeight( _ codec.BinaryCodec, _ exported.Height, ) (uint64, error) { - return 0, nil + return 0, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "localhost client has no consensus state") } // Status always returns Active. The localhost status cannot be changed. diff --git a/modules/light-clients/09-localhost/types/client_state_test.go b/modules/light-clients/09-localhost/types/client_state_test.go index f9d53cb3640..70d15c0f1fb 100644 --- a/modules/light-clients/09-localhost/types/client_state_test.go +++ b/modules/light-clients/09-localhost/types/client_state_test.go @@ -534,20 +534,19 @@ func (suite *LocalhostTestSuite) TestGetTimestampAtHeight() { clientState *types.ClientState malleate func() checkHeight exported.Height - expValue int // this is always 0 for localhost client }{ { - name: "get timestamp at height returns 0", + name: "get timestamp at height returns error", clientState: types.NewClientState("chainID", clientHeight), checkHeight: clientHeight, }, { - name: "get timestamp at client height + 1 returns 0", + name: "get timestamp at client height + 1 returns error", clientState: types.NewClientState("chainID", clientHeight), checkHeight: clientHeight.Increment(), }, { - name: "get timestamp at client height + 2 returns 0", + name: "get timestamp at client height + 2 returns error", clientState: types.NewClientState("chainID", clientHeight), checkHeight: clientHeight.Increment().Increment(), }, @@ -558,14 +557,12 @@ func (suite *LocalhostTestSuite) TestGetTimestampAtHeight() { suite.Run(tc.name, func() { suite.SetupTest() - tc.malleate() - ts, err := tc.clientState.GetTimestampAtHeight( + _, err := tc.clientState.GetTimestampAtHeight( suite.ctx, suite.store, suite.cdc, clientHeight, ) - suite.Require().NoError(err) - suite.Require().Equal(tc.expValue, ts) + suite.Require().Error(err) }) } } From 64573b5b9f2abd6415d39a6f499550c386cd4390 Mon Sep 17 00:00:00 2001 From: Bo Du Date: Fri, 25 Mar 2022 01:26:23 -0400 Subject: [PATCH 6/7] fix: expected height is next height --- modules/light-clients/06-solomachine/types/client_state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/light-clients/06-solomachine/types/client_state.go b/modules/light-clients/06-solomachine/types/client_state.go index 99914a34731..cca6f90ca2f 100644 --- a/modules/light-clients/06-solomachine/types/client_state.go +++ b/modules/light-clients/06-solomachine/types/client_state.go @@ -46,7 +46,7 @@ func (cs ClientState) GetTimestampAtHeight( cdc codec.BinaryCodec, height exported.Height, ) (uint64, error) { - if !cs.GetLatestHeight().EQ(height) { + if !cs.GetLatestHeight().Increment().EQ(height) { return 0, sdkerrors.Wrapf(ErrInvalidSequence, "not latest height (%s)", height) } return cs.ConsensusState.Timestamp, nil From dcec344211bf4e70b7c4643a227f4f05392218d3 Mon Sep 17 00:00:00 2001 From: Bo Du Date: Fri, 25 Mar 2022 01:31:04 -0400 Subject: [PATCH 7/7] chore: add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c302b4c759e..663b11f848b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -92,6 +92,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* (client) [\#888](https://github.com/cosmos/ibc-go/pull/888) Add `GetTimestampAtHeight` to `ClientState` * (interchain-accounts) [\#1037](https://github.com/cosmos/ibc-go/pull/1037) Add a function `InitModule` to the interchain accounts `AppModule`. This function should be called within the upgrade handler when adding the interchain accounts module to a chain. It should be called in place of InitGenesis (set the consensus version in the version map). * (testing) [\#1003](https://github.com/cosmos/ibc-go/pull/1003) Testing chain's `Signer` fields has changed from `[]tmtypes.PrivValidator` to `map[string]tmtypes.PrivValidator` to accomodate valset updates changing the order of the ValidatorSet. * (testing) [\#1003](https://github.com/cosmos/ibc-go/pull/1003) `SignAndDeliver` will now just deliver the transaction without creating and committing a block. Thus, it requires that `BeginBlock` MUST be called before `SignAndDeliver`