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.

Sign up for GitHub

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
Merged

chore: add solomachine client storage #1144

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9ce3b69
WIP: solomachine UpdateState now handles storage of ClientState and C…
damiannolan Mar 18, 2022
ae972c3
applying suggestions for review
damiannolan Mar 21, 2022
c749c70
removing unnecessary consensus state storage
damiannolan Mar 22, 2022
9628a60
Merge branch '02-client-refactor' into damian/solomachine-client-storage
damiannolan Mar 22, 2022
663ede8
updating tests to use store in favour of return values
damiannolan Mar 23, 2022
ca18137
Merge branch 'damian/solomachine-client-storage' of github.com:cosmos…
damiannolan Mar 23, 2022
cdf7d35
Merge branch '02-client-refactor' into damian/solomachine-client-storage
damiannolan Mar 28, 2022
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
10 changes: 5 additions & 5 deletions modules/light-clients/06-solomachine/types/misbehaviour_handle.go
View file
Open in desktop
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
View file
Open in desktop
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
Contributor 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
Contributor 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
View file
Open in desktop
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