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 solomachine client storage #1144

Merged
merged 7 commits into from
Mar 28, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ func (cs ClientState) CheckMisbehaviourAndUpdateState(
// misbehaviour.ValidateBasic which is called by the 02-client keeper.

// verify first signature
if err := verifySignatureAndData(cdc, cs, soloMisbehaviour, soloMisbehaviour.SignatureOne); err != nil {
if err := cs.verifySignatureAndData(cdc, soloMisbehaviour, soloMisbehaviour.SignatureOne); err != nil {
return nil, sdkerrors.Wrap(err, "failed to verify signature one")
}

// verify second signature
if err := verifySignatureAndData(cdc, cs, soloMisbehaviour, soloMisbehaviour.SignatureTwo); err != nil {
if err := cs.verifySignatureAndData(cdc, soloMisbehaviour, soloMisbehaviour.SignatureTwo); err != nil {
return nil, sdkerrors.Wrap(err, "failed to verify signature two")
}

Expand All @@ -50,7 +50,7 @@ func (cs ClientState) CheckMisbehaviourAndUpdateState(
// verifySignatureAndData verifies that the currently registered public key has signed
// over the provided data and that the data is valid. The data is valid if it can be
// unmarshaled into the specified data type.
func verifySignatureAndData(cdc codec.BinaryCodec, clientState ClientState, misbehaviour *Misbehaviour, sigAndData *SignatureAndData) error {
func (cs ClientState) verifySignatureAndData(cdc codec.BinaryCodec, misbehaviour *Misbehaviour, sigAndData *SignatureAndData) error {

// do not check misbehaviour timestamp since we want to allow processing of past misbehaviour

Expand All @@ -62,7 +62,7 @@ func verifySignatureAndData(cdc codec.BinaryCodec, clientState ClientState, misb
data, err := MisbehaviourSignBytes(
cdc,
misbehaviour.Sequence, sigAndData.Timestamp,
clientState.ConsensusState.Diversifier,
cs.ConsensusState.Diversifier,
sigAndData.DataType,
sigAndData.Data,
)
Expand All @@ -75,7 +75,7 @@ func verifySignatureAndData(cdc codec.BinaryCodec, clientState ClientState, misb
return err
}

publicKey, err := clientState.ConsensusState.GetPubKey()
publicKey, err := cs.ConsensusState.GetPubKey()
if err != nil {
return err
}
Expand Down
28 changes: 22 additions & 6 deletions modules/light-clients/06-solomachine/types/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

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"
)

Expand Down Expand Up @@ -89,12 +90,12 @@ func (cs ClientState) verifyMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec,
// NOTE: a check that the misbehaviour message data are not equal is done by
// misbehaviour.ValidateBasic which is called by the 02-client keeper.
// verify first signature
if err := verifySignatureAndData(cdc, cs, misbehaviour, misbehaviour.SignatureOne); err != nil {
if err := cs.verifySignatureAndData(cdc, misbehaviour, misbehaviour.SignatureOne); err != nil {
return sdkerrors.Wrap(err, "failed to verify signature one")
}

// verify second signature
if err := verifySignatureAndData(cdc, cs, misbehaviour, misbehaviour.SignatureTwo); err != nil {
if err := cs.verifySignatureAndData(cdc, misbehaviour, misbehaviour.SignatureTwo); err != nil {
return sdkerrors.Wrap(err, "failed to verify signature two")
}

Expand All @@ -103,7 +104,12 @@ func (cs ClientState) verifyMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec,

// UpdateState updates the consensus state to the new public key and an incremented sequence.
func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, clientMsg exported.ClientMessage) (exported.ClientState, exported.ConsensusState, error) {
smHeader := clientMsg.(*Header)
smHeader, ok := clientMsg.(*Header)
if !ok {
return nil, nil, sdkerrors.Wrapf(clienttypes.ErrInvalidClientType, "expected %T got %T", Header{}, clientMsg)
}

// create new solomachine ConsensusState
consensusState := &ConsensusState{
PublicKey: smHeader.NewPublicKey,
Diversifier: smHeader.NewDiversifier,
Expand All @@ -112,6 +118,15 @@ func (cs ClientState) UpdateState(ctx sdk.Context, cdc codec.BinaryCodec, client

cs.Sequence++
cs.ConsensusState = consensusState

clientStore.Set(host.ClientStateKey(), clienttypes.MustMarshalClientState(cdc, &cs))

// set default consensus height with header height
consensusHeight := smHeader.GetHeight()
if cs.ClientType() != exported.Localhost {
clientStore.Set(host.ConsensusStateKey(consensusHeight), clienttypes.MustMarshalConsensusState(cdc, consensusState))
}
damiannolan marked this conversation as resolved.
Show resolved Hide resolved

return &cs, consensusState, nil
}

Expand All @@ -126,9 +141,10 @@ func (cs ClientState) CheckForMisbehaviour(_ sdk.Context, _ codec.BinaryCodec, _

// UpdateStateOnMisbehaviour updates state upon misbehaviour. This method should only be called on misbehaviour
// as it does not perform any misbehaviour checks.
func (cs ClientState) UpdateStateOnMisbehaviour(
_ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, // prematurely include args for self storage of consensus state
) (*ClientState, exported.ConsensusState, error) {
func (cs ClientState) UpdateStateOnMisbehaviour(ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore) (*ClientState, exported.ConsensusState, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a test for this function? Can you check that the correct client state is set in the store?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, there is a test function for this. I was initially just doing an assertion on the returned value but I can adapt it to query the store. Will do the same as you suggest for UpdateState 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Sweet, I just figure better to do now so when we remove the return values we don't have to add testing code

Copy link
Member Author

Choose a reason for hiding this comment

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

Definitely! Agreed 💯

cs.IsFrozen = true

clientStore.Set(host.ClientStateKey(), clienttypes.MustMarshalClientState(cdc, &cs))

return &cs, cs.ConsensusState, nil
}
10 changes: 9 additions & 1 deletion modules/light-clients/06-solomachine/types/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,14 @@ func (suite *SoloMachineTestSuite) TestUpdateState() {
},
true,
},
{
"invalid type misbehaviour",
func() {
clientState = solomachine.ClientState()
clientMsg = solomachine.CreateMisbehaviour()
},
false,
},
}

for _, tc := range testCases {
Expand All @@ -615,7 +623,7 @@ func (suite *SoloMachineTestSuite) TestUpdateState() {
suite.Require().Equal(consensusState, cs.(*types.ClientState).ConsensusState)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add checks on the successful case that the correct client state is set in the store?

} else {
suite.Require().Error(err)
suite.Require().Nil(clientState)
suite.Require().Nil(cs)
suite.Require().Nil(consensusState)
}
}
Expand Down