From a27a16e524160a1de93f61a366e3a71d4466ce66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 14 Apr 2021 17:28:53 +0200 Subject: [PATCH 1/2] modify solo machine verify functions to use a pointer to ensure the sequence is incremented --- .../06-solomachine/types/client_state.go | 34 +++++++++---------- .../06-solomachine/types/client_state_test.go | 5 +++ 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/modules/light-clients/06-solomachine/types/client_state.go b/modules/light-clients/06-solomachine/types/client_state.go index d008ac81be9..c208e23b333 100644 --- a/modules/light-clients/06-solomachine/types/client_state.go +++ b/modules/light-clients/06-solomachine/types/client_state.go @@ -100,7 +100,7 @@ func (cs ClientState) VerifyUpgradeAndUpdateState( // VerifyClientState verifies a proof of the client state of the running chain // stored on the solo machine. -func (cs ClientState) VerifyClientState( +func (cs *ClientState) VerifyClientState( store sdk.KVStore, cdc codec.BinaryMarshaler, height exported.Height, @@ -131,13 +131,13 @@ func (cs ClientState) VerifyClientState( cs.Sequence++ cs.ConsensusState.Timestamp = timestamp - setClientState(store, cdc, &cs) + setClientState(store, cdc, cs) return nil } // VerifyClientConsensusState verifies a proof of the consensus state of the // running chain stored on the solo machine. -func (cs ClientState) VerifyClientConsensusState( +func (cs *ClientState) VerifyClientConsensusState( store sdk.KVStore, cdc codec.BinaryMarshaler, height exported.Height, @@ -169,13 +169,13 @@ func (cs ClientState) VerifyClientConsensusState( cs.Sequence++ cs.ConsensusState.Timestamp = timestamp - setClientState(store, cdc, &cs) + setClientState(store, cdc, cs) return nil } // VerifyConnectionState verifies a proof of the connection state of the // specified connection end stored on the target machine. -func (cs ClientState) VerifyConnectionState( +func (cs *ClientState) VerifyConnectionState( store sdk.KVStore, cdc codec.BinaryMarshaler, height exported.Height, @@ -206,13 +206,13 @@ func (cs ClientState) VerifyConnectionState( cs.Sequence++ cs.ConsensusState.Timestamp = timestamp - setClientState(store, cdc, &cs) + setClientState(store, cdc, cs) return nil } // VerifyChannelState verifies a proof of the channel state of the specified // channel end, under the specified port, stored on the target machine. -func (cs ClientState) VerifyChannelState( +func (cs *ClientState) VerifyChannelState( store sdk.KVStore, cdc codec.BinaryMarshaler, height exported.Height, @@ -244,13 +244,13 @@ func (cs ClientState) VerifyChannelState( cs.Sequence++ cs.ConsensusState.Timestamp = timestamp - setClientState(store, cdc, &cs) + setClientState(store, cdc, cs) return nil } // VerifyPacketCommitment verifies a proof of an outgoing packet commitment at // the specified port, specified channel, and specified sequence. -func (cs ClientState) VerifyPacketCommitment( +func (cs *ClientState) VerifyPacketCommitment( store sdk.KVStore, cdc codec.BinaryMarshaler, height exported.Height, @@ -285,13 +285,13 @@ func (cs ClientState) VerifyPacketCommitment( cs.Sequence++ cs.ConsensusState.Timestamp = timestamp - setClientState(store, cdc, &cs) + setClientState(store, cdc, cs) return nil } // VerifyPacketAcknowledgement verifies a proof of an incoming packet // acknowledgement at the specified port, specified channel, and specified sequence. -func (cs ClientState) VerifyPacketAcknowledgement( +func (cs *ClientState) VerifyPacketAcknowledgement( store sdk.KVStore, cdc codec.BinaryMarshaler, height exported.Height, @@ -326,14 +326,14 @@ func (cs ClientState) VerifyPacketAcknowledgement( cs.Sequence++ cs.ConsensusState.Timestamp = timestamp - setClientState(store, cdc, &cs) + setClientState(store, cdc, cs) return nil } // VerifyPacketReceiptAbsence verifies a proof of the absence of an // incoming packet receipt at the specified port, specified channel, and // specified sequence. -func (cs ClientState) VerifyPacketReceiptAbsence( +func (cs *ClientState) VerifyPacketReceiptAbsence( store sdk.KVStore, cdc codec.BinaryMarshaler, height exported.Height, @@ -367,13 +367,13 @@ func (cs ClientState) VerifyPacketReceiptAbsence( cs.Sequence++ cs.ConsensusState.Timestamp = timestamp - setClientState(store, cdc, &cs) + setClientState(store, cdc, cs) return nil } // VerifyNextSequenceRecv verifies a proof of the next sequence number to be // received of the specified channel at the specified port. -func (cs ClientState) VerifyNextSequenceRecv( +func (cs *ClientState) VerifyNextSequenceRecv( store sdk.KVStore, cdc codec.BinaryMarshaler, height exported.Height, @@ -407,7 +407,7 @@ func (cs ClientState) VerifyNextSequenceRecv( cs.Sequence++ cs.ConsensusState.Timestamp = timestamp - setClientState(store, cdc, &cs) + setClientState(store, cdc, cs) return nil } @@ -417,7 +417,7 @@ func (cs ClientState) VerifyNextSequenceRecv( // along with the solo-machine sequence encoded in the proofHeight. func produceVerificationArgs( cdc codec.BinaryMarshaler, - cs ClientState, + cs *ClientState, height exported.Height, prefix exported.Prefix, proof []byte, diff --git a/modules/light-clients/06-solomachine/types/client_state_test.go b/modules/light-clients/06-solomachine/types/client_state_test.go index 6666f2d493a..6c7661ef629 100644 --- a/modules/light-clients/06-solomachine/types/client_state_test.go +++ b/modules/light-clients/06-solomachine/types/client_state_test.go @@ -247,6 +247,7 @@ func (suite *SoloMachineTestSuite) TestVerifyClientState() { if tc.expPass { suite.Require().NoError(err) + suite.Require().Equal(expSeq, tc.clientState.Sequence) suite.Require().Equal(expSeq, suite.GetSequenceFromStore(), "sequence not updated in the store (%d) on valid test case %s", suite.GetSequenceFromStore(), tc.name) } else { suite.Require().Error(err) @@ -375,6 +376,7 @@ func (suite *SoloMachineTestSuite) TestVerifyClientConsensusState() { if tc.expPass { suite.Require().NoError(err) + suite.Require().Equal(expSeq, tc.clientState.Sequence) suite.Require().Equal(expSeq, suite.GetSequenceFromStore(), "sequence not updated in the store (%d) on valid test case %s", suite.GetSequenceFromStore(), tc.name) } else { suite.Require().Error(err) @@ -465,6 +467,7 @@ func (suite *SoloMachineTestSuite) TestVerifyConnectionState() { if tc.expPass { suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.name) + suite.Require().Equal(expSeq, tc.clientState.Sequence) suite.Require().Equal(expSeq, suite.GetSequenceFromStore(), "sequence not updated in the store (%d) on valid test case %d: %s", suite.GetSequenceFromStore(), i, tc.name) } else { suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.name) @@ -554,6 +557,7 @@ func (suite *SoloMachineTestSuite) TestVerifyChannelState() { if tc.expPass { suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.name) + suite.Require().Equal(expSeq, tc.clientState.Sequence) suite.Require().Equal(expSeq, suite.GetSequenceFromStore(), "sequence not updated in the store (%d) on valid test case %d: %s", suite.GetSequenceFromStore(), i, tc.name) } else { suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.name) @@ -903,6 +907,7 @@ func (suite *SoloMachineTestSuite) TestVerifyNextSeqRecv() { if tc.expPass { suite.Require().NoError(err, "valid test case %d failed: %s", i, tc.name) + suite.Require().Equal(expSeq, tc.clientState.Sequence) suite.Require().Equal(expSeq, suite.GetSequenceFromStore(), "sequence not updated in the store (%d) on valid test case %d: %s", suite.GetSequenceFromStore(), i, tc.name) } else { suite.Require().Error(err, "invalid test case %d passed: %s", i, tc.name) From 832ee88d603dd6a73ab009f7692fec2a4de3b82d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 14 Apr 2021 17:33:32 +0200 Subject: [PATCH 2/2] update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 234a38be084..57d1dcbbd7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +### Bug Fixes + +* (modules/light-clients/06-solomachine) [\#120](https://github.com/cosmos/ibc-go/pull/120) Fix solo machine handshake verification bug. + ### State Machine Breaking * (modules/light-clients/07-tendermint) [\#99](https://github.com/cosmos/ibc-go/pull/99) Enforce maximum chain-id length for tendermint client.