Skip to content

Commit

Permalink
refactor: removing CheckHeaderAndUpdateState from ClientState (#1210)
Browse files Browse the repository at this point in the history 
* refactor: removing CheckHeaderAndUpdateState from ClientState interface & light client implementations

* chore: fix changelog
  • Loading branch information
@seantking
seantking authored Apr 4, 2022
1 parent e2f37b8 commit 8a9978c
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 440 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,14 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements

* (02-client) [\#1208](https://github.com/cosmos/ibc-go/pull/1208) Replace `CheckHeaderAndUpdateState` usage in 02-client with calls to `VerifyClientMessage`, `CheckForMisbehaviour`, `UpdateStateOnMisbehaviour` and `UpdateState`.
* (modules/core/02-client) [\#1208](https://github.com/cosmos/ibc-go/pull/1208) Replace `CheckHeaderAndUpdateState` usage in 02-client with calls to `VerifyClientMessage`, `CheckForMisbehaviour`, `UpdateStateOnMisbehaviour` and `UpdateState`.
* (modules/light-clients/09-localhost) [\#1187](https://github.com/cosmos/ibc-go/pull/1187/) Removing localhost light client implementation as it is not functional.
* [\#1186](https://github.com/cosmos/ibc-go/pull/1186/files) Removing `GetRoot` function from ConsensusState interface in `02-client`. `GetRoot` is unused by core IBC.
* (modules/core/02-client) [\#1196](https://github.com/cosmos/ibc-go/pull/1196) Adding VerifyClientMessage to ClientState interface.
* (modules/core/02-client) [\#1198](https://github.com/cosmos/ibc-go/pull/1198) Adding UpdateStateOnMisbehaviour to ClientState interface.
* (modules/core/02-client) [\#1170](https://github.com/cosmos/ibc-go/pull/1170) Updating `ClientUpdateProposal` to set client state in lightclient implementations `CheckSubstituteAndUpdateState` methods.
* (modules/core/02-client) [\#1197](https://github.com/cosmos/ibc-go/pull/1197) Adding `CheckForMisbehaviour` to `ClientState` interface.
* (modules/core/02-client) [\#1195](https://github.com/cosmos/ibc-go/pull/1210) Removing `CheckHeaderAndUpdateState` from `ClientState` interface & associated light client implementations.
* (modules/core/exported) [\#1206](https://github.com/cosmos/ibc-go/pull/1206) Adding new method `UpdateState` to `ClientState` interface.

### Features
Expand Down
1 change: 0 additions & 1 deletion modules/core/exported/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ type ClientState interface {
UpdateState(sdk.Context, codec.BinaryCodec, sdk.KVStore, ClientMessage) error

// Update and Misbehaviour functions
CheckHeaderAndUpdateState(sdk.Context, codec.BinaryCodec, sdk.KVStore, ClientMessage) (ClientState, ConsensusState, error)
CheckMisbehaviourAndUpdateState(sdk.Context, codec.BinaryCodec, sdk.KVStore, ClientMessage) (ClientState, error)
CheckSubstituteAndUpdateState(ctx sdk.Context, cdc codec.BinaryCodec, subjectClientStore, substituteClientStore sdk.KVStore, substituteClient ClientState) (ClientState, error)

Expand Down
29 changes: 0 additions & 29 deletions modules/light-clients/06-solomachine/types/update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,35 +10,6 @@ import (
"github.com/cosmos/ibc-go/v3/modules/core/exported"
)

// CheckHeaderAndUpdateState checks if the provided header is valid and updates
// the consensus state if appropriate. It returns an error if:
// - the header provided is not parseable to a solo machine header
// - the header sequence does not match the current sequence
// - the header timestamp is less than the consensus state timestamp
// - the currently registered public key did not provide the update signature
func (cs ClientState) CheckHeaderAndUpdateState(
ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore,
msg exported.ClientMessage,
) (exported.ClientState, exported.ConsensusState, error) {
if err := cs.VerifyClientMessage(ctx, cdc, clientStore, msg); err != nil {
return nil, nil, err
}

foundMisbehaviour := cs.CheckForMisbehaviour(ctx, cdc, clientStore, msg)
if foundMisbehaviour {
cs.UpdateStateOnMisbehaviour(ctx, cdc, clientStore, msg)
return &cs, cs.ConsensusState, nil
}

if err := cs.UpdateState(ctx, cdc, clientStore, msg); err != nil {
return nil, nil, err
}

newClientState := clienttypes.MustUnmarshalClientState(cdc, clientStore.Get(host.ClientStateKey())).(*ClientState)

return newClientState, newClientState.ConsensusState, nil
}

// VerifyClientMessage introspects the provided ClientMessage and checks its validity
// A Solomachine Header is considered valid if the currently registered public key has signed over the new public key with the correct sequence
// A Solomachine Misbehaviour is considered valid if duplicate signatures of the current public key are found on two different messages at a given sequence
Expand Down
171 changes: 0 additions & 171 deletions modules/light-clients/06-solomachine/types/update_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,177 +12,6 @@ import (
ibctesting "github.com/cosmos/ibc-go/v3/testing"
)

func (suite *SoloMachineTestSuite) TestCheckHeaderAndUpdateState() {
var (
clientState exported.ClientState
header exported.ClientMessage
)

// test singlesig and multisig public keys
for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} {

testCases := []struct {
name string
setup func()
expPass bool
}{
{
"successful update",
func() {
clientState = solomachine.ClientState()
header = solomachine.CreateHeader()
},
true,
},
{
"wrong client state type",
func() {
clientState = &ibctmtypes.ClientState{}
header = solomachine.CreateHeader()
},
false,
},
{
"invalid header type",
func() {
clientState = solomachine.ClientState()
header = &ibctmtypes.Header{}
},
false,
},
{
"wrong sequence in header",
func() {
clientState = solomachine.ClientState()
// store in temp before assigning to interface type
h := solomachine.CreateHeader()
h.Sequence++
header = h
},
false,
},
{
"invalid header Signature",
func() {
clientState = solomachine.ClientState()
h := solomachine.CreateHeader()
h.Signature = suite.GetInvalidProof()
header = h
}, false,
},
{
"invalid timestamp in header",
func() {
clientState = solomachine.ClientState()
h := solomachine.CreateHeader()
h.Timestamp--
header = h
}, false,
},
{
"signature uses wrong sequence",
func() {
clientState = solomachine.ClientState()
solomachine.Sequence++
header = solomachine.CreateHeader()
},
false,
},
{
"signature uses new pubkey to sign",
func() {
// store in temp before assinging to interface type
cs := solomachine.ClientState()
h := solomachine.CreateHeader()

publicKey, err := codectypes.NewAnyWithValue(solomachine.PublicKey)
suite.NoError(err)

data := &types.HeaderData{
NewPubKey: publicKey,
NewDiversifier: h.NewDiversifier,
}

dataBz, err := suite.chainA.Codec.Marshal(data)
suite.Require().NoError(err)

// generate invalid signature
signBytes := &types.SignBytes{
Sequence: cs.Sequence,
Timestamp: solomachine.Time,
Diversifier: solomachine.Diversifier,
DataType: types.CLIENT,
Data: dataBz,
}

signBz, err := suite.chainA.Codec.Marshal(signBytes)
suite.Require().NoError(err)

sig := solomachine.GenerateSignature(signBz)
suite.Require().NoError(err)
h.Signature = sig

clientState = cs
header = h

},
false,
},
{
"signature signs over old pubkey",
func() {
// store in temp before assinging to interface type
cs := solomachine.ClientState()
oldPubKey := solomachine.PublicKey
h := solomachine.CreateHeader()

// generate invalid signature
data := append(sdk.Uint64ToBigEndian(cs.Sequence), oldPubKey.Bytes()...)
sig := solomachine.GenerateSignature(data)
h.Signature = sig

clientState = cs
header = h
},
false,
},
{
"consensus state public key is nil",
func() {
cs := solomachine.ClientState()
cs.ConsensusState.PublicKey = nil
clientState = cs
header = solomachine.CreateHeader()
},
false,
},
}

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

suite.Run(tc.name, func() {
// setup test
tc.setup()

clientState, consensusState, err := clientState.CheckHeaderAndUpdateState(suite.chainA.GetContext(), suite.chainA.Codec, suite.store, header)

if tc.expPass {
suite.Require().NoError(err)
suite.Require().Equal(header.(*types.Header).NewPublicKey, clientState.(*types.ClientState).ConsensusState.PublicKey)
suite.Require().Equal(false, clientState.(*types.ClientState).IsFrozen)
suite.Require().Equal(header.(*types.Header).Sequence+1, clientState.(*types.ClientState).Sequence)
suite.Require().Equal(consensusState, clientState.(*types.ClientState).ConsensusState)
} else {
suite.Require().Error(err)
suite.Require().Nil(clientState)
suite.Require().Nil(consensusState)
}
})
}
}
}

func (suite *SoloMachineTestSuite) TestVerifyClientMessageHeader() {
var (
clientMsg exported.ClientMessage
Expand Down
84 changes: 0 additions & 84 deletions modules/light-clients/07-tendermint/types/update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,90 +16,6 @@ import (
"github.com/cosmos/ibc-go/v3/modules/core/exported"
)

// CheckHeaderAndUpdateState checks if the provided header is valid, and if valid it will:
// create the consensus state for the header.Height
// and update the client state if the header height is greater than the latest client state height
// It returns an error if:
// - the client or header provided are not parseable to tendermint types
// - the header is invalid
// - header height is less than or equal to the trusted header height
// - header revision is not equal to trusted header revision
// - header valset commit verification fails
// - header timestamp is past the trusting period in relation to the consensus state
// - header timestamp is less than or equal to the consensus state timestamp
//
// Tendermint client validity checking uses the bisection algorithm described
// in the [Tendermint spec](https://github.com/tendermint/spec/blob/master/spec/consensus/light-client.md).
//
// Misbehaviour Detection:
// UpdateClient will detect implicit misbehaviour by enforcing certain invariants on any new update call and will return a frozen client.
// 1. Any valid update that creates a different consensus state for an already existing height is evidence of misbehaviour and will freeze client.
// 2. Any valid update that breaks time monotonicity with respect to its neighboring consensus states is evidence of misbehaviour and will freeze client.
// Misbehaviour sets frozen height to {0, 1} since it is only used as a boolean value (zero or non-zero).
//
func (cs ClientState) CheckHeaderAndUpdateState(
ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore,
header exported.ClientMessage,
) (exported.ClientState, exported.ConsensusState, error) {
tmHeader, ok := header.(*Header)
if !ok {
return nil, nil, sdkerrors.Wrapf(
clienttypes.ErrInvalidHeader, "expected type %T, got %T", &Header{}, header,
)
}

// Check if the Client store already has a consensus state for the header's height
// If the consensus state exists, and it matches the header then we return early
// since header has already been submitted in a previous UpdateClient.
var conflictingHeader bool
prevConsState, _ := GetConsensusState(clientStore, cdc, header.GetHeight())
if prevConsState != nil {
// This header has already been submitted and the necessary state is already stored
// in client store, thus we can return early without further validation.
if reflect.DeepEqual(prevConsState, tmHeader.ConsensusState()) {
return &cs, prevConsState, nil
}
// A consensus state already exists for this height, but it does not match the provided header.
// Thus, we must check that this header is valid, and if so we will freeze the client.
conflictingHeader = true
}

if err := cs.VerifyClientMessage(ctx, cdc, clientStore, tmHeader); err != nil {
return nil, nil, err
}

consState := tmHeader.ConsensusState()
// Header is different from existing consensus state and also valid, so freeze the client and return
if conflictingHeader {
cs.FrozenHeight = FrozenHeight
return &cs, consState, nil
}
// Check that consensus state timestamps are monotonic
prevCons, prevOk := GetPreviousConsensusState(clientStore, cdc, header.GetHeight())
nextCons, nextOk := GetNextConsensusState(clientStore, cdc, header.GetHeight())
// if previous consensus state exists, check consensus state time is greater than previous consensus state time
// if previous consensus state is not before current consensus state, freeze the client and return.
if prevOk && !prevCons.Timestamp.Before(consState.Timestamp) {
cs.FrozenHeight = FrozenHeight
return &cs, consState, nil
}
// if next consensus state exists, check consensus state time is less than next consensus state time
// if next consensus state is not after current consensus state, freeze the client and return.
if nextOk && !nextCons.Timestamp.After(consState.Timestamp) {
cs.FrozenHeight = FrozenHeight
return &cs, consState, nil
}

if err := cs.UpdateState(ctx, cdc, clientStore, tmHeader); err != nil {
return nil, nil, err
}

newClientState := clienttypes.MustUnmarshalClientState(cdc, clientStore.Get(host.ClientStateKey()))
newConsensusState := clienttypes.MustUnmarshalConsensusState(cdc, clientStore.Get(host.ConsensusStateKey(header.GetHeight())))

return newClientState, newConsensusState, nil
}

// checkTrustedHeader checks that consensus state matches trusted fields of Header
func checkTrustedHeader(header *Header, consState *ConsensusState) error {
tmTrustedValidators, err := tmtypes.ValidatorSetFromProto(header.TrustedValidators)
Expand Down
Loading

0 comments on commit 8a9978c

Please sign in to comment.