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

02-client-refactor: rename update to UpdateState for 07-tendermint #1117

Merged
merged 9 commits into from
Mar 24, 2022
87 changes: 54 additions & 33 deletions modules/light-clients/07-tendermint/types/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package types

import (
"bytes"
"fmt"
"reflect"
"time"

Expand Down Expand Up @@ -110,35 +111,10 @@ func (cs ClientState) CheckHeaderAndUpdateState(
return &cs, consState, nil
}

// Check the earliest consensus state to see if it is expired, if so then set the prune height
// so that we can delete consensus state and all associated metadata.
var (
pruneHeight exported.Height
pruneError error
)
pruneCb := func(height exported.Height) bool {
consState, err := GetConsensusState(clientStore, cdc, height)
// this error should never occur
if err != nil {
pruneError = err
return true
}
if cs.IsExpired(consState.Timestamp, ctx.BlockTime()) {
pruneHeight = height
}
return true
}
IterateConsensusStateAscending(clientStore, pruneCb)
if pruneError != nil {
return nil, nil, pruneError
}
// if pruneHeight is set, delete consensus state and metadata
if pruneHeight != nil {
deleteConsensusState(clientStore, pruneHeight)
deleteConsensusMetadata(clientStore, pruneHeight)
newClientState, consensusState, err := cs.UpdateState(ctx, cdc, clientStore, tmHeader)
if err != nil {
return nil, nil, err
}

newClientState, consensusState := update(ctx, clientStore, &cs, tmHeader)
return newClientState, consensusState, nil
}

Expand Down Expand Up @@ -244,11 +220,24 @@ func checkValidity(
return nil
}

// update the consensus state from a new header and set processed time metadata
func update(ctx sdk.Context, clientStore sdk.KVStore, clientState *ClientState, header *Header) (*ClientState, *ConsensusState) {
// UpdateState sets the consensus state associated with the provided header and sets the consensus metadata.
func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, msg exported.ClientMessage) (*ClientState, *ConsensusState, error) {
header, ok := msg.(*Header)
if !ok {
panic(fmt.Sprintf("client state can only be updated with a Header: expected %T, got %T)", &Header{}, msg))
}

// check for duplicate update
if consensusState, err := GetConsensusState(clientStore, cdc, header.GetHeight()); err != nil {
// perform no-op
return &cs, consensusState, nil
}

cs.pruneOldestConsensusState(ctx, cdc, clientStore)

height := header.GetHeight().(clienttypes.Height)
if height.GT(clientState.LatestHeight) {
clientState.LatestHeight = height
if height.GT(cs.LatestHeight) {
cs.LatestHeight = height
}
consensusState := &ConsensusState{
Timestamp: header.GetTime(),
Expand All @@ -259,5 +248,37 @@ func update(ctx sdk.Context, clientStore sdk.KVStore, clientState *ClientState,
// set metadata for this consensus state
setConsensusMetadata(ctx, clientStore, header.GetHeight())

return clientState, consensusState
return &cs, consensusState, nil
}

// pruneOldestConsensusState attempts to prune the oldest consensus state. The consensus state will only be pruned
// if it is expired.
func (cs ClientState) pruneOldestConsensusState(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add some logical spacing between code in this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good. Copied from existing code, but agree it could use spacing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated!

// Check the earliest consensus state to see if it is expired, if so then set the prune height
// so that we can delete consensus state and all associated metadata.
var (
pruneHeight exported.Height
pruneError error
)
pruneCb := func(height exported.Height) bool {
consState, err := GetConsensusState(clientStore, cdc, height)
// this error should never occur
if err != nil {
pruneError = err
return true
}
if cs.IsExpired(consState.Timestamp, ctx.BlockTime()) {
pruneHeight = height
}
return true
}
IterateConsensusStateAscending(clientStore, pruneCb)
if pruneError != nil {
panic(pruneError)
}
Comment on lines +276 to +278
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should just panic within the iterate function, but I can fix in a followup pr

// if pruneHeight is set, delete consensus state and metadata
if pruneHeight != nil {
deleteConsensusState(clientStore, pruneHeight)
deleteConsensusMetadata(clientStore, pruneHeight)
}
}