Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add GetTimestampAtHeight to client state #888

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
7 changes: 7 additions & 0 deletions modules/core/02-client/legacy/v100/solomachine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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!")
Expand Down
6 changes: 6 additions & 0 deletions modules/core/exported/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions modules/light-clients/06-solomachine/types/client_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,19 @@ func (cs ClientState) GetLatestHeight() exported.Height {
return clienttypes.NewHeight(0, cs.Sequence)
}

// GetTimestampAtHeight returns the timestamp in nanoseconds of the consensus state at the given height.
func (cs ClientState) GetTimestampAtHeight(
_ sdk.Context,
clientStore sdk.KVStore,
cdc codec.BinaryCodec,
height exported.Height,
) (uint64, error) {
if !cs.GetLatestHeight().Increment().EQ(height) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if !cs.GetLatestHeight().Increment().EQ(height) {
if !cs.GetLatestHeight().EQ(height) {

Sorry I was incorrect!

The solo machine stores the current sequence of the next proof it'll verify. In the timeout case it'll either be verifying absence of next seq recv (ordered channels) or packet receipt absence (unordered channels). I'm not sure why I got mixed up, but the sequence is incremented after verifying and this function is always called before verifying. So the height passed in should match the current sequence. The only time it won't is during connection handshake verification which doesn't need to rely on GetTimestampAtHeight

return 0, sdkerrors.Wrapf(ErrInvalidSequence, "not latest height (%s)", height)
}
return cs.ConsensusState.Timestamp, nil
}

// Status returns the status of the solo machine client.
// The client may be:
// - Active: if frozen sequence is 0
Expand Down
50 changes: 49 additions & 1 deletion modules/light-clients/06-solomachine/types/client_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,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)

Expand Down Expand Up @@ -845,3 +845,51 @@ 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
height exported.Height
expValue uint64
expPass bool
}{
{
name: "get timestamp at height exists",
clientState: suite.solomachine.ClientState(),
height: suite.solomachine.ClientState().GetLatestHeight(),
expValue: suite.solomachine.ClientState().ConsensusState.Timestamp,
expPass: true,
},
{
name: "get timestamp at height not exists",
clientState: suite.solomachine.ClientState(),
height: suite.solomachine.ClientState().GetLatestHeight().Increment(),
},
}

for i, tc := range testCases {
tc := tc

suite.Run(tc.name, func() {
ctx := suite.chainA.GetContext()

ts, err := tc.clientState.GetTimestampAtHeight(
ctx, suite.store, suite.chainA.Codec, tc.height,
)

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)
}
})
}
}
15 changes: 15 additions & 0 deletions modules/light-clients/07-tendermint/types/client_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,21 @@ 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 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)
}
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
Expand Down
47 changes: 47 additions & 0 deletions modules/light-clients/07-tendermint/types/client_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}
10 changes: 10 additions & 0 deletions modules/light-clients/09-localhost/types/client_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "localhost client has no consensus state")
}

// Status always returns Active. The localhost status cannot be changed.
func (cs ClientState) Status(_ sdk.Context, _ sdk.KVStore, _ codec.BinaryCodec,
) exported.Status {
Expand Down
39 changes: 39 additions & 0 deletions modules/light-clients/09-localhost/types/client_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -527,3 +527,42 @@ func (suite *LocalhostTestSuite) TestVerifyNextSeqRecv() {
})
}
}

func (suite *LocalhostTestSuite) TestGetTimestampAtHeight() {
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
testCases := []struct {
name string
clientState *types.ClientState
malleate func()
checkHeight exported.Height
}{
{
name: "get timestamp at height returns error",
clientState: types.NewClientState("chainID", clientHeight),
checkHeight: clientHeight,
},
{
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 error",
clientState: types.NewClientState("chainID", clientHeight),
checkHeight: clientHeight.Increment().Increment(),
},
}

for _, tc := range testCases {
tc := tc

suite.Run(tc.name, func() {
suite.SetupTest()

_, err := tc.clientState.GetTimestampAtHeight(
suite.ctx, suite.store, suite.cdc, clientHeight,
)

suite.Require().Error(err)
})
}
}