From 63f3a4fbab5349072970447b54a78fe1751f4de5 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Fri, 11 Mar 2022 19:51:53 +0100 Subject: [PATCH 1/6] adding VerifyClientMessage and tests --- .../06-solomachine/types/misbehaviour.go | 6 + .../06-solomachine/types/update.go | 93 +++-- .../06-solomachine/types/update_test.go | 351 ++++++++++++++++++ 3 files changed, 409 insertions(+), 41 deletions(-) diff --git a/modules/light-clients/06-solomachine/types/misbehaviour.go b/modules/light-clients/06-solomachine/types/misbehaviour.go index 31b9b1dc97c..217720294f4 100644 --- a/modules/light-clients/06-solomachine/types/misbehaviour.go +++ b/modules/light-clients/06-solomachine/types/misbehaviour.go @@ -22,6 +22,12 @@ func (misbehaviour Misbehaviour) Type() string { return exported.TypeClientMisbehaviour } +// TODO: Remove GetHeight() when new interface type ClientMessage is introduced +// GetHeight implements the exported.Header interface +func (misbehaviour Misbehaviour) GetHeight() exported.Height { + return nil +} + // ValidateBasic implements Misbehaviour interface. func (misbehaviour Misbehaviour) ValidateBasic() error { if err := host.ClientIdentifierValidator(misbehaviour.ClientId); err != nil { diff --git a/modules/light-clients/06-solomachine/types/update.go b/modules/light-clients/06-solomachine/types/update.go index 3896d2dddec..06d9541e9bb 100644 --- a/modules/light-clients/06-solomachine/types/update.go +++ b/modules/light-clients/06-solomachine/types/update.go @@ -17,61 +17,72 @@ import ( // - the currently registered public key did not provide the update signature func (cs ClientState) CheckHeaderAndUpdateState( ctx sdk.Context, cdc codec.BinaryCodec, clientStore sdk.KVStore, - header exported.Header, + msg exported.Header, // TODO: Update to exported.ClientMessage ) (exported.ClientState, exported.ConsensusState, error) { - smHeader, ok := header.(*Header) - if !ok { - return nil, nil, sdkerrors.Wrapf( - clienttypes.ErrInvalidHeader, "header type %T, expected %T", header, &Header{}, - ) - } - - if err := checkHeader(cdc, &cs, smHeader); err != nil { + if err := cs.VerifyClientMessage(cdc, msg); err != nil { return nil, nil, err } + smHeader := msg.(*Header) clientState, consensusState := update(&cs, smHeader) return clientState, consensusState, nil } -// checkHeader checks if the Solo Machine update signature is valid. -func checkHeader(cdc codec.BinaryCodec, clientState *ClientState, header *Header) error { - // assert update sequence is current sequence - if header.Sequence != clientState.Sequence { - return sdkerrors.Wrapf( - clienttypes.ErrInvalidHeader, - "header sequence does not match the client state sequence (%d != %d)", header.Sequence, clientState.Sequence, - ) - } +// VerifyClientMessage checks if the Solo Machine update signature(s) is valid. +func (cs ClientState) VerifyClientMessage(cdc codec.BinaryCodec, clientMsg exported.Header) error { + switch msg := clientMsg.(type) { + case *Header: + // assert update sequence is current sequence + if msg.Sequence != cs.Sequence { + return sdkerrors.Wrapf( + clienttypes.ErrInvalidHeader, + "header sequence does not match the client state sequence (%d != %d)", msg.Sequence, cs.Sequence, + ) + } - // assert update timestamp is not less than current consensus state timestamp - if header.Timestamp < clientState.ConsensusState.Timestamp { - return sdkerrors.Wrapf( - clienttypes.ErrInvalidHeader, - "header timestamp is less than to the consensus state timestamp (%d < %d)", header.Timestamp, clientState.ConsensusState.Timestamp, - ) - } + // assert update timestamp is not less than current consensus state timestamp + if msg.Timestamp < cs.ConsensusState.Timestamp { + return sdkerrors.Wrapf( + clienttypes.ErrInvalidHeader, + "header timestamp is less than to the consensus state timestamp (%d < %d)", msg.Timestamp, cs.ConsensusState.Timestamp, + ) + } - // assert currently registered public key signed over the new public key with correct sequence - data, err := HeaderSignBytes(cdc, header) - if err != nil { - return err - } + // assert currently registered public key signed over the new public key with correct sequence + data, err := HeaderSignBytes(cdc, msg) + if err != nil { + return err + } - sigData, err := UnmarshalSignatureData(cdc, header.Signature) - if err != nil { - return err - } + sigData, err := UnmarshalSignatureData(cdc, msg.Signature) + if err != nil { + return err + } - publicKey, err := clientState.ConsensusState.GetPubKey() - if err != nil { - return err - } + publicKey, err := cs.ConsensusState.GetPubKey() + if err != nil { + return err + } - if err := VerifySignature(publicKey, data, sigData); err != nil { - return sdkerrors.Wrap(ErrInvalidHeader, err.Error()) - } + if err := VerifySignature(publicKey, data, sigData); err != nil { + return sdkerrors.Wrap(ErrInvalidHeader, err.Error()) + } + case *Misbehaviour: + // 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, msg, msg.SignatureOne); err != nil { + return sdkerrors.Wrap(err, "failed to verify signature one") + } + // verify second signature + if err := verifySignatureAndData(cdc, cs, msg, msg.SignatureTwo); err != nil { + return sdkerrors.Wrap(err, "failed to verify signature two") + } + + default: + return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected type %T, got type %T", Header{}, msg) + } return nil } diff --git a/modules/light-clients/06-solomachine/types/update_test.go b/modules/light-clients/06-solomachine/types/update_test.go index f13d5f198d1..11784ad9215 100644 --- a/modules/light-clients/06-solomachine/types/update_test.go +++ b/modules/light-clients/06-solomachine/types/update_test.go @@ -180,3 +180,354 @@ func (suite *SoloMachineTestSuite) TestCheckHeaderAndUpdateState() { } } } + +func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { + var ( + clientState *types.ClientState + clientMsg exported.Header // TODO: Update to ClientMessage interface + ) + + // test singlesig and multisig public keys + for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} { + + testCases := []struct { + name string + setup func() + expPass bool + }{ + { + "successful header", + func() { + clientState = solomachine.ClientState() + clientMsg = solomachine.CreateHeader() + }, + true, + }, + { + "successful misbehaviour", + func() { + clientState = solomachine.ClientState() + clientMsg = solomachine.CreateMisbehaviour() + }, + true, + }, + { + "invalid client message type", + func() { + clientState = solomachine.ClientState() + clientMsg = &ibctmtypes.Header{} + }, + false, + }, + { + "wrong sequence in header", + func() { + clientState = solomachine.ClientState() + // store in temp before assigning to interface type + h := solomachine.CreateHeader() + h.Sequence++ + clientMsg = h + }, + false, + }, + { + "invalid header Signature", + func() { + clientState = solomachine.ClientState() + h := solomachine.CreateHeader() + h.Signature = suite.GetInvalidProof() + clientMsg = h + }, false, + }, + { + "invalid timestamp in header", + func() { + clientState = solomachine.ClientState() + h := solomachine.CreateHeader() + h.Timestamp-- + clientMsg = h + }, false, + }, + { + "signature uses wrong sequence", + func() { + clientState = solomachine.ClientState() + solomachine.Sequence++ + clientMsg = 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 + clientMsg = 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 + clientMsg = h + }, + false, + }, + { + "consensus state public key is nil", + func() { + cs := solomachine.ClientState() + cs.ConsensusState.PublicKey = nil + clientState = cs + clientMsg = solomachine.CreateHeader() + }, + false, + }, + { + "invalid SignatureOne SignatureData", + func() { + clientState = solomachine.ClientState() + m := solomachine.CreateMisbehaviour() + + m.SignatureOne.Signature = suite.GetInvalidProof() + clientMsg = m + }, false, + }, + { + "invalid SignatureTwo SignatureData", + func() { + clientState = solomachine.ClientState() + m := solomachine.CreateMisbehaviour() + + m.SignatureTwo.Signature = suite.GetInvalidProof() + clientMsg = m + }, false, + }, + { + "invalid SignatureOne timestamp", + func() { + clientState = solomachine.ClientState() + m := solomachine.CreateMisbehaviour() + + m.SignatureOne.Timestamp = 1000000000000 + clientMsg = m + }, false, + }, + { + "invalid SignatureTwo timestamp", + func() { + clientState = solomachine.ClientState() + m := solomachine.CreateMisbehaviour() + + m.SignatureTwo.Timestamp = 1000000000000 + clientMsg = m + }, false, + }, + { + "invalid first signature data", + func() { + clientState = solomachine.ClientState() + + // store in temp before assigning to interface type + m := solomachine.CreateMisbehaviour() + + msg := []byte("DATA ONE") + signBytes := &types.SignBytes{ + Sequence: solomachine.Sequence + 1, + Timestamp: solomachine.Time, + Diversifier: solomachine.Diversifier, + DataType: types.CLIENT, + Data: msg, + } + + data, err := suite.chainA.Codec.Marshal(signBytes) + suite.Require().NoError(err) + + sig := solomachine.GenerateSignature(data) + + m.SignatureOne.Signature = sig + m.SignatureOne.Data = msg + clientMsg = m + }, + false, + }, + { + "invalid second signature data", + func() { + clientState = solomachine.ClientState() + + // store in temp before assigning to interface type + m := solomachine.CreateMisbehaviour() + + msg := []byte("DATA TWO") + signBytes := &types.SignBytes{ + Sequence: solomachine.Sequence + 1, + Timestamp: solomachine.Time, + Diversifier: solomachine.Diversifier, + DataType: types.CLIENT, + Data: msg, + } + + data, err := suite.chainA.Codec.Marshal(signBytes) + suite.Require().NoError(err) + + sig := solomachine.GenerateSignature(data) + + m.SignatureTwo.Signature = sig + m.SignatureTwo.Data = msg + clientMsg = m + }, + false, + }, + { + "wrong pubkey generates first signature", + func() { + clientState = solomachine.ClientState() + badMisbehaviour := solomachine.CreateMisbehaviour() + + // update public key to a new one + solomachine.CreateHeader() + m := solomachine.CreateMisbehaviour() + + // set SignatureOne to use the wrong signature + m.SignatureOne = badMisbehaviour.SignatureOne + clientMsg = m + }, false, + }, + { + "wrong pubkey generates second signature", + func() { + clientState = solomachine.ClientState() + badMisbehaviour := solomachine.CreateMisbehaviour() + + // update public key to a new one + solomachine.CreateHeader() + m := solomachine.CreateMisbehaviour() + + // set SignatureTwo to use the wrong signature + m.SignatureTwo = badMisbehaviour.SignatureTwo + clientMsg = m + }, false, + }, + { + "signatures sign over different sequence", + func() { + clientState = solomachine.ClientState() + + // store in temp before assigning to interface type + m := solomachine.CreateMisbehaviour() + + // Signature One + msg := []byte("DATA ONE") + // sequence used is plus 1 + signBytes := &types.SignBytes{ + Sequence: solomachine.Sequence + 1, + Timestamp: solomachine.Time, + Diversifier: solomachine.Diversifier, + DataType: types.CLIENT, + Data: msg, + } + + data, err := suite.chainA.Codec.Marshal(signBytes) + suite.Require().NoError(err) + + sig := solomachine.GenerateSignature(data) + + m.SignatureOne.Signature = sig + m.SignatureOne.Data = msg + + // Signature Two + msg = []byte("DATA TWO") + // sequence used is minus 1 + + signBytes = &types.SignBytes{ + Sequence: solomachine.Sequence - 1, + Timestamp: solomachine.Time, + Diversifier: solomachine.Diversifier, + DataType: types.CLIENT, + Data: msg, + } + data, err = suite.chainA.Codec.Marshal(signBytes) + suite.Require().NoError(err) + + sig = solomachine.GenerateSignature(data) + + m.SignatureTwo.Signature = sig + m.SignatureTwo.Data = msg + + clientMsg = m + }, + false, + }, + { + "consensus state pubkey is nil", + func() { + cs := solomachine.ClientState() + cs.ConsensusState.PublicKey = nil + clientState = cs + clientMsg = solomachine.CreateMisbehaviour() + }, + false, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + // setup test + tc.setup() + + err := clientState.VerifyClientMessage(suite.chainA.Codec, clientMsg) + + if tc.expPass { + suite.Require().NoError(err) + } else { + suite.Require().Error(err) + } + }) + } + } +} From d6d9920cdd3c43ad3991e975c399c9d92dab09da Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Fri, 11 Mar 2022 21:10:49 +0100 Subject: [PATCH 2/6] splitting tests for verify header and misbehaviour --- .../06-solomachine/types/update_test.go | 100 ++++++++++++------ 1 file changed, 66 insertions(+), 34 deletions(-) diff --git a/modules/light-clients/06-solomachine/types/update_test.go b/modules/light-clients/06-solomachine/types/update_test.go index 11784ad9215..43380d8efed 100644 --- a/modules/light-clients/06-solomachine/types/update_test.go +++ b/modules/light-clients/06-solomachine/types/update_test.go @@ -181,10 +181,10 @@ func (suite *SoloMachineTestSuite) TestCheckHeaderAndUpdateState() { } } -func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { +func (suite *SoloMachineTestSuite) TestVerifyClientMessageHeader() { var ( - clientState *types.ClientState clientMsg exported.Header // TODO: Update to ClientMessage interface + clientState *types.ClientState ) // test singlesig and multisig public keys @@ -198,7 +198,6 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { { "successful header", func() { - clientState = solomachine.ClientState() clientMsg = solomachine.CreateHeader() }, true, @@ -206,7 +205,6 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { { "successful misbehaviour", func() { - clientState = solomachine.ClientState() clientMsg = solomachine.CreateMisbehaviour() }, true, @@ -214,7 +212,6 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { { "invalid client message type", func() { - clientState = solomachine.ClientState() clientMsg = &ibctmtypes.Header{} }, false, @@ -222,7 +219,6 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { { "wrong sequence in header", func() { - clientState = solomachine.ClientState() // store in temp before assigning to interface type h := solomachine.CreateHeader() h.Sequence++ @@ -233,7 +229,6 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { { "invalid header Signature", func() { - clientState = solomachine.ClientState() h := solomachine.CreateHeader() h.Signature = suite.GetInvalidProof() clientMsg = h @@ -242,7 +237,6 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { { "invalid timestamp in header", func() { - clientState = solomachine.ClientState() h := solomachine.CreateHeader() h.Timestamp-- clientMsg = h @@ -251,7 +245,7 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { { "signature uses wrong sequence", func() { - clientState = solomachine.ClientState() + solomachine.Sequence++ clientMsg = solomachine.CreateHeader() }, @@ -316,19 +310,75 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { false, }, { - "consensus state public key is nil", + "consensus state public key is nil - header", func() { - cs := solomachine.ClientState() - cs.ConsensusState.PublicKey = nil - clientState = cs + clientState.ConsensusState.PublicKey = nil clientMsg = solomachine.CreateHeader() }, false, }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + clientState = solomachine.ClientState() + + // setup test + tc.setup() + + err := clientState.VerifyClientMessage(suite.chainA.Codec, clientMsg) + + if tc.expPass { + suite.Require().NoError(err) + } else { + suite.Require().Error(err) + } + }) + } + } +} + +func (suite *SoloMachineTestSuite) TestVerifyClientMessageMisbehaviour() { + var ( + clientMsg exported.Header // TODO: Update to ClientMessage interface + clientState *types.ClientState + ) + + // test singlesig and multisig public keys + for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} { + + testCases := []struct { + name string + setup func() + expPass bool + }{ + { + "successful misbehaviour", + func() { + clientMsg = solomachine.CreateMisbehaviour() + }, + true, + }, + { + "invalid client message type", + func() { + clientMsg = &ibctmtypes.Header{} + }, + false, + }, + { + "consensus state pubkey is nil", + func() { + clientState.ConsensusState.PublicKey = nil + clientMsg = solomachine.CreateMisbehaviour() + }, + false, + }, { "invalid SignatureOne SignatureData", func() { - clientState = solomachine.ClientState() m := solomachine.CreateMisbehaviour() m.SignatureOne.Signature = suite.GetInvalidProof() @@ -338,7 +388,6 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { { "invalid SignatureTwo SignatureData", func() { - clientState = solomachine.ClientState() m := solomachine.CreateMisbehaviour() m.SignatureTwo.Signature = suite.GetInvalidProof() @@ -348,7 +397,6 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { { "invalid SignatureOne timestamp", func() { - clientState = solomachine.ClientState() m := solomachine.CreateMisbehaviour() m.SignatureOne.Timestamp = 1000000000000 @@ -358,7 +406,6 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { { "invalid SignatureTwo timestamp", func() { - clientState = solomachine.ClientState() m := solomachine.CreateMisbehaviour() m.SignatureTwo.Timestamp = 1000000000000 @@ -368,8 +415,6 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { { "invalid first signature data", func() { - clientState = solomachine.ClientState() - // store in temp before assigning to interface type m := solomachine.CreateMisbehaviour() @@ -396,8 +441,6 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { { "invalid second signature data", func() { - clientState = solomachine.ClientState() - // store in temp before assigning to interface type m := solomachine.CreateMisbehaviour() @@ -424,7 +467,6 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { { "wrong pubkey generates first signature", func() { - clientState = solomachine.ClientState() badMisbehaviour := solomachine.CreateMisbehaviour() // update public key to a new one @@ -439,7 +481,6 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { { "wrong pubkey generates second signature", func() { - clientState = solomachine.ClientState() badMisbehaviour := solomachine.CreateMisbehaviour() // update public key to a new one @@ -454,7 +495,6 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { { "signatures sign over different sequence", func() { - clientState = solomachine.ClientState() // store in temp before assigning to interface type m := solomachine.CreateMisbehaviour() @@ -501,22 +541,14 @@ func (suite *SoloMachineTestSuite) TestVerifyClientMessage() { }, false, }, - { - "consensus state pubkey is nil", - func() { - cs := solomachine.ClientState() - cs.ConsensusState.PublicKey = nil - clientState = cs - clientMsg = solomachine.CreateMisbehaviour() - }, - false, - }, } for _, tc := range testCases { tc := tc suite.Run(tc.name, func() { + clientState = solomachine.ClientState() + // setup test tc.setup() From 51465ee562bf9b471e7accceff24aafcc46fa78e Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Fri, 11 Mar 2022 21:16:53 +0100 Subject: [PATCH 3/6] rename update to UpdateState --- .../06-solomachine/types/update.go | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/modules/light-clients/06-solomachine/types/update.go b/modules/light-clients/06-solomachine/types/update.go index 06d9541e9bb..eba2e10bcb0 100644 --- a/modules/light-clients/06-solomachine/types/update.go +++ b/modules/light-clients/06-solomachine/types/update.go @@ -23,9 +23,7 @@ func (cs ClientState) CheckHeaderAndUpdateState( return nil, nil, err } - smHeader := msg.(*Header) - clientState, consensusState := update(&cs, smHeader) - return clientState, consensusState, nil + return cs.UpdateState(ctx, cdc, clientStore, msg) } // VerifyClientMessage checks if the Solo Machine update signature(s) is valid. @@ -86,16 +84,19 @@ func (cs ClientState) VerifyClientMessage(cdc codec.BinaryCodec, clientMsg expor return nil } -// update the consensus state to the new public key and an incremented sequence -func update(clientState *ClientState, header *Header) (*ClientState, *ConsensusState) { +// 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, + header exported.Header, +) (exported.ClientState, exported.ConsensusState, error) { + smHeader := header.(*Header) consensusState := &ConsensusState{ - PublicKey: header.NewPublicKey, - Diversifier: header.NewDiversifier, - Timestamp: header.Timestamp, + PublicKey: smHeader.NewPublicKey, + Diversifier: smHeader.NewDiversifier, + Timestamp: smHeader.Timestamp, } - // increment sequence number - clientState.Sequence++ - clientState.ConsensusState = consensusState - return clientState, consensusState + cs.Sequence++ + cs.ConsensusState = consensusState + return &cs, consensusState, nil } From 8636c445ba9bc65f1c08ecefd29dcc92f711d868 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Fri, 11 Mar 2022 21:18:52 +0100 Subject: [PATCH 4/6] adding CheckForMisbehaviour and UpdateStateOnMisbehaviour --- .../06-solomachine/types/update.go | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/modules/light-clients/06-solomachine/types/update.go b/modules/light-clients/06-solomachine/types/update.go index eba2e10bcb0..7e7fdad5887 100644 --- a/modules/light-clients/06-solomachine/types/update.go +++ b/modules/light-clients/06-solomachine/types/update.go @@ -84,6 +84,31 @@ func (cs ClientState) VerifyClientMessage(cdc codec.BinaryCodec, clientMsg expor return nil } +// CheckForMisbehaviour returns true for type Misbehaviour (passed VerifyClientMessage check), otherwise returns false +func (cs ClientState) CheckForMisbehaviour( + _ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, + msg exported.Header, // TODO: Update to exported.ClientMessage +) bool { + var foundMisbehaviour bool + switch msg.(type) { + case *Header: + foundMisbehaviour = false + case *Misbehaviour: + foundMisbehaviour = true + } + + return foundMisbehaviour +} + +// 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, error) { + cs.IsFrozen = true + return &cs, nil +} + // 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, From 0158c18025085837f44d20619df99bacb8dfeb49 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Fri, 11 Mar 2022 21:22:19 +0100 Subject: [PATCH 5/6] adding misbehaviour checks --- modules/light-clients/06-solomachine/types/update.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/modules/light-clients/06-solomachine/types/update.go b/modules/light-clients/06-solomachine/types/update.go index 7e7fdad5887..84fe2f0b922 100644 --- a/modules/light-clients/06-solomachine/types/update.go +++ b/modules/light-clients/06-solomachine/types/update.go @@ -23,6 +23,11 @@ func (cs ClientState) CheckHeaderAndUpdateState( return nil, nil, err } + foundMisbehaviour := cs.CheckForMisbehaviour(ctx, cdc, clientStore, msg) + if foundMisbehaviour { + return cs.UpdateStateOnMisbehaviour(ctx, cdc, clientStore) + } + return cs.UpdateState(ctx, cdc, clientStore, msg) } @@ -104,9 +109,9 @@ func (cs ClientState) CheckForMisbehaviour( // 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, error) { +) (*ClientState, exported.ConsensusState, error) { cs.IsFrozen = true - return &cs, nil + return &cs, cs.ConsensusState, nil } // UpdateState updates the consensus state to the new public key and an incremented sequence. From ae9290efde17cff24c45cfb7ebb47aad038686cc Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Mon, 14 Mar 2022 15:45:13 +0100 Subject: [PATCH 6/6] updating CheckForMisbehaviour codestyle and adding basic test cases --- .../06-solomachine/types/update.go | 10 +-- .../06-solomachine/types/update_test.go | 87 +++++++++++++++++++ 2 files changed, 90 insertions(+), 7 deletions(-) diff --git a/modules/light-clients/06-solomachine/types/update.go b/modules/light-clients/06-solomachine/types/update.go index 84fe2f0b922..07e3a94fe57 100644 --- a/modules/light-clients/06-solomachine/types/update.go +++ b/modules/light-clients/06-solomachine/types/update.go @@ -94,15 +94,11 @@ func (cs ClientState) CheckForMisbehaviour( _ sdk.Context, _ codec.BinaryCodec, _ sdk.KVStore, msg exported.Header, // TODO: Update to exported.ClientMessage ) bool { - var foundMisbehaviour bool - switch msg.(type) { - case *Header: - foundMisbehaviour = false - case *Misbehaviour: - foundMisbehaviour = true + if _, ok := msg.(*Misbehaviour); ok { + return true } - return foundMisbehaviour + return false } // UpdateStateOnMisbehaviour updates state upon misbehaviour. This method should only be called on misbehaviour diff --git a/modules/light-clients/06-solomachine/types/update_test.go b/modules/light-clients/06-solomachine/types/update_test.go index 43380d8efed..bfe3162f3ed 100644 --- a/modules/light-clients/06-solomachine/types/update_test.go +++ b/modules/light-clients/06-solomachine/types/update_test.go @@ -181,6 +181,93 @@ func (suite *SoloMachineTestSuite) TestCheckHeaderAndUpdateState() { } } +func (suite *SoloMachineTestSuite) TestCheckForMisbehaviour() { + var ( + clientMsg exported.Header // TODO: Update to ClientMessage interface + clientState 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 + }{ + { + "success", + func() { + clientMsg = solomachine.CreateMisbehaviour() + }, + true, + }, + { + "normal header returns false", + func() { + clientMsg = solomachine.CreateHeader() + }, + false, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + clientState = solomachine.ClientState() + + tc.malleate() + + // TODO: Remove type assertion when ClientState interface includes CheckForMisbehaviour + smClientState, ok := clientState.(*types.ClientState) + if ok { + foundMisbehaviour := smClientState.CheckForMisbehaviour(suite.chainA.GetContext(), suite.chainA.Codec, nil, clientMsg) + + if tc.expPass { + suite.Require().True(foundMisbehaviour) + } else { + suite.Require().False(foundMisbehaviour) + } + } + }) + } + } +} + +func (suite *SoloMachineTestSuite) TestUpdateStateOnMisbehaviour() { + // test singlesig and multisig public keys + for _, solomachine := range []*ibctesting.Solomachine{suite.solomachine, suite.solomachineMulti} { + testCases := []struct { + name string + malleate func() + expPass bool + }{ + { + "success", + func() {}, + true, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + clientState := solomachine.ClientState() + + tc.malleate() + + // TODO: Update to pass client store and make assertions on state changes + cs, _, _ := clientState.UpdateStateOnMisbehaviour(suite.chainA.GetContext(), suite.chainA.Codec, nil) + + if tc.expPass { + suite.Require().True(cs.IsFrozen) + } + }) + } + } +} + func (suite *SoloMachineTestSuite) TestVerifyClientMessageHeader() { var ( clientMsg exported.Header // TODO: Update to ClientMessage interface