Skip to content

Commit

Permalink
chore: replace require statements with custom errors (#42)
Browse files Browse the repository at this point in the history
* chore: replace require statements with custom errors

* Update src/contracts/libraries/MerkleTrie.sol

---------

Co-authored-by: Oleksii Matiiasevych <[email protected]>
  • Loading branch information
nmlinaric and lastperson authored Mar 18, 2024
1 parent 556d31a commit 30cb6c4
Show file tree
Hide file tree
Showing 32 changed files with 303 additions and 193 deletions.
11 changes: 8 additions & 3 deletions src/contracts/ERC20Safe.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol";
@notice This contract is intended to be used with ERC20Handler contract.
*/
contract ERC20Safe {

error ERC20NonContractCall();
error ERC20CallFailed();
error ERC20OperationFailed();

/**
@notice Used to gain custody of deposited token.
@param tokenAddress Address of ERC20 to transfer.
Expand Down Expand Up @@ -88,13 +93,13 @@ contract ERC20Safe {
assembly {
tokenSize := extcodesize(token)
}
require(tokenSize > 0, "ERC20: not a contract");
if (tokenSize <= 0) revert ERC20NonContractCall();

(bool success, bytes memory returndata) = address(token).call(data);
require(success, "ERC20: call failed");
if (!success) revert ERC20CallFailed();

if (returndata.length > 0) {
require(abi.decode(returndata, (bool)), "ERC20: operation did not succeed");
if (!abi.decode(returndata, (bool))) revert ERC20OperationFailed();
}
}
}
3 changes: 2 additions & 1 deletion src/contracts/Executor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ contract Executor is Context {
error EmptyProposalsArray();
error BridgeIsPaused();
error ZeroAddressProvided();
error NoVerifierAddressProvided();
error AccessNotAllowed(address sender, bytes4 funcSig);
error TransferHashDoesNotMatchSlotValue(bytes32 transferHash);
error StateRootDoesNotMatch(IStateRootStorage stateRootStorage, bytes32 stateRoot);
Expand Down Expand Up @@ -107,7 +108,7 @@ contract Executor is Context {
@param verifiersAddresses Array of verifiers addresses which store state roots.
*/
function adminSetVerifiers(uint8 securityModel, address[] memory verifiersAddresses) external onlyAllowed {
require(verifiersAddresses.length > 0, "Should provide at least one verifier address");
if (verifiersAddresses.length <= 0) revert NoVerifierAddressProvided();
_securityModels[securityModel] = verifiersAddresses;
}

Expand Down
7 changes: 5 additions & 2 deletions src/contracts/Router.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ contract Router is Context {
error AccessNotAllowed(address sender, bytes4 funcSig);
error BridgeIsPaused();
error ZeroAddressProvided();
error NonceDecrementNotAllowed(uint64 currentNonce);
error MsgValueNotZero();

event Deposit(
uint8 destinationDomainID,
Expand Down Expand Up @@ -74,7 +76,8 @@ contract Router is Context {
@param nonce The nonce value to be set.
*/
function adminSetDepositNonce(uint8 domainID, uint64 nonce) external onlyAllowed {
require(nonce > _depositCounts[domainID], "Does not allow decrements of the nonce");
uint64 currentNonce = _depositCounts[domainID];
if (nonce <= currentNonce) revert NonceDecrementNotAllowed(currentNonce);
_depositCounts[domainID] = nonce;
}

Expand Down Expand Up @@ -103,7 +106,7 @@ contract Router is Context {
if (handler == address(0)) revert ResourceIDNotMappedToHandler();

if (address(feeHandler) == address(0)) {
require(msg.value == 0, "no FeeHandler, msg.value != 0");
if (msg.value != 0) revert MsgValueNotZero();
} else {
// Reverts on failure
feeHandler.collectFee{value: msg.value}(
Expand Down
1 change: 1 addition & 0 deletions src/contracts/handlers/ERCHandlerHelpers.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ abstract contract ERCHandlerHelpers is IERCHandler {

error ContractAddressNotWhitelisted(address contractAddress);
error DepositAmountTooSmall(uint256 depositAmount);
error SenderNotBridgeRouterOrExecutor();

// resourceID => token contract address
mapping(bytes32 => address) public _resourceIDToTokenContractAddress;
Expand Down
6 changes: 4 additions & 2 deletions src/contracts/handlers/FeeHandlerRouter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,16 @@ contract FeeHandlerRouter is IFeeHandler, AccessControl {
event WhitelistChanged(address whitelistAddress, bool isWhitelisted);

error IncorrectFeeSupplied(uint256);
error SenderNotRouterContract();
error SenderNotAdmin();

modifier onlyRouter() {
_onlyRouter();
_;
}

function _onlyRouter() private view {
require(msg.sender == _routerAddress, "sender must be router contract");
if (msg.sender != _routerAddress) revert SenderNotRouterContract();
}

modifier onlyAdmin() {
Expand All @@ -40,7 +42,7 @@ contract FeeHandlerRouter is IFeeHandler, AccessControl {
}

function _onlyAdmin() private view {
require(hasRole(DEFAULT_ADMIN_ROLE, _msgSender()), "sender doesn't have admin role");
if (!hasRole(DEFAULT_ADMIN_ROLE, _msgSender())) revert SenderNotAdmin();
}

/**
Expand Down
16 changes: 11 additions & 5 deletions src/contracts/handlers/PermissionlessGenericHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,14 @@ contract PermissionlessGenericHandler is IHandler {
_;
}

error SenderNotBridgeContract();
error SenderNotExecutorContract();
error IncorrectDataLength(uint256 dataLength);
error RequestedFeeTooLarge(uint256 maxFee);
error InvalidExecutioDataDepositor(address executioDataDepositor);

function _onlyBridge() private view {
require(msg.sender == _bridgeAddress, "sender must be bridge contract");
if (msg.sender != _bridgeAddress) revert SenderNotBridgeContract();
}

modifier onlyExecutor() {
Expand All @@ -30,7 +36,7 @@ contract PermissionlessGenericHandler is IHandler {
}

function _onlyExecutor() private view {
require(msg.sender == _executorAddress, "sender must be executor contract");
if (msg.sender != _executorAddress) revert SenderNotExecutorContract();
}

/**
Expand Down Expand Up @@ -113,7 +119,7 @@ contract PermissionlessGenericHandler is IHandler {
executeFuncSignature(address executionDataDepositor, uint256[] uintArray, address addr)
*/
function deposit(bytes32 resourceID, address depositor, bytes calldata data) external pure returns (bytes memory) {
require(data.length >= 76, "Incorrect data length"); // 32 + 2 + 1 + 1 + 20 + 20
if (data.length < 76) revert IncorrectDataLength(data.length); // 32 + 2 + 1 + 1 + 20 + 20

uint256 maxFee;
uint16 lenExecuteFuncSignature;
Expand Down Expand Up @@ -142,8 +148,8 @@ contract PermissionlessGenericHandler is IHandler {
)
);

require(maxFee < MAX_FEE, "requested fee too large");
require(depositor == executionDataDepositor, "incorrect depositor in deposit data");
if (maxFee > MAX_FEE) revert RequestedFeeTooLarge(maxFee);
if (depositor != executionDataDepositor) revert InvalidExecutioDataDepositor(executionDataDepositor);
return data;
}

Expand Down
39 changes: 24 additions & 15 deletions src/contracts/handlers/fee/BasicFeeHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,26 +22,32 @@ contract BasicFeeHandler is IFeeHandler, AccessControl {

event FeeChanged(uint256 newFee);

error SenderNotBridgeOrRouter();
error IncorrectFeeSupplied(uint256);
error ZeroAddressProvided();
error SenderNotAdmin();
error CannotRenounceOneself();
error NewFeeEqualsCurrentFee(uint256 currentFee);
error AddressesAndAmountsArraysDifferentLength(
uint256 addressesLength,
uint256 amountsLength
);
error EtherFeeTransferFailed();

modifier onlyAdmin() {
require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "sender doesn't have admin role");
if (!hasRole(DEFAULT_ADMIN_ROLE, msg.sender)) revert SenderNotAdmin();
_;
}

modifier onlyBridgeOrRouter() {
_onlyBridgeOrRouter();
modifier onlyRouterOrFeeRouter() {
_onlyRouterOrFeeRouter();
_;
}

function _onlyBridgeOrRouter() private view {
require(
msg.sender == _bridgeAddress ||
msg.sender == _feeHandlerRouterAddress ||
msg.sender == _routerAddress,
"sender must be bridge or fee router contract"
);
function _onlyRouterOrFeeRouter() private view {
if (msg.sender != _feeHandlerRouterAddress &&
msg.sender != _routerAddress
) revert SenderNotBridgeOrRouter();
}

/**
Expand All @@ -67,7 +73,7 @@ contract BasicFeeHandler is IFeeHandler, AccessControl {
*/
function renounceAdmin(address newAdmin) external {
address sender = _msgSender();
require(sender != newAdmin, "Cannot renounce oneself");
if (sender == newAdmin) revert CannotRenounceOneself();
grantRole(DEFAULT_ADMIN_ROLE, newAdmin);
renounceRole(DEFAULT_ADMIN_ROLE, sender);
}
Expand All @@ -90,7 +96,7 @@ contract BasicFeeHandler is IFeeHandler, AccessControl {
uint8 securityModel,
bytes calldata depositData,
bytes calldata feeData
) external virtual payable onlyBridgeOrRouter {
) external virtual payable onlyRouterOrFeeRouter {
uint256 currentFee = _domainResourceIDSecurityModelToFee[destinationDomainID][resourceID][securityModel];
if (msg.value != currentFee) revert IncorrectFeeSupplied(msg.value);
emit FeeCollected(sender, fromDomainID, destinationDomainID, resourceID, currentFee, address(0));
Expand Down Expand Up @@ -135,7 +141,7 @@ contract BasicFeeHandler is IFeeHandler, AccessControl {
uint256 newFee
) external onlyAdmin {
uint256 currentFee = _domainResourceIDSecurityModelToFee[destinationDomainID][resourceID][securityModel];
require(currentFee != newFee, "Current fee is equal to new fee");
if (currentFee == newFee) revert NewFeeEqualsCurrentFee(currentFee);
_domainResourceIDSecurityModelToFee[destinationDomainID][resourceID][securityModel] = newFee;
emit FeeChanged(newFee);
}
Expand All @@ -148,10 +154,13 @@ contract BasicFeeHandler is IFeeHandler, AccessControl {
@param amounts Array of amounts to transfer to {addrs}.
*/
function transferFee(address payable[] calldata addrs, uint256[] calldata amounts) external onlyAdmin {
require(addrs.length == amounts.length, "addrs[], amounts[]: diff length");
if (addrs.length != amounts.length) revert AddressesAndAmountsArraysDifferentLength(
addrs.length,
amounts.length
);
for (uint256 i = 0; i < addrs.length; i++) {
(bool success, ) = addrs[i].call{value: amounts[i]}("");
require(success, "Fee ether transfer failed");
if (!success) revert EtherFeeTransferFailed();
emit FeeDistributed(address(0), addrs[i], amounts[i]);
}
}
Expand Down
28 changes: 17 additions & 11 deletions src/contracts/handlers/fee/PercentageERC20FeeHandlerEVM.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ contract PercentageERC20FeeHandlerEVM is BasicFeeHandler, ERC20Safe {

event FeeBoundsChanged(uint256 newLowerBound, uint256 newUpperBound);

error MsgValueNotZero();
error InvalidBoundsRatio(uint128 newLowerBound, uint128 newUpperBound);
error NewBoundsEqualCurrentBounds(uint128 currentLowerBound, uint128 currentUpperBound);

/**
@param bridgeAddress Contract address of previously deployed Bridge.
@param feeHandlerRouterAddress Contract address of previously deployed FeeHandlerRouter.
Expand Down Expand Up @@ -121,8 +125,8 @@ contract PercentageERC20FeeHandlerEVM is BasicFeeHandler, ERC20Safe {
uint8 securityModel,
bytes calldata depositData,
bytes calldata feeData
) external payable override onlyBridgeOrRouter {
require(msg.value == 0, "collectFee: msg.value != 0");
) external payable override onlyRouterOrFeeRouter {
if (msg.value != 0) revert MsgValueNotZero();

(uint256 fee, address tokenAddress) = _calculateFee(
sender,
Expand All @@ -146,15 +150,14 @@ contract PercentageERC20FeeHandlerEVM is BasicFeeHandler, ERC20Safe {
@param newUpperBound Value {_newUpperBound} will be updated to.
*/
function changeFeeBounds(bytes32 resourceID, uint128 newLowerBound, uint128 newUpperBound) external onlyAdmin {
require(
newUpperBound == 0 || (newUpperBound > newLowerBound),
"Upper bound must be larger than lower bound or 0"
);
if (newUpperBound != 0 && (newUpperBound <= newLowerBound)) {
revert InvalidBoundsRatio(newLowerBound, newUpperBound);
}

Bounds memory existingBounds = _resourceIDToFeeBounds[resourceID];
require(
existingBounds.lowerBound != newLowerBound || existingBounds.upperBound != newUpperBound,
"Current bounds are equal to new bounds"
);
if (existingBounds.lowerBound == newLowerBound && existingBounds.upperBound == newUpperBound) {
revert NewBoundsEqualCurrentBounds(existingBounds.lowerBound, existingBounds.upperBound);
}

Bounds memory newBounds = Bounds(newLowerBound, newUpperBound);
_resourceIDToFeeBounds[resourceID] = newBounds;
Expand All @@ -175,7 +178,10 @@ contract PercentageERC20FeeHandlerEVM is BasicFeeHandler, ERC20Safe {
address[] calldata addrs,
uint256[] calldata amounts
) external onlyAdmin {
require(addrs.length == amounts.length, "addrs[], amounts[]: diff length");
if (addrs.length != amounts.length) revert AddressesAndAmountsArraysDifferentLength(
addrs.length,
amounts.length
);
address tokenHandler = IBridge(_bridgeAddress)._resourceIDToHandlerAddress(resourceID);
address tokenAddress = IERCHandler(tokenHandler)._resourceIDToTokenContractAddress(resourceID);
for (uint256 i = 0; i < addrs.length; i++) {
Expand Down
10 changes: 7 additions & 3 deletions src/contracts/libraries/Bytes.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ pragma solidity ^0.8.11;
* @notice Bytes is a library for manipulating byte arrays.
*/
library Bytes {

error SliceOwerflow();
error SliceOutOfBounds();

/**
* @notice Slices a byte array with a given starting index and length. Returns a new byte array
* as opposed to a pointer to the original array. Will throw if trying to slice more
Expand All @@ -24,9 +28,9 @@ library Bytes {
uint256 _length
) internal pure returns (bytes memory) {
unchecked {
require(_length + 31 >= _length, "slice_overflow");
require(_start + _length >= _start, "slice_overflow");
require(_bytes.length >= _start + _length, "slice_outOfBounds");
if (_length + 31 < _length) revert SliceOwerflow();
if (_start + _length < _start) revert SliceOwerflow();
if (_bytes.length < _start + _length) revert SliceOutOfBounds();
}

bytes memory tempBytes;
Expand Down
Loading

0 comments on commit 30cb6c4

Please sign in to comment.