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

Test: bitmap utils unit tests #101

Merged
merged 5 commits into from
Jan 10, 2024
Merged

Test: bitmap utils unit tests #101

merged 5 commits into from
Jan 10, 2024

Conversation

8sunyuan
Copy link
Collaborator

@8sunyuan 8sunyuan commented Dec 12, 2023

Remove unused functions from BitmapUtils.sol, bytesArrayToBitmap, orderedBytesArrayToBitmap_Yul, bytesArrayToBitmap_Yul.
bytesArrayToBitmap is used only once in the sigchecker and replaced with orderedBytesArrayToBitmap, need to confirm this small change is fine in terms of gas increase and quorumNumbers ordering.
Added some additional unit tests for functions to BitmapUtils library.

@8sunyuan 8sunyuan force-pushed the test/bitmap-utils-unit branch from 77aa80b to 4fceeda Compare January 8, 2024 16:46
Copy link
Collaborator

@wadealexc wadealexc left a comment

Choose a reason for hiding this comment

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

Looks great!

@8sunyuan 8sunyuan merged commit c53bb81 into m2-mainnet Jan 10, 2024
3 checks passed
@8sunyuan 8sunyuan deleted the test/bitmap-utils-unit branch January 10, 2024 15:54
8sunyuan added a commit that referenced this pull request Jan 10, 2024
* test: refactor and add tests to bitmap unit

* test: added tests and using asserts

* chore: remove single quote

* feat: addNumberToBitmap function

* test: tree file and Alexs bitmap fix

* test: add back updated stakeRegistry tests

* test: config tests

setMinimumStake tests
addStrategies tests
initializeQuorum tests

* refactor: using MockAVSDeployer for test file

* test: more config tests

* refactor: stake registry harness weighting

* fix: stake weighting refactor

removed overriding weightOfOperator functions
in StakeRegistry and fixed broken tests using the helper
_setOperatorWeight()

* test: refactor and add tests to bitmap unit

* test: added tests and using asserts

* chore: remove single quote

* feat: addNumberToBitmap function

* test: tree file and Alexs bitmap fix

* test: add back updated stakeRegistry tests

* test: config tests

setMinimumStake tests
addStrategies tests
initializeQuorum tests

* refactor: using MockAVSDeployer for test file

* test: more config tests

* refactor: stake registry harness weighting

* fix: stake weighting refactor

removed overriding weightOfOperator functions
in StakeRegistry and fixed broken tests using the helper
_setOperatorWeight()

* fix: undo change

* chore: rebase fixes

* test: voteweighing tests

* chore: updated eigenlayer-contract ref to m2-mainnet head (#119)

* updated eigenlayer-contract ref to m2-mainnet head

* fixed test - interface mismatch from previous update

* docs: update reg coord to include pubkey registration and service man… (#116)

* docs: update reg coord to include pubkey registration and service manager usage

* docs: add service manager to tech docs intro

* fix: update table of contents

* docs: add docs for BLSSignatureChecker and OperatorStateRetriever

* docs: address feedback

* docs: add documentation for each registry (#125)

* docs: standardize capitalization of Operator since thats what we do everywhere else

* docs: add BLSApkRegistry docs

* chore: remove old files

- i've incorporated all the info from these files into the current docs, so i'm removing them

* docs: add wip for IndexRegistry and StakeRegistry, and fix spacing in StakeRegistry

* docs: add IndexRegistry docs

* docs: Add StakeRegistry

* docs: clarify wording

* test: fix unit tests

* feat: registry coordinator unit test improvements (#121)

* feat: add tree diagram for RegistryManager

* chore: reorder and rename tests

* feat: add a couple simple tests

note that the `test_createQuorum` test is currently failing because
initialization already sets up the max number of quorums

something will need to be adjusted here

* docs: update reg coord to include pubkey registration and service man… (#116)

* docs: update reg coord to include pubkey registration and service manager usage

* docs: add service manager to tech docs intro

* fix: update table of contents

* docs: add docs for BLSSignatureChecker and OperatorStateRetriever

* docs: address feedback

* docs: add documentation for each registry (#125)

* docs: standardize capitalization of Operator since thats what we do everywhere else

* docs: add BLSApkRegistry docs

* chore: remove old files

- i've incorporated all the info from these files into the current docs, so i'm removing them

* docs: add wip for IndexRegistry and StakeRegistry, and fix spacing in StakeRegistry

* docs: add IndexRegistry docs

* docs: Add StakeRegistry

* docs: clarify wording

* chore: fix tree file name

* chore: add a couple post-checks on state

also fix a couple test names

* chore: fix breaking test

set up one less than the max number of quorums in a single test's setup,
rather than the full max number,
so that the test will properly allow the creation of a new quorum

also fix a typo in a test name

* feat: add tree diagram for RegistryManager

* chore: reorder and rename tests

* feat: add a couple simple tests

note that the `test_createQuorum` test is currently failing because
initialization already sets up the max number of quorums

something will need to be adjusted here

* chore: fix tree file name

* chore: add a couple post-checks on state

also fix a couple test names

* chore: fix breaking test

set up one less than the max number of quorums in a single test's setup,
rather than the full max number,
so that the test will properly allow the creation of a new quorum

also fix a typo in a test name

* feat: add a test for partial deregistration

also improve some formatting + fix some typos, and add a touch more documentation

* feat: expose more internal functions in harnessed contract

* feat: add some simple coverage for the internal `_registerOperator` fnc

* feat: add test coverage for the internal `deregisterOperator` fnc

also clarify somewhat ambiguous wording in the tree file

* feat: add simple test coverage for the internal `_updateOperatorBitmap` fnc

* chore: remove commitlint job that reviews all commits in PR from CI

This was causing a *lot* of CI failures, including for merge commits

With this commit, CI will still check the _latest_ commit for meeting conventions,
it just won't run over all commits in a PR

This may lead to a few more "unconventional" commits making it through,
but the CI should still flag when someone is just not using conventional commits at all,
which I think was the original goal.

* feat: add some coverage for `updateOperators(ForQuorums)` fncs

* feat: add testing for `updateOperatorsForQuorum` function

* feat: add some coverage for complex view functions

note: this commit also adds a TODO around a currently-failing test.
I plan to discuss the correct path forwards here and then push another commit.

* chore: clarify NatSpec comments

* fix: make `getQuorumBitmapIndicesAtBlockNumber` revert if operator was registered

the logic is now more in-line with the logic in the StakeRegistry -- for reference, see:

https://github.com/layr-labs/eigenlayer-middleware/blob/
98f8844/src/StakeRegistry.sol#L297-L299

* feat: add simple unit test for `getQuorumBitmapIndicesAtBlockNumber`

also improve wording in the 'tree' file

* chore: move `_getQuorumBitmapIndexAtBlockNumber` into the section with other internal fncs

* chore: remove unnecessary require statement + improve code clarity

* fix: correct a compiler error for implicit type conversion

* feat: address TODOs in tests

* feat: add tree diagram for RegistryManager

* chore: fix tree file name

* feat: add a test for partial deregistration

also improve some formatting + fix some typos, and add a touch more documentation

* feat: expose more internal functions in harnessed contract

* feat: add some simple coverage for the internal `_registerOperator` fnc

* feat: add test coverage for the internal `deregisterOperator` fnc

also clarify somewhat ambiguous wording in the tree file

* feat: add simple test coverage for the internal `_updateOperatorBitmap` fnc

* chore: remove commitlint job that reviews all commits in PR from CI

This was causing a *lot* of CI failures, including for merge commits

With this commit, CI will still check the _latest_ commit for meeting conventions,
it just won't run over all commits in a PR

This may lead to a few more "unconventional" commits making it through,
but the CI should still flag when someone is just not using conventional commits at all,
which I think was the original goal.

* feat: add some coverage for `updateOperators(ForQuorums)` fncs

* feat: add testing for `updateOperatorsForQuorum` function

* feat: add some coverage for complex view functions

note: this commit also adds a TODO around a currently-failing test.
I plan to discuss the correct path forwards here and then push another commit.

* chore: clarify NatSpec comments

* fix: make `getQuorumBitmapIndicesAtBlockNumber` revert if operator was registered

the logic is now more in-line with the logic in the StakeRegistry -- for reference, see:

https://github.com/layr-labs/eigenlayer-middleware/blob/
98f8844/src/StakeRegistry.sol#L297-L299

* feat: add simple unit test for `getQuorumBitmapIndicesAtBlockNumber`

also improve wording in the 'tree' file

* chore: move `_getQuorumBitmapIndexAtBlockNumber` into the section with other internal fncs

* chore: remove unnecessary require statement + improve code clarity

* fix: correct a compiler error for implicit type conversion

* feat: address TODOs in tests

---------

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

* Test: bitmap utils unit tests (#101)

* test: refactor and add tests to bitmap unit

* test: added tests and using asserts

* chore: remove single quote

* feat: addNumberToBitmap function

* chore: remove unused bitmap functions

* feat: addNumberToBitmap function

* test: tree file and Alexs bitmap fix

* test: add back updated stakeRegistry tests

* test: config tests

setMinimumStake tests
addStrategies tests
initializeQuorum tests

* refactor: using MockAVSDeployer for test file

* test: more config tests

* refactor: stake registry harness weighting

* fix: stake weighting refactor

removed overriding weightOfOperator functions
in StakeRegistry and fixed broken tests using the helper
_setOperatorWeight()

* feat: addNumberToBitmap function

* test: add back updated stakeRegistry tests

* test: config tests

setMinimumStake tests
addStrategies tests
initializeQuorum tests

* refactor: using MockAVSDeployer for test file

* test: more config tests

* refactor: stake registry harness weighting

* fix: stake weighting refactor

removed overriding weightOfOperator functions
in StakeRegistry and fixed broken tests using the helper
_setOperatorWeight()

* chore: rebase fixes

* fix: undo change

* test: voteweighing tests

* test: fix unit tests

* fix: rebase errors

* fix: broken tests from rebase

---------

Co-authored-by: Samuel Laferriere <[email protected]>
Co-authored-by: Alex <[email protected]>
Co-authored-by: ChaoticWalrus <[email protected]>
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.

2 participants