diff --git a/contracts/Bridge.sol b/contracts/Bridge.sol index 7601f5cb..419882bb 100644 --- a/contracts/Bridge.sol +++ b/contracts/Bridge.sol @@ -79,13 +79,33 @@ contract Bridge is Pausable, Context, EIP712 { event Retry(string txHash); + error AccessNotAllowed(address sender, bytes4 funcSig); + + error ResourceIDNotMappedToHandler(); + + error DepositToCurrentDomain(); + + error InvalidProposalSigner(); + + error EmptyProposalsArray(); + + error NonceDecrementsNotAllowed(); + + error MPCAddressAlreadySet(); + + error MPCAddressNotSet(); + + error MPCAddressIsNotUpdatable(); + + error MPCAddressZeroAddress(); + modifier onlyAllowed() { _onlyAllowed(msg.sig, _msgSender()); _; } function _onlyAllowed(bytes4 sig, address sender) private view { - require(_accessControl.hasAccess(sig, sender), "sender doesn't have access to function"); + if (!_accessControl.hasAccess(sig, sender)) revert AccessNotAllowed(sender, sig); } function _msgSender() internal override view returns (address) { @@ -127,7 +147,7 @@ contract Bridge is Pausable, Context, EIP712 { @notice MPC address has to be set before Bridge can be unpaused */ function adminUnpauseTransfers() external onlyAllowed { - require(_MPCAddress != address(0), "MPC address not set"); + if (_MPCAddress == address(0)) revert MPCAddressNotSet(); _unpause(_msgSender()); } @@ -228,16 +248,16 @@ contract Bridge is Pausable, Context, EIP712 { @param feeData Additional data to be passed to the fee handler. @notice Emits {Deposit} event with all necessary parameters and a handler response. @return depositNonce deposit nonce for the destination domain. - @return handlerResponse a handler response: + @return handlerResponse a handler response: - ERC20Handler: responds with an empty data. - ERC721Handler: responds with the deposited token metadata acquired by calling a tokenURI method in the token contract. - PermissionedGenericHandler: responds with the raw bytes returned from the call to the target contract. - PermissionlessGenericHandler: responds with an empty data. */ - function deposit(uint8 destinationDomainID, bytes32 resourceID, bytes calldata depositData, bytes calldata feeData) - external payable whenNotPaused + function deposit(uint8 destinationDomainID, bytes32 resourceID, bytes calldata depositData, bytes calldata feeData) + external payable whenNotPaused returns (uint64 depositNonce, bytes memory handlerResponse) { - require(destinationDomainID != _domainID, "Can't deposit to current domain"); + if (destinationDomainID == _domainID) revert DepositToCurrentDomain(); address sender = _msgSender(); if (address(_feeHandler) == address(0)) { @@ -247,7 +267,7 @@ contract Bridge is Pausable, Context, EIP712 { _feeHandler.collectFee{value: msg.value}(sender, _domainID, destinationDomainID, resourceID, depositData, feeData); } address handler = _resourceIDToHandlerAddress[resourceID]; - require(handler != address(0), "resourceID not mapped to handler"); + if (handler == address(0)) revert ResourceIDNotMappedToHandler(); depositNonce = ++_depositCounts[destinationDomainID]; @@ -294,8 +314,8 @@ contract Bridge is Pausable, Context, EIP712 { In the case of {PermissionedGenericHandler}, when execution fails, the handler will emit a failure event and terminate the function normally. */ function executeProposals(Proposal[] memory proposals, bytes calldata signature) public whenNotPaused { - require(proposals.length > 0, "Proposals can't be an empty array"); - require(verify(proposals, signature), "Invalid proposal signer"); + if (proposals.length == 0) revert EmptyProposalsArray(); + if (!verify(proposals, signature)) revert InvalidProposalSigner(); for (uint256 i = 0; i < proposals.length; i++) { if(isProposalExecuted(proposals[i].originDomainID, proposals[i].depositNonce)) { @@ -327,7 +347,7 @@ contract Bridge is Pausable, Context, EIP712 { which is mapped in {functionAccess} in AccessControlSegregator contract. */ function startKeygen() external onlyAllowed { - require(_MPCAddress == address(0), "MPC address is already set"); + if (_MPCAddress != address(0)) revert MPCAddressAlreadySet(); emit StartKeygen(); } @@ -339,8 +359,8 @@ contract Bridge is Pausable, Context, EIP712 { @param MPCAddress Address that will be set as MPC address. */ function endKeygen(address MPCAddress) external onlyAllowed { - require(MPCAddress != address(0), "MPC address can't be null-address"); - require(_MPCAddress == address(0), "MPC address can't be updated"); + if( MPCAddress == address(0)) revert MPCAddressZeroAddress(); + if (_MPCAddress != address(0)) revert MPCAddressIsNotUpdatable(); _MPCAddress = MPCAddress; _unpause(_msgSender()); emit EndKeygen(); diff --git a/contracts/ERC20Safe.sol b/contracts/ERC20Safe.sol index 3373899e..d769cedd 100644 --- a/contracts/ERC20Safe.sol +++ b/contracts/ERC20Safe.sol @@ -101,5 +101,4 @@ contract ERC20Safe { require(abi.decode(returndata, (bool)), "ERC20: operation did not succeed"); } } - } diff --git a/contracts/TestContracts.sol b/contracts/TestContracts.sol index 5ffb9b96..f86fbf83 100644 --- a/contracts/TestContracts.sol +++ b/contracts/TestContracts.sol @@ -2,7 +2,6 @@ // SPDX-License-Identifier: BUSL-1.1 pragma solidity 0.8.11; -import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Wrapper.sol"; import "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetMinterPauser.sol"; import "./handlers/ERCHandlerHelpers.sol"; import "./interfaces/IERC20Plus.sol"; @@ -160,6 +159,37 @@ contract XC20Test is ERC20 { } } +/** + @dev This contract mocks XC20Test where "transferFrom()" always fails + */ +contract XC20TestMock is XC20Test { + + function transferFrom(address from, address to, uint256 amount) public virtual override(ERC20) returns (bool) { + address spender = _msgSender(); + _spendAllowance(from, spender, amount); + _transfer(from, to, amount); + return false; + } +} + +/** + @dev This contract mocks ERC20PresetMinterPauser where and "transferFrom()" always fails + */ +contract ERC20PresetMinterPauserMock is ERC20PresetMinterPauser { + + constructor( + string memory name, + string memory symbol + ) ERC20PresetMinterPauser(name, symbol) {} + + function transferFrom(address from, address to, uint256 amount) public virtual override(ERC20) returns (bool) { + address spender = _msgSender(); + _spendAllowance(from, spender, amount); + _transfer(from, to, amount); + return false; + } +} + contract ERC20PresetMinterPauserDecimals is ERC20PresetMinterPauser { uint8 private immutable customDecimals; diff --git a/contracts/handlers/ERC1155Handler.sol b/contracts/handlers/ERC1155Handler.sol index 503a26e9..4ccfafdb 100644 --- a/contracts/handlers/ERC1155Handler.sol +++ b/contracts/handlers/ERC1155Handler.sol @@ -77,7 +77,7 @@ contract ERC1155Handler is IHandler, ERCHandlerHelpers, ERC1155Safe, ERC1155Hold } address tokenAddress = _resourceIDToTokenContractAddress[resourceID]; - require(_tokenContractAddressToTokenProperties[address(tokenAddress)].isWhitelisted, "provided tokenAddress is not whitelisted"); + if (!_tokenContractAddressToTokenProperties[tokenAddress].isWhitelisted) revert ContractAddressNotWhitelisted(tokenAddress); if (_tokenContractAddressToTokenProperties[tokenAddress].isBurnable) { mintBatchERC1155(tokenAddress, address(recipientAddress), tokenIDs, amounts, transferData); diff --git a/contracts/handlers/ERC20Handler.sol b/contracts/handlers/ERC20Handler.sol index 91b7c9e4..5cd7c52c 100644 --- a/contracts/handlers/ERC20Handler.sol +++ b/contracts/handlers/ERC20Handler.sol @@ -42,7 +42,7 @@ contract ERC20Handler is IHandler, ERCHandlerHelpers, ERC20Safe { (amount) = abi.decode(data, (uint)); address tokenAddress = _resourceIDToTokenContractAddress[resourceID]; - require(_tokenContractAddressToTokenProperties[tokenAddress].isWhitelisted, "provided tokenAddress is not whitelisted"); + if (!_tokenContractAddressToTokenProperties[tokenAddress].isWhitelisted) revert ContractAddressNotWhitelisted(tokenAddress); if (_tokenContractAddressToTokenProperties[tokenAddress].isBurnable) { burnERC20(tokenAddress, depositor, amount); @@ -79,7 +79,7 @@ contract ERC20Handler is IHandler, ERCHandlerHelpers, ERC20Safe { recipientAddress := mload(add(destinationRecipientAddress, 0x20)) } - require(_tokenContractAddressToTokenProperties[tokenAddress].isWhitelisted, "provided tokenAddress is not whitelisted"); + if (!_tokenContractAddressToTokenProperties[tokenAddress].isWhitelisted) revert ContractAddressNotWhitelisted(tokenAddress); if (_tokenContractAddressToTokenProperties[tokenAddress].isBurnable) { mintERC20(tokenAddress, address(recipientAddress), convertToExternalBalance(tokenAddress, amount)); diff --git a/contracts/handlers/ERC721Handler.sol b/contracts/handlers/ERC721Handler.sol index 8bc0158f..dc0d9fbe 100644 --- a/contracts/handlers/ERC721Handler.sol +++ b/contracts/handlers/ERC721Handler.sol @@ -54,7 +54,7 @@ contract ERC721Handler is IHandler, ERCHandlerHelpers, ERC721Safe { (tokenID) = abi.decode(data, (uint)); address tokenAddress = _resourceIDToTokenContractAddress[resourceID]; - require(_tokenContractAddressToTokenProperties[tokenAddress].isWhitelisted, "provided tokenAddress is not whitelisted"); + if (!_tokenContractAddressToTokenProperties[tokenAddress].isWhitelisted) revert ContractAddressNotWhitelisted(tokenAddress); // Check if the contract supports metadata, fetch it if it does if (tokenAddress.supportsInterface(_INTERFACE_ERC721_METADATA)) { @@ -103,7 +103,7 @@ contract ERC721Handler is IHandler, ERCHandlerHelpers, ERC721Safe { } address tokenAddress = _resourceIDToTokenContractAddress[resourceID]; - require(_tokenContractAddressToTokenProperties[address(tokenAddress)].isWhitelisted, "provided tokenAddress is not whitelisted"); + if (!_tokenContractAddressToTokenProperties[tokenAddress].isWhitelisted) revert ContractAddressNotWhitelisted(tokenAddress); if (_tokenContractAddressToTokenProperties[tokenAddress].isBurnable) { mintERC721(tokenAddress, address(recipientAddress), tokenID, metaData); diff --git a/contracts/handlers/ERCHandlerHelpers.sol b/contracts/handlers/ERCHandlerHelpers.sol index 287703c6..ff6c4f2a 100644 --- a/contracts/handlers/ERCHandlerHelpers.sol +++ b/contracts/handlers/ERCHandlerHelpers.sol @@ -26,6 +26,8 @@ contract ERCHandlerHelpers is IERCHandler { Decimals decimals; } + error ContractAddressNotWhitelisted(address contractAddress); + // resourceID => token contract address mapping (bytes32 => address) public _resourceIDToTokenContractAddress; @@ -71,7 +73,7 @@ contract ERCHandlerHelpers is IERCHandler { } function _setBurnable(address contractAddress) internal { - require(_tokenContractAddressToTokenProperties[contractAddress].isWhitelisted, "provided contract is not whitelisted"); + if (!_tokenContractAddressToTokenProperties[contractAddress].isWhitelisted) revert ContractAddressNotWhitelisted(contractAddress); _tokenContractAddressToTokenProperties[contractAddress].isBurnable = true; } @@ -83,7 +85,7 @@ contract ERCHandlerHelpers is IERCHandler { @param externalDecimals Decimal places of token that is transferred. */ function _setDecimals(address contractAddress, uint8 externalDecimals) internal { - require(_tokenContractAddressToTokenProperties[contractAddress].isWhitelisted, "provided contract is not whitelisted"); + if (!_tokenContractAddressToTokenProperties[contractAddress].isWhitelisted) revert ContractAddressNotWhitelisted(contractAddress); _tokenContractAddressToTokenProperties[contractAddress].decimals = Decimals({ isSet: true, externalDecimals: externalDecimals diff --git a/contracts/handlers/PermissionedGenericHandler.sol b/contracts/handlers/PermissionedGenericHandler.sol index d654d754..8e2036b0 100644 --- a/contracts/handlers/PermissionedGenericHandler.sol +++ b/contracts/handlers/PermissionedGenericHandler.sol @@ -20,6 +20,8 @@ contract PermissionedGenericHandler is IHandler { bool isWhitelisted; } + error ContractAddressNotWhitelisted(address contractAddress); + // token contract address => TokenContractProperties mapping (address => TokenContractProperties) public _tokenContractAddressToTokenProperties; @@ -106,7 +108,7 @@ contract PermissionedGenericHandler is IHandler { require(depositor == address(uint160(metadataDepositor >> 96)), 'incorrect depositor in the data'); } - require(_tokenContractAddressToTokenProperties[contractAddress].isWhitelisted, "provided contractAddress is not whitelisted"); + if (!_tokenContractAddressToTokenProperties[contractAddress].isWhitelisted) revert ContractAddressNotWhitelisted(contractAddress); bytes4 sig = _tokenContractAddressToTokenProperties[contractAddress].depositFunctionSignature; if (sig != bytes4(0)) { @@ -136,7 +138,7 @@ contract PermissionedGenericHandler is IHandler { metaData = bytes(data[32:32 + lenMetadata]); address contractAddress = _resourceIDToContractAddress[resourceID]; - require(_tokenContractAddressToTokenProperties[contractAddress].isWhitelisted, "provided contractAddress is not whitelisted"); + if (!_tokenContractAddressToTokenProperties[contractAddress].isWhitelisted) revert ContractAddressNotWhitelisted(contractAddress); bytes4 sig = _tokenContractAddressToTokenProperties[contractAddress].executeFunctionSignature; if (sig != bytes4(0)) { diff --git a/contracts/handlers/XC20Handler.sol b/contracts/handlers/XC20Handler.sol index f9629321..efffd5bc 100644 --- a/contracts/handlers/XC20Handler.sol +++ b/contracts/handlers/XC20Handler.sol @@ -42,7 +42,7 @@ contract XC20Handler is IHandler, ERCHandlerHelpers, XC20Safe { (amount) = abi.decode(data, (uint)); address tokenAddress = _resourceIDToTokenContractAddress[resourceID]; - require(_tokenContractAddressToTokenProperties[tokenAddress].isWhitelisted, "provided tokenAddress is not whitelisted"); + if (!_tokenContractAddressToTokenProperties[tokenAddress].isWhitelisted) revert ContractAddressNotWhitelisted(tokenAddress); if (_tokenContractAddressToTokenProperties[tokenAddress].isBurnable) { burnERC20(tokenAddress, depositor, amount); @@ -79,7 +79,7 @@ contract XC20Handler is IHandler, ERCHandlerHelpers, XC20Safe { recipientAddress := mload(add(destinationRecipientAddress, 0x20)) } - require(_tokenContractAddressToTokenProperties[tokenAddress].isWhitelisted, "provided tokenAddress is not whitelisted"); + if (!_tokenContractAddressToTokenProperties[tokenAddress].isWhitelisted) revert ContractAddressNotWhitelisted(tokenAddress); if (_tokenContractAddressToTokenProperties[tokenAddress].isBurnable) { mintERC20(tokenAddress, address(recipientAddress), convertToExternalBalance(tokenAddress, amount)); diff --git a/contracts/handlers/fee/BasicFeeHandler.sol b/contracts/handlers/fee/BasicFeeHandler.sol index 3159fb7f..ce34f351 100644 --- a/contracts/handlers/fee/BasicFeeHandler.sol +++ b/contracts/handlers/fee/BasicFeeHandler.sol @@ -20,6 +20,8 @@ contract BasicFeeHandler is IFeeHandler, AccessControl { uint256 newFee ); + error IncorrectFeeSupplied(uint256); + modifier onlyAdmin() { require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "sender doesn't have admin role"); _; @@ -69,7 +71,7 @@ contract BasicFeeHandler is IFeeHandler, AccessControl { @param feeData Additional data to be passed to the fee handler. */ function collectFee(address sender, uint8 fromDomainID, uint8 destinationDomainID, bytes32 resourceID, bytes calldata depositData, bytes calldata feeData) payable external onlyBridgeOrRouter { - require(msg.value == _fee, "Incorrect fee supplied"); + if (msg.value != _fee) revert IncorrectFeeSupplied(msg.value); emit FeeCollected(sender, fromDomainID, destinationDomainID, resourceID, _fee, address(0)); } diff --git a/contracts/handlers/fee/DynamicERC20FeeHandlerEVM.sol b/contracts/handlers/fee/DynamicERC20FeeHandlerEVM.sol index 34d7a85b..653dda16 100644 --- a/contracts/handlers/fee/DynamicERC20FeeHandlerEVM.sol +++ b/contracts/handlers/fee/DynamicERC20FeeHandlerEVM.sol @@ -57,7 +57,7 @@ contract DynamicERC20FeeHandlerEVM is DynamicFeeHandler { total: 353 */ - require(feeData.length == 353, "Incorrect feeData length"); + if (feeData.length != 353) revert IncorrectFeeDataLength(feeData.length); FeeDataType memory feeDataDecoded; uint256 txCost; @@ -66,12 +66,18 @@ contract DynamicERC20FeeHandlerEVM is DynamicFeeHandler { feeDataDecoded.sig = bytes(feeData[256: 321]); OracleMessageType memory oracleMessage = abi.decode(feeDataDecoded.message, (OracleMessageType)); - require(block.timestamp <= oracleMessage.expiresAt, "Obsolete oracle data"); - require((oracleMessage.fromDomainID == fromDomainID) - && (oracleMessage.toDomainID == destinationDomainID) - && (oracleMessage.resourceID == resourceID), - "Incorrect deposit params" - ); + if (block.timestamp > oracleMessage.expiresAt) revert ObsoleteOracleData(); + if ((oracleMessage.fromDomainID != fromDomainID) || + (oracleMessage.toDomainID != destinationDomainID) || + (oracleMessage.resourceID != resourceID) + ) { + revert IncorrectDepositParams( + oracleMessage.fromDomainID, + oracleMessage.toDomainID, + oracleMessage.resourceID + ); + } + bytes32 messageHash = keccak256(feeDataDecoded.message); diff --git a/contracts/handlers/fee/DynamicERC20FeeHandlerSubstrate.sol b/contracts/handlers/fee/DynamicERC20FeeHandlerSubstrate.sol index cb8412c3..bedd8cbb 100644 --- a/contracts/handlers/fee/DynamicERC20FeeHandlerSubstrate.sol +++ b/contracts/handlers/fee/DynamicERC20FeeHandlerSubstrate.sol @@ -71,7 +71,7 @@ contract DynamicERC20FeeHandlerSubstrate is DynamicFeeHandler { total: 353 */ - require(feeData.length == 353, "Incorrect feeData length"); + if (feeData.length != 353) revert IncorrectFeeDataLength(feeData.length); FeeDataType memory feeDataDecoded; uint256 txCost; @@ -80,12 +80,17 @@ contract DynamicERC20FeeHandlerSubstrate is DynamicFeeHandler { feeDataDecoded.sig = bytes(feeData[256: 321]); SubstrateOracleMessageType memory oracleMessage = abi.decode(feeDataDecoded.message, (SubstrateOracleMessageType)); - require(block.timestamp <= oracleMessage.expiresAt, "Obsolete oracle data"); - require((oracleMessage.fromDomainID == fromDomainID) - && (oracleMessage.toDomainID == destinationDomainID) - && (oracleMessage.resourceID == resourceID), - "Incorrect deposit params" - ); + if (block.timestamp > oracleMessage.expiresAt) revert ObsoleteOracleData(); + if ((oracleMessage.fromDomainID != fromDomainID) || + (oracleMessage.toDomainID != destinationDomainID) || + (oracleMessage.resourceID != resourceID) + ) { + revert IncorrectDepositParams( + oracleMessage.fromDomainID, + oracleMessage.toDomainID, + oracleMessage.resourceID + ); + } bytes32 messageHash = keccak256(feeDataDecoded.message); diff --git a/contracts/handlers/fee/DynamicFeeHandler.sol b/contracts/handlers/fee/DynamicFeeHandler.sol index 1315068b..7ba6f947 100644 --- a/contracts/handlers/fee/DynamicFeeHandler.sol +++ b/contracts/handlers/fee/DynamicFeeHandler.sol @@ -47,6 +47,16 @@ abstract contract DynamicFeeHandler is IFeeHandler, AccessControl, ERC20Safe { event FeeOraclePropertiesSet(uint32 gasUsed, uint16 feePercent); + error InvalidSignature(); + + error IncorrectFeeDataLength(uint256); + + error IncorrectFeeSupplied(uint256); + + error ObsoleteOracleData(); + + error IncorrectDepositParams(uint8 fromDomainID, uint8 destinationDomainID, bytes32 resourceID); + modifier onlyAdmin() { require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "sender doesn't have admin role"); _; @@ -121,7 +131,7 @@ abstract contract DynamicFeeHandler is IFeeHandler, AccessControl, ERC20Safe { function collectFee(address sender, uint8 fromDomainID, uint8 destinationDomainID, bytes32 resourceID, bytes calldata depositData, bytes calldata feeData) payable external onlyBridgeOrRouter { (uint256 fee, address tokenAddress) = _calculateFee(sender, fromDomainID, destinationDomainID, resourceID, depositData, feeData); if(tokenAddress == address(0)){ - require(msg.value == fee, "Incorrect fee supplied"); + if (msg.value != fee) revert IncorrectFeeSupplied(msg.value); } else { require(msg.value == 0, "collectFee: msg.value != 0"); lockERC20(tokenAddress, sender, address(this), fee); @@ -166,6 +176,6 @@ abstract contract DynamicFeeHandler is IFeeHandler, AccessControl, ERC20Safe { function verifySig(bytes32 message, bytes memory signature, address signerAddress) internal pure { address signerAddressRecovered = ECDSA.recover(message, signature); - require(signerAddressRecovered == signerAddress, 'Invalid signature'); + if (signerAddressRecovered != signerAddress) revert InvalidSignature(); } } diff --git a/contracts/handlers/fee/DynamicGenericFeeHandlerEVM.sol b/contracts/handlers/fee/DynamicGenericFeeHandlerEVM.sol index 71f34c47..f26178b2 100644 --- a/contracts/handlers/fee/DynamicGenericFeeHandlerEVM.sol +++ b/contracts/handlers/fee/DynamicGenericFeeHandlerEVM.sol @@ -54,7 +54,7 @@ contract DynamicGenericFeeHandlerEVM is DynamicFeeHandler { total: 353 */ - require(feeData.length == 353, "Incorrect feeData length"); + if (feeData.length != 353) revert IncorrectFeeDataLength(feeData.length); FeeDataType memory feeDataDecoded; uint256 txCost; @@ -63,12 +63,17 @@ contract DynamicGenericFeeHandlerEVM is DynamicFeeHandler { feeDataDecoded.sig = bytes(feeData[256: 321]); OracleMessageType memory oracleMessage = abi.decode(feeDataDecoded.message, (OracleMessageType)); - require(block.timestamp <= oracleMessage.expiresAt, "Obsolete oracle data"); - require((oracleMessage.fromDomainID == fromDomainID) - && (oracleMessage.toDomainID == destinationDomainID) - && (oracleMessage.resourceID == resourceID), - "Incorrect deposit params" - ); + if (block.timestamp > oracleMessage.expiresAt) revert ObsoleteOracleData(); + if ((oracleMessage.fromDomainID != fromDomainID) || + (oracleMessage.toDomainID != destinationDomainID) || + (oracleMessage.resourceID != resourceID) + ) { + revert IncorrectDepositParams( + oracleMessage.fromDomainID, + oracleMessage.toDomainID, + oracleMessage.resourceID + ); + } require(oracleMessage.msgGasLimit > 0, "msgGasLimit == 0"); bytes32 messageHash = keccak256(feeDataDecoded.message); diff --git a/test/contractBridge/admin.js b/test/contractBridge/admin.js index 27f9ae0a..79a798be 100644 --- a/test/contractBridge/admin.js +++ b/test/contractBridge/admin.js @@ -42,10 +42,10 @@ contract("Bridge - [admin]", async (accounts) => { let withdrawData = ""; - const assertOnlyAdmin = (method, ...params) => { - return TruffleAssert.reverts( - method(...params, {from: nonAdminAddress}), - "sender doesn't have access to function" + const assertOnlyAdmin = (method) => { + return Helpers.expectToRevertWithCustomError( + method(), + "AccessNotAllowed(address,bytes4)" ); }; @@ -112,15 +112,17 @@ contract("Bridge - [admin]", async (accounts) => { }); it("Should fail if \"StartKeygen\" is called by non admin", async () => { - await assertOnlyAdmin(BridgeInstance.startKeygen); + await assertOnlyAdmin(() => + BridgeInstance.startKeygen({from: nonAdminAddress}) + ); }); it("Should fail if \"StartKeygen\" is called after MPC address is set", async () => { await BridgeInstance.endKeygen(Helpers.mpcAddress); - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( BridgeInstance.startKeygen(), - "MPC address is already set" + "MPCAddressAlreadySet()" ); }); @@ -133,22 +135,27 @@ contract("Bridge - [admin]", async (accounts) => { }); it("Should fail if \"endKeygen\" is called by non admin", async () => { - await assertOnlyAdmin(BridgeInstance.endKeygen, someAddress); + await assertOnlyAdmin(() => + BridgeInstance.endKeygen( + someAddress, + {from: nonAdminAddress} + ) + ) }); it("Should fail if null address is passed as MPC address", async () => { - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( BridgeInstance.endKeygen(nullAddress), - "MPC address can't be null-address" + "MPCAddressZeroAddress()" ); }); it("Should fail if admin tries to update MPC address", async () => { await BridgeInstance.endKeygen(Helpers.mpcAddress); - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( BridgeInstance.endKeygen(someAddress), - "MPC address can't be updated" + "MPCAddressIsNotUpdatable()" ); }); @@ -161,7 +168,12 @@ contract("Bridge - [admin]", async (accounts) => { }); it("Should fail if \"refreshKey\" is called by non admin", async () => { - await assertOnlyAdmin(BridgeInstance.refreshKey, topologyHash); + await assertOnlyAdmin(() => + BridgeInstance.refreshKey( + topologyHash, + {from: nonAdminAddress} + ) + ) }); // Set Handler Address @@ -239,12 +251,14 @@ contract("Bridge - [admin]", async (accounts) => { }); it("Should require admin role to set a ERC20 Resource ID and contract address", async () => { - await assertOnlyAdmin( - BridgeInstance.adminSetResource, - someAddress, - bytes32, - someAddress, - genericHandlerSetResourceData + await assertOnlyAdmin(() => + BridgeInstance.adminSetResource( + someAddress, + bytes32, + someAddress, + genericHandlerSetResourceData, + {from: nonAdminAddress} + ) ); }); @@ -313,12 +327,14 @@ contract("Bridge - [admin]", async (accounts) => { }); it("Should require admin role to set a Generic Resource ID and contract address", async () => { - await assertOnlyAdmin( - BridgeInstance.adminSetResource, - someAddress, - bytes32, - someAddress, - genericHandlerSetResourceData + await assertOnlyAdmin(() => + BridgeInstance.adminSetResource( + someAddress, + bytes32, + someAddress, + genericHandlerSetResourceData, + {from: nonAdminAddress} + ) ); }); @@ -359,10 +375,12 @@ contract("Bridge - [admin]", async (accounts) => { }); it("Should require admin role to set ERC20MintableInstance.address as burnable", async () => { - await assertOnlyAdmin( - BridgeInstance.adminSetBurnable, - someAddress, - someAddress + await assertOnlyAdmin(() => + BridgeInstance.adminSetBurnable( + someAddress, + someAddress, + {from: nonAdminAddress} + ) ); }); @@ -426,7 +444,13 @@ contract("Bridge - [admin]", async (accounts) => { }); it("Should require admin role to withdraw funds", async () => { - await assertOnlyAdmin(BridgeInstance.adminWithdraw, someAddress, "0x0"); + await assertOnlyAdmin(() => + BridgeInstance.adminWithdraw( + someAddress, + "0x0", + {from: nonAdminAddress} + ) + ) }); // Set nonce @@ -439,7 +463,13 @@ contract("Bridge - [admin]", async (accounts) => { }); it("Should require admin role to set nonce", async () => { - await assertOnlyAdmin(BridgeInstance.adminSetDepositNonce, 1, 3); + await assertOnlyAdmin(() => + BridgeInstance.adminSetDepositNonce( + 1, + 3, + {from: nonAdminAddress} + ) + ) }); it("Should not allow for decrements of the nonce", async () => { @@ -455,13 +485,23 @@ contract("Bridge - [admin]", async (accounts) => { // Change access control contract it("Should require admin role to change access control contract", async () => { - await assertOnlyAdmin(BridgeInstance.adminChangeAccessControl, someAddress); + await assertOnlyAdmin(() => + BridgeInstance.adminChangeAccessControl( + someAddress, + {from: nonAdminAddress} + ) + ) }); // Retry it("Should require admin role to retry deposit", async () => { - await assertOnlyAdmin(BridgeInstance.retry, txHash); + await assertOnlyAdmin(() => + BridgeInstance.retry( + txHash, + {from: nonAdminAddress} + ) + ) }); it("Should successfully emit Retry event", async () => { diff --git a/test/contractBridge/depositERC1155.js b/test/contractBridge/depositERC1155.js index 39c62b02..f25645de 100644 --- a/test/contractBridge/depositERC1155.js +++ b/test/contractBridge/depositERC1155.js @@ -187,20 +187,20 @@ contract("Bridge - [deposit - ERC1155]", async (accounts) => { }); it("deposit requires resourceID that is mapped to a handler", async () => { - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( BridgeInstance.deposit(destinationDomainID, "0x0", depositData, feeData, { from: depositorAddress, }), - "resourceID not mapped to handler" + "ResourceIDNotMappedToHandler()" ); }); it("Deposit destination domain can not be current bridge domain ", async () => { - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( BridgeInstance.deposit(originDomainID, "0x0", depositData, feeData, { from: depositorAddress, }), - "Can't deposit to current domain" + "DepositToCurrentDomain()" ); }); }); diff --git a/test/contractBridge/depositERC20.js b/test/contractBridge/depositERC20.js index 36207e52..ea9d0fce 100644 --- a/test/contractBridge/depositERC20.js +++ b/test/contractBridge/depositERC20.js @@ -8,6 +8,7 @@ const TruffleAssert = require("truffle-assertions"); const Helpers = require("../helpers"); const ERC20MintableContract = artifacts.require("ERC20PresetMinterPauser"); +const ERC20MintableContractMock = artifacts.require("ERC20PresetMinterPauserMock"); const ERC20HandlerContract = artifacts.require("ERC20Handler"); contract("Bridge - [deposit - ERC20]", async (accounts) => { @@ -25,24 +26,35 @@ contract("Bridge - [deposit - ERC20]", async (accounts) => { let BridgeInstance; let OriginERC20MintableInstance; + let OriginERC20MintableInstanceMock; let OriginERC20HandlerInstance; let depositData; + let initialResourceIDs; beforeEach(async () => { await Promise.all([ ERC20MintableContract.new("token", "TOK").then( (instance) => (OriginERC20MintableInstance = instance) ), + ERC20MintableContractMock.new("token", "TOK").then( + (instance) => (OriginERC20MintableInstanceMock = instance) + ), (BridgeInstance = await Helpers.deployBridge( originDomainID, adminAddress )), ]); - resourceID = Helpers.createResourceID( + const resourceID1 = Helpers.createResourceID( OriginERC20MintableInstance.address, originDomainID ); + const resourceID2 = Helpers.createResourceID( + OriginERC20MintableInstanceMock.address, + originDomainID + ); + + initialResourceIDs = [resourceID1, resourceID2]; OriginERC20HandlerInstance = await ERC20HandlerContract.new( BridgeInstance.address @@ -51,20 +63,35 @@ contract("Bridge - [deposit - ERC20]", async (accounts) => { await Promise.all([ BridgeInstance.adminSetResource( OriginERC20HandlerInstance.address, - resourceID, + initialResourceIDs[0], OriginERC20MintableInstance.address, emptySetResourceData ), + BridgeInstance.adminSetResource( + OriginERC20HandlerInstance.address, + initialResourceIDs[1], + OriginERC20MintableInstanceMock.address, + emptySetResourceData + ), OriginERC20MintableInstance.mint( depositorAddress, originChainInitialTokenAmount ), + OriginERC20MintableInstanceMock.mint( + depositorAddress, + originChainInitialTokenAmount + ), ]); await OriginERC20MintableInstance.approve( OriginERC20HandlerInstance.address, depositAmount * 2, {from: depositorAddress} ); + await OriginERC20MintableInstanceMock.approve( + OriginERC20HandlerInstance.address, + depositAmount, + {from: depositorAddress} + ); depositData = Helpers.createERCDepositData( depositAmount, @@ -101,7 +128,7 @@ contract("Bridge - [deposit - ERC20]", async (accounts) => { await TruffleAssert.passes( BridgeInstance.deposit( destinationDomainID, - resourceID, + initialResourceIDs[0], depositData, feeData, {from: depositorAddress} @@ -112,7 +139,7 @@ contract("Bridge - [deposit - ERC20]", async (accounts) => { it("_depositCounts should be increments from 0 to 1", async () => { await BridgeInstance.deposit( destinationDomainID, - resourceID, + initialResourceIDs[0], depositData, feeData, {from: depositorAddress} @@ -127,7 +154,7 @@ contract("Bridge - [deposit - ERC20]", async (accounts) => { it("ERC20 can be deposited with correct balances", async () => { await BridgeInstance.deposit( destinationDomainID, - resourceID, + initialResourceIDs[0], depositData, feeData, {from: depositorAddress} @@ -150,7 +177,7 @@ contract("Bridge - [deposit - ERC20]", async (accounts) => { it("Deposit event is fired with expected value", async () => { let depositTx = await BridgeInstance.deposit( destinationDomainID, - resourceID, + initialResourceIDs[0], depositData, feeData, {from: depositorAddress} @@ -159,14 +186,14 @@ contract("Bridge - [deposit - ERC20]", async (accounts) => { TruffleAssert.eventEmitted(depositTx, "Deposit", (event) => { return ( event.destinationDomainID.toNumber() === destinationDomainID && - event.resourceID === resourceID.toLowerCase() && + event.resourceID === initialResourceIDs[0].toLowerCase() && event.depositNonce.toNumber() === expectedDepositNonce ); }); depositTx = await BridgeInstance.deposit( destinationDomainID, - resourceID, + initialResourceIDs[0], depositData, feeData, {from: depositorAddress} @@ -175,27 +202,40 @@ contract("Bridge - [deposit - ERC20]", async (accounts) => { TruffleAssert.eventEmitted(depositTx, "Deposit", (event) => { return ( event.destinationDomainID.toNumber() === destinationDomainID && - event.resourceID === resourceID.toLowerCase() && + event.resourceID === initialResourceIDs[0].toLowerCase() && event.depositNonce.toNumber() === expectedDepositNonce + 1 ); }); }); it("deposit requires resourceID that is mapped to a handler", async () => { - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( BridgeInstance.deposit(destinationDomainID, "0x0", depositData, feeData, { from: depositorAddress, }), - "resourceID not mapped to handler" + "ResourceIDNotMappedToHandler()" ); }); it("Deposit destination domain can not be current bridge domain ", async () => { - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( BridgeInstance.deposit(originDomainID, "0x0", depositData, feeData, { from: depositorAddress, }), - "Can't deposit to current domain" + "DepositToCurrentDomain()" + ); + }); + + it("should revert if ERC20Safe contract call fails", async () => { + await TruffleAssert.reverts( + BridgeInstance.deposit( + destinationDomainID, + initialResourceIDs[1], + depositData, + feeData, + {from: depositorAddress} + ), + "ERC20: operation did not succeed" ); }); }); diff --git a/test/contractBridge/depositERC721.js b/test/contractBridge/depositERC721.js index 7fbe93e7..291d49c1 100644 --- a/test/contractBridge/depositERC721.js +++ b/test/contractBridge/depositERC721.js @@ -181,11 +181,11 @@ contract("Bridge - [deposit - ERC721]", async (accounts) => { }); it("Deposit destination domain can not be current bridge domain ", async () => { - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( BridgeInstance.deposit(originDomainID, "0x0", depositData, feeData, { from: depositorAddress, }), - "Can't deposit to current domain" + "DepositToCurrentDomain()" ); }); }); diff --git a/test/contractBridge/depositGeneric.js b/test/contractBridge/depositGeneric.js index b12af9d5..739e8989 100644 --- a/test/contractBridge/depositGeneric.js +++ b/test/contractBridge/depositGeneric.js @@ -120,11 +120,11 @@ contract("Bridge - [deposit - Generic]", async (accounts) => { }); it("Deposit destination domain can not be current bridge domain ", async () => { - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( BridgeInstance.deposit(originDomainID, "0x0", depositData, feeData, { from: depositorAddress, }), - "Can't deposit to current domain" + "DepositToCurrentDomain()" ); }); }); diff --git a/test/contractBridge/depositXC20.js b/test/contractBridge/depositXC20.js index 3479592e..5d662a70 100644 --- a/test/contractBridge/depositXC20.js +++ b/test/contractBridge/depositXC20.js @@ -9,6 +9,7 @@ const Helpers = require("../helpers"); const XC20HandlerContract = artifacts.require("XC20Handler"); const XC20TestContract = artifacts.require("XC20Test"); +const XC20TestContractMock = artifacts.require("XC20TestMock"); contract("Bridge - [deposit - XRC20]", async (accounts) => { const originDomainID = 1; @@ -25,8 +26,10 @@ contract("Bridge - [deposit - XRC20]", async (accounts) => { let BridgeInstance; let OriginXC20TestInstance; + let OriginXC20TestInstanceMock; let OriginXC20HandlerInstance; let depositData; + let initialResourceIDs; beforeEach(async () => { await Promise.all([ @@ -37,12 +40,21 @@ contract("Bridge - [deposit - XRC20]", async (accounts) => { XC20TestContract.new().then( (instance) => (OriginXC20TestInstance = instance) ), + XC20TestContractMock.new().then( + (instance) => (OriginXC20TestInstanceMock = instance) + ), ]); - resourceID = Helpers.createResourceID( + const resourceID1 = Helpers.createResourceID( OriginXC20TestInstance.address, originDomainID ); + const resourceID2 = Helpers.createResourceID( + OriginXC20TestInstanceMock.address, + originDomainID + ); + + initialResourceIDs = [resourceID1, resourceID2]; OriginXC20HandlerInstance = await XC20HandlerContract.new( BridgeInstance.address @@ -51,20 +63,35 @@ contract("Bridge - [deposit - XRC20]", async (accounts) => { await Promise.all([ BridgeInstance.adminSetResource( OriginXC20HandlerInstance.address, - resourceID, + initialResourceIDs[0], OriginXC20TestInstance.address, emptySetResourceData ), + BridgeInstance.adminSetResource( + OriginXC20HandlerInstance.address, + initialResourceIDs[1], + OriginXC20TestInstanceMock.address, + emptySetResourceData + ), OriginXC20TestInstance.mint( depositorAddress, originChainInitialTokenAmount ), + OriginXC20TestInstanceMock.mint( + depositorAddress, + originChainInitialTokenAmount + ), ]); await OriginXC20TestInstance.approve( OriginXC20HandlerInstance.address, depositAmount, {from: depositorAddress} ); + await OriginXC20TestInstanceMock.approve( + OriginXC20HandlerInstance.address, + depositAmount, + {from: depositorAddress} + ); depositData = Helpers.createERCDepositData( depositAmount, @@ -90,7 +117,7 @@ contract("Bridge - [deposit - XRC20]", async (accounts) => { await TruffleAssert.passes( BridgeInstance.deposit( destinationDomainID, - resourceID, + initialResourceIDs[0], depositData, feeData, {from: depositorAddress} @@ -101,7 +128,7 @@ contract("Bridge - [deposit - XRC20]", async (accounts) => { it("_depositCounts should be increments from 0 to 1", async () => { await BridgeInstance.deposit( destinationDomainID, - resourceID, + initialResourceIDs[0], depositData, feeData, {from: depositorAddress} @@ -116,7 +143,7 @@ contract("Bridge - [deposit - XRC20]", async (accounts) => { it("XC20 can be deposited with correct balances", async () => { await BridgeInstance.deposit( destinationDomainID, - resourceID, + initialResourceIDs[0], depositData, feeData, {from: depositorAddress} @@ -145,7 +172,7 @@ contract("Bridge - [deposit - XRC20]", async (accounts) => { let depositTx = await BridgeInstance.deposit( destinationDomainID, - resourceID, + initialResourceIDs[0], depositData, feeData, {from: depositorAddress} @@ -154,14 +181,14 @@ contract("Bridge - [deposit - XRC20]", async (accounts) => { TruffleAssert.eventEmitted(depositTx, "Deposit", (event) => { return ( event.destinationDomainID.toNumber() === destinationDomainID && - event.resourceID === resourceID.toLowerCase() && + event.resourceID === initialResourceIDs[0].toLowerCase() && event.depositNonce.toNumber() === expectedDepositNonce ); }); depositTx = await BridgeInstance.deposit( destinationDomainID, - resourceID, + initialResourceIDs[0], depositData, feeData, {from: depositorAddress} @@ -170,14 +197,14 @@ contract("Bridge - [deposit - XRC20]", async (accounts) => { TruffleAssert.eventEmitted(depositTx, "Deposit", (event) => { return ( event.destinationDomainID.toNumber() === destinationDomainID && - event.resourceID === resourceID.toLowerCase() && + event.resourceID === initialResourceIDs[0].toLowerCase() && event.depositNonce.toNumber() === expectedDepositNonce + 1 ); }); }); it("deposit requires resourceID that is mapped to a handler", async () => { - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( BridgeInstance.deposit( destinationDomainID, "0x0", @@ -185,16 +212,29 @@ contract("Bridge - [deposit - XRC20]", async (accounts) => { feeData, {from: depositorAddress} ), - "resourceID not mapped to handler" + "ResourceIDNotMappedToHandler()" ); }); it("Deposit destination domain can not be current bridge domain ", async () => { - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( BridgeInstance.deposit(originDomainID, "0x0", depositData, feeData, { from: depositorAddress, }), - "Can't deposit to current domain" + "DepositToCurrentDomain()" + ); + }); + + it("should if XC20Safe contract call fails", async () => { + await TruffleAssert.reverts( + BridgeInstance.deposit( + destinationDomainID, + initialResourceIDs[1], + depositData, + feeData, + {from: depositorAddress} + ), + "ERC20: operation did not succeed" ); }); }); @@ -220,7 +260,7 @@ contract("Bridge - [deposit - XRC20]", async (accounts) => { await TruffleAssert.passes( BridgeInstance.deposit( destinationDomainID, - resourceID, + initialResourceIDs[0], depositData, feeData, {from: depositorAddress} @@ -231,7 +271,7 @@ contract("Bridge - [deposit - XRC20]", async (accounts) => { it("_depositCounts should be increments from 0 to 1", async () => { await BridgeInstance.deposit( destinationDomainID, - resourceID, + initialResourceIDs[0], depositData, feeData, {from: depositorAddress} @@ -246,7 +286,7 @@ contract("Bridge - [deposit - XRC20]", async (accounts) => { it("XC20 can be deposited with correct balances", async () => { await BridgeInstance.deposit( destinationDomainID, - resourceID, + initialResourceIDs[0], depositData, feeData, {from: depositorAddress} @@ -269,7 +309,7 @@ contract("Bridge - [deposit - XRC20]", async (accounts) => { it("Deposit event is fired with expected value", async () => { let depositTx = await BridgeInstance.deposit( destinationDomainID, - resourceID, + initialResourceIDs[0], depositData, feeData, {from: depositorAddress} @@ -278,14 +318,14 @@ contract("Bridge - [deposit - XRC20]", async (accounts) => { TruffleAssert.eventEmitted(depositTx, "Deposit", (event) => { return ( event.destinationDomainID.toNumber() === destinationDomainID && - event.resourceID === resourceID.toLowerCase() && + event.resourceID === initialResourceIDs[0].toLowerCase() && event.depositNonce.toNumber() === expectedDepositNonce ); }); depositTx = await BridgeInstance.deposit( destinationDomainID, - resourceID, + initialResourceIDs[0], depositData, feeData, {from: depositorAddress} @@ -294,14 +334,14 @@ contract("Bridge - [deposit - XRC20]", async (accounts) => { TruffleAssert.eventEmitted(depositTx, "Deposit", (event) => { return ( event.destinationDomainID.toNumber() === destinationDomainID && - event.resourceID === resourceID.toLowerCase() && + event.resourceID === initialResourceIDs[0].toLowerCase() && event.depositNonce.toNumber() === expectedDepositNonce + 1 ); }); }); - it("deposit requires resourceID that is mapped to a handler", async () => { - await TruffleAssert.reverts( + it("deposit requires initialResourceIDs[0] that is mapped to a handler", async () => { + await Helpers.expectToRevertWithCustomError( BridgeInstance.deposit( destinationDomainID, "0x0", @@ -309,16 +349,16 @@ contract("Bridge - [deposit - XRC20]", async (accounts) => { feeData, {from: depositorAddress} ), - "resourceID not mapped to handler" + "ResourceIDNotMappedToHandler()" ); }); it("Deposit destination domain can not be current bridge domain ", async () => { - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( BridgeInstance.deposit(originDomainID, "0x0", depositData, feeData, { from: depositorAddress, }), - "Can't deposit to current domain" + "DepositToCurrentDomain()" ); }); }); diff --git a/test/contractBridge/executeProposalERC20.js b/test/contractBridge/executeProposalERC20.js index 8247bec1..b2232b38 100644 --- a/test/contractBridge/executeProposalERC20.js +++ b/test/contractBridge/executeProposalERC20.js @@ -244,11 +244,11 @@ contract("Bridge - [execute proposal - ERC20]", async (accounts) => { }) ); - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( BridgeInstance.executeProposal(proposal, proposalSignedData, { from: relayer1Address, }), - "Invalid proposal signer" + "InvalidProposalSigner()" ); }); }); diff --git a/test/contractBridge/executeProposalXC20.js b/test/contractBridge/executeProposalXC20.js index 3e1298da..be077503 100644 --- a/test/contractBridge/executeProposalXC20.js +++ b/test/contractBridge/executeProposalXC20.js @@ -268,11 +268,11 @@ contract("Bridge - [execute proposal - XC20]", async (accounts) => { ) ); - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( BridgeInstance.executeProposal(proposal, proposalSignedData, { from: relayer1Address, }), - "Invalid proposal signer" + "InvalidProposalSigner()" ); }); }); @@ -436,11 +436,11 @@ contract("Bridge - [execute proposal - XC20]", async (accounts) => { ) ); - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( BridgeInstance.executeProposal(proposal, proposalSignedData, { from: relayer1Address, }), - "Invalid proposal signer" + "InvalidProposalSigner()" ); }); }); diff --git a/test/contractBridge/executeProposals.js b/test/contractBridge/executeProposals.js index 954e8b23..6863b243 100644 --- a/test/contractBridge/executeProposals.js +++ b/test/contractBridge/executeProposals.js @@ -381,11 +381,11 @@ contract("Bridge - [execute proposal]", async (accounts) => { proposalsForExecution ); - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( BridgeInstance.executeProposals([], proposalSignedData, { from: relayer1Address, }), - "Proposals can't be an empty array" + "EmptyProposalsArray()" ); }); @@ -519,13 +519,13 @@ contract("Bridge - [execute proposal]", async (accounts) => { ) ); - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( BridgeInstance.executeProposals( proposalsForExecution, proposalSignedData, {from: relayer1Address} ), - "Invalid proposal signer" + "InvalidProposalSigner()" ); }); }); diff --git a/test/handlers/erc20/decimals.js b/test/handlers/erc20/decimals.js index 0f968835..d52e8e1e 100644 --- a/test/handlers/erc20/decimals.js +++ b/test/handlers/erc20/decimals.js @@ -74,8 +74,6 @@ contract("ERC20Handler - [decimals]", async (accounts) => { await ERC20MintableInstance.address )).decimals; - console.log("ERC20MintableInstanceDecimals",ERC20MintableInstanceDecimals["externalDecimals"]) - assert.strictEqual(ERC20MintableInstanceDecimals.isSet, false) assert.strictEqual(ERC20MintableInstanceDecimals["externalDecimals"], "0") }); diff --git a/test/handlers/fee/basic/collectFee.js b/test/handlers/fee/basic/collectFee.js index 65d435ec..56a00c65 100644 --- a/test/handlers/fee/basic/collectFee.js +++ b/test/handlers/fee/basic/collectFee.js @@ -149,8 +149,9 @@ contract("BasicFeeHandler - [collectFee]", async (accounts) => { it("deposit should revert if invalid fee amount supplied", async () => { // current fee is set to 0 assert.equal(await ERC20BasicFeeHandlerInstance._fee.call(), 0); + const incorrectFee = Ethers.utils.parseEther("1.0"); - await TruffleAssert.reverts( + const errorValues = await Helpers.expectToRevertWithCustomError( BridgeInstance.deposit( destinationDomainID, erc20ResourceID, @@ -158,11 +159,13 @@ contract("BasicFeeHandler - [collectFee]", async (accounts) => { feeData, { from: depositorAddress, - value: Ethers.utils.parseEther("1.0"), + value: incorrectFee, } ), - "Incorrect fee supplied" + "IncorrectFeeSupplied(uint256)" ); + + assert.equal(errorValues[0].toString(), incorrectFee.toString()); }); it("deposit should pass if valid fee amount supplied for ERC20 deposit", async () => { diff --git a/test/handlers/fee/dynamic/calculateFeeERC20EVM.js b/test/handlers/fee/dynamic/calculateFeeERC20EVM.js index f3fdadb8..0755749a 100644 --- a/test/handlers/fee/dynamic/calculateFeeERC20EVM.js +++ b/test/handlers/fee/dynamic/calculateFeeERC20EVM.js @@ -3,7 +3,6 @@ * SPDX-License-Identifier: LGPL-3.0-only */ -const TruffleAssert = require("truffle-assertions"); const Ethers = require("ethers"); const Helpers = require("../../../helpers"); @@ -231,7 +230,7 @@ contract("DynamicERC20FeeHandlerEVM - [calculateFee]", async (accounts) => { oracle.privateKey, feeDataAmount ) + "11"; - await TruffleAssert.reverts( + const errorValues = await Helpers.expectToRevertWithCustomError( FeeHandlerRouterInstance.calculateFee( sender, originDomainID, @@ -240,8 +239,10 @@ contract("DynamicERC20FeeHandlerEVM - [calculateFee]", async (accounts) => { depositData, feeData ), - "Incorrect feeData length" + "IncorrectFeeDataLength(uint256)" ); + + assert.equal(errorValues[0].toNumber(), feeData.substring(2).length / 2); }); it("should not calculate fee if deposit data differ from fee data", async () => { @@ -269,7 +270,7 @@ contract("DynamicERC20FeeHandlerEVM - [calculateFee]", async (accounts) => { oracle.privateKey, feeDataAmount ); - await TruffleAssert.reverts( + const errorValues = await Helpers.expectToRevertWithCustomError( FeeHandlerRouterInstance.calculateFee( sender, originDomainID, @@ -278,8 +279,12 @@ contract("DynamicERC20FeeHandlerEVM - [calculateFee]", async (accounts) => { depositData, feeData ), - "Incorrect deposit params" + "IncorrectDepositParams(uint8,uint8,bytes32)" ); + + assert.equal(errorValues[0], originDomainID); + assert.equal(errorValues[1], otherDestinationDomainID); + assert.equal(errorValues[2], resourceID); }); it("should not calculate fee if oracle signature is incorrect", async () => { @@ -308,7 +313,7 @@ contract("DynamicERC20FeeHandlerEVM - [calculateFee]", async (accounts) => { oracle2.privateKey, feeDataAmount ); - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( FeeHandlerRouterInstance.calculateFee( sender, originDomainID, @@ -317,7 +322,7 @@ contract("DynamicERC20FeeHandlerEVM - [calculateFee]", async (accounts) => { depositData, feeData ), - "Invalid signature" + "InvalidSignature()" ); }); @@ -347,7 +352,7 @@ contract("DynamicERC20FeeHandlerEVM - [calculateFee]", async (accounts) => { oracle.privateKey, feeDataAmount ); - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( FeeHandlerRouterInstance.calculateFee( sender, originDomainID, @@ -356,7 +361,7 @@ contract("DynamicERC20FeeHandlerEVM - [calculateFee]", async (accounts) => { depositData, feeData ), - "Obsolete oracle data" + "ObsoleteOracleData()" ); }); }); diff --git a/test/handlers/fee/dynamic/calculateFeeERC20Substrate.js b/test/handlers/fee/dynamic/calculateFeeERC20Substrate.js index 634ef518..8d54659a 100644 --- a/test/handlers/fee/dynamic/calculateFeeERC20Substrate.js +++ b/test/handlers/fee/dynamic/calculateFeeERC20Substrate.js @@ -3,7 +3,6 @@ * SPDX-License-Identifier: LGPL-3.0-only */ -const TruffleAssert = require("truffle-assertions"); const Ethers = require("ethers"); const Helpers = require("../../../helpers"); @@ -235,7 +234,7 @@ contract("DynamicERC20FeeHandlerSubstrate - [calculateFee]", async (accounts) => oracle.privateKey, feeDataAmount ) + "11"; - await TruffleAssert.reverts( + const errorValues = await Helpers.expectToRevertWithCustomError( FeeHandlerRouterInstance.calculateFee( sender, originDomainID, @@ -244,8 +243,10 @@ contract("DynamicERC20FeeHandlerSubstrate - [calculateFee]", async (accounts) => depositData, feeData ), - "Incorrect feeData length" + "IncorrectFeeDataLength(uint256)" ); + + assert.equal(errorValues[0].toNumber(), feeData.substring(2).length / 2); }); it("should not calculate fee if deposit data differ from fee data", async () => { @@ -274,7 +275,7 @@ contract("DynamicERC20FeeHandlerSubstrate - [calculateFee]", async (accounts) => oracle.privateKey, feeDataAmount ); - await TruffleAssert.reverts( + const errorValues = await Helpers.expectToRevertWithCustomError( FeeHandlerRouterInstance.calculateFee( sender, originDomainID, @@ -283,8 +284,12 @@ contract("DynamicERC20FeeHandlerSubstrate - [calculateFee]", async (accounts) => depositData, feeData ), - "Incorrect deposit params" + "IncorrectDepositParams(uint8,uint8,bytes32)" ); + + assert.equal(errorValues[0], originDomainID); + assert.equal(errorValues[1], otherDestinationDomainID); + assert.equal(errorValues[2], resourceID); }); it("should not calculate fee if oracle signature is incorrect", async () => { @@ -314,7 +319,7 @@ contract("DynamicERC20FeeHandlerSubstrate - [calculateFee]", async (accounts) => oracle2.privateKey, feeDataAmount ); - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( FeeHandlerRouterInstance.calculateFee( sender, originDomainID, @@ -323,7 +328,7 @@ contract("DynamicERC20FeeHandlerSubstrate - [calculateFee]", async (accounts) => depositData, feeData ), - "Invalid signature" + "InvalidSignature()" ); }); @@ -354,7 +359,7 @@ contract("DynamicERC20FeeHandlerSubstrate - [calculateFee]", async (accounts) => oracle.privateKey, feeDataAmount ); - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( FeeHandlerRouterInstance.calculateFee( sender, originDomainID, @@ -363,7 +368,7 @@ contract("DynamicERC20FeeHandlerSubstrate - [calculateFee]", async (accounts) => depositData, feeData ), - "Obsolete oracle data" + "ObsoleteOracleData()" ); }); }); diff --git a/test/handlers/fee/dynamic/calculateFeeGenericEVM.js b/test/handlers/fee/dynamic/calculateFeeGenericEVM.js index d12b0c3b..585560d0 100644 --- a/test/handlers/fee/dynamic/calculateFeeGenericEVM.js +++ b/test/handlers/fee/dynamic/calculateFeeGenericEVM.js @@ -192,7 +192,7 @@ contract("DynamicGenericFeeHandlerEVM - [calculateFee]", async (accounts) => { feeDataAmount ) + "11"; - await TruffleAssert.reverts( + const errorValues = await Helpers.expectToRevertWithCustomError( FeeHandlerRouterInstance.calculateFee( sender, originDomainID, @@ -201,8 +201,10 @@ contract("DynamicGenericFeeHandlerEVM - [calculateFee]", async (accounts) => { depositData, feeData ), - "Incorrect feeData length" + "IncorrectFeeDataLength(uint256)" ); + + assert.equal(errorValues[0].toNumber(), feeData.substring(2).length / 2); }); it("should not calculate fee if deposit data differ from fee data", async () => { @@ -224,7 +226,7 @@ contract("DynamicGenericFeeHandlerEVM - [calculateFee]", async (accounts) => { oracle.privateKey, feeDataAmount ); - await TruffleAssert.reverts( + const errorValues = await Helpers.expectToRevertWithCustomError( FeeHandlerRouterInstance.calculateFee( sender, originDomainID, @@ -233,8 +235,12 @@ contract("DynamicGenericFeeHandlerEVM - [calculateFee]", async (accounts) => { depositData, feeData ), - "Incorrect deposit params" + "IncorrectDepositParams(uint8,uint8,bytes32)" ); + + assert.equal(errorValues[0], originDomainID); + assert.equal(errorValues[1], otherDestinationDomainID); + assert.equal(errorValues[2], resourceID); }); it("should not calculate fee if oracle signature is incorrect", async () => { @@ -256,7 +262,7 @@ contract("DynamicGenericFeeHandlerEVM - [calculateFee]", async (accounts) => { oracle2.privateKey, feeDataAmount ); - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( FeeHandlerRouterInstance.calculateFee( sender, originDomainID, @@ -265,7 +271,7 @@ contract("DynamicGenericFeeHandlerEVM - [calculateFee]", async (accounts) => { depositData, feeData ), - "Invalid signature" + "InvalidSignature()" ); }); @@ -285,7 +291,7 @@ contract("DynamicGenericFeeHandlerEVM - [calculateFee]", async (accounts) => { oracle.privateKey, feeDataAmount ); - await TruffleAssert.reverts( + await Helpers.expectToRevertWithCustomError( FeeHandlerRouterInstance.calculateFee( sender, originDomainID, @@ -294,7 +300,7 @@ contract("DynamicGenericFeeHandlerEVM - [calculateFee]", async (accounts) => { depositData, feeData ), - "Obsolete oracle data" + "ObsoleteOracleData()" ); }); diff --git a/test/handlers/fee/dynamic/collectFeeGenericEVM.js b/test/handlers/fee/dynamic/collectFeeGenericEVM.js index 03fa0337..aed3efec 100644 --- a/test/handlers/fee/dynamic/collectFeeGenericEVM.js +++ b/test/handlers/fee/dynamic/collectFeeGenericEVM.js @@ -182,7 +182,9 @@ contract("DynamicGenericFeeHandlerEVM - [collectFee]", async (accounts) => { }); it("deposit should revert if invalid fee (msg.value) amount supplied", async () => { - await TruffleAssert.reverts( + const incorrectFee = Ethers.utils.parseEther("1.0"); + + const errorValues = await Helpers.expectToRevertWithCustomError( BridgeInstance.deposit( destinationDomainID, resourceID, @@ -190,11 +192,13 @@ contract("DynamicGenericFeeHandlerEVM - [collectFee]", async (accounts) => { feeData, { from: depositorAddress, - value: Ethers.utils.parseEther("1.0"), + value: incorrectFee, } ), - "Incorrect fee supplied" + "IncorrectFeeSupplied(uint256)" ); + + assert.equal(errorValues[0].toString(), incorrectFee.toString()); }); it("deposit should revert if not called by router on DynamicFeeHandler contract", async () => { diff --git a/test/helpers.js b/test/helpers.js index dae6bb9e..d763e869 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -349,7 +349,7 @@ const createDepositProposalDataFromHandlerResponse = ( // This function packs the parameters together with a fake address and removes the address. // After repacking the data in the handler together with depositorAddress, the offsets will be correct. // Usage: use this function to prepare execution data, -// then pack the result together with executeFunctionSignature, maxFee etc +// then pack the result together with executeFunctionSignature, maxFee etc // (using the createPermissionlessGenericDepositData() helper) // and then pass the data to Bridge.deposit(). const createPermissionlessGenericExecutionData = ( @@ -361,6 +361,38 @@ const createPermissionlessGenericExecutionData = ( return "0x" + abiEncode(types, values).substr(66); }; +// truffle doesn't support decoding custom errors, this is adapted from +// https://github.com/trufflesuite/truffle/issues/4123 +const expectToRevertWithCustomError = async function(promise, expectedErrorSignature) { + try { + await promise; + } catch (error) { + const encoded = web3.eth.abi.encodeFunctionSignature(expectedErrorSignature); + const returnValue = Object.entries(error.data).filter( + (it) => it.length > 1 + ).map( + (it) => it[1] + ).find( + (it) => it != null && it.constructor.name === "Object" && "return" in it + ).return; + // expect event error and provided error signatures to match + assert.equal(returnValue.slice(0, 10), encoded); + + let inputParams; + // match everything between () in function signature + const regex = RegExp(/\(([^)]+)\)/); + if(regex.exec(expectedErrorSignature)) { + const types = regex.exec(expectedErrorSignature)[1].split(","); + inputParams = Ethers.utils.defaultAbiCoder.decode( + types, + Ethers.utils.hexDataSlice(returnValue, 4) + ); + } + return inputParams; + } + assert.fail("Expected an exception but none was received"); +} + module.exports = { advanceBlock, advanceTime, @@ -391,5 +423,6 @@ module.exports = { signTypedProposal, mockSignTypedProposalWithInvalidChainID, createDepositProposalDataFromHandlerResponse, - createPermissionlessGenericExecutionData + createPermissionlessGenericExecutionData, + expectToRevertWithCustomError };