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

[Update]: Improve coverage #293

Open
wants to merge 52 commits into
base: main
Choose a base branch
from
Open

[Update]: Improve coverage #293

wants to merge 52 commits into from

Conversation

Mohsen-T
Copy link
Contributor

  • refactored the skip files array generation process in the .solcover.js
  • solidity-coverage does not offer native support *.sol.
  • implemented a programmatic function to dynamically generate the skip files array based on folder names.
  • defined the depth of coverage

mtabasco and others added 30 commits November 10, 2023 14:25
@mtabasco
event ValidatorExited -> indexed owner
c4dfe87
@mtabasco
Merge branch 'main' of https://github.com/bloxapp/ssv-network
2d2a608
@mtabasco
Merge branch 'main' of https://github.com/bloxapp/ssv-network
61fd8ed
@Mohsen-T
fix: added a test case in balance.ts
b979f71
@Mohsen-T
fix: reviewed the balance calculation when an operator is removed
71d61a0
@Mohsen-T
Update balances.ts
68cc9a8
@Mohsen-T
fix: added a test case
6951d28
@Mohsen-T
fix: removed only
c5776f7
@Mohsen-T
fix: progressing
6521a99
@Mohsen-T
fix: updated test case
df0e0ad
@Mohsen-T
fix: removed log
d0dcb41
@mtabasco
Merge branch 'main' of https://github.com/bloxapp/ssv-network
6cfe19a
@Mohsen-T
[Feature] - Integration ssv-keys in ssv-network for generating keysha…
7564dfe 
…res (#276)

* feat: updated validators & operators management using ssv-keys
@Mohsen-T
[Update]: deployment tests and update task and documentation (#282)
4471f05 
* fix: updated documentations
@Mohsen-T
[Update] Added triggers for compilation in a few tasks (upgrading / u…
c9a90b1 
…pdating) (#285)

* feat: added trigger for compilation in tasks
@Mohsen-T
Merge branch 'v1.1.0' into mohsen-fix-balance-calculation-operators
899ccd7
@Mohsen-T
fix: changed before test
88e22a3
@Mohsen-T
Merge pull request #284 from bloxapp/mohsen-fix-balance-calculation-o…
a886dc8 
…perators

[Review] Balance calculation when operators will be removed
@mtabasco
bulk validator registration (init)
c5b63db
@mtabasco
2 register validator functions, all tests pass
d24c4a2
@mtabasco
refactor cluster and operators libraries
331420f
@mtabasco
sync with 1.1.0
a0b451b
@mtabasco
V1.0.2 deployment metadata (#283)
335f008 
* v1.0.2 deployment metadata
@mtabasco
stage metadata, update version
7943326
@mtabasco
Merge branch 'main' of https://github.com/bloxapp/ssv-network
71a9cf6
@mtabasco
Bulk registration of validators (#287)
9c609cd 
* bulk validator registration
@mtabasco
update CHANGELOG
5638b7f
@mtabasco
bulk remove validators
6431a00
@mtabasco
upgrade goerli prod
5483446
@mtabasco
merge from v1.0.2-bulk registration
de9484f
mtabasco and others added 17 commits January 16, 2024 02:25
@mtabasco
hlsky dev metadata
49ba157
@mtabasco
add exit validator tests
c80dc3b
@mtabasco
update CHANGELOG
a8f814d
@mtabasco
Merge branch 'main' into bulk-features
968279e
@mtabasco
update gas usage
7ce9440
@mtabasco
update roles doc
af99def
@mtabasco
minor improvements
56d1184
@mtabasco
v1.1.0 goerli_prod metadata
584b2cb
@mtabasco
remove abis from git
31389e1
@mtabasco
remove liquidation check (duplicate)
f51d071
@Mohsen-T
Fix: updated the test coverage
1156139 
Enhanced test coverage by incorporating additional test cases to address previously overlooked scenarios
@mtabasco
fix: bulkRemoveValidators reduce validator counter
a58162e
@mtabasco
Merge branch 'bulk-features' of https://github.com/bloxapp/ssv-network …
acc3393 
…into bulk-features
@mtabasco
fix: bulkRemoveValidator all tests passed
9508cf3
@mtabasco
update gas numbers
c20aaca
@mtabasco
update NatSpec comments
27a6b99
@Mohsen-T
feat: improve the solidity coverage
d61b03d 
- refactored the skip files array generation process in the .solcover.js
- solidity-coverage does not offer native support *.sol.
- implemented a programmatic function to dynamically generate the skip files array based on folder names.
- defined the depth of coverage
@mtabasco
Copy link
Collaborator

image @Mohsen-T to reach this point, create a test in `test/validators/remove.ts` trying to remove an existing validator (`publicKey` correct) with the wrong parameters (`msg.sender` or `operatorIds`)

@mtabasco
Copy link
Collaborator

image

@Mohsen-T please create a test to fire the revert PublicKeysSharesLengthMismatch

@mtabasco
Copy link
Collaborator

image

In ClusterLib.sol seems this line is skipped.
The test Register validator in a new cluster with incorrect input data passes the wrong cluster data. Maybe you can pass specifically that parameter wrong to include that line in the coverage? thanks

@mtabasco
Copy link
Collaborator

mtabasco commented Feb 14, 2024 •
edited
Loading

image image

In OperatorsLib.sol, we haven't tested ExceedValidatorLimit error😱

@mtabasco
Copy link
Collaborator

mtabasco commented Feb 14, 2024 •
edited
Loading

image ProtocolLib.sol Please test the `MaxValueExceeded` error

@mtabasco
Copy link
Collaborator

mtabasco commented Feb 14, 2024 •
edited
Loading

image image

ValidatorLib.sol
Please test InvalidOperatorIdsLength and InvalidPublicKeyLength

@mtabasco
Copy link
Collaborator

image

please skip contracts/upgrades from the coverage test

mtabasco
mtabasco requested changes Feb 14, 2024
View reviewed changes
Copy link
Collaborator

@mtabasco mtabasco left a comment

Choose a reason for hiding this comment

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

Please see the attached comments. thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mtabasco mtabasco mtabasco approved these changes

At least 3 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Mohsen-T @mtabasco