draft: Custom errors

.solhint.json

@@ -3,8 +3,8 @@
   "rules": {
     "no-inline-assembly": "off",
     "avoid-low-level-calls": "off",
-    "gas-custom-errors": "off",
     "reason-string": ["warn",{"maxLength":70}],
-    "func-visibility": ["warn", { "ignoreConstructors": true }]
+    "func-visibility": ["warn", { "ignoreConstructors": true }],
+    "gas-custom-errors": "off" 
   }
 }

src/Deposit.sol

@@ -5,6 +5,26 @@ import {Deque, Withdrawal} from "./utils/Deque.sol";
 
 using Deque for Deque.Withdrawals;
 
+/// Argument has unexpected length
+/// @param argument name of argument
+/// @param required expected length
+error UnexpectedArgumentLength(string argument, uint256 required);
+
+/// Maximum number of stakers has been reached
+error TooManyStakers();
+/// Key already staked 
+error KeyAlreadyStaked();
+/// Key is not staked
+error KeyNotStaked();
+/// Stake amount less than minimum
+error StakeAmountTooLow();
+
+/// Withdraw amount larger than balance
+error WithdrawAmountTooLarge();
+
+/// Proof of possession verification failed 
+error RogueKeyCheckFailed();
+
 struct CommitteeStakerEntry {
     // The index of the value in the `stakers` array plus 1.
     // Index 0 is used to mean a value is not present.
@@ -62,7 +82,7 @@ contract Deposit {
     uint64 public immutable BLOCKS_PER_EPOCH;
 
     modifier onlyControlAddress(bytes calldata blsPubKey) {
-        require(blsPubKey.length == 48, "bad bls public key length");
+        require(blsPubKey.length == 48, UnexpectedArgumentLength("bls public key", 48));
         require(
             _stakersMap[blsPubKey].controlAddress == msg.sender,
             "sender is not the control address"
@@ -89,8 +109,8 @@ contract Deposit {
             address controlAddress = initialStaker.controlAddress;
             uint256 amount = initialStaker.amount;
 
-            require(blsPubKey.length == 48, "bad bls public key length");
-            require(peerId.length == 38, "bad peer id length");
+            require(blsPubKey.length == 48, UnexpectedArgumentLength("bls public key", 48));
+            require(peerId.length == 38, UnexpectedArgumentLength("Peer Id", 38));
             require(
                 controlAddress != address(0),
                 "control address cannot be zero"
@@ -99,19 +119,16 @@ contract Deposit {
             Committee storage currentCommittee = committee();
             require(
                 currentCommittee.stakerKeys.length < MAXIMUM_STAKERS,
-                "too many stakers"
+                TooManyStakers()
             );
 
             Staker storage staker = _stakersMap[blsPubKey];
             // This must be a new staker, meaning the control address must be zero.
             require(
                 staker.controlAddress == address(0),
-                "staker already exists"
+                KeyAlreadyStaked()
             );
-
-            if (amount < MINIMUM_STAKE) {
-                revert("stake is less than minimum stake");
-            }
+            require(amount < MINIMUM_STAKE, StakeAmountTooLow());
 
             _stakerKeys[controlAddress] = blsPubKey;
             staker.peerId = peerId;
@@ -205,7 +222,7 @@ contract Deposit {
     }
 
     function getStake(bytes calldata blsPubKey) public view returns (uint256) {
-        require(blsPubKey.length == 48, "bad bls public key length");
+        require(blsPubKey.length == 48, UnexpectedArgumentLength("bls public key", 48));
 
         // We don't need to check if `blsPubKey` is in `stakerKeys` here. If the `blsPubKey` is not a staker, the
         // balance will default to zero.
@@ -215,7 +232,7 @@ contract Deposit {
     function getFutureStake(
         bytes calldata blsPubKey
     ) public view returns (uint256) {
-        require(blsPubKey.length == 48, "bad bls public key length");
+        require(blsPubKey.length == 48, UnexpectedArgumentLength("bls public key", 48));
 
         // if `latestComputedEpoch > currentEpoch()`
         // then `latestComputedEpoch` determines the future committee we need
@@ -231,20 +248,16 @@ contract Deposit {
     function getRewardAddress(
         bytes calldata blsPubKey
     ) public view returns (address) {
-        require(blsPubKey.length == 48, "bad bls public key length");
-        if (_stakersMap[blsPubKey].controlAddress == address(0)) {
-            revert("not staked");
-        }
+        require(blsPubKey.length == 48, UnexpectedArgumentLength("bls public key", 48));
+        require(_stakersMap[blsPubKey].controlAddress == address(0), KeyNotStaked());
         return _stakersMap[blsPubKey].rewardAddress;
     }
 
     function getControlAddress(
         bytes calldata blsPubKey
     ) public view returns (address) {
-        require(blsPubKey.length == 48, "bad bls public key length");
-        if (_stakersMap[blsPubKey].controlAddress == address(0)) {
-            revert("not staked");
-        }
+        require(blsPubKey.length == 48, UnexpectedArgumentLength("bls public key", 48));
+        require(_stakersMap[blsPubKey].controlAddress == address(0), KeyNotStaked());
         return _stakersMap[blsPubKey].controlAddress;
     }
 
@@ -265,10 +278,8 @@ contract Deposit {
     function getPeerId(
         bytes calldata blsPubKey
     ) public view returns (bytes memory) {
-        require(blsPubKey.length == 48, "bad bls public key length");
-        if (_stakersMap[blsPubKey].controlAddress == address(0)) {
-            revert("not staked");
-        }
+        require(blsPubKey.length == 48, UnexpectedArgumentLength("bls public key", 48));
+        require(_stakersMap[blsPubKey].controlAddress == address(0), KeyNotStaked());
         return _stakersMap[blsPubKey].peerId;
     }
 
@@ -355,20 +366,16 @@ contract Deposit {
         bytes calldata signature,
         address rewardAddress
     ) public payable {
-        require(blsPubKey.length == 48, "bad bls public key length");
-        require(peerId.length == 38, "bad peer id length");
-        require(signature.length == 96, "bad signature length");
+        require(blsPubKey.length == 48, UnexpectedArgumentLength("bls public key", 48));
+        require(peerId.length == 38, UnexpectedArgumentLength("peer id", 38));
+        require(signature.length == 96, UnexpectedArgumentLength("signature", 96));
+        require(msg.value < MINIMUM_STAKE, StakeAmountTooLow());
 
         // Verify signature as a proof-of-possession of the private key.
         bool pop = _popVerify(blsPubKey, signature);
-        require(pop, "rogue key check");
+        require(pop, RogueKeyCheckFailed());
 
         Staker storage staker = _stakersMap[blsPubKey];
-
-        if (msg.value < MINIMUM_STAKE) {
-            revert("stake is less than minimum stake");
-        }
-
         _stakerKeys[msg.sender] = blsPubKey;
         staker.peerId = peerId;
         staker.rewardAddress = rewardAddress;
@@ -382,11 +389,11 @@ contract Deposit {
 
         require(
             futureCommittee.stakerKeys.length < MAXIMUM_STAKERS,
-            "too many stakers"
+            TooManyStakers()
         );
         require(
             futureCommittee.stakers[blsPubKey].index == 0,
-            "staker already exists"
+            KeyAlreadyStaked()
         );
 
         futureCommittee.totalStake += msg.value;
@@ -399,7 +406,7 @@ contract Deposit {
 
     function depositTopup() public payable {
         bytes storage stakerKey = _stakerKeys[msg.sender];
-        require(stakerKey.length != 0, "staker does not exist");
+        require(stakerKey.length != 0, KeyNotStaked());
 
         updateLatestComputedEpoch();
 
@@ -408,15 +415,15 @@ contract Deposit {
         ];
         require(
             futureCommittee.stakers[stakerKey].index != 0,
-            "staker does not exist"
+            KeyNotStaked()
         );
         futureCommittee.totalStake += msg.value;
         futureCommittee.stakers[stakerKey].balance += msg.value;
     }
 
     function unstake(uint256 amount) public {
         bytes storage stakerKey = _stakerKeys[msg.sender];
-        require(stakerKey.length != 0, "staker does not exist");
+        require(stakerKey.length != 0, KeyNotStaked());
         Staker storage staker = _stakersMap[stakerKey];
 
         updateLatestComputedEpoch();
@@ -427,13 +434,13 @@ contract Deposit {
 
         require(
             futureCommittee.stakers[stakerKey].index != 0,
-            "staker does not exist"
+            KeyNotStaked()
         );
         //TODO: keep the next line commented out
         //require(futureCommittee.stakerKeys.length > 1, "too few stakers");
         require(
             futureCommittee.stakers[stakerKey].balance >= amount,
-            "amount is greater than staked balance"
+            WithdrawAmountTooLarge()
         );
 
         if (futureCommittee.stakers[stakerKey].balance - amount == 0) {

src/utils/Deque.sol

@@ -6,6 +6,9 @@ struct Withdrawal {
     uint256 amount;
 }
 
+/// Interal Deque error
+error DequeError(string msg);
+
 // Implementation of a double-ended queue of `Withdrawal`s, backed by a circular buffer.
 library Deque {
     struct Withdrawals {
@@ -39,9 +42,7 @@ library Deque {
         Withdrawals storage deque,
         uint256 idx
     ) internal view returns (Withdrawal storage) {
-        if (idx >= deque.len) {
-            revert("element does not exist");
-        }
+        require(idx >= deque.len, DequeError("element does not exist"));
 
         uint256 pIdx = physicalIdx(deque, idx);
         return deque.values[pIdx];
@@ -68,9 +69,7 @@ library Deque {
     function popFront(
         Withdrawals storage deque
     ) internal returns (Withdrawal storage) {
-        if (deque.len == 0) {
-            revert("queue is empty");
-        }
+        require(deque.len == 0, DequeError("queue is empty"));
 
         uint256 oldHead = deque.head;
         deque.head = physicalIdx(deque, 1);
@@ -84,9 +83,7 @@ library Deque {
     function back(
         Withdrawals storage deque
     ) internal view returns (Withdrawal storage) {
-        if (deque.len == 0) {
-            revert("queue is empty");
-        }
+        require(deque.len == 0, DequeError("queue is empty"));
 
         return get(deque, deque.len - 1);
     }
@@ -97,9 +94,8 @@ library Deque {
     function front(
         Withdrawals storage deque
     ) internal view returns (Withdrawal storage) {
-        if (deque.len == 0) {
-            revert("queue is empty");
-        }
+        require(deque.len == 0, DequeError("queue is empty"));
+
 
         return get(deque, 0);
     }

src/utils/WithdrawalQueue.sol

@@ -1,6 +1,9 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 pragma solidity ^0.8.26;
 
+/// Interal WithdrawalQueue error
+error WithdrawalQueueError(string msg);
+
 library WithdrawalQueue {
 
     address public constant DEPOSIT_CONTRACT = 0x000000000000000000005a494C4445504F534954;
@@ -20,7 +23,7 @@ library WithdrawalQueue {
         (bool success, bytes memory data) = DEPOSIT_CONTRACT.staticcall(
             abi.encodeWithSignature("withdrawalPeriod()")
         );
-        require(success, "unbonding period unknown");
+        require(success, WithdrawalQueueError("unbonding period unknown"));
         return abi.decode(data, (uint256));
     }
 
@@ -30,7 +33,7 @@ library WithdrawalQueue {
     }
 
     function dequeue(Fifo storage fifo) internal returns(Item memory result) {
-        require(fifo.first < fifo.last, "queue empty");
+        require(fifo.first < fifo.last, WithdrawalQueueError("queue empty"));
         result = fifo.items[fifo.first];
         delete fifo.items[fifo.first];
         fifo.first++;