diff --git a/.solhint.json b/.solhint.json index 5bde826..907f063 100644 --- a/.solhint.json +++ b/.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" } } diff --git a/src/Deposit.sol b/src/Deposit.sol index cc63677..6a141b5 100644 --- a/src/Deposit.sol +++ b/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,7 +415,7 @@ contract Deposit { ]; require( futureCommittee.stakers[stakerKey].index != 0, - "staker does not exist" + KeyNotStaked() ); futureCommittee.totalStake += msg.value; futureCommittee.stakers[stakerKey].balance += msg.value; @@ -416,7 +423,7 @@ contract Deposit { 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) { diff --git a/src/utils/Deque.sol b/src/utils/Deque.sol index dff4a7a..4b6758c 100644 --- a/src/utils/Deque.sol +++ b/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); } diff --git a/src/utils/WithdrawalQueue.sol b/src/utils/WithdrawalQueue.sol index c8e4841..3cd1dde 100644 --- a/src/utils/WithdrawalQueue.sol +++ b/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++;