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: registry coordinator unit test improvements #121

Merged
merged 50 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
2eb591d
feat: add tree diagram for RegistryManager
ChaoticWalrus Dec 20, 2023
0a6d3cd
chore: reorder and rename tests
ChaoticWalrus Dec 21, 2023
f9448fa
feat: add a couple simple tests
ChaoticWalrus Dec 21, 2023
ebe657e
docs: update reg coord to include pubkey registration and service man…
wadealexc Dec 28, 2023
98f8844
docs: add documentation for each registry (#125)
wadealexc Jan 1, 2024
d2f6b63
chore: fix tree file name
ChaoticWalrus Jan 2, 2024
ba24f6e
chore: add a couple post-checks on state
ChaoticWalrus Jan 3, 2024
0bbd089
chore: fix breaking test
ChaoticWalrus Jan 3, 2024
fb36bff
feat: add tree diagram for RegistryManager
ChaoticWalrus Dec 20, 2023
007cc92
chore: reorder and rename tests
ChaoticWalrus Dec 21, 2023
891f76d
feat: add a couple simple tests
ChaoticWalrus Dec 21, 2023
bf3bf38
chore: fix tree file name
ChaoticWalrus Jan 2, 2024
eb9a8da
chore: add a couple post-checks on state
ChaoticWalrus Jan 3, 2024
f4f9cf1
chore: fix breaking test
ChaoticWalrus Jan 3, 2024
ffefd02
Merge branch 'feat-RegistryManager-Unit-Test-Improvements' of https:/…
ChaoticWalrus Jan 3, 2024
798c461
feat: add a test for partial deregistration
ChaoticWalrus Jan 3, 2024
42e6838
feat: expose more internal functions in harnessed contract
ChaoticWalrus Jan 3, 2024
3e95552
feat: add some simple coverage for the internal `_registerOperator` fnc
ChaoticWalrus Jan 3, 2024
83c5aa6
feat: add test coverage for the internal `deregisterOperator` fnc
ChaoticWalrus Jan 3, 2024
e43114d
feat: add simple test coverage for the internal `_updateOperatorBitma…
ChaoticWalrus Jan 4, 2024
4dae487
chore: remove commitlint job that reviews all commits in PR from CI
ChaoticWalrus Jan 4, 2024
500d7ba
feat: add some coverage for `updateOperators(ForQuorums)` fncs
ChaoticWalrus Jan 4, 2024
5e5115f
feat: add testing for `updateOperatorsForQuorum` function
ChaoticWalrus Jan 4, 2024
814855c
feat: add some coverage for complex view functions
ChaoticWalrus Jan 8, 2024
8882f7c
chore: clarify NatSpec comments
ChaoticWalrus Jan 9, 2024
26565d6
fix: make `getQuorumBitmapIndicesAtBlockNumber` revert if operator wa…
ChaoticWalrus Jan 9, 2024
4e70e81
feat: add simple unit test for `getQuorumBitmapIndicesAtBlockNumber`
ChaoticWalrus Jan 9, 2024
ab44a82
chore: move `_getQuorumBitmapIndexAtBlockNumber` into the section wit…
ChaoticWalrus Jan 9, 2024
8cd21f4
chore: remove unnecessary require statement + improve code clarity
ChaoticWalrus Jan 9, 2024
278b660
fix: correct a compiler error for implicit type conversion
ChaoticWalrus Jan 9, 2024
1f1a840
feat: address TODOs in tests
ChaoticWalrus Jan 9, 2024
22fb866
feat: add tree diagram for RegistryManager
ChaoticWalrus Dec 20, 2023
fd56b76
chore: fix tree file name
ChaoticWalrus Jan 2, 2024
6b30068
feat: add a test for partial deregistration
ChaoticWalrus Jan 3, 2024
21a4ad2
feat: expose more internal functions in harnessed contract
ChaoticWalrus Jan 3, 2024
2fd5089
feat: add some simple coverage for the internal `_registerOperator` fnc
ChaoticWalrus Jan 3, 2024
9b1eabf
feat: add test coverage for the internal `deregisterOperator` fnc
ChaoticWalrus Jan 3, 2024
98dd6a7
feat: add simple test coverage for the internal `_updateOperatorBitma…
ChaoticWalrus Jan 4, 2024
a81fdc0
chore: remove commitlint job that reviews all commits in PR from CI
ChaoticWalrus Jan 4, 2024
c899a38
feat: add some coverage for `updateOperators(ForQuorums)` fncs
ChaoticWalrus Jan 4, 2024
ada8b4c
feat: add testing for `updateOperatorsForQuorum` function
ChaoticWalrus Jan 4, 2024
97e942a
feat: add some coverage for complex view functions
ChaoticWalrus Jan 8, 2024
4b7de19
chore: clarify NatSpec comments
ChaoticWalrus Jan 9, 2024
23c7895
fix: make `getQuorumBitmapIndicesAtBlockNumber` revert if operator wa…
ChaoticWalrus Jan 9, 2024
8f931ba
feat: add simple unit test for `getQuorumBitmapIndicesAtBlockNumber`
ChaoticWalrus Jan 9, 2024
5f18583
chore: move `_getQuorumBitmapIndexAtBlockNumber` into the section wit…
ChaoticWalrus Jan 9, 2024
9f965c1
chore: remove unnecessary require statement + improve code clarity
ChaoticWalrus Jan 9, 2024
9a328e5
fix: correct a compiler error for implicit type conversion
ChaoticWalrus Jan 9, 2024
f7244a2
feat: address TODOs in tests
ChaoticWalrus Jan 9, 2024
be57485
Merge branch 'feat-RegistryManager-Unit-Test-Improvements' of https:/…
ChaoticWalrus Jan 9, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions .github/workflows/commitlint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,4 @@ jobs:

- name: Validate current commit (last commit) with commitlint
if: github.event_name == 'push'
run: npx commitlint --from HEAD~1 --to HEAD --verbose

- name: Validate PR commits with commitlint
if: github.event_name == 'pull_request'
run: npx commitlint --from ${{ github.event.pull_request.head.sha }}~${{ github.event.pull_request.commits }} --to ${{ github.event.pull_request.head.sha }} --verbose
run: npx commitlint --from HEAD~1 --to HEAD --verbose
201 changes: 200 additions & 1 deletion docs/BLSSignatureChecker.md
Original file line number Diff line number Diff line change
@@ -1 +1,200 @@
TODO!
[core-docs-m2]: https://github.com/Layr-Labs/eigenlayer-contracts/tree/m2-mainnet/docs
[core-dmgr-docs]: https://github.com/Layr-Labs/eigenlayer-contracts/blob/m2-mainnet/docs/core/DelegationManager.md
[core-dmgr-register]: https://github.com/Layr-Labs/eigenlayer-contracts/blob/m2-mainnet/docs/core/DelegationManager.md#registeroperatortoavs
[core-dmgr-deregister]: https://github.com/Layr-Labs/eigenlayer-contracts/blob/m2-mainnet/docs/core/DelegationManager.md#deregisteroperatorfromavs

[eigenda-service-manager]: https://github.com/Layr-Labs/eigenda/blob/m2-mainnet-contracts/contracts/src/core/EigenDAServiceManager.sol

## BLSSignatureChecker

| File | Type | Proxy |
| -------- | -------- | -------- |
| [`BLSSignatureChecker.sol`](../src/BLSSignatureChecker.sol) | Singleton | Transparent proxy |
| [`OperatorStateRetriever.sol`](../src/OperatorStateRetriever.sol) | Singleton | None |

`BLSSignatureChecker` and `OperatorStateRetriever` perform (respectively) the onchain and offchain portions of BLS signature validation for the aggregate of a quorum's registered Operators.

The `OperatorStateRetriever` has various view methods intended to be called by offchain infrastructure in order to prepare a call to `BLSSignatureChecker.checkSignatures`. These methods traverse the state histories kept by the various registry contracts (see [./RegistryCoordinator.md](./RegistryCoordinator.md)) to query states at specific block numbers.

These historical states are then used within `BLSSignatureChecker` to validate a BLS signature formed from an aggregated subset of the Operators registered for one or more quorums at some specific block number.

#### High-level Concepts

This document organizes methods according to the following themes (click each to be taken to the relevant section):
* [Onchain](#onchain)
* [Offchain](#offchain)
* [System Configuration](#system-configuration)

---

### Onchain

#### `BLSSignatureChecker.checkSignatures`

```solidity
function checkSignatures(
bytes32 msgHash,
bytes calldata quorumNumbers,
uint32 referenceBlockNumber,
NonSignerStakesAndSignature memory params
)
public
view
returns (QuorumStakeTotals memory, bytes32)

struct NonSignerStakesAndSignature {
uint32[] nonSignerQuorumBitmapIndices;
BN254.G1Point[] nonSignerPubkeys;
BN254.G1Point[] quorumApks;
BN254.G2Point apkG2;
BN254.G1Point sigma;
uint32[] quorumApkIndices;
uint32[] totalStakeIndices;
uint32[][] nonSignerStakeIndices;
}

struct QuorumStakeTotals {
uint96[] signedStakeForQuorum;
uint96[] totalStakeForQuorum;
}
```

The goal of this method is to allow an AVS to validate a BLS signature formed from the aggregate pubkey ("apk") of Operators registered in one or more quorums at some `referenceBlockNumber`.

Some notes on method parameters:
* `referenceBlockNumber` is the reason each registry contract keeps historical states: so that lookups can be performed on each registry's info at a particular block. This is important because Operators may sign some data on behalf of an AVS, then deregister from one or more of the AVS's quorums. Historical states allow signature validation to be performed against a "fixed point" in AVS/quorum history.
* `quorumNumbers` is used to perform signature validation across one *or more* quorums. Also, Operators may be registered for more than one quorum - and for each quorum an Operator is registered for, that Operator's pubkey is included in that quorum's apk within the `BLSApkRegistry`. This means that, when calculating an apk across multiple `quorumNumbers`, Operators registered for more than one of these quorums will have their pubkey included more than once in the total apk.
* `params` contains both a signature from all signing Operators, as well as several fields that identify registered, non-signing Operators. While non-signing Operators haven't contributed to the signature, but need to be accounted for because, as Operators registered for one or more signing quorums, their public keys are included in that quorum's apk. Essentially, in order to validate the signature, nonsigners' public keys need to be subtracted out from the total apk to derive the apk that actually signed the message.

This method performs the following steps. Note that each step involves lookups of historical state from `referenceBlockNumber`, but the writing in this section will use the present tense because adding "at the `referenceBlockNumber`" everywhere gets confusing. Steps follow:
1. Calculate the *total nonsigner apk*, an aggregate pubkey composed of all nonsigner pubkeys. For each nonsigner:
* Query the `RegistryCoordinator` to get the nonsigner's registered quorums.
* Multiply the nonsigner's pubkey by the number of quorums in `quorumNumbers` the nonsigner is registered for.
* Add the result to the *total nonsigner apk*.
2. Calculate the negative of the *total nonsigner apk*.
3. For each quorum:
* Query the `BLSApkRegistry` to get the *quorum apk*: the aggregate pubkey of all Operators registered for that quorum.
* Add the *quorum apk* to the *total nonsigner apk*. This effectively subtracts out any pubkeys belonging to nonsigning Operators in the quorum, leaving only pubkeys of signing Operators. We'll call the result the *total signing apk*.
* Query the `StakeRegistry` to get the total stake for the quorum.
* For each nonsigner, if the nonsigner is registered for the quorum, query the `StakeRegistry` for their stake and subtract it from the total. This leaves only stake belonging to signing Operators.
4. Use the `msgHash`, the *total signing apk*, `params.apkG2`, and `params.sigma` to validate the BLS signature.
5. Return the total stake and signing stakes for each quorum, along with a hash identifying the `referenceBlockNumber` and non-signers

*Entry Points* (EigenDA):
* Called by [`EigenDAServiceManager.confirmBatch`][eigenda-service-manager]

*Requirements*:
* Input validation:
* Quorum-related fields MUST have equal lengths: `quorumNumbers`, `params.quorumApks`, `params.quorumApkIndices`, `params.totalStakeIndices`, `params.nonSignerStakeIndices`
* Nonsigner-related fields MUST have equal lengths: `params.nonSignerPubkeys`, `params.nonSignerQuorumBitmapIndices`
* `referenceBlockNumber` MUST NOT be greater than `block.number`
* `quorumNumbers` MUST be an ordered list of valid, initialized quorums
* `params.nonSignerPubkeys` MUST ONLY contain unique pubkeys, in ascending order of their pubkey hash
* For each quorum:
* If stale stakes are forbidden (see [`BLSSignatureChecker.setStaleStakesForbidden`](#blssignaturecheckersetstalestakesforbidden)), check the last `quorumUpdateBlockNumber` is within `DelegationManager.withdrawalDelayBlocks` of `referenceBlockNumber`. This references a value in the EigenLayer core contracts - see [EigenLayer core docs][core-docs-m2] for more info.
* Validate that each `params.quorumApks` corresponds to the quorum's apk at the `referenceBlockNumber`
* For each historical state lookup, the `referenceBlockNumber` and provided index MUST point to a valid historical entry:
* `referenceBlockNumber` MUST come after the entry's `updateBlockNumber`
* The entry's `nextUpdateBlockNumber` MUST EITHER be 0, OR greater than `referenceBlockNumber`

---

### Offchain

These methods perform very gas-heavy lookups through various registry states, and are called by offchain infrastructure to construct calldata for a call to `BLSSignatureChecker.checkSignatures`:
* [`OperatorStateRetriever.getOperatorState (operatorId)`](#operatorstateretrievergetoperatorstate-operatorid)
* [`OperatorStateRetriever.getOperatorState (quorumNumbers)`](#operatorstateretrievergetoperatorstate-quorumnumbers)
* [`OperatorStateRetriever.getCheckSignaturesIndices`](#operatorstateretrievergetchecksignaturesindices)

#### `OperatorStateRetriever.getOperatorState (operatorId)`

```solidity
function getOperatorState(
IRegistryCoordinator registryCoordinator,
bytes32 operatorId,
uint32 blockNumber
)
external
view
returns (uint256, Operator[][] memory)

struct Operator {
bytes32 operatorId;
uint96 stake;
}
```

Traverses history in the `RegistryCoordinator`, `IndexRegistry`, and `StakeRegistry` to retrieve information on an Operator (given by `operatorId`) and the quorums they are registered for at a specific `blockNumber`. Returns:
* `uint256`: a bitmap of the quorums the Operator was registered for at `blockNumber`
* `Operator[][]`: For each of the quorums mentioned above, this is a list of the Operators registered for that quorum at `blockNumber`, containing each Operator's `operatorId` and `stake`.

#### `OperatorStateRetriever.getOperatorState (quorumNumbers)`

```solidity
function getOperatorState(
IRegistryCoordinator registryCoordinator,
bytes memory quorumNumbers,
uint32 blockNumber
)
public
view
returns(Operator[][] memory)
```

Traverses history in the `RegistryCoordinator`, `IndexRegistry`, and `StakeRegistry` to retrieve information on the Operator set registered for each quorum in `quorumNumbers` at `blockNumber`. Returns:
* `Operator[][]`: For each quorum in `quorumNumbers`, this is a list of the Operators registered for that quorum at `blockNumber`, containing each Operator's `operatorId` and `stake`.

#### `OperatorStateRetriever.getCheckSignaturesIndices`

```solidity
function getCheckSignaturesIndices(
IRegistryCoordinator registryCoordinator,
uint32 referenceBlockNumber,
bytes calldata quorumNumbers,
bytes32[] calldata nonSignerOperatorIds
)
external
view
returns (CheckSignaturesIndices memory)

struct CheckSignaturesIndices {
uint32[] nonSignerQuorumBitmapIndices;
uint32[] quorumApkIndices;
uint32[] totalStakeIndices;
uint32[][] nonSignerStakeIndices; // nonSignerStakeIndices[quorumNumberIndex][nonSignerIndex]
}
```

Traverses histories in the `RegistryCoordinator`, `IndexRegistry`, `StakeRegistry`, and `BLSApkRegistry` to retrieve information on one or more quorums' Operator sets and nonsigning Operators at a given `referenceBlockNumber`.

The return values are all "indices," because of the linear historical state each registry keeps. Offchain code calls this method to compute indices into historical state, which later is leveraged for cheap lookups in `BLSSignatureChecker.checkSignatures` (rather than traversing over the history during an onchain operation).

For each quorum, this returns:
* `uint32[] nonSignerQuorumBitmapIndices`: The indices in `RegistryCoordinator._operatorBitmapHistory` where each nonsigner's registered quorum bitmap can be found at `referenceBlockNumber`. Length is equal to the number of nonsigners included in `nonSignerOperatorIds`
* `uint32[] quorumApkIndices`: The indices in `BLSApkRegistry.apkHistory` where the quorum's apk can be found at `referenceBlockNumber`. Length is equal to the number of quorums in `quorumNumbers`.
* `uint32[] totalStakeIndices`: The indices in `StakeRegistry._totalStakeHistory` where each quorum's total stake can be found at `referenceBlockNumber`. Length is equal to the number of quorums in `quorumNumbers`.
* `uint32[][] nonSignerStakeIndices`: For each quorum, a list of the indices of each nonsigner's `StakeRegistry.operatorStakeHistory` entry at `referenceBlockNumber`. Length is equal to the number of quorums in `quorumNumbers`, and each sub-list is equal in length to the number of nonsigners in `nonSignerOperatorIds` registered for that quorum at `referenceBlockNumber`

---

### System Configuration

#### `BLSSignatureChecker.setStaleStakesForbidden`

```solidity
function setStaleStakesForbidden(
bool value
)
external
onlyCoordinatorOwner
```

This method allows the `RegistryCoordinator` Owner to update `staleStakesForbidden` in the `BLSSignatureChecker`. If stale stakes are forbidden, `BLSSignatureChecker.checkSignatures` will perform an additional check when querying each quorum's apk, Operator stakes, and total stakes.

This additional check requires that each quorum was updated within a certain block window of the `referenceBlockNumber` passed into `BLSSignatureChecker.checkSignatures`.

*Effects*:
* Sets `staleStakesForbidden` to `value`

*Requirements*:
* Caller MUST be the `RegistryCoordinator` Owner
Loading