-
Notifications
You must be signed in to change notification settings - Fork 647
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
02-client refactor: Adding VerifyClientMessage helper fn #1119
Merged
seantking
merged 26 commits into
02-client-refactor
from
sean/issue#879-create-verify-helper-fn
Mar 28, 2022
Merged
Changes from 17 commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
73047c4
refactor: Adding VerifyClientMessage helper fn to ClientState
seantking 628401d
Merge branch '02-client-refactor' into sean/issue#879-create-verify-h…
seantking 8dd6dab
refactor: creating verifyHeader priv fn and respective test
seantking b04e491
refactor: adding initial test cases
seantking afb2ff5
refactor: add more test cases
seantking 465b801
nit: move fns
seantking c6f9909
remove clientState var
seantking c5c5eb0
refactor: adding different val set test case
seantking f27b7b2
refactor: add test case for header with next height and diff validato…
seantking 3676401
refactor: adding remaining test cases
seantking 64dda21
chore: uncomment previous tests:
seantking 76b600d
fix: chainA -> chainB
seantking 76eec4f
chore: comment
seantking 98310be
refactor: remove consState from api + fix tests
seantking ec00c35
refactor: add verifyHeader to clientState
seantking 98599f8
Merge branch '02-client-refactor' into sean/issue#879-create-verify-h…
seantking 7839463
Merge branch '02-client-refactor' into sean/issue#879-create-verify-h…
seantking 0148f35
fix: incorret trusted validators for concensus state test
seantking 795a4fc
Update modules/light-clients/07-tendermint/types/update_test.go
seantking 7a1d046
chore: add comment
seantking 926de94
fix: params
seantking 36b5ebc
refactor: remove timestamp from api
seantking cbbf854
refactor: switch and type
seantking d42c9c0
Merge branch '02-client-refactor' into sean/issue#879-create-verify-h…
seantking fa4184f
fix: remove height+1
seantking 4e5fca4
02-client refactor: add tests for verifyMisbehaviour (#1166)
colin-axner File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,15 +65,7 @@ func (cs ClientState) CheckHeaderAndUpdateState( | |
conflictingHeader = true | ||
} | ||
|
||
// get consensus state from clientStore | ||
trustedConsState, err := GetConsensusState(clientStore, cdc, tmHeader.TrustedHeight) | ||
if err != nil { | ||
return nil, nil, sdkerrors.Wrapf( | ||
err, "could not get consensus state from clientstore at TrustedHeight: %s", tmHeader.TrustedHeight, | ||
) | ||
} | ||
|
||
if err := checkValidity(&cs, trustedConsState, tmHeader, ctx.BlockTime()); err != nil { | ||
if err := cs.VerifyClientMessage(ctx, clientStore, cdc, tmHeader); err != nil { | ||
return nil, nil, err | ||
} | ||
|
||
|
@@ -126,12 +118,74 @@ func checkTrustedHeader(header *Header, consState *ConsensusState) error { | |
return nil | ||
} | ||
|
||
// checkValidity checks if the Tendermint header is valid. | ||
// CONTRACT: consState.Height == header.TrustedHeight | ||
func checkValidity( | ||
clientState *ClientState, consState *ConsensusState, | ||
header *Header, currentTimestamp time.Time, | ||
// VerifyClientMessage checks if the clientMessage is of type Header or Misbehaviour and verifies the message | ||
func (cs *ClientState) VerifyClientMessage( | ||
ctx sdk.Context, clientStore sdk.KVStore, cdc codec.BinaryCodec, | ||
header exported.ClientMessage, | ||
) error { | ||
switch header.(type) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And here do |
||
case *Header: | ||
return cs.verifyHeader(ctx, clientStore, cdc, header, ctx.BlockTime()) | ||
case *Misbehaviour: | ||
misbehaviour := header.(*Misbehaviour) | ||
// Regardless of the type of misbehaviour, ensure that both headers are valid and would have been accepted by light-client | ||
|
||
// Retrieve trusted consensus states for each Header in misbehaviour | ||
// and unmarshal from clientStore | ||
|
||
// Get consensus bytes from clientStore | ||
tmConsensusState1, err := GetConsensusState(clientStore, cdc, misbehaviour.Header1.TrustedHeight) | ||
if err != nil { | ||
return sdkerrors.Wrapf(err, "could not get trusted consensus state from clientStore for Header1 at TrustedHeight: %s", misbehaviour.Header1) | ||
} | ||
|
||
// Get consensus bytes from clientStore | ||
tmConsensusState2, err := GetConsensusState(clientStore, cdc, misbehaviour.Header2.TrustedHeight) | ||
if err != nil { | ||
return sdkerrors.Wrapf(err, "could not get trusted consensus state from clientStore for Header2 at TrustedHeight: %s", misbehaviour.Header2) | ||
} | ||
|
||
// Check the validity of the two conflicting headers against their respective | ||
// trusted consensus states | ||
// NOTE: header height and commitment root assertions are checked in | ||
// misbehaviour.ValidateBasic by the client keeper and msg.ValidateBasic | ||
// by the base application. | ||
if err := checkMisbehaviourHeader( | ||
cs, tmConsensusState1, misbehaviour.Header1, ctx.BlockTime(), | ||
); err != nil { | ||
return sdkerrors.Wrap(err, "verifying Header1 in Misbehaviour failed") | ||
} | ||
if err := checkMisbehaviourHeader( | ||
cs, tmConsensusState2, misbehaviour.Header2, ctx.BlockTime(), | ||
); err != nil { | ||
return sdkerrors.Wrap(err, "verifying Header2 in Misbehaviour failed") | ||
} | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// verifyHeader returns an error if: | ||
// - the client or header provided are not parseable to tendermint types | ||
// - the header is invalid | ||
// - header height is less than or equal to the trusted header height | ||
// - header revision is not equal to trusted header revision | ||
// - header valset commit verification fails | ||
// - header timestamp is past the trusting period in relation to the consensus state | ||
// - header timestamp is less than or equal to the consensus state timestamp | ||
func (cs *ClientState) verifyHeader( | ||
ctx sdk.Context, clientStore sdk.KVStore, cdc codec.BinaryCodec, | ||
tmHeader exported.ClientMessage, currentTimestamp time.Time, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you could change
colin-axner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) error { | ||
// TODO: check ok | ||
header := tmHeader.(*Header) | ||
|
||
// Retrieve trusted consensus states for each Header in misbehaviour | ||
consState, err := GetConsensusState(clientStore, cdc, header.TrustedHeight) | ||
if err != nil { | ||
return sdkerrors.Wrapf(err, "could not get trusted consensus state from clientStore for Header at TrustedHeight: %s", header.TrustedHeight) | ||
} | ||
|
||
if err := checkTrustedHeader(header, consState); err != nil { | ||
return err | ||
} | ||
|
@@ -169,7 +223,7 @@ func checkValidity( | |
) | ||
} | ||
|
||
chainID := clientState.GetChainID() | ||
chainID := cs.GetChainID() | ||
// If chainID is in revision format, then set revision number of chainID with the revision number | ||
// of the header we are verifying | ||
// This is useful if the update is at a previous revision rather than an update to the latest revision | ||
|
@@ -200,11 +254,12 @@ func checkValidity( | |
err = light.Verify( | ||
&signedHeader, | ||
tmTrustedValidators, tmSignedHeader, tmValidatorSet, | ||
clientState.TrustingPeriod, currentTimestamp, clientState.MaxClockDrift, clientState.TrustLevel.ToTendermint(), | ||
cs.TrustingPeriod, currentTimestamp, cs.MaxClockDrift, cs.TrustLevel.ToTendermint(), | ||
) | ||
if err != nil { | ||
return sdkerrors.Wrap(err, "failed to verify header") | ||
} | ||
|
||
return nil | ||
} | ||
|
||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would suggest this to be named
clientMsg