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.

Sign up for GitHub

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

Jump to bottom

Drop unnecessary nolint comments #477

Merged
merged 4 commits into from
Aug 20, 2024
Merged

Drop unnecessary nolint comments #477

merged 4 commits into from
Aug 20, 2024

Conversation

MatheusFranco99
Copy link
Contributor

@MatheusFranco99 MatheusFranco99 commented Aug 13, 2024 •
edited
Loading

Overview

This PR drops unnecessary nolint comments.

There were lint errors related to the unused variables quickTimeoutThreshold, quickTimeout and slowTimeout in timeout.go. To resolve these errors while preserving their intended use as informative constants for implementation, these variables have been capitalised to make them exported (accessible outside their package), avoiding the lint errors.

Note: There are still nolint comments in encryption.go but the related function will be fixed in another PR.

@MatheusFranco99 MatheusFranco99 self-assigned this Aug 13, 2024
@GalRogozinski GalRogozinski mentioned this pull request Aug 14, 2024
Open
9 tasks
@MatheusFranco99 MatheusFranco99 merged commit 642d9d2 into ssvlabs:dev Aug 20, 2024
2 checks passed
@MatheusFranco99 MatheusFranco99 deleted the drop-nolint branch August 20, 2024 10:46
GalRogozinski pushed a commit that referenced this pull request Sep 12, 2024
@MatheusFranco99 @GalRogozinski
Meta - Drop unnecessary nolint comments (#477)
90a174e 
* Remove nolint comment and export timeout variables

* Drop unnecessary nolint

* Add comment

* Fix lint issue
GalRogozinski pushed a commit that referenced this pull request Nov 12, 2024
@alan-ssvlabs @y0sher @MatheusFranco99
Spec - Move domainType from CommitteeMember and Share to Network (#494)
ec462ef 
* remove domaintype from committeeMember

* remove domaintype from share

* add config for ssv containing domainType

* add custom unmarshal for baserunner to avoid error in unmarshalling config

* move domainType to Network

* add testing domain in NewTestingNetwork function

* Spec - filter committee duties per slot validator duties (#467)

* Filter shares for slot `CommitteeRunner` based on validators that have duty for that slot.

* Filter duty and create share map according to owned validators

* Add test: start duty with no shares for duty's validators

* Add test: happy flow for committee with fraction of duty's validators

* Generate JSON tests

* Apply suggestions

---------

Co-authored-by: MatheusFranco99 <[email protected]>

* Spec - check att and sync duties exist before submitting (#468)

* Meta - Update to go1.22 (#474)

* Update go1.20 to go1.22

* Update go.sum with mod tidy

* Meta - Update dependencies (#483)

* Update dependencies

* Fix lint issue

* Generate JSON tests to trigger actions

* Update fastssz

* Generate JSON tests and align ssz error

* Revert go-eth2-client version change

* Revert fastssz upgrade

* Generate SSZ and JSON tests

* Meta - Fix static analysis issues (#480)

* Solve potential file inclusion via variable

* Fix file permission (0644 to 0600)

* Add nosec comment for PRNG (pseudo-random number generator) used for testing

* Fix lint issue on nil check in []byte type

* Update permission from 0444 to 0600

* Update 0444 to 0400

* Meta - Drop unnecessary nolint comments (#477)

* Remove nolint comment and export timeout variables

* Drop unnecessary nolint

* Add comment

* Fix lint issue

* Spec - Share length validation (#478)

* Add share length validation in runner construction

* Align to error handling in runners constructions

* Add validation to committee runner

* Add runner construction tests

* Refactor runner construction in testingutil to deal with creation errors

* Generate JSON tests

* Fix lint issue

* Fix comments

* Meta - Drop redundant error (#475)

* Spec - Drop redundant validation for decided messages (#476)

* Remove redundant validation

* Align error string

* Spec - Sort signers in decided message (#484)

* Sort signers in decided message

* Add test for sorted signers in decided msg

* Generate JSON tests

* Fix lint issue

* Spec - Stop processing after decided (#487)

* Stop processing consensus messages after instance is decided

* Align error in qbft tests

* Align errors in ssv tests

* Generate JSON tests

* Fix lint issue

* Spec - Drop leftover error check (#469)

* Remove leftover err check

* Align argument variable name to type

* Spec - Secure key storage (#481)

* Implement secure key storage using PKCS8

* Fix lint issue

* Switch back PKCS8 to PKCS1

* Meta - Remove residual DKG (#502)

* Remove DKG signature type

* Remove DKG msg type

* Remove DKGOperators field from TestKeySet

* Remove unused ecdsaKeys field from TestingKeyStorage

* Remove unused "ecdsaSKFromHex" function

* Generate JSON tests

* Spec - Add GoSec action and fix issues (#505)

* Add github action and makefile command

* Fix issues in round robin proposer function

* Fix bad PutUint32 in GetCommitteeID

* Fix issue with HasQuorum and HasPartialQuorum

* Add role sanitization in GetRoleType and NewMessageType

* Add sanitization to BeaconNetwork methods

* Add sanitization in testingutils

* Add sanitization to height usage in test files

* Fix uint64 conversion in runner/postconsensus/valid_msg test

* Sanitize ValidatorIndex conversion

* Update action name

* Fix tests to use valid RunnerRoles

* Generate SSZ

* Generate JSON tests

* Revert the change on GetCommitteeID

* Add nosec G115 to GetCommitteeID

* revert the merge because it was merged with origin main by accident

---------

Co-authored-by: rehs0y <[email protected]>
Co-authored-by: MatheusFranco99 <[email protected]>
GalRogozinski pushed a commit that referenced this pull request Nov 12, 2024
@alan-ssvlabs @y0sher
@MatheusFranco99 @GalRogozinski
Spec - Move domainType from CommitteeMember and Share to Network (#494)
cf804b6 
* remove domaintype from committeeMember

* remove domaintype from share

* add config for ssv containing domainType

* add custom unmarshal for baserunner to avoid error in unmarshalling config

* move domainType to Network

* add testing domain in NewTestingNetwork function

* Spec - filter committee duties per slot validator duties (#467)

* Filter shares for slot `CommitteeRunner` based on validators that have duty for that slot.

* Filter duty and create share map according to owned validators

* Add test: start duty with no shares for duty's validators

* Add test: happy flow for committee with fraction of duty's validators

* Generate JSON tests

* Apply suggestions

---------

Co-authored-by: MatheusFranco99 <[email protected]>

* Spec - check att and sync duties exist before submitting (#468)

* Meta - Update to go1.22 (#474)

* Update go1.20 to go1.22

* Update go.sum with mod tidy

* Meta - Update dependencies (#483)

* Update dependencies

* Fix lint issue

* Generate JSON tests to trigger actions

* Update fastssz

* Generate JSON tests and align ssz error

* Revert go-eth2-client version change

* Revert fastssz upgrade

* Generate SSZ and JSON tests

* Meta - Fix static analysis issues (#480)

* Solve potential file inclusion via variable

* Fix file permission (0644 to 0600)

* Add nosec comment for PRNG (pseudo-random number generator) used for testing

* Fix lint issue on nil check in []byte type

* Update permission from 0444 to 0600

* Update 0444 to 0400

* Meta - Drop unnecessary nolint comments (#477)

* Remove nolint comment and export timeout variables

* Drop unnecessary nolint

* Add comment

* Fix lint issue

* Spec - Share length validation (#478)

* Add share length validation in runner construction

* Align to error handling in runners constructions

* Add validation to committee runner

* Add runner construction tests

* Refactor runner construction in testingutil to deal with creation errors

* Generate JSON tests

* Fix lint issue

* Fix comments

* Meta - Drop redundant error (#475)

* Spec - Drop redundant validation for decided messages (#476)

* Remove redundant validation

* Align error string

* Spec - Sort signers in decided message (#484)

* Sort signers in decided message

* Add test for sorted signers in decided msg

* Generate JSON tests

* Fix lint issue

* Spec - Stop processing after decided (#487)

* Stop processing consensus messages after instance is decided

* Align error in qbft tests

* Align errors in ssv tests

* Generate JSON tests

* Fix lint issue

* Spec - Drop leftover error check (#469)

* Remove leftover err check

* Align argument variable name to type

* Spec - Secure key storage (#481)

* Implement secure key storage using PKCS8

* Fix lint issue

* Switch back PKCS8 to PKCS1

* Meta - Remove residual DKG (#502)

* Remove DKG signature type

* Remove DKG msg type

* Remove DKGOperators field from TestKeySet

* Remove unused ecdsaKeys field from TestingKeyStorage

* Remove unused "ecdsaSKFromHex" function

* Generate JSON tests

* Spec - Add GoSec action and fix issues (#505)

* Add github action and makefile command

* Fix issues in round robin proposer function

* Fix bad PutUint32 in GetCommitteeID

* Fix issue with HasQuorum and HasPartialQuorum

* Add role sanitization in GetRoleType and NewMessageType

* Add sanitization to BeaconNetwork methods

* Add sanitization in testingutils

* Add sanitization to height usage in test files

* Fix uint64 conversion in runner/postconsensus/valid_msg test

* Sanitize ValidatorIndex conversion

* Update action name

* Fix tests to use valid RunnerRoles

* Generate SSZ

* Generate JSON tests

* Revert the change on GetCommitteeID

* Add nosec G115 to GetCommitteeID

* revert the merge because it was merged with origin main by accident

---------

Co-authored-by: rehs0y <[email protected]>
Co-authored-by: MatheusFranco99 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alan-ssvlabs alan-ssvlabs alan-ssvlabs approved these changes

@GalRogozinski GalRogozinski GalRogozinski approved these changes

Assignees

@MatheusFranco99 MatheusFranco99

Labels
meta
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MatheusFranco99 @GalRogozinski @alan-ssvlabs