diff --git a/src/contracts/ERC20Safe.sol b/src/contracts/ERC20Safe.sol index 4b5b02e2..7a0fc3de 100755 --- a/src/contracts/ERC20Safe.sol +++ b/src/contracts/ERC20Safe.sol @@ -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. @@ -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(); } } } diff --git a/src/contracts/Executor.sol b/src/contracts/Executor.sol index 660e0b6b..e5ec36e1 100644 --- a/src/contracts/Executor.sol +++ b/src/contracts/Executor.sol @@ -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); @@ -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; } diff --git a/src/contracts/Router.sol b/src/contracts/Router.sol index 1de14d11..12143649 100644 --- a/src/contracts/Router.sol +++ b/src/contracts/Router.sol @@ -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, @@ -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; } @@ -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}( diff --git a/src/contracts/handlers/ERCHandlerHelpers.sol b/src/contracts/handlers/ERCHandlerHelpers.sol index d6c622f1..def207e6 100755 --- a/src/contracts/handlers/ERCHandlerHelpers.sol +++ b/src/contracts/handlers/ERCHandlerHelpers.sol @@ -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; diff --git a/src/contracts/handlers/FeeHandlerRouter.sol b/src/contracts/handlers/FeeHandlerRouter.sol index 5e1cfa0c..727a71ea 100755 --- a/src/contracts/handlers/FeeHandlerRouter.sol +++ b/src/contracts/handlers/FeeHandlerRouter.sol @@ -24,6 +24,8 @@ contract FeeHandlerRouter is IFeeHandler, AccessControl { event WhitelistChanged(address whitelistAddress, bool isWhitelisted); error IncorrectFeeSupplied(uint256); + error SenderNotRouterContract(); + error SenderNotAdmin(); modifier onlyRouter() { _onlyRouter(); @@ -31,7 +33,7 @@ contract FeeHandlerRouter is IFeeHandler, AccessControl { } function _onlyRouter() private view { - require(msg.sender == _routerAddress, "sender must be router contract"); + if (msg.sender != _routerAddress) revert SenderNotRouterContract(); } modifier onlyAdmin() { @@ -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(); } /** diff --git a/src/contracts/handlers/PermissionlessGenericHandler.sol b/src/contracts/handlers/PermissionlessGenericHandler.sol index f2770745..cf966e37 100755 --- a/src/contracts/handlers/PermissionlessGenericHandler.sol +++ b/src/contracts/handlers/PermissionlessGenericHandler.sol @@ -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() { @@ -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(); } /** @@ -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; @@ -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; } diff --git a/src/contracts/handlers/fee/BasicFeeHandler.sol b/src/contracts/handlers/fee/BasicFeeHandler.sol index 1ebe8455..d5b6033a 100755 --- a/src/contracts/handlers/fee/BasicFeeHandler.sol +++ b/src/contracts/handlers/fee/BasicFeeHandler.sol @@ -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(); } /** @@ -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); } @@ -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)); @@ -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); } @@ -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]); } } diff --git a/src/contracts/handlers/fee/PercentageERC20FeeHandlerEVM.sol b/src/contracts/handlers/fee/PercentageERC20FeeHandlerEVM.sol index 298a265e..0d4d0a4a 100755 --- a/src/contracts/handlers/fee/PercentageERC20FeeHandlerEVM.sol +++ b/src/contracts/handlers/fee/PercentageERC20FeeHandlerEVM.sol @@ -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. @@ -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, @@ -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; @@ -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++) { diff --git a/src/contracts/libraries/Bytes.sol b/src/contracts/libraries/Bytes.sol index 2c1b0198..da649aa3 100644 --- a/src/contracts/libraries/Bytes.sol +++ b/src/contracts/libraries/Bytes.sol @@ -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 @@ -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; diff --git a/src/contracts/libraries/MerkleTrie.sol b/src/contracts/libraries/MerkleTrie.sol index 8b333abe..9c522d9b 100644 --- a/src/contracts/libraries/MerkleTrie.sol +++ b/src/contracts/libraries/MerkleTrie.sol @@ -21,6 +21,19 @@ library MerkleTrie { RLPReader.RLPItem[] decoded; } + error MerkleTrieEmptyKey(); + error MerkleTrieKeyIndexExceedsKeyLength(); + error MerkleTrieInvalidRootHash(); + error MerkleTrieInvalidInternalNodeHash(); + error MerkleTrieInvalidLargeInternalHash(); + error MerkleTrieValueNodeNotLastInProof(); + error RLPReaderDecodedItemIsNotAListItem(); + error MerkleTriePathMustShareAllNibblesWithKey(); + error MerkleTrieKeyRemainderMustEqualPathRemainder(); + error MerkleTrieValueLengthIsZero(); + error MerkleTrieNodeWithUnknownPrefix(); + error MerkleTrieUnparsableNode(); + /// @notice Determines the number of elements per branch node. uint256 internal constant TREE_RADIX = 16; @@ -70,7 +83,7 @@ library MerkleTrie { /// @param _root Known root of the Merkle trie. /// @return value_ Value of the key if it exists. function get(bytes memory _key, bytes[] memory _proof, bytes32 _root) internal pure returns (bytes memory value_) { - require(_key.length > 0, "MerkleTrie: empty key"); + if (_key.length <= 0) revert MerkleTrieEmptyKey(); TrieNode[] memory proof = _parseProof(_proof); bytes memory key = Bytes.toNibbles(_key); @@ -82,23 +95,23 @@ library MerkleTrie { TrieNode memory currentNode = proof[i]; // Key index should never exceed total key length or we'll be out of bounds. - require(currentKeyIndex <= key.length, "MerkleTrie: key index exceeds total key length"); + if (currentKeyIndex > key.length) revert MerkleTrieKeyIndexExceedsKeyLength(); if (currentKeyIndex == 0) { // First proof element is always the root node. - require( - Bytes.equal(abi.encodePacked(keccak256(currentNode.encoded)), currentNodeID), - "MerkleTrie: invalid root hash" - ); + if (!Bytes.equal(abi.encodePacked(keccak256(currentNode.encoded)), currentNodeID)) { + revert MerkleTrieInvalidRootHash(); + } } else if (currentNode.encoded.length >= 32) { // Nodes 32 bytes or larger are hashed inside branch nodes. - require( - Bytes.equal(abi.encodePacked(keccak256(currentNode.encoded)), currentNodeID), - "MerkleTrie: invalid large internal hash" - ); + if (!Bytes.equal(abi.encodePacked(keccak256(currentNode.encoded)), currentNodeID)) { + revert MerkleTrieInvalidLargeInternalHash(); + } } else { // Nodes smaller than 32 bytes aren't hashed. - require(Bytes.equal(currentNode.encoded, currentNodeID), "MerkleTrie: invalid internal node hash"); + if (!Bytes.equal(currentNode.encoded, currentNodeID)) { + revert MerkleTrieInvalidInternalNodeHash(); + } } if (currentNode.decoded.length == BRANCH_NODE_LENGTH) { @@ -109,10 +122,10 @@ library MerkleTrie { // even when the value wasn't explicitly placed there. Geth treats a value of // bytes(0) as "key does not exist" and so we do the same. value_ = RLPReader.readBytes(currentNode.decoded[TREE_RADIX]); - require(value_.length > 0, "MerkleTrie: value length must be greater than zero (branch)"); + if (value_.length <= 0) revert MerkleTrieValueLengthIsZero(); // Extra proof elements are not allowed. - require(i == proof.length - 1, "MerkleTrie: value node must be last node in proof (branch)"); + if (i != proof.length - 1) revert MerkleTrieValueNodeNotLastInProof(); return value_; } else { @@ -134,10 +147,7 @@ library MerkleTrie { // Whether this is a leaf node or an extension node, the path remainder MUST be a // prefix of the key remainder (or be equal to the key remainder) or the proof is // considered invalid. - require( - pathRemainder.length == sharedNibbleLength, - "MerkleTrie: path remainder must share all nibbles with key" - ); + if (pathRemainder.length != sharedNibbleLength) revert MerkleTriePathMustShareAllNibblesWithKey(); if (prefix == PREFIX_LEAF_EVEN || prefix == PREFIX_LEAF_ODD) { // Prefix of 2 or 3 means this is a leaf node. For the leaf node to be valid, @@ -146,20 +156,19 @@ library MerkleTrie { // the key remainder length equals the shared nibble length, which implies // equality with the path remainder (since we already did the same check with // the path remainder and the shared nibble length). - require( - keyRemainder.length == sharedNibbleLength, - "MerkleTrie: key remainder must be identical to path remainder" - ); + if (keyRemainder.length != sharedNibbleLength) { + revert MerkleTrieKeyRemainderMustEqualPathRemainder(); + } // Our Merkle Trie is designed specifically for the purposes of the Ethereum // state trie. Empty values are not allowed in the state trie, so we can safely // say that if the value is empty, the key should not exist and the proof is // invalid. value_ = RLPReader.readBytes(currentNode.decoded[1]); - require(value_.length > 0, "MerkleTrie: value length must be greater than zero (leaf)"); + if (value_.length <= 0) revert MerkleTrieValueLengthIsZero(); // Extra proof elements are not allowed. - require(i == proof.length - 1, "MerkleTrie: value node must be last node in proof (leaf)"); + if (i != proof.length - 1) revert MerkleTrieValueNodeNotLastInProof(); return value_; } else if (prefix == PREFIX_EXTENSION_EVEN || prefix == PREFIX_EXTENSION_ODD) { @@ -169,10 +178,10 @@ library MerkleTrie { currentNodeID = _getNodeID(currentNode.decoded[1]); currentKeyIndex += sharedNibbleLength; } else { - revert("MerkleTrie: received a node with an unknown prefix"); + revert MerkleTrieNodeWithUnknownPrefix(); } } else { - revert("MerkleTrie: received an unparseable node"); + revert MerkleTrieUnparsableNode(); } } diff --git a/src/contracts/libraries/RLPReader.sol b/src/contracts/libraries/RLPReader.sol index 2e915014..03b57569 100644 --- a/src/contracts/libraries/RLPReader.sol +++ b/src/contracts/libraries/RLPReader.sol @@ -41,6 +41,18 @@ library RLPReader { */ uint256 internal constant MAX_LIST_LENGTH = 32; + error RLPReaderDecodedItemIsNotAListItem(); + error RLPReaderListItemHasInvalidDataRemainder(); + error RLPReaderInvalidContentLength(); + error RLPItemTotalLengthGreaterThanContentLength(); + error RLPReaderListLenghtGreaterThanContentLength(); + error RLPReaderInvalidLeadingZeros(); + error RLPReaderItemLengthIsZero(); + error RLPReaderInvalidPrefix(); + error RLPItemStringLengthGreaterThanContentLength(); + error RLPReaderInvalidBytes32Value(); + error RLPReaderInvalidAddressValue(); + /** * @notice Converts bytes to a reference to memory position and length. * @@ -50,10 +62,7 @@ library RLPReader { */ function toRLPItem(bytes memory _in) internal pure returns (RLPItem memory) { // Empty arrays are not RLP items. - require( - _in.length > 0, - "RLPReader: length of an RLP item must be greater than zero to be decodable" - ); + if ( _in.length <= 0) revert RLPReaderItemLengthIsZero(); MemoryPointer ptr; assembly { @@ -73,15 +82,9 @@ library RLPReader { function readList(RLPItem memory _in) internal pure returns (RLPItem[] memory) { (uint256 listOffset, uint256 listLength, RLPItemType itemType) = _decodeLength(_in); - require( - itemType == RLPItemType.LIST_ITEM, - "RLPReader: decoded item type for list is not a list item" - ); + if (itemType != RLPItemType.LIST_ITEM) revert RLPReaderDecodedItemIsNotAListItem(); - require( - listOffset + listLength == _in.length, - "RLPReader: list item has an invalid data remainder" - ); + if (listOffset + listLength != _in.length) revert RLPReaderListItemHasInvalidDataRemainder(); // Solidity in-memory arrays can't be increased in size, but *can* be decreased in size by // writing to the length. Since we can't know the number of RLP items without looping over @@ -139,15 +142,9 @@ library RLPReader { function readBytes(RLPItem memory _in) internal pure returns (bytes memory) { (uint256 itemOffset, uint256 itemLength, RLPItemType itemType) = _decodeLength(_in); - require( - itemType == RLPItemType.DATA_ITEM, - "RLPReader: decoded item type for bytes is not a data item" - ); + if (itemType != RLPItemType.DATA_ITEM) revert RLPReaderDecodedItemIsNotAListItem(); - require( - _in.length == itemOffset + itemLength, - "RLPReader: bytes value contains an invalid remainder" - ); + if (_in.length != itemOffset + itemLength) revert RLPReaderListItemHasInvalidDataRemainder(); return _copy(_in.ptr, itemOffset, itemLength); } @@ -195,10 +192,7 @@ library RLPReader { // Short-circuit if there's nothing to decode, note that we perform this check when // the user creates an RLP item via toRLPItem, but it's always possible for them to bypass // that function and create an RLP item directly. So we need to check this anyway. - require( - _in.length > 0, - "RLPReader: length of an RLP item must be greater than zero to be decodable" - ); + if (_in.length <= 0) revert RLPReaderItemLengthIsZero(); MemoryPointer ptr = _in.ptr; uint256 prefix; @@ -215,55 +209,37 @@ library RLPReader { // slither-disable-next-line variable-scope uint256 strLen = prefix - 0x80; - require( - _in.length > strLen, - "RLPReader: length of content must be greater than string length (short string)" - ); + if (_in.length <= strLen) revert RLPItemStringLengthGreaterThanContentLength(); bytes1 firstByteOfContent; assembly { firstByteOfContent := and(mload(add(ptr, 1)), shl(248, 0xff)) } - require( - strLen != 1 || firstByteOfContent >= 0x80, - "RLPReader: invalid prefix, single byte < 0x80 are not prefixed (short string)" - ); + if (strLen == 1 && firstByteOfContent < 0x80) revert RLPReaderInvalidPrefix(); return (1, strLen, RLPItemType.DATA_ITEM); } else if (prefix <= 0xbf) { // Long string. uint256 lenOfStrLen = prefix - 0xb7; - require( - _in.length > lenOfStrLen, - "RLPReader: length of content must be > than length of string length (long string)" - ); + if (_in.length <= lenOfStrLen) revert RLPItemStringLengthGreaterThanContentLength(); bytes1 firstByteOfContent; assembly { firstByteOfContent := and(mload(add(ptr, 1)), shl(248, 0xff)) } - require( - firstByteOfContent != 0x00, - "RLPReader: length of content must not have any leading zeros (long string)" - ); + if (firstByteOfContent == 0x00) revert RLPReaderInvalidLeadingZeros(); uint256 strLen; assembly { strLen := shr(sub(256, mul(8, lenOfStrLen)), mload(add(ptr, 1))) } - require( - strLen > 55, - "RLPReader: length of content must be greater than 55 bytes (long string)" - ); + if (strLen <= 55) revert RLPReaderInvalidContentLength(); - require( - _in.length > lenOfStrLen + strLen, - "RLPReader: length of content must be greater than total length (long string)" - ); + if (_in.length <= lenOfStrLen + strLen) revert RLPItemTotalLengthGreaterThanContentLength(); return (1 + lenOfStrLen, strLen, RLPItemType.DATA_ITEM); } else if (prefix <= 0xf7) { @@ -271,45 +247,30 @@ library RLPReader { // slither-disable-next-line variable-scope uint256 listLen = prefix - 0xc0; - require( - _in.length > listLen, - "RLPReader: length of content must be greater than list length (short list)" - ); + if (_in.length <= listLen) revert RLPReaderListLenghtGreaterThanContentLength(); return (1, listLen, RLPItemType.LIST_ITEM); } else { // Long list. uint256 lenOfListLen = prefix - 0xf7; - require( - _in.length > lenOfListLen, - "RLPReader: length of content must be > than length of list length (long list)" - ); + if (_in.length <= lenOfListLen) revert RLPReaderListLenghtGreaterThanContentLength(); bytes1 firstByteOfContent; assembly { firstByteOfContent := and(mload(add(ptr, 1)), shl(248, 0xff)) } - require( - firstByteOfContent != 0x00, - "RLPReader: length of content must not have any leading zeros (long list)" - ); + if (firstByteOfContent == 0x00) revert RLPReaderInvalidLeadingZeros(); uint256 listLen; assembly { listLen := shr(sub(256, mul(8, lenOfListLen)), mload(add(ptr, 1))) } - require( - listLen > 55, - "RLPReader: length of content must be greater than 55 bytes (long list)" - ); + if (listLen <= 55) revert RLPReaderInvalidContentLength(); - require( - _in.length > lenOfListLen + listLen, - "RLPReader: length of content must be greater than total length (long list)" - ); + if (_in.length <= lenOfListLen + listLen) revert RLPItemTotalLengthGreaterThanContentLength(); return (1 + lenOfListLen, listLen, RLPItemType.LIST_ITEM); } @@ -363,11 +324,11 @@ library RLPReader { * @return Decoded bytes32. */ function readBytes32(RLPItem memory _in) internal pure returns (bytes32) { - require(_in.length <= 33, "Invalid RLP bytes32 value."); + if (_in.length > 33) revert RLPReaderInvalidBytes32Value(); (uint256 itemOffset, uint256 itemLength, RLPItemType itemType) = _decodeLength(_in); - require(itemType == RLPItemType.DATA_ITEM, "Invalid RLP bytes32 value."); + if (itemType != RLPItemType.DATA_ITEM) revert RLPReaderInvalidBytes32Value(); uint256 ptr = MemoryPointer.unwrap(_in.ptr) + itemOffset; bytes32 out; @@ -402,7 +363,7 @@ library RLPReader { return address(0); } - require(_in.length == 21, "Invalid RLP address value."); + if (_in.length != 21) revert RLPReaderInvalidAddressValue(); return address(uint160(readUint256(_in))); } diff --git a/src/contracts/libraries/StorageProof.sol b/src/contracts/libraries/StorageProof.sol index 9682149b..a91b389a 100644 --- a/src/contracts/libraries/StorageProof.sol +++ b/src/contracts/libraries/StorageProof.sol @@ -8,13 +8,17 @@ library StorageProof { using RLPReader for RLPReader.RLPItem; using RLPReader for bytes; + error StorageValueDoesNotExist(); + error AccountDoesNotExist(); + error InvalidAccountListLength(); + function getStorageValue(bytes32 slotHash, bytes32 storageRoot, bytes[] memory stateProof) internal pure returns (bytes32) { bytes memory valueRlpBytes = MerkleTrie.get(abi.encodePacked(slotHash), stateProof, storageRoot); - require(valueRlpBytes.length > 0, "Storage value does not exist"); + if (valueRlpBytes.length <= 0) revert StorageValueDoesNotExist(); return valueRlpBytes.toRLPItem().readBytes32(); } @@ -24,9 +28,9 @@ library StorageProof { { bytes32 addressHash = keccak256(abi.encodePacked(contractAddress)); bytes memory acctRlpBytes = MerkleTrie.get(abi.encodePacked(addressHash), proof, stateRoot); - require(acctRlpBytes.length > 0, "Account does not exist"); + if (acctRlpBytes.length <= 0) revert AccountDoesNotExist(); RLPReader.RLPItem[] memory acctFields = acctRlpBytes.toRLPItem().readList(); - require(acctFields.length == 4); + if (acctFields.length != 4) revert InvalidAccountListLength(); return bytes32(acctFields[2].readUint256()); } } diff --git a/src/contracts/proxies/SpectreProxy.sol b/src/contracts/proxies/SpectreProxy.sol index 7689e0c9..f404096d 100644 --- a/src/contracts/proxies/SpectreProxy.sol +++ b/src/contracts/proxies/SpectreProxy.sol @@ -25,13 +25,18 @@ contract SpectreProxy is AccessControl { event CommitteeRotated(uint8 sourceDomainID, uint256 slot); event StateRootSubmitted(uint8 sourceDomainID, uint256 slot, bytes32 stateRoot); + error SenderNotAdmin(); + error ArrayLengthsDoNotMatch(); + error SpectreAddressNotFound(); + error InvalidMerkleProof(); + modifier onlyAdmin() { _onlyAdmin(); _; } function _onlyAdmin() private view { - require(hasRole(DEFAULT_ADMIN_ROLE, _msgSender()), "sender doesn't have admin role"); + if (!hasRole(DEFAULT_ADMIN_ROLE, _msgSender())) revert SenderNotAdmin(); } /** @@ -41,7 +46,7 @@ contract SpectreProxy is AccessControl { @param spectreAddresses List of spectre addresses. */ constructor(uint8[] memory domainIDS, address[] memory spectreAddresses) { - require(domainIDS.length == spectreAddresses.length, "array length should be equal"); + if (domainIDS.length != spectreAddresses.length) revert ArrayLengthsDoNotMatch(); for (uint8 i = 0; i < domainIDS.length; i++) { spectreContracts[domainIDS[i]] = spectreAddresses[i]; } @@ -72,7 +77,7 @@ contract SpectreProxy is AccessControl { bytes calldata stepProof ) external { address spectreAddress = spectreContracts[sourceDomainID]; - require(spectreAddress != address(0), "no spectre address found"); + if (spectreAddress == address(0)) revert SpectreAddressNotFound(); ISpectre spectre = ISpectre(spectreAddress); spectre.rotate(rotateProof, stepInput, stepProof); @@ -95,16 +100,15 @@ contract SpectreProxy is AccessControl { bytes[] calldata stateRootProof ) external { address spectreAddress = spectreContracts[sourceDomainID]; - require(spectreAddress != address(0), "no spectre address found"); + if (spectreAddress == address(0)) revert SpectreAddressNotFound(); ISpectre spectre = ISpectre(spectreAddress); spectre.step(input, stepProof); bytes32 executionRoot = spectre.executionPayloadRoots(input.finalizedSlot); - require( - verifyMerkleBranch(stateRoot, executionRoot, stateRootProof, STATE_ROOT_INDEX), - "invalid merkle proof" - ); + if (!verifyMerkleBranch(stateRoot, executionRoot, stateRootProof, STATE_ROOT_INDEX)) { + revert InvalidMerkleProof(); + } stateRoots[sourceDomainID][input.finalizedSlot] = stateRoot; emit StateRootSubmitted(sourceDomainID, input.finalizedSlot, stateRoot); diff --git a/src/contracts/utils/AccessControlSegregator.sol b/src/contracts/utils/AccessControlSegregator.sol index fd2e7324..181845fb 100755 --- a/src/contracts/utils/AccessControlSegregator.sol +++ b/src/contracts/utils/AccessControlSegregator.sol @@ -13,6 +13,9 @@ contract AccessControlSegregator { bytes4 public constant GRANT_ACCESS_SIG = AccessControlSegregator(address(0)).grantAccess.selector; + error ArrayLengthsDoNotMatch(); + error SenderWithoutAccessRights(); + /** @notice Initializes access control to functions and sets initial access to grantAccess function. @@ -20,7 +23,7 @@ contract AccessControlSegregator { @param accounts List of accounts. */ constructor(bytes4[] memory functions, address[] memory accounts) { - require(accounts.length == functions.length, "array length should be equal"); + if (accounts.length != functions.length) revert ArrayLengthsDoNotMatch(); _grantAccess(GRANT_ACCESS_SIG, msg.sender); for (uint8 i = 0; i < accounts.length; i++) { @@ -45,7 +48,7 @@ contract AccessControlSegregator { @param account Address of account. */ function grantAccess(bytes4 sig, address account) public { - require(hasAccess(GRANT_ACCESS_SIG, msg.sender), "sender doesn't have grant access rights"); + if (!hasAccess(GRANT_ACCESS_SIG, msg.sender)) revert SenderWithoutAccessRights(); _grantAccess(sig, account); } diff --git a/src/contracts/utils/Pausable.sol b/src/contracts/utils/Pausable.sol index b66f1fa9..467380a5 100755 --- a/src/contracts/utils/Pausable.sol +++ b/src/contracts/utils/Pausable.sol @@ -11,6 +11,10 @@ pragma solidity 0.8.11; * */ contract Pausable { + + error ContractIsPaused(); + error ContractIsNotPaused(); + /** * @dev Emitted when the pause is triggered by `account`. */ @@ -50,7 +54,7 @@ contract Pausable { } function _whenNotPaused() private view { - require(!_paused, "Pausable: paused"); + if (_paused) revert ContractIsPaused(); } /** @@ -66,7 +70,7 @@ contract Pausable { } function _whenPaused() private view { - require(_paused, "Pausable: not paused"); + if (!_paused) revert ContractIsNotPaused(); } /** diff --git a/test/contractBridge/admin.test.ts b/test/contractBridge/admin.test.ts index ae165d4f..4f493c04 100644 --- a/test/contractBridge/admin.test.ts +++ b/test/contractBridge/admin.test.ts @@ -328,7 +328,10 @@ describe("Bridge - [admin]", () => { routerInstance .connect(tokenOwnerAccount) .adminSetDepositNonce(domainID, newNonce), - ).to.be.revertedWith("Does not allow decrements of the nonce"); + ).to.be.revertedWithCustomError( + routerInstance, + "NonceDecrementNotAllowed(uint64)", + ); }); it("Should require admin role to mark nonce as used", async () => { diff --git a/test/contractBridge/depositERC20.test.ts b/test/contractBridge/depositERC20.test.ts index 4d6d7406..8163610d 100755 --- a/test/contractBridge/depositERC20.test.ts +++ b/test/contractBridge/depositERC20.test.ts @@ -9,6 +9,7 @@ import type { Depositor, ERC20Handler, ERC20PresetMinterPauser, + ERC20Safe__factory, Executor, } from "../../typechain-types"; import { @@ -35,6 +36,7 @@ describe("Bridge - [deposit - ERC20]", () => { let ERC20MintableInstance1: ERC20PresetMinterPauser; let ERC20MintableInstance2: ERC20PresetMinterPauser; let ERC20HandlerInstance: ERC20Handler; + let erc20SafeContract: ERC20Safe__factory; let depositorAccount: HardhatEthersSigner; let recipientAccount: HardhatEthersSigner; @@ -111,6 +113,8 @@ describe("Bridge - [deposit - ERC20]", () => { 20, await recipientAccount.getAddress(), ); + + erc20SafeContract = await ethers.getContractFactory("ERC20Safe"); }); it("[sanity] test depositorAccount' balance", async () => { @@ -284,6 +288,6 @@ describe("Bridge - [deposit - ERC20]", () => { depositData, feeData, ), - ).to.be.revertedWith("ERC20: operation did not succeed"); + ).to.be.revertedWithCustomError(erc20SafeContract, "ERC20OperationFailed"); }); }); diff --git a/test/contractBridge/executeProposalERC20.test.ts b/test/contractBridge/executeProposalERC20.test.ts index 734691a0..a7b4e49c 100755 --- a/test/contractBridge/executeProposalERC20.test.ts +++ b/test/contractBridge/executeProposalERC20.test.ts @@ -15,6 +15,7 @@ import type { ERC20Handler, ERC20PresetMinterPauser, StateRootStorage, + MerkleTrie__factory, } from "../../typechain-types"; describe("Bridge - [execute proposal - ERC20]", () => { @@ -38,6 +39,7 @@ describe("Bridge - [execute proposal - ERC20]", () => { let ERC20MintableInstance: ERC20PresetMinterPauser; let ERC20HandlerInstance: ERC20Handler; let stateRootStorageInstance: StateRootStorage; + let MerkleTrieContract: MerkleTrie__factory; let depositorAccount: HardhatEthersSigner; let recipientAccount: HardhatEthersSigner; let relayer1: HardhatEthersSigner; @@ -126,6 +128,8 @@ describe("Bridge - [execute proposal - ERC20]", () => { slot, stateRoot, ); + + MerkleTrieContract = await ethers.getContractFactory("MerkleTrie"); }); it("isProposalExecuted returns false if depositNonce is not used", async () => { @@ -299,8 +303,9 @@ describe("Bridge - [execute proposal - ERC20]", () => { .connect(relayer1) .executeProposal(invalidProposal, accountProof1, slot); - await expect(proposalTx).to.be.revertedWith( - "MerkleTrie: invalid large internal hash", + await expect(proposalTx).to.be.revertedWithCustomError( + MerkleTrieContract, + "MerkleTrieInvalidLargeInternalHash", ); // check that deposit nonce has not been marked as used in bitmap @@ -347,7 +352,10 @@ describe("Bridge - [execute proposal - ERC20]", () => { it("[sanity] should fail if verifiers array is empty", async () => { await expect( executorInstance.adminSetVerifiers(1, []), - ).to.be.rejectedWith("Should provide at least one verifier address"); + ).to.be.revertedWithCustomError( + executorInstance, + "NoVerifierAddressProvided", + ); }); it("should successfully execute a proposal", async () => { diff --git a/test/handlers/erc20/deposit.test.ts b/test/handlers/erc20/deposit.test.ts index b669983e..9e41b261 100755 --- a/test/handlers/erc20/deposit.test.ts +++ b/test/handlers/erc20/deposit.test.ts @@ -15,6 +15,7 @@ import type { Executor, ERC20Handler, ERC20PresetMinterPauser, + ERC20Safe__factory, } from "../../../typechain-types"; describe("ERC20Handler - [Deposit ERC20]", () => { @@ -32,6 +33,7 @@ describe("ERC20Handler - [Deposit ERC20]", () => { let executorInstance: Executor; let ERC20MintableInstance: ERC20PresetMinterPauser; let ERC20HandlerInstance: ERC20Handler; + let erc20SafeContract: ERC20Safe__factory; let adminAccount: HardhatEthersSigner; let depositorAccount: HardhatEthersSigner; @@ -73,6 +75,8 @@ describe("ERC20Handler - [Deposit ERC20]", () => { emptySetResourceData, ), ]); + + erc20SafeContract = await ethers.getContractFactory("ERC20Safe"); }); it("[sanity] depositor owns tokenAmount of ERC20", async () => { @@ -203,7 +207,7 @@ describe("ERC20Handler - [Deposit ERC20]", () => { ), feeData, ), - ).to.be.revertedWith("ERC20: not a contract"); + ).to.be.revertedWithCustomError(erc20SafeContract, "ERC20NonContractCall"); await expect( routerInstance @@ -219,6 +223,6 @@ describe("ERC20Handler - [Deposit ERC20]", () => { ), feeData, ), - ).to.be.revertedWith("ERC20: not a contract"); + ).to.be.revertedWithCustomError(erc20SafeContract, "ERC20NonContractCall"); }); }); diff --git a/test/handlers/fee/basic/admin.test.ts b/test/handlers/fee/basic/admin.test.ts index 22754e39..c1bbfc8a 100755 --- a/test/handlers/fee/basic/admin.test.ts +++ b/test/handlers/fee/basic/admin.test.ts @@ -149,7 +149,7 @@ describe("BasicFeeHandler - [admin]", () => { basicFeeHandlerInstance .connect(nonAdminAccount) .changeFee(destinationDomainID, resourceID, securityModel, fee), - ).to.be.revertedWith("sender doesn't have admin role"); + ).to.be.revertedWithCustomError(basicFeeHandlerInstance, "SenderNotAdmin"); }); it("BasicFeeHandler admin should be changed to newBasicFeeHandlerAdmin", async () => { diff --git a/test/handlers/fee/basic/changeFee.test.ts b/test/handlers/fee/basic/changeFee.test.ts index 246a6963..b7af0e81 100755 --- a/test/handlers/fee/basic/changeFee.test.ts +++ b/test/handlers/fee/basic/changeFee.test.ts @@ -106,7 +106,10 @@ describe("BasicFeeHandler - [changeFee]", () => { securityModel, 0, ), - ).to.be.rejectedWith("Current fee is equal to new fee"); + ).to.be.revertedWithCustomError( + basicFeeHandlerInstance, + "NewFeeEqualsCurrentFee", + ); }); it("should require admin role to change fee", async () => { @@ -114,6 +117,6 @@ describe("BasicFeeHandler - [changeFee]", () => { basicFeeHandlerInstance .connect(nonAdminAccount) .changeFee(destinationDomainID, resourceID, securityModel, 1), - ).to.be.revertedWith("sender doesn't have admin role"); + ).to.be.revertedWithCustomError(basicFeeHandlerInstance, "SenderNotAdmin"); }); }); diff --git a/test/handlers/fee/basic/collectFee.test.ts b/test/handlers/fee/basic/collectFee.test.ts index a223dd5c..df2cfbd3 100755 --- a/test/handlers/fee/basic/collectFee.test.ts +++ b/test/handlers/fee/basic/collectFee.test.ts @@ -246,7 +246,7 @@ describe("BasicFeeHandler - [collectFee]", () => { value: ethers.parseEther("1.0"), }, ), - ).to.be.rejectedWith("no FeeHandler, msg.value != 0"); + ).to.be.revertedWithCustomError(routerInstance, "MsgValueNotZero"); }); it("deposit should pass if fee handler not set and fee not supplied", async () => { @@ -314,7 +314,10 @@ describe("BasicFeeHandler - [collectFee]", () => { value: ethers.parseEther("0.5"), }, ), - ).to.be.revertedWith("sender must be bridge or fee router contract"); + ).to.be.revertedWithCustomError( + basicFeeHandlerInstance, + "SenderNotBridgeOrRouter", + ); const balanceAfter = await ethers.provider.getBalance( await basicFeeHandlerInstance.getAddress(), @@ -373,7 +376,10 @@ describe("BasicFeeHandler - [collectFee]", () => { value: ethers.parseEther("0.5"), }, ), - ).to.be.revertedWith("sender must be router contract"); + ).to.be.revertedWithCustomError( + feeHandlerRouterInstance, + "SenderNotRouterContract", + ); const balanceAfter = await ethers.provider.getBalance( await basicFeeHandlerInstance.getAddress(), diff --git a/test/handlers/fee/basic/distributeFee.test.ts b/test/handlers/fee/basic/distributeFee.test.ts index b78e1730..937683d6 100755 --- a/test/handlers/fee/basic/distributeFee.test.ts +++ b/test/handlers/fee/basic/distributeFee.test.ts @@ -243,7 +243,7 @@ describe("BasicFeeHandler - [distributeFee]", () => { ], [payout, payout], ), - ).to.be.revertedWith("sender doesn't have admin role"); + ).to.be.revertedWithCustomError(basicFeeHandlerInstance, "SenderNotAdmin"); }); it("should revert if addrs and amounts arrays have different length", async () => { @@ -283,6 +283,9 @@ describe("BasicFeeHandler - [distributeFee]", () => { [await recipientAccount1.getAddress(), recipientAccount2.getAddress()], [payout, payout, payout], ), - ).to.be.revertedWith("addrs[], amounts[]: diff length"); + ).to.be.revertedWithCustomError( + basicFeeHandlerInstance, + "AddressesAndAmountsArraysDifferentLength", + ); }); }); diff --git a/test/handlers/fee/handlerRouter.test.ts b/test/handlers/fee/handlerRouter.test.ts index 9bf1b641..0e794d05 100755 --- a/test/handlers/fee/handlerRouter.test.ts +++ b/test/handlers/fee/handlerRouter.test.ts @@ -108,7 +108,7 @@ describe("FeeHandlerRouter", () => { securityModel, feeHandlerAccount.getAddress(), ), - ).to.be.revertedWith("sender doesn't have admin role"); + ).to.be.revertedWithCustomError(feeHandlerRouterInstance, "SenderNotAdmin"); }); it("should successfully set whitelist on an address", async () => { @@ -139,7 +139,7 @@ describe("FeeHandlerRouter", () => { feeHandlerRouterInstance .connect(nonAdminAccount) .adminSetWhitelist(await nonWhitelistedAccount.getAddress(), true), - ).to.be.revertedWith("sender doesn't have admin role"); + ).to.be.revertedWithCustomError(feeHandlerRouterInstance, "SenderNotAdmin"); }); it("should return fee 0 if address whitelisted", async () => { diff --git a/test/handlers/fee/percentage/admin.test.ts b/test/handlers/fee/percentage/admin.test.ts index aeefc08d..353371d2 100755 --- a/test/handlers/fee/percentage/admin.test.ts +++ b/test/handlers/fee/percentage/admin.test.ts @@ -140,7 +140,10 @@ describe("PercentageFeeHandler - [admin]", () => { percentageFeeHandlerInstance .connect(nonAdminAccount) .changeFee(destinationDomainID, resourceID, securityModel, fee), - ).to.be.revertedWith("sender doesn't have admin role"); + ).to.be.revertedWithCustomError( + percentageFeeHandlerInstance, + "SenderNotAdmin", + ); }); it("should set fee bounds", async () => { @@ -182,7 +185,10 @@ describe("PercentageFeeHandler - [admin]", () => { percentageFeeHandlerInstance .connect(nonAdminAccount) .changeFeeBounds(resourceID, lowerBound, upperBound), - ).to.be.revertedWith("sender doesn't have admin role"); + ).to.be.revertedWithCustomError( + percentageFeeHandlerInstance, + "SenderNotAdmin", + ); }); it("PercentageFeeHandler admin should be changed to newPercentageFeeHandlerAdmin", async () => { diff --git a/test/handlers/fee/percentage/changeFee.test.ts b/test/handlers/fee/percentage/changeFee.test.ts index 210fa075..d8f153a0 100755 --- a/test/handlers/fee/percentage/changeFee.test.ts +++ b/test/handlers/fee/percentage/changeFee.test.ts @@ -93,7 +93,10 @@ describe("PercentageFeeHandler - [change fee and bounds]", () => { securityModel, 0, ), - ).to.be.revertedWith("Current fee is equal to new fee"); + ).to.be.revertedWithCustomError( + percentageFeeHandlerInstance, + "NewFeeEqualsCurrentFee", + ); }); it("should require admin role to change fee", async () => { @@ -101,7 +104,10 @@ describe("PercentageFeeHandler - [change fee and bounds]", () => { percentageFeeHandlerInstance .connect(nonAdminAddress) .changeFee(destinationDomainID, resourceID, securityModel, 1), - ).to.be.revertedWith("sender doesn't have admin role"); + ).to.be.revertedWithCustomError( + percentageFeeHandlerInstance, + "SenderNotAdmin", + ); }); it("should set fee bounds", async () => { @@ -131,10 +137,13 @@ describe("PercentageFeeHandler - [change fee and bounds]", () => { await percentageFeeHandlerInstance.changeFeeBounds(resourceID, 25, 50); await expect( percentageFeeHandlerInstance.changeFeeBounds(resourceID, 25, 50), - ).to.be.revertedWith("Current bounds are equal to new bounds"); + ).to.be.revertedWithCustomError( + percentageFeeHandlerInstance, + "NewBoundsEqualCurrentBounds", + ); }); - it("should fail to set lower bound larger than upper bound ", async () => { + it("should fail to set lower bound larger than upper bound", async () => { const percentageFeeHandlerInstance = await PercentageFeeHandlerContract.deploy( await bridgeInstance.getAddress(), @@ -143,7 +152,10 @@ describe("PercentageFeeHandler - [change fee and bounds]", () => { ); await expect( percentageFeeHandlerInstance.changeFeeBounds(resourceID, 50, 25), - ).to.be.revertedWith("Upper bound must be larger than lower bound or 0"); + ).to.be.revertedWithCustomError( + percentageFeeHandlerInstance, + "InvalidBoundsRatio", + ); }); it("should set only lower bound", async () => { @@ -197,6 +209,9 @@ describe("PercentageFeeHandler - [change fee and bounds]", () => { percentageFeeHandlerInstance .connect(nonAdminAddress) .changeFeeBounds(resourceID, 50, 100), - ).to.be.revertedWith("sender doesn't have admin role"); + ).to.be.revertedWithCustomError( + percentageFeeHandlerInstance, + "SenderNotAdmin", + ); }); }); diff --git a/test/handlers/fee/percentage/collectFee.test.ts b/test/handlers/fee/percentage/collectFee.test.ts index 57fa7b8f..3e9b39c9 100755 --- a/test/handlers/fee/percentage/collectFee.test.ts +++ b/test/handlers/fee/percentage/collectFee.test.ts @@ -185,7 +185,10 @@ describe("PercentageFeeHandler - [collectFee]", () => { value: ethers.parseEther("0.5"), }, ), - ).to.be.revertedWith("collectFee: msg.value != 0"); + ).to.be.revertedWithCustomError( + percentageFeeHandlerInstance, + "MsgValueNotZero", + ); }); it("deposit should revert if fee collection fails", async () => { @@ -238,7 +241,10 @@ describe("PercentageFeeHandler - [collectFee]", () => { value: ethers.parseEther("0.5").toString(), }, ), - ).to.be.revertedWith("sender must be bridge or fee router contract"); + ).to.be.revertedWithCustomError( + percentageFeeHandlerInstance, + "SenderNotBridgeOrRouter", + ); }); it("deposit should revert if not called by bridge on FeeHandlerRouter contract", async () => { @@ -266,7 +272,10 @@ describe("PercentageFeeHandler - [collectFee]", () => { value: ethers.parseEther("0.5"), }, ), - ).to.be.revertedWith("sender must be router contract"); + ).to.be.revertedWithCustomError( + feeHandlerRouterInstance, + "SenderNotRouterContract", + ); }); it("should successfully change fee handler from FeeRouter to PercentageFeeHandler and collect fee", async () => { diff --git a/test/handlers/fee/percentage/distributeFee.test.ts b/test/handlers/fee/percentage/distributeFee.test.ts index 3ba391f3..b9eca9ff 100644 --- a/test/handlers/fee/percentage/distributeFee.test.ts +++ b/test/handlers/fee/percentage/distributeFee.test.ts @@ -256,7 +256,10 @@ describe("PercentageFeeHandler - [distributeFee]", () => { ], [payout, payout], ), - ).to.be.revertedWith("sender doesn't have admin role"); + ).to.be.revertedWithCustomError( + percentageFeeHandlerInstance, + "SenderNotAdmin", + ); }); it("should revert if addrs and amounts arrays have different length", async () => { @@ -285,6 +288,9 @@ describe("PercentageFeeHandler - [distributeFee]", () => { ], [payout, payout, payout], ), - ).to.be.revertedWith("addrs[], amounts[]: diff length"); + ).to.be.revertedWithCustomError( + percentageFeeHandlerInstance, + "AddressesAndAmountsArraysDifferentLength", + ); }); }); diff --git a/test/handlers/generic/permissionlessDeposit.test.ts b/test/handlers/generic/permissionlessDeposit.test.ts index f354088a..11bd2f60 100755 --- a/test/handlers/generic/permissionlessDeposit.test.ts +++ b/test/handlers/generic/permissionlessDeposit.test.ts @@ -131,7 +131,10 @@ describe("PermissionlessGenericHandler - [deposit]", () => { invalidDepositData, feeData, ), - ).to.be.revertedWith("Incorrect data length"); + ).to.be.revertedWithCustomError( + permissionlessGenericHandlerInstance, + "IncorrectDataLength", + ); }); it("should revert if metadata encoded depositor does not match deposit depositor", async () => { @@ -153,7 +156,10 @@ describe("PermissionlessGenericHandler - [deposit]", () => { invalidDepositData, feeData, ), - ).to.be.revertedWith("incorrect depositor in deposit data"); + ).to.be.revertedWithCustomError( + permissionlessGenericHandlerInstance, + "InvalidExecutioDataDepositor", + ); }); it("should revert if max fee exceeds 1000000", async () => { @@ -179,6 +185,9 @@ describe("PermissionlessGenericHandler - [deposit]", () => { from: depositorAccount, }, ), - ).to.be.revertedWith("requested fee too large"); + ).to.be.revertedWithCustomError( + permissionlessGenericHandlerInstance, + "RequestedFeeTooLarge", + ); }); }); diff --git a/test/proxies/spectre/spectreProxy.test.ts b/test/proxies/spectre/spectreProxy.test.ts index 33266c78..e2700cbc 100644 --- a/test/proxies/spectre/spectreProxy.test.ts +++ b/test/proxies/spectre/spectreProxy.test.ts @@ -80,7 +80,7 @@ describe("Spectre Proxy", () => { spectreProxyInstance .connect(nonAdminAccount) .adminSetSpectreAddress(originDomainID, spectreAddress), - ).to.be.revertedWith("sender doesn't have admin role"); + ).to.be.revertedWithCustomError(spectreProxyInstance, "SenderNotAdmin"); }); it("should set spectre address with an admin role", async () => { @@ -103,7 +103,10 @@ describe("Spectre Proxy", () => { stepInput, stepProof, ), - ).to.be.revertedWith("no spectre address found"); + ).to.be.revertedWithCustomError( + spectreProxyInstance, + "SpectreAddressNotFound", + ); }); it("should emit event even if rotate successful", async () => { @@ -128,7 +131,10 @@ describe("Spectre Proxy", () => { validStateRoot, validStateRootProof, ), - ).to.be.revertedWith("no spectre address found"); + ).to.be.revertedWithCustomError( + spectreProxyInstance, + "SpectreAddressNotFound", + ); }); it("should revert if step proof not valid", async () => { @@ -140,7 +146,7 @@ describe("Spectre Proxy", () => { validStateRoot, invalidStateRootProof, ), - ).to.be.revertedWith("invalid merkle proof"); + ).to.be.revertedWithCustomError(spectreProxyInstance, "InvalidMerkleProof"); }); it("should revert if step state root not valid", async () => { @@ -152,7 +158,7 @@ describe("Spectre Proxy", () => { invalidStateRoot, validStateRootProof, ), - ).to.be.revertedWith("invalid merkle proof"); + ).to.be.revertedWithCustomError(spectreProxyInstance, "InvalidMerkleProof"); }); it("should emit event and store state root if step valid", async () => { diff --git a/test/utils/accessControlSegregator/constructor.test.ts b/test/utils/accessControlSegregator/constructor.test.ts index c733c95f..127811da 100755 --- a/test/utils/accessControlSegregator/constructor.test.ts +++ b/test/utils/accessControlSegregator/constructor.test.ts @@ -68,7 +68,10 @@ describe("AccessControlSegregator - [constructor]", () => { ["0xa973ec93", "0x78728c73"], [adminAccount], ), - ).to.be.revertedWith("array length should be equal"); + ).to.be.revertedWithCustomError( + accessControlSegregatorInstance, + "ArrayLengthsDoNotMatch", + ); }); it("should grant deployer grant access rights", async () => { diff --git a/test/utils/accessControlSegregator/grantAccess.test.ts b/test/utils/accessControlSegregator/grantAccess.test.ts index f6763af1..256178d7 100755 --- a/test/utils/accessControlSegregator/grantAccess.test.ts +++ b/test/utils/accessControlSegregator/grantAccess.test.ts @@ -39,7 +39,10 @@ describe("AccessControlSegregator - [grant access]", () => { accessControlSegregatorInstance .connect(accountWithoutAccess) .grantAccess(functionSignature, receivingAccessAccount), - ).to.be.revertedWith("sender doesn't have grant access rights"); + ).to.be.revertedWithCustomError( + accessControlSegregatorInstance, + "SenderWithoutAccessRights", + ); }); it("should successfully grant access to a function", async () => {