Skip to content

Commit

Permalink
Fix misbehaviour handling for solo machine (#7515)
Browse files Browse the repository at this point in the history
* add timestamp to SignatureAndData

Add timestamp field to signature and data. Add ValidateBasic check for timestamp. Add ValidateBasic test. Update misbehaviour handler to use supplied timestamp.

* fix typo

* add timestamp check

* fix lint

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
  • Loading branch information
2 people authored and clevinson committed Oct 19, 2020
1 parent 6aec47b commit f3be396
Show file tree
Hide file tree
Showing 7 changed files with 173 additions and 89 deletions.
1 change: 1 addition & 0 deletions proto/ibc/lightclients/solomachine/v1/solomachine.proto
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ message SignatureAndData {
bytes signature = 1;
DataType data_type = 2 [(gogoproto.moretags) = "yaml:\"data_type\""];
bytes data = 3;
uint64 timestamp = 4;
}

// TimestampedSignatureData contains the signature data and the timestamp of the
Expand Down
3 changes: 3 additions & 0 deletions x/ibc/light-clients/06-solomachine/types/misbehaviour.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ func (sd SignatureAndData) ValidateBasic() error {
if sd.DataType == UNSPECIFIED {
return sdkerrors.Wrap(ErrInvalidSignatureAndData, "data type cannot be UNSPECIFIED")
}
if sd.Timestamp == 0 {
return sdkerrors.Wrap(ErrInvalidSignatureAndData, "timestamp cannot be 0")
}

return nil
}
11 changes: 9 additions & 2 deletions x/ibc/light-clients/06-solomachine/types/misbehaviour_handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,23 @@ 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.
// unmarshaled into the specified data type or the timestamp of the signature is less
// than the consensus state timestamp.
func verifySignatureAndData(cdc codec.BinaryMarshaler, clientState ClientState, misbehaviour *Misbehaviour, sigAndData *SignatureAndData) error {
// timestamp less than consensus state would always fail and not succeed in fooling the
// light client
if sigAndData.Timestamp < clientState.ConsensusState.Timestamp {
return sdkerrors.Wrapf(clienttypes.ErrInvalidMisbehaviour, "timestamp is less than consensus state timestamp (%d < %d)", sigAndData.Timestamp, clientState.ConsensusState.Timestamp)
}

// ensure data can be unmarshaled to the specified data type
if _, err := UnmarshalDataByType(cdc, sigAndData.DataType, sigAndData.Data); err != nil {
return err
}

data, err := MisbehaviourSignBytes(
cdc,
misbehaviour.Sequence, clientState.ConsensusState.Timestamp,
misbehaviour.Sequence, sigAndData.Timestamp,
clientState.ConsensusState.Diversifier,
sigAndData.DataType,
sigAndData.Data,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,34 @@ func (suite *SoloMachineTestSuite) TestCheckMisbehaviourAndUpdateState() {
misbehaviour = m
}, false,
},
{
"invalid SignatureOne timestamp",
func() {
clientState = solomachine.ClientState()
m := solomachine.CreateMisbehaviour()

m.SignatureOne.Timestamp = 1000000000000
misbehaviour = m
}, false,
},
{
"invalid SignatureTwo timestamp",
func() {
clientState = solomachine.ClientState()
m := solomachine.CreateMisbehaviour()

m.SignatureTwo.Timestamp = 1000000000000
misbehaviour = m
}, false,
},
{
"timestamp is less than consensus state timestamp",
func() {
clientState = solomachine.ClientState()
solomachine.Time = solomachine.Time - 5
misbehaviour = solomachine.CreateMisbehaviour()
}, false,
},
{
"invalid first signature data",
func() {
Expand Down
12 changes: 12 additions & 0 deletions x/ibc/light-clients/06-solomachine/types/misbehaviour_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,18 @@ func (suite *SoloMachineTestSuite) TestMisbehaviourValidateBasic() {
misbehaviour.SignatureTwo.DataType = types.UNSPECIFIED
}, false,
},
{
"timestamp for SignatureOne is zero",
func(misbehaviour *types.Misbehaviour) {
misbehaviour.SignatureOne.Timestamp = 0
}, false,
},
{
"timestamp for SignatureTwo is zero",
func(misbehaviour *types.Misbehaviour) {
misbehaviour.SignatureTwo.Timestamp = 0
}, false,
},
}

for _, tc := range testCases {
Expand Down
Loading

0 comments on commit f3be396

Please sign in to comment.