Skip to content

Commit

Permalink
chore: CheckSubstituteAndUpdateState stores client state in lightclie…
Browse files Browse the repository at this point in the history
…nts impl (cosmos#1170)

* set client state in store in lightclient

* adding changelog entry
  • Loading branch information
damiannolan authored and seunlanlege committed Aug 9, 2022
1 parent d9f29cc commit d1551da
Show file tree
Hide file tree
Showing 5 changed files with 602 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Improvements
* [\#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) [\#1170](https://github.com/cosmos/ibc-go/pull/1170) Updating `ClientUpdateProposal` to set client state in lightclient implementations `CheckSubstituteAndUpdateState` methods.

### Features

Expand Down
38 changes: 24 additions & 14 deletions modules/light-clients/06-solomachine/proposal_handle.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package solomachine
package types

import (
"reflect"
Expand All @@ -7,8 +7,8 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

clienttypes "github.com/cosmos/ibc-go/v5/modules/core/02-client/types"
"github.com/cosmos/ibc-go/v5/modules/core/exported"
clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
"github.com/cosmos/ibc-go/v3/modules/core/exported"
)

// CheckSubstituteAndUpdateState verifies that the subject is allowed to be updated by
Expand All @@ -21,36 +21,46 @@ import (
func (cs ClientState) CheckSubstituteAndUpdateState(
ctx sdk.Context, cdc codec.BinaryCodec, subjectClientStore,
_ sdk.KVStore, substituteClient exported.ClientState,
) error {
) (exported.ClientState, error) {

if !cs.AllowUpdateAfterProposal {
return sdkerrors.Wrapf(clienttypes.ErrUpdateClientFailed, "solo machine client is not allowed to updated with a proposal")
return nil, sdkerrors.Wrapf(
clienttypes.ErrUpdateClientFailed,
"solo machine client is not allowed to updated with a proposal",
)
}

substituteClientState, ok := substituteClient.(*ClientState)
if !ok {
return sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "substitute client state type %T, expected %T", substituteClient, &ClientState{})
return nil, sdkerrors.Wrapf(
clienttypes.ErrInvalidClientType, "substitute client state type %T, expected %T", substituteClient, &ClientState{},
)
}

subjectPublicKey, err := cs.ConsensusState.GetPubKey()
if err != nil {
return sdkerrors.Wrap(err, "failed to get consensus public key")
return nil, sdkerrors.Wrap(err, "failed to get consensus public key")
}

substitutePublicKey, err := substituteClientState.ConsensusState.GetPubKey()
if err != nil {
return sdkerrors.Wrap(err, "failed to get substitute client public key")
return nil, sdkerrors.Wrap(err, "failed to get substitute client public key")
}

if reflect.DeepEqual(subjectPublicKey, substitutePublicKey) {
return sdkerrors.Wrapf(clienttypes.ErrInvalidHeader, "subject and substitute have the same public key")
return nil, sdkerrors.Wrapf(
clienttypes.ErrInvalidHeader, "subject and substitute have the same public key",
)
}

clientState := &cs

// update to substitute parameters
cs.Sequence = substituteClientState.Sequence
cs.ConsensusState = substituteClientState.ConsensusState
cs.IsFrozen = false
clientState.Sequence = substituteClientState.Sequence
clientState.ConsensusState = substituteClientState.ConsensusState
clientState.IsFrozen = false

setClientState(subjectClientStore, cdc, &cs)
setClientState(subjectClientStore, cdc, clientState)

return nil
return clientState, nil
}
94 changes: 94 additions & 0 deletions modules/light-clients/06-solomachine/types/proposal_handle_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package types_test

import (
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"
"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"
ibctesting "github.com/cosmos/ibc-go/v3/testing"
)

func (suite *SoloMachineTestSuite) TestCheckSubstituteAndUpdateState() {
var (
subjectClientState *types.ClientState
substituteClientState exported.ClientState
)

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

testCases := []struct {
name string
malleate func()
expPass bool
}{
{
"valid substitute", func() {
subjectClientState.AllowUpdateAfterProposal = true
}, true,
},
{
"subject not allowed to be updated", func() {
subjectClientState.AllowUpdateAfterProposal = false
}, false,
},
{
"substitute is not the solo machine", func() {
substituteClientState = &ibctmtypes.ClientState{}
}, false,
},
{
"subject public key is nil", func() {
subjectClientState.ConsensusState.PublicKey = nil
}, false,
},

{
"substitute public key is nil", func() {
substituteClientState.(*types.ClientState).ConsensusState.PublicKey = nil
}, false,
},
{
"subject and substitute use the same public key", func() {
substituteClientState.(*types.ClientState).ConsensusState.PublicKey = subjectClientState.ConsensusState.PublicKey
}, false,
},
}

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

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

subjectClientState = solomachine.ClientState()
subjectClientState.AllowUpdateAfterProposal = true
substitute := ibctesting.NewSolomachine(suite.T(), suite.chainA.Codec, "substitute", "testing", 5)
substituteClientState = substitute.ClientState()

tc.malleate()

subjectClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), solomachine.ClientID)
substituteClientStore := suite.chainA.App.GetIBCKeeper().ClientKeeper.ClientStore(suite.chainA.GetContext(), substitute.ClientID)

updatedClient, err := subjectClientState.CheckSubstituteAndUpdateState(suite.chainA.GetContext(), suite.chainA.App.AppCodec(), subjectClientStore, substituteClientStore, substituteClientState)

if tc.expPass {
suite.Require().NoError(err)

suite.Require().Equal(substituteClientState.(*types.ClientState).ConsensusState, updatedClient.(*types.ClientState).ConsensusState)
suite.Require().Equal(substituteClientState.(*types.ClientState).Sequence, updatedClient.(*types.ClientState).Sequence)
suite.Require().Equal(false, updatedClient.(*types.ClientState).IsFrozen)

// ensure updated client state is set in store
bz := subjectClientStore.Get(host.ClientStateKey())
suite.Require().Equal(clienttypes.MustMarshalClientState(suite.chainA.Codec, updatedClient), bz)
} else {
suite.Require().Error(err)
suite.Require().Nil(updatedClient)
}
})
}
}
}
107 changes: 107 additions & 0 deletions modules/light-clients/07-tendermint/types/proposal_handle.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
package types

import (
"reflect"

"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"
"github.com/cosmos/ibc-go/v3/modules/core/exported"
)

// CheckSubstituteAndUpdateState will try to update the client with the state of the
// substitute if and only if the proposal passes and one of the following conditions are
// satisfied:
// 1) AllowUpdateAfterMisbehaviour and Status() == Frozen
// 2) AllowUpdateAfterExpiry=true and Status() == Expired
//
// The following must always be true:
// - The substitute client is the same type as the subject client
// - The subject and substitute client states match in all parameters (expect frozen height, latest height, and chain-id)
//
// In case 1) before updating the client, the client will be unfrozen by resetting
// the FrozenHeight to the zero Height. If a client is frozen and AllowUpdateAfterMisbehaviour
// is set to true, the client will be unexpired even if AllowUpdateAfterExpiry is set to false.
func (cs ClientState) CheckSubstituteAndUpdateState(
ctx sdk.Context, cdc codec.BinaryCodec, subjectClientStore,
substituteClientStore sdk.KVStore, substituteClient exported.ClientState,
) (exported.ClientState, error) {
substituteClientState, ok := substituteClient.(*ClientState)
if !ok {
return nil, sdkerrors.Wrapf(
clienttypes.ErrInvalidClient, "expected type %T, got %T", &ClientState{}, substituteClient,
)
}

if !IsMatchingClientState(cs, *substituteClientState) {
return nil, sdkerrors.Wrap(clienttypes.ErrInvalidSubstitute, "subject client state does not match substitute client state")
}

switch cs.Status(ctx, subjectClientStore, cdc) {

case exported.Frozen:
if !cs.AllowUpdateAfterMisbehaviour {
return nil, sdkerrors.Wrap(clienttypes.ErrUpdateClientFailed, "client is not allowed to be unfrozen")
}

// unfreeze the client
cs.FrozenHeight = clienttypes.ZeroHeight()

case exported.Expired:
if !cs.AllowUpdateAfterExpiry {
return nil, sdkerrors.Wrap(clienttypes.ErrUpdateClientFailed, "client is not allowed to be unexpired")
}

default:
return nil, sdkerrors.Wrap(clienttypes.ErrUpdateClientFailed, "client cannot be updated with proposal")
}

// copy consensus states and processed time from substitute to subject
// starting from initial height and ending on the latest height (inclusive)
height := substituteClientState.GetLatestHeight()

consensusState, err := GetConsensusState(substituteClientStore, cdc, height)
if err != nil {
return nil, sdkerrors.Wrap(err, "unable to retrieve latest consensus state for substitute client")
}

SetConsensusState(subjectClientStore, cdc, consensusState, height)

// set metadata stored for the substitute consensus state
processedHeight, found := GetProcessedHeight(substituteClientStore, height)
if !found {
return nil, sdkerrors.Wrap(clienttypes.ErrUpdateClientFailed, "unable to retrieve processed height for substitute client latest height")
}

processedTime, found := GetProcessedTime(substituteClientStore, height)
if !found {
return nil, sdkerrors.Wrap(clienttypes.ErrUpdateClientFailed, "unable to retrieve processed time for substitute client latest height")
}

setConsensusMetadataWithValues(subjectClientStore, height, processedHeight, processedTime)

cs.LatestHeight = substituteClientState.LatestHeight
cs.ChainId = substituteClientState.ChainId

// no validation is necessary since the substitute is verified to be Active
// in 02-client.
setClientState(subjectClientStore, cdc, &cs)

return &cs, nil
}

// IsMatchingClientState returns true if all the client state parameters match
// except for frozen height, latest height, and chain-id.
func IsMatchingClientState(subject, substitute ClientState) bool {
// zero out parameters which do not need to match
subject.LatestHeight = clienttypes.ZeroHeight()
subject.FrozenHeight = clienttypes.ZeroHeight()
substitute.LatestHeight = clienttypes.ZeroHeight()
substitute.FrozenHeight = clienttypes.ZeroHeight()
subject.ChainId = ""
substitute.ChainId = ""

return reflect.DeepEqual(subject, substitute)
}
Loading

0 comments on commit d1551da

Please sign in to comment.