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.

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: add tests for verifyMisbehaviour #1166

Conversation

colin-axner
Copy link
Contributor

@colin-axner colin-axner commented Mar 23, 2022

Description

Revision height tests will be done in a followup pr

test cases not copied from TestCheckMisbehaviourAndUpdateState:

  • "invalid misbehavior misbehaviour with trusted height different from trusted consensus state" -> I'm not sure what this is testing, this would just be trusted consensus state does not exist.
  • "already frozen client state" - this check should be performed by 02-client
  • "trusted validators is incorrect for given consensus state" -> duplicate of "misbehaviour trusted validators does not match validator hash in trusted consensus state"
  • "provided height > header height" -> what was this testing? Seems like it was testing trusted consensus state DNE

renamed:

  • "invalid misbehavior misbehaviour with trusted validators different from trusted consensus state" -> "misbehaviour trusted validators does not match validator hash in trusted consensus state"

ref: #879


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

@seantking seantking force-pushed the sean/issue#879-create-verify-helper-fn branch from 70c2973 to b04e491 Compare March 24, 2022 07:58
@colin-axner colin-axner marked this pull request as ready for review March 24, 2022 15:02
@colin-axner
Copy link
Contributor Author

marking as r4r, if we decide to merge before all test cases are uncommented, I'll open an issue and followup on the rest of the test cases next week

Copy link
Member

@damiannolan damiannolan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@seantking
Copy link
Contributor

I'm going to resolve the conflicts in my branch now (I guess there will be more conflicts for you once I do this).

@seantking seantking merged commit 4e5fca4 into sean/issue#879-create-verify-helper-fn Mar 28, 2022
@seantking seantking deleted the colin/879-tm-verify-misbehaviour branch March 28, 2022 16:15
seantking added a commit that referenced this pull request Mar 28, 2022
* refactor: Adding VerifyClientMessage helper fn to ClientState

* refactor: creating verifyHeader priv fn and respective test

* refactor: adding initial test cases

* refactor: add more test cases

* nit: move fns

* remove clientState var

* refactor: adding different val set test case

* refactor: add test case for header with next height and diff validator set

* refactor: adding remaining test cases

* chore: uncomment previous tests:

* fix: chainA -> chainB

* chore: comment

* refactor: remove consState from api + fix tests

* refactor: add verifyHeader to clientState

* fix: incorret trusted validators for concensus state test

* Update modules/light-clients/07-tendermint/types/update_test.go

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>

* chore: add comment

* fix: params

* refactor: remove timestamp from api

* refactor: switch and type

* fix: remove height+1

* 02-client refactor: add tests for verifyMisbehaviour (#1166)

* refactor: move misbehaviour validation into verifyMisbehaviour function

* begin writing misbehaviour tests

* fix misbehaviour test

* continue adding misbehaviour test cases

* add more test cases to verifyMisbehaviour test

* add changing validator set tests

* finish rest of tests except revision height testing

* Update modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* add back misbehaviour type assertion

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
seunlanlege pushed a commit to ComposableFi/ibc-go that referenced this pull request Aug 9, 2022
* refactor: Adding VerifyClientMessage helper fn to ClientState

* refactor: creating verifyHeader priv fn and respective test

* refactor: adding initial test cases

* refactor: add more test cases

* nit: move fns

* remove clientState var

* refactor: adding different val set test case

* refactor: add test case for header with next height and diff validator set

* refactor: adding remaining test cases

* chore: uncomment previous tests:

* fix: chainA -> chainB

* chore: comment

* refactor: remove consState from api + fix tests

* refactor: add verifyHeader to clientState

* fix: incorret trusted validators for concensus state test

* Update modules/light-clients/07-tendermint/types/update_test.go

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>

* chore: add comment

* fix: params

* refactor: remove timestamp from api

* refactor: switch and type

* fix: remove height+1

* 02-client refactor: add tests for verifyMisbehaviour (cosmos#1166)

* refactor: move misbehaviour validation into verifyMisbehaviour function

* begin writing misbehaviour tests

* fix misbehaviour test

* continue adding misbehaviour test cases

* add more test cases to verifyMisbehaviour test

* add changing validator set tests

* finish rest of tests except revision height testing

* Update modules/light-clients/07-tendermint/types/misbehaviour_handle_test.go

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* add back misbehaviour type assertion

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
CosmosCar pushed a commit to caelus-labs/ibc-go that referenced this pull request Nov 6, 2023
Depends on celestiaorg/celestia-openrpc#43

---------

Co-authored-by: Manav Aggarwal <manavaggarwal1234@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants