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

refactor: custom errors in middleware #355

Merged
merged 2 commits into from
Jan 10, 2025
Merged

refactor: custom errors in middleware #355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c770e7e
refactor: custom errors
8sunyuan Jan 10, 2025
00e0832
fix: unit tests
8sunyuan Jan 10, 2025
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
34 changes: 10 additions & 24 deletions src/BLSApkRegistry.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
* @param quorumNumber The number of the new quorum
*/
function initializeQuorum(uint8 quorumNumber) public virtual onlyRegistryCoordinator {
require(apkHistory[quorumNumber].length == 0, "BLSApkRegistry.initializeQuorum: quorum already exists");
require(apkHistory[quorumNumber].length == 0, QuorumAlreadyExists());

apkHistory[quorumNumber].push(ApkUpdate({
apkHash: bytes24(0),
Expand All @@ -100,17 +100,9 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
BN254.G1Point calldata pubkeyRegistrationMessageHash
) external onlyRegistryCoordinator returns (bytes32 operatorId) {
bytes32 pubkeyHash = BN254.hashG1Point(params.pubkeyG1);
require(
pubkeyHash != ZERO_PK_HASH, "BLSApkRegistry.registerBLSPublicKey: cannot register zero pubkey"
);
require(
operatorToPubkeyHash[operator] == bytes32(0),
"BLSApkRegistry.registerBLSPublicKey: operator already registered pubkey"
);
require(
pubkeyHashToOperator[pubkeyHash] == address(0),
"BLSApkRegistry.registerBLSPublicKey: public key already registered"
);
require(pubkeyHash != ZERO_PK_HASH, ZeroPubKey());
require(operatorToPubkeyHash[operator] == bytes32(0), OperatorAlreadyRegistered());
require(pubkeyHashToOperator[pubkeyHash] == address(0), BLSPubkeyAlreadyRegistered());

// gamma = h(sigma, P, P', H(m))
uint256 gamma = uint256(keccak256(abi.encodePacked(
Expand All @@ -130,7 +122,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
BN254.negGeneratorG2(),
pubkeyRegistrationMessageHash.plus(BN254.generatorG1().scalar_mul(gamma)),
params.pubkeyG2
), "BLSApkRegistry.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match");
), InvalidBLSSignatureOrPrivateKey());

operatorToPubkey[operator] = params.pubkeyG1;
operatorToPubkeyHash[operator] = pubkeyHash;
Expand All @@ -151,7 +143,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
// Validate quorum exists and get history length
uint8 quorumNumber = uint8(quorumNumbers[i]);
uint256 historyLength = apkHistory[quorumNumber].length;
require(historyLength != 0, "BLSApkRegistry._processQuorumApkUpdate: quorum does not exist");
require(historyLength != 0, QuorumDoesNotExist());

// Update aggregate public key for this quorum
newApk = currentApk[quorumNumber].plus(point);
Expand Down Expand Up @@ -185,10 +177,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
BN254.G1Point memory pubkey = operatorToPubkey[operator];
bytes32 pubkeyHash = operatorToPubkeyHash[operator];

require(
pubkeyHash != bytes32(0),
"BLSApkRegistry.getRegisteredPubkey: operator is not registered"
);
require(pubkeyHash != bytes32(0), OperatorNotRegistered());

return (pubkey, pubkeyHash);
}
Expand Down Expand Up @@ -253,11 +242,11 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
*/
require(
blockNumber >= quorumApkUpdate.updateBlockNumber,
"BLSApkRegistry.getApkHashAtBlockNumberAndIndex: index too recent"
BlockNumberTooRecent()
);
require(
quorumApkUpdate.nextUpdateBlockNumber == 0 || blockNumber < quorumApkUpdate.nextUpdateBlockNumber,
"BLSApkRegistry.getApkHashAtBlockNumberAndIndex: not latest apk update"
BlockNumberNotLatest()
);

return quorumApkUpdate.apkHash;
Expand All @@ -280,9 +269,6 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
}

function _checkRegistryCoordinator() internal view {
require(
msg.sender == address(registryCoordinator),
"BLSApkRegistry._checkRegistryCoordinator: caller is not the registry coordinator"
);
require(msg.sender == address(registryCoordinator), OnlyRegistryCoordinatorOwner());
}
}
35 changes: 10 additions & 25 deletions src/BLSSignatureChecker.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,7 @@ contract BLSSignatureChecker is IBLSSignatureChecker {
bool public staleStakesForbidden;

modifier onlyCoordinatorOwner() {
require(
msg.sender == registryCoordinator.owner(),
"BLSSignatureChecker.onlyCoordinatorOwner: caller is not the owner of the registryCoordinator"
);
require(msg.sender == registryCoordinator.owner(), OnlyRegistryCoordinatorOwner());
_;
}

Expand Down Expand Up @@ -91,29 +88,23 @@ contract BLSSignatureChecker is IBLSSignatureChecker {
uint32 referenceBlockNumber,
NonSignerStakesAndSignature memory params
) public view returns (QuorumStakeTotals memory, bytes32) {
require(
quorumNumbers.length != 0,
"BLSSignatureChecker.checkSignatures: empty quorum input"
);
require(quorumNumbers.length != 0, InputEmptyQuorumNumbers());

require(
(quorumNumbers.length == params.quorumApks.length) &&
(quorumNumbers.length == params.quorumApkIndices.length) &&
(quorumNumbers.length == params.totalStakeIndices.length) &&
(quorumNumbers.length == params.nonSignerStakeIndices.length),
"BLSSignatureChecker.checkSignatures: input quorum length mismatch"
InputArrayLengthMismatch()
);

require(
params.nonSignerPubkeys.length ==
params.nonSignerQuorumBitmapIndices.length,
"BLSSignatureChecker.checkSignatures: input nonsigner length mismatch"
InputNonSignerLengthMismatch()
);

require(
referenceBlockNumber < uint32(block.number),
"BLSSignatureChecker.checkSignatures: invalid reference block"
);
require(referenceBlockNumber < uint32(block.number), InvalidReferenceBlocknumber());

// This method needs to calculate the aggregate pubkey for all signing operators across
// all signing quorums. To do that, we can query the aggregate pubkey for each quorum
Expand Down Expand Up @@ -155,7 +146,7 @@ contract BLSSignatureChecker is IBLSSignatureChecker {
require(
uint256(nonSigners.pubkeyHashes[j]) >
uint256(nonSigners.pubkeyHashes[j - 1]),
"BLSSignatureChecker.checkSignatures: nonSignerPubkeys not sorted"
NonSignerPubkeysNotSorted()
);
}

Expand Down Expand Up @@ -207,7 +198,7 @@ contract BLSSignatureChecker is IBLSSignatureChecker {
) +
withdrawalDelayBlocks >
referenceBlockNumber,
"BLSSignatureChecker.checkSignatures: StakeRegistry updates must be within withdrawalDelayBlocks window"
StaleStakesForbidden()
);
}

Expand All @@ -220,7 +211,7 @@ contract BLSSignatureChecker is IBLSSignatureChecker {
blockNumber: referenceBlockNumber,
index: params.quorumApkIndices[i]
}),
"BLSSignatureChecker.checkSignatures: quorumApk hash in storage does not match provided quorum apk"
InvalidQuorumApkHash()
);
apk = apk.plus(params.quorumApks[i]);

Expand Down Expand Up @@ -274,14 +265,8 @@ contract BLSSignatureChecker is IBLSSignatureChecker {
params.apkG2,
params.sigma
);
require(
pairingSuccessful,
"BLSSignatureChecker.checkSignatures: pairing precompile call failed"
);
require(
signatureIsValid,
"BLSSignatureChecker.checkSignatures: signature is invalid"
);
require(pairingSuccessful, InvalidBLSPairingKey());
require(signatureIsValid, InvalidBLSSignature());
}
// set signatoryRecordHash variable used for fraudproofs
bytes32 signatoryRecordHash = keccak256(
Expand Down
4 changes: 2 additions & 2 deletions src/EjectionManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ contract EjectionManager is IEjectionManager, OwnableUpgradeable {
* @dev The owner can eject operators without recording of stake ejection
*/
function ejectOperators(bytes32[][] memory _operatorIds) external {
require(isEjector[msg.sender] || msg.sender == owner(), "EjectionManager.ejectOperators: Only owner or ejector can eject");
require(isEjector[msg.sender] || msg.sender == owner(), OnlyOwnerOrEjector());

for(uint i = 0; i < _operatorIds.length; ++i) {
uint8 quorumNumber = uint8(i);
Expand Down Expand Up @@ -137,7 +137,7 @@ contract EjectionManager is IEjectionManager, OwnableUpgradeable {

///@dev internal function to set the quorum ejection params
function _setQuorumEjectionParams(uint8 _quorumNumber, QuorumEjectionParams memory _quorumEjectionParams) internal {
require(_quorumNumber < MAX_QUORUM_COUNT, "EjectionManager._setQuorumEjectionParams: Quorum number exceeds MAX_QUORUM_COUNT");
require(_quorumNumber < MAX_QUORUM_COUNT, MaxQuorumCount());
quorumEjectionParams[_quorumNumber] = _quorumEjectionParams;
emit QuorumEjectionParamsSet(_quorumNumber, _quorumEjectionParams.rateLimitWindow, _quorumEjectionParams.ejectableStakePercent);
}
Expand Down
10 changes: 5 additions & 5 deletions src/IndexRegistry.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ contract IndexRegistry is IndexRegistryStorage {
// Validate quorum exists and get current operator count
uint8 quorumNumber = uint8(quorumNumbers[i]);
uint256 historyLength = _operatorCountHistory[quorumNumber].length;
require(historyLength != 0, "IndexRegistry.registerOperator: quorum does not exist");
require(historyLength != 0, QuorumDoesNotExist());

/**
* Increase the number of operators currently active for this quorum,
Expand Down Expand Up @@ -87,7 +87,7 @@ contract IndexRegistry is IndexRegistryStorage {
// Validate quorum exists and get the operatorIndex of the operator being deregistered
uint8 quorumNumber = uint8(quorumNumbers[i]);
uint256 historyLength = _operatorCountHistory[quorumNumber].length;
require(historyLength != 0, "IndexRegistry.registerOperator: quorum does not exist");
require(historyLength != 0, QuorumDoesNotExist());
uint32 operatorIndexToRemove = currentOperatorIndex[quorumNumber][operatorId];

/**
Expand All @@ -113,7 +113,7 @@ contract IndexRegistry is IndexRegistryStorage {
* @param quorumNumber The number of the new quorum
*/
function initializeQuorum(uint8 quorumNumber) public virtual onlyRegistryCoordinator {
require(_operatorCountHistory[quorumNumber].length == 0, "IndexRegistry.createQuorum: quorum already exists");
require(_operatorCountHistory[quorumNumber].length == 0, QuorumDoesNotExist());

_operatorCountHistory[quorumNumber].push(QuorumUpdate({
numOperators: 0,
Expand Down Expand Up @@ -329,7 +329,7 @@ contract IndexRegistry is IndexRegistryStorage {
operatorList[i] = _operatorIdForIndexAtBlockNumber(quorumNumber, uint32(i), blockNumber);
require(
operatorList[i] != OPERATOR_DOES_NOT_EXIST_ID,
"IndexRegistry.getOperatorListAtBlockNumber: operator does not exist at the given block number"
OperatorIdDoesNotExist()
);
}
return operatorList;
Expand All @@ -342,6 +342,6 @@ contract IndexRegistry is IndexRegistryStorage {
}

function _checkRegistryCoordinator() internal view {
require(msg.sender == address(registryCoordinator), "IndexRegistry._checkRegistryCoordinator: caller is not the registry coordinator");
require(msg.sender == address(registryCoordinator), OnlyRegistryCoordinator());
}
}
4 changes: 3 additions & 1 deletion src/OperatorStateRetriever.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ contract OperatorStateRetriever {
uint32[][] nonSignerStakeIndices; // nonSignerStakeIndices[quorumNumberIndex][nonSignerIndex]
}

error OperatorNotRegistered();

/**
* @notice This function is intended to to be called by AVS operators every time a new task is created (i.e.)
* the AVS coordinator makes a request to AVS operators. Since all of the crucial information is kept onchain,
Expand Down Expand Up @@ -131,7 +133,7 @@ contract OperatorStateRetriever {
checkSignaturesIndices.nonSignerQuorumBitmapIndices[i]
);

require(nonSignerQuorumBitmap != 0, "OperatorStateRetriever.getCheckSignaturesIndices: operator must be registered at blocknumber");
require(nonSignerQuorumBitmap != 0, OperatorNotRegistered());

// if the operator was a part of the quorum and the quorum is a part of the provided quorumNumbers
if ((nonSignerQuorumBitmap >> uint8(quorumNumbers[quorumNumberIndex])) & 1 == 1) {
Expand Down
23 changes: 5 additions & 18 deletions src/ServiceManagerBase.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,7 @@ abstract contract ServiceManagerBase is ServiceManagerBaseStorage {

/// @notice when applied to a function, only allows the RegistryCoordinator to call it
modifier onlyRegistryCoordinator() {
require(
msg.sender == address(_registryCoordinator),
"ServiceManagerBase.onlyRegistryCoordinator: caller is not the registry coordinator"
);
require(msg.sender == address(_registryCoordinator), OnlyRegistryCoordinator());
_;
}

Expand Down Expand Up @@ -209,7 +206,7 @@ abstract contract ServiceManagerBase is ServiceManagerBaseStorage {
function acceptProposedSlasher() external onlyOwner {
require(
block.timestamp >= slasherProposalTimestamp + SLASHER_PROPOSAL_DELAY,
"ServiceManager: Slasher proposal delay not met"
DelayPeriodNotPassed()
);
_setSlasher(proposedSlasher);
delete proposedSlasher;
Expand Down Expand Up @@ -314,24 +311,14 @@ abstract contract ServiceManagerBase is ServiceManagerBaseStorage {
}

function _checkRewardsInitiator() internal view {
require(
msg.sender == rewardsInitiator,
"ServiceManagerBase.onlyRewardsInitiator: caller is not the rewards initiator"
);
require(msg.sender == rewardsInitiator, OnlyRewardsInitiator());
}

function _checkStakeRegistry() internal view {
require(
msg.sender == address(_stakeRegistry),
"ServiceManagerBase.onlyStakeRegistry: caller is not the stake registry"
);
require(msg.sender == address(_stakeRegistry), OnlyStakeRegistry());
}


function _checkSlasher() internal view {
require(
msg.sender == slasher,
"ServiceManagerBase.onlySlasher: caller is not the slasher"
);
require(msg.sender == slasher, OnlySlasher());
}
}
Loading
Loading