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

feat: ecdsa basic registry #194

Merged
merged 73 commits into from
Mar 20, 2024
Merged

feat: ecdsa basic registry #194

merged 73 commits into from
Mar 20, 2024

Conversation

stevennevins
Copy link
Collaborator

@stevennevins stevennevins commented Feb 26, 2024

This PR introduces a simpler registry implementation as an alternative to the existing StakeRegistry.sol. This is based on the teams asking for more examples and a general requirement to use ECDSA as their signature scheme.

Also, standardizing to use an isValidSignature function is proposed to query if a set of signatures from operators are valid at a given block height which isn't contained in the other implementation but seems like a useful standardization

ypatil12 and others added 30 commits January 26, 2024 15:34
* refactor: minWithdrawalDelayBLocks from core

* fix: core contracts commit and tests

* feat: add avs directory to service manager

* fix: rebase off updated bls sig checker; update integration tests

* Add AVS Directory Support to Service Manager (#156)

* update: change core submodule branch

* feat: add avs directory to service manager

* fix: rebase off updated bls sig checker; update integration tests

* fix: core contracts commit and tests

* feat: add avs directory to service manager

* fix: rebase off updated bls sig checker; update integration tests

* fix: conflicts

---------

Co-authored-by: 8sunyuan <[email protected]>

* fix: submodule commit

* fix: rebase changes

* Add AVS Directory Support to Service Manager (#156)

* update: change core submodule branch

* feat: add avs directory to service manager

* fix: rebase off updated bls sig checker; update integration tests

* fix: core contracts commit and tests

* feat: add avs directory to service manager

* fix: rebase off updated bls sig checker; update integration tests

* fix: conflicts

---------

Co-authored-by: 8sunyuan <[email protected]>

* fix: submodule commit

* fix: rebase changes

* Add AVS Directory Support to Service Manager (#156)

* update: change core submodule branch

* feat: add avs directory to service manager

* fix: rebase off updated bls sig checker; update integration tests

* fix: core contracts commit and tests

* feat: add avs directory to service manager

* fix: rebase off updated bls sig checker; update integration tests

* fix: conflicts

---------

Co-authored-by: 8sunyuan <[email protected]>

* fix: submodule commit

* docs: AVSRegistry -> AVSDirectory

---------

Co-authored-by: Yash Patil <[email protected]>
* test: fix flaky tests by removing bogosort

* test: fix flaky test by rejecting empty addr inputs
* chore: add storage gaps to BLSSignatureChecker and ServiceManagerBase

* chore: ServiceManagerBase to abstract and create mock

* chore: use onlyInitializing in Base and initializer in mock

* chore: add storage gaps to BLSSignatureChecker and ServiceManagerBase

* chore: ServiceManagerBase to abstract and create mock

* chore: use onlyInitializing in Base and initializer in mock

* fix: core submodules

---------

Co-authored-by: steven <[email protected]>
* test: gas scenarios for updateOperators

* refactor: using one call for operatorShares

* test: updateOperators 200 operators

* test: gas scenarios for updateOperators

* refactor: using one call for operatorShares

* test: updateOperators 200 operators

* fix: comments
slightly less memory ops in `orderedBytesArrayToBitmap` fnc
* fix: include missing fields in TYPEHASH defintion

the `salt` and `expiry` fields were missing from the `OPERATOR_CHURN_APPROVAL_TYPEHASH` def

* fix: correct definition of OperatorKickParam inside of typehash def
…180)

* chore: clean up loops to iterate downward and remove unneeded checks

* docs: clarifying comments around quorum existence checks
@NimaVaziri
Copy link

@stevennevins seems like @ChaoticWalrus is good with this / the remaining comments are minor - any blockers to get this over the line?

@stevennevins
Copy link
Collaborator Author

@stevennevins seems like @ChaoticWalrus is good with this / the remaining comments are minor - any blockers to get this over the line?

@ChaoticWalrus, I just wanted to confirm we're aligned on the changes for the remaining unresolved items. Also if you see any hurdles to getting this in

@ChaoticWalrus
Copy link
Contributor

@ChaoticWalrus, I just wanted to confirm we're aligned on the changes for the remaining unresolved items. Also if you see any hurdles to getting this in

Things are looking fine to me, overall.
Some more explanatory comments / noting somewhere about some of the finer points we discussed seems reasonable.

revert InvalidSignedWeight();
}
uint256 thresholdStake = _getThresholdStake(_referenceBlock);
if ((thresholdStake * BPS)/ totalWeight > (_signedWeight * BPS) / totalWeight){
Copy link
Contributor

@ChaoticWalrus ChaoticWalrus Mar 19, 2024

Choose a reason for hiding this comment

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

looks like you actually totally changed the meaning of thresholdStake to be the literal amount required to sign?
this check could now just be if( thresholdStake > _signedWeight ) because you're doing the exact same manipulations on both sides of the inequality

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in 79d95aa

Copy link
Contributor

@ChaoticWalrus ChaoticWalrus left a comment

Choose a reason for hiding this comment

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

I'm OK merging this into the 'unaudited' folder at this point, personally.
would be good to note any outstanding discussions, probably with some comments?
also might be nice to run this file structure by someone else, but I think it's quite clear having an "unaudited" folder 🤷

@stevennevins stevennevins merged commit eae9114 into dev Mar 20, 2024
3 checks passed
@stevennevins stevennevins deleted the feat/ecdsa-basic branch March 20, 2024 14:30
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.

8 participants