chore: address solomachine testing refactor follow ups #6085
Conversation
WalkthroughThe changes involve a minor adjustment in the order of variable declarations within a test function and improvements in how client state and signatures are managed within test cases. This adjustment is part of enhancing the test suite for better readability and efficiency. Changes
Possibly related issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
| suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), clientID, sm.ClientState()) | ||
|
|
||
| oldPubKey := sm.PublicKey | ||
| h := sm.CreateHeader(sm.Diversifier) |
There was a problem hiding this comment.
sm.CreateHeader creates a new header and then increments solomachine timestamp. Previously, we then set below the new solomachine client state with updated timestamp causing failure on due to - header.Timestamp < cs.ConsensusState.Timestamp in verifyHeader
There was a problem hiding this comment.
ah, nice, makes sense to me why this was happening.
| clientState.ConsensusState.PublicKey = nil | ||
| clientMsg = sm.CreateHeader(sm.Diversifier) | ||
| suite.chainA.App.GetIBCKeeper().ClientKeeper.SetClientState(suite.chainA.GetContext(), clientID, sm.ClientState()) | ||
| h := sm.CreateHeader(sm.Diversifier) | ||
| h.NewPublicKey = nil | ||
| clientMsg = h | ||
| }, | ||
| // TODO(jim): Doesn't fail on VerifySignature | ||
| clienttypes.ErrInvalidHeader, | ||
| solomachine.ErrSignatureVerificationFailed, |
There was a problem hiding this comment.
Reorganised the code here as it was triggering something similar.
damiannolan
requested review from
colin-axner,
AdityaSripal and
chatton
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Review Status
Configuration used: .coderabbit.yml
Files selected for processing (1)
- modules/light-clients/06-solomachine/light_client_module_test.go (4 hunks)
Additional Context Used
Path-based Instructions (1)
modules/light-clients/06-solomachine/light_client_module_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
modules/light-clients/06-solomachine/light_client_module_test.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: .coderabbit.yml
Files selected for processing (1)
- modules/light-clients/06-solomachine/light_client_module_test.go (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- modules/light-clients/06-solomachine/light_client_module_test.go
|
Description
Addresses todos in solomachine testing refactor. Follow up from #6038
closes: #XXXX
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.
docs/).godoccomments.Files changedin the GitHub PR explorer.SonarCloud Reportin the comment section below once CI passes.Summary by CodeRabbit
SoloMachineTestSuitefor improved readability and efficiency in handling client state and signatures during test cases.