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

fix genesis exporting of tendermint metadata #210

Merged
merged 3 commits into from
Jun 8, 2021
Merged
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 @@ -38,6 +38,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Bug Fixes

* (07-tendermint) [#\210](https://github.com/cosmos/ibc-go/pull/210) Export all consensus metadata on genesis restarts for tendermint clients.
* (core) [\#200](https://github.com/cosmos/ibc-go/pull/200) Fixes incorrect export of IBC identifier sequences. Previously, the next identifier sequence for clients/connections/channels was not set during genesis export. This resulted in the next identifiers being generated on the new chain to reuse old identifiers (the sequences began again from 0).
* (02-client) [\#192](https://github.com/cosmos/ibc-go/pull/192) Fix IBC `query ibc client header` cli command. Support historical queries for query header/node-state commands.
* (modules/light-clients/06-solomachine) [\#153](https://github.com/cosmos/ibc-go/pull/153) Fix solo machine proof height sequence mismatch bug.
Expand Down
4 changes: 2 additions & 2 deletions modules/light-clients/07-tendermint/types/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ import (
"github.com/cosmos/ibc-go/modules/core/exported"
)

// ExportMetadata exports all the processed times in the client store so they can be included in clients genesis
// ExportMetadata exports all the consensus metadata in the client store so they can be included in clients genesis
// and imported by a ClientKeeper
func (cs ClientState) ExportMetadata(store sdk.KVStore) []exported.GenesisMetadata {
gm := make([]exported.GenesisMetadata, 0)
IterateProcessedTime(store, func(key, val []byte) bool {
IterateConsensusMetadata(store, func(key, val []byte) bool {
gm = append(gm, clienttypes.NewGenesisMetadata(key, val))
return false
})
Expand Down
93 changes: 72 additions & 21 deletions modules/light-clients/07-tendermint/types/genesis_test.go
Original file line number Diff line number Diff line change
@@ -1,38 +1,89 @@
package types_test

import (
"time"

sdk "github.com/cosmos/cosmos-sdk/types"

clienttypes "github.com/cosmos/ibc-go/modules/core/02-client/types"
commitmenttypes "github.com/cosmos/ibc-go/modules/core/23-commitment/types"
"github.com/cosmos/ibc-go/modules/light-clients/07-tendermint/types"
ibctesting "github.com/cosmos/ibc-go/testing"
)

// expected export ordering:
// processed height and processed time per height
// then all iteration keys
func (suite *TendermintTestSuite) TestExportMetadata() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test could be better using a for loop for the updates, but it doesn't feel worth the effort. I think this covers the necessary purposes

clientState := types.NewClientState(chainID, types.DefaultTrustLevel, trustingPeriod, ubdPeriod, maxClockDrift, height, commitmenttypes.GetSDKSpecs(), upgradePath, false, false)
suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), "clientA", clientState)
// test intializing client and exporting metadata
path := ibctesting.NewPath(suite.chainA, suite.chainB)
suite.coordinator.SetupClients(path)
clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID)
clientState := path.EndpointA.GetClientState()
height := clientState.GetLatestHeight()

initIteration := types.GetIterationKey(clientStore, height)
suite.Require().NotEqual(0, len(initIteration))
initProcessedTime, found := types.GetProcessedTime(clientStore, height)
suite.Require().True(found)
initProcessedHeight, found := types.GetProcessedHeight(clientStore, height)
suite.Require().True(found)

gm := clientState.ExportMetadata(suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID))
suite.Require().NotNil(gm, "client with metadata returned nil exported metadata")
suite.Require().Len(gm, 3, "exported metadata has unexpected length")

suite.Require().Equal(types.ProcessedHeightKey(height), gm[0].GetKey(), "metadata has unexpected key")
actualProcessedHeight, err := clienttypes.ParseHeight(string(gm[0].GetValue()))
suite.Require().NoError(err)
suite.Require().Equal(initProcessedHeight, actualProcessedHeight, "metadata has unexpected value")

suite.Require().Equal(types.ProcessedTimeKey(height), gm[1].GetKey(), "metadata has unexpected key")
suite.Require().Equal(initProcessedTime, sdk.BigEndianToUint64(gm[1].GetValue()), "metadata has unexpected value")

gm := clientState.ExportMetadata(suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), "clientA"))
suite.Require().Nil(gm, "client with no metadata returned non-nil exported metadata")
suite.Require().Equal(types.IterationKey(height), gm[2].GetKey(), "metadata has unexpected key")
suite.Require().Equal(initIteration, gm[2].GetValue(), "metadata has unexpected value")

clientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), "clientA")
// test updating client and exporting metadata
err = path.EndpointA.UpdateClient()
suite.Require().NoError(err)

// set some processed times
timestamp1 := uint64(time.Now().UnixNano())
timestamp2 := uint64(time.Now().Add(time.Minute).UnixNano())
timestampBz1 := sdk.Uint64ToBigEndian(timestamp1)
timestampBz2 := sdk.Uint64ToBigEndian(timestamp2)
types.SetProcessedTime(clientStore, clienttypes.NewHeight(0, 1), timestamp1)
types.SetProcessedTime(clientStore, clienttypes.NewHeight(0, 2), timestamp2)
clientState = path.EndpointA.GetClientState()
updateHeight := clientState.GetLatestHeight()

gm = clientState.ExportMetadata(suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), "clientA"))
iteration := types.GetIterationKey(clientStore, updateHeight)
suite.Require().NotEqual(0, len(initIteration))
processedTime, found := types.GetProcessedTime(clientStore, updateHeight)
suite.Require().True(found)
processedHeight, found := types.GetProcessedHeight(clientStore, updateHeight)
suite.Require().True(found)

gm = clientState.ExportMetadata(suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), path.EndpointA.ClientID))
suite.Require().NotNil(gm, "client with metadata returned nil exported metadata")
suite.Require().Len(gm, 2, "exported metadata has unexpected length")
suite.Require().Len(gm, 6, "exported metadata has unexpected length")

// expected ordering:
// initProcessedHeight, initProcessedTime, processedHeight, processedTime, initIteration, iteration

// check init processed height and time
suite.Require().Equal(types.ProcessedHeightKey(height), gm[0].GetKey(), "metadata has unexpected key")
actualProcessedHeight, err = clienttypes.ParseHeight(string(gm[0].GetValue()))
suite.Require().NoError(err)
suite.Require().Equal(initProcessedHeight, actualProcessedHeight, "metadata has unexpected value")

suite.Require().Equal(types.ProcessedTimeKey(height), gm[1].GetKey(), "metadata has unexpected key")
suite.Require().Equal(initProcessedTime, sdk.BigEndianToUint64(gm[1].GetValue()), "metadata has unexpected value")

// check processed height and time after update
suite.Require().Equal(types.ProcessedHeightKey(updateHeight), gm[2].GetKey(), "metadata has unexpected key")
actualProcessedHeight, err = clienttypes.ParseHeight(string(gm[2].GetValue()))
suite.Require().NoError(err)
suite.Require().Equal(processedHeight, actualProcessedHeight, "metadata has unexpected value")

suite.Require().Equal(types.ProcessedTimeKey(updateHeight), gm[3].GetKey(), "metadata has unexpected key")
suite.Require().Equal(processedTime, sdk.BigEndianToUint64(gm[3].GetValue()), "metadata has unexpected value")

suite.Require().Equal(types.ProcessedTimeKey(clienttypes.NewHeight(0, 1)), gm[0].GetKey(), "metadata has unexpected key")
suite.Require().Equal(timestampBz1, gm[0].GetValue(), "metadata has unexpected value")
// check iteration keys
suite.Require().Equal(types.IterationKey(height), gm[4].GetKey(), "metadata has unexpected key")
suite.Require().Equal(initIteration, gm[4].GetValue(), "metadata has unexpected value")

suite.Require().Equal(types.ProcessedTimeKey(clienttypes.NewHeight(0, 2)), gm[1].GetKey(), "metadata has unexpected key")
suite.Require().Equal(timestampBz2, gm[1].GetValue(), "metadata has unexpected value")
suite.Require().Equal(types.IterationKey(updateHeight), gm[5].GetKey(), "metadata has unexpected key")
suite.Require().Equal(iteration, gm[5].GetValue(), "metadata has unexpected value")
}
23 changes: 20 additions & 3 deletions modules/light-clients/07-tendermint/types/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,20 +82,37 @@ func deleteConsensusState(clientStore sdk.KVStore, height exported.Height) {
clientStore.Delete(key)
}

// IterateProcessedTime iterates through the prefix store and applies the callback.
// IterateConsensusMetadata iterates through the prefix store and applies the callback.
// If the cb returns true, then iterator will close and stop.
func IterateProcessedTime(store sdk.KVStore, cb func(key, val []byte) bool) {
func IterateConsensusMetadata(store sdk.KVStore, cb func(key, val []byte) bool) {
iterator := sdk.KVStorePrefixIterator(store, []byte(host.KeyConsensusStatePrefix))

// iterate over processed time and processed height
defer iterator.Close()
for ; iterator.Valid(); iterator.Next() {
keySplit := strings.Split(string(iterator.Key()), "/")
// processed time key in prefix store has format: "consensusState/<height>/processedTime"
if len(keySplit) != 3 || keySplit[2] != "processedTime" {
if len(keySplit) != 3 {
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
// ignore all consensus state keys
continue

}

if keySplit[2] != "processedTime" && keySplit[2] != "processedHeight" {
// only perform callback on consensus metadata
continue
}

if cb(iterator.Key(), iterator.Value()) {
break
}
}

// iterate over iteration keys
iterator = sdk.KVStorePrefixIterator(store, []byte(KeyIterateConsensusStatePrefix))

defer iterator.Close()
for ; iterator.Valid(); iterator.Next() {
Comment on lines +112 to +115
Copy link
Member

Choose a reason for hiding this comment

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

Ahh I see this, good work!

if cb(iterator.Key(), iterator.Value()) {
break
}
Expand Down