From eaabe1586374a25439c880ada348d87a5b3df740 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 21 Jun 2018 16:53:05 +0200 Subject: Update core tests to actually check revert message --- packages/contracts/src/utils/assertions.ts | 12 +++++++++++ packages/contracts/src/utils/types.ts | 32 ++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/assertions.ts b/packages/contracts/src/utils/assertions.ts index 615e648f3..29489e648 100644 --- a/packages/contracts/src/utils/assertions.ts +++ b/packages/contracts/src/utils/assertions.ts @@ -51,6 +51,18 @@ export function expectRevertOrAlwaysFailingTransactionAsync(p: Promise): P return expectRevertOrOtherErrorAsync(p, 'always failing transaction'); } +/** + * Rejects if the given Promise does not reject with the given revert reason or "always + * failing transaction" error. + * @param p the Promise which is expected to reject + * @param reason a specific revert reason + * @returns a new Promise which will reject if the conditions are not met and + * otherwise resolve with no value. + */ +export function expectRevertReasonOrAlwaysFailingTransactionAsync(p: Promise, reason: string): PromiseLike { + return _expectEitherErrorAsync(p, 'always failing transaction', reason); +} + /** * Rejects if the given Promise does not reject with a "revert" or "Contract * call failed" error. diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index bb8c12088..da8ea588f 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -150,3 +150,35 @@ export interface MatchOrder { leftSignature: string; rightSignature: string; } + +export enum ContractLibErrors { + OrderUnfillable = 'ORDER_UNFILLABLE', + InvalidMaker = 'INVALID_MAKER', + InvalidTaker = 'INVALID_TAKER', + InvalidSender = 'INVALID_SENDER', + InvalidOrderSignature = 'INVALID_ORDER_SIGNATURE', + InvalidTakerAmount = 'INVALID_TAKER_AMOUNT', + RoundingError = 'ROUNDING_ERROR', + InvalidSignature = 'INVALID_SIGNATURE', + SignatureIllegal = 'SIGNATURE_ILLEGAL', + SignatureUnsupported = 'SIGNATURE_UNSUPPORTED', + InvalidNewOrderEpoch = 'INVALID_NEW_ORDER_EPOCH', + CompleteFillFailed = 'COMPLETE_FILL_FAILED', + NegativeSpreadRequired = 'NEGATIVE_SPREAD_REQUIRED', + ReentrancyIllegal = 'REENTRANCY_ILLEGAL', + InvalidTxHash = 'INVALID_TX_HASH', + InvalidTxSignature = 'INVALID_TX_SIGNATURE', + FailedExecution = 'FAILED_EXECUTION', + AssetProxyMismatch = 'ASSET_PROXY_MISMATCH', + AssetProxyIdMismatch = 'ASSET_PROXY_ID_MISMATCH', + LengthGreaterThan0Required = 'LENGTH_GREATER_THAN_0_REQUIRED', + Length1Required = 'LENGTH_1_REQUIRED', + Length66Required = 'LENGTH_66_REQUIRED', + InvalidAmount = 'INVALID_AMOUNT', + TransferFailed = 'TRANSFER_FAILED', + SenderNotAuthorized = 'SENDER_NOT_AUTHORIZED', + TargetNotAuthorized = 'TARGET_NOT_AUTHORIZED', + TargetAlreadyAuthorized = 'TARGET_ALREADY_AUTHORIZED', + IndexOutOfBounds = 'INDEX_OUT_OF_BOUNDS', + AuthorizedAddressMismatch = 'AUTHORIZED_ADDRESS_MISMATCH', +} -- cgit v1.2.3 From 7869c19245c4f5c69bda8726b66df405aca8754c Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 21 Jun 2018 17:23:42 +0200 Subject: Change revert reason in ownable to be similar to all other revert reasons --- packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol b/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol index 296c6c856..048fdb46f 100644 --- a/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol +++ b/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol @@ -22,7 +22,7 @@ contract Ownable is IOwnable { modifier onlyOwner() { require( msg.sender == owner, - "Only contract owner is allowed to call this method." + 'ONLY_CONTRACT_OWNER' ); _; } -- cgit v1.2.3 From ada5428df7f056293d826d28362f8be08874771c Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 21 Jun 2018 17:23:59 +0200 Subject: Check revert reasons in Authorizable tests --- packages/contracts/src/utils/types.ts | 1 + 1 file changed, 1 insertion(+) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index da8ea588f..db590213c 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -181,4 +181,5 @@ export enum ContractLibErrors { TargetAlreadyAuthorized = 'TARGET_ALREADY_AUTHORIZED', IndexOutOfBounds = 'INDEX_OUT_OF_BOUNDS', AuthorizedAddressMismatch = 'AUTHORIZED_ADDRESS_MISMATCH', + OnlyContractOwner = 'ONLY_CONTRACT_OWNER', } -- cgit v1.2.3 From 914b00936199526f67398d5b9823f893f1773cdd Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Fri, 22 Jun 2018 17:12:17 +0200 Subject: Change Whitelist error messages to conform to rest and added revert reason checks to transactions tests --- .../src/contracts/current/test/Whitelist/Whitelist.sol | 10 +++++----- packages/contracts/src/utils/types.ts | 2 ++ 2 files changed, 7 insertions(+), 5 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol index d35815474..8b52858b1 100644 --- a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol +++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol @@ -23,13 +23,13 @@ import "../../protocol/Exchange/interfaces/IExchange.sol"; import "../../protocol/Exchange/libs/LibOrder.sol"; import "../../utils/Ownable/Ownable.sol"; -contract Whitelist is +contract Whitelist is Ownable { // Revert reasons - string constant MAKER_NOT_WHITELISTED = "Maker address not whitelisted."; - string constant TAKER_NOT_WHITELISTED = "Taker address not whitelisted."; - string constant INVALID_SENDER = "Sender must equal transaction origin."; + string constant MAKER_NOT_WHITELISTED = "MAKER_NOT_WHITELISTED"; // Maker address not whitelisted. + string constant TAKER_NOT_WHITELISTED = "TAKER_NOT_WHITELISTED"; // Taker address not whitelisted. + string constant INVALID_SENDER = "INVALID_SENDER"; // Sender must equal transaction origin. // Mapping of address => whitelist status. mapping (address => bool) public isWhitelisted; @@ -77,7 +77,7 @@ contract Whitelist is public { address takerAddress = msg.sender; - + // This contract must be the entry point for the transaction. require( takerAddress == tx.origin, diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index 24183b549..c1eb72099 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -183,4 +183,6 @@ export enum ContractLibErrors { IndexOutOfBounds = 'INDEX_OUT_OF_BOUNDS', AuthorizedAddressMismatch = 'AUTHORIZED_ADDRESS_MISMATCH', OnlyContractOwner = 'ONLY_CONTRACT_OWNER', + MakerNotWhitelisted = 'MAKER_NOT_WHITELISTED', + TakerNotWhitelisted = 'TAKER_NOT_WHITELISTED', } -- cgit v1.2.3 From 59d3a219937a80fa6f8d6096e9a54534103c0cd7 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Fri, 22 Jun 2018 17:39:41 +0200 Subject: Fix test now that contract reverts with message --- packages/contracts/src/utils/types.ts | 1 + 1 file changed, 1 insertion(+) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index c1eb72099..43c9109e7 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -185,4 +185,5 @@ export enum ContractLibErrors { OnlyContractOwner = 'ONLY_CONTRACT_OWNER', MakerNotWhitelisted = 'MAKER_NOT_WHITELISTED', TakerNotWhitelisted = 'TAKER_NOT_WHITELISTED', + AssetProxyDoesNotExist = 'ASSET_PROXY_DOES_NOT_EXIST', } -- cgit v1.2.3 From ba14850c9a3f774169db1b67446618bb84daae54 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Fri, 22 Jun 2018 18:35:35 +0200 Subject: Standardize ERC20 error strings given convention --- .../contracts/src/contracts/current/tokens/ERC20Token/ERC20Token.sol | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/tokens/ERC20Token/ERC20Token.sol b/packages/contracts/src/contracts/current/tokens/ERC20Token/ERC20Token.sol index f0bcdafef..b6961a6ec 100644 --- a/packages/contracts/src/contracts/current/tokens/ERC20Token/ERC20Token.sol +++ b/packages/contracts/src/contracts/current/tokens/ERC20Token/ERC20Token.sol @@ -23,8 +23,8 @@ import "./IERC20Token.sol"; contract ERC20Token is IERC20Token { - string constant INSUFFICIENT_BALANCE = "Insufficient balance to complete transfer."; - string constant INSUFFICIENT_ALLOWANCE = "Insufficient allowance to complete transfer."; + string constant INSUFFICIENT_BALANCE = "ERC20_INSUFFICIENT_BALANCE"; + string constant INSUFFICIENT_ALLOWANCE = "ERC20_INSUFFICIENT_ALLOWANCE"; string constant OVERFLOW = "Transfer would result in an overflow."; mapping (address => uint256) balances; @@ -97,4 +97,3 @@ contract ERC20Token is IERC20Token { return allowed[_owner][_spender]; } } - -- cgit v1.2.3 From 0e7c254b938045f0c6e0a57c78bb1db8a23557b6 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Fri, 22 Jun 2018 18:36:15 +0200 Subject: Move constants over to ContractLibError enum and update all tests --- packages/contracts/src/utils/constants.ts | 13 ------------- packages/contracts/src/utils/types.ts | 12 ++++++++++-- 2 files changed, 10 insertions(+), 15 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts index f21b8c7a0..2b2bd2425 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -19,19 +19,6 @@ const TESTRPC_PRIVATE_KEYS_STRINGS = [ export const constants = { INVALID_OPCODE: 'invalid opcode', REVERT: 'revert', - LIB_BYTES_GREATER_THAN_ZERO_LENGTH_REQUIRED: 'GREATER_THAN_ZERO_LENGTH_REQUIRED', - LIB_BYTES_GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED', - LIB_BYTES_GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED', - LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED', - LIB_BYTES_GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED', - LIB_BYTES_GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED', - ERC20_INSUFFICIENT_BALANCE: 'Insufficient balance to complete transfer.', - ERC20_INSUFFICIENT_ALLOWANCE: 'Insufficient allowance to complete transfer.', - EXCHANGE_LENGTH_GREATER_THAN_0_REQUIRED: 'LENGTH_GREATER_THAN_0_REQUIRED', - EXCHANGE_SIGNATURE_UNSUPPORTED: 'SIGNATURE_UNSUPPORTED', - EXCHANGE_SIGNATURE_ILLEGAL: 'SIGNATURE_ILLEGAL', - EXCHANGE_LENGTH_0_REQUIRED: 'LENGTH_0_REQUIRED', - EXCHANGE_LENGTH_65_REQUIRED: 'LENGTH_65_REQUIRED', TESTRPC_NETWORK_ID: 50, // Note(albrow): In practice V8 and most other engines limit the minimum // interval for setInterval to 10ms. We still set it to 0 here in order to diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index 43c9109e7..f65a9858b 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -173,8 +173,8 @@ export enum ContractLibErrors { AssetProxyMismatch = 'ASSET_PROXY_MISMATCH', AssetProxyIdMismatch = 'ASSET_PROXY_ID_MISMATCH', LengthGreaterThan0Required = 'LENGTH_GREATER_THAN_0_REQUIRED', - Length1Required = 'LENGTH_1_REQUIRED', - Length66Required = 'LENGTH_66_REQUIRED', + ExchangeLength0Required = 'LENGTH_0_REQUIRED', + ExchangeLength65Required = 'LENGTH_65_REQUIRED', InvalidAmount = 'INVALID_AMOUNT', TransferFailed = 'TRANSFER_FAILED', SenderNotAuthorized = 'SENDER_NOT_AUTHORIZED', @@ -186,4 +186,12 @@ export enum ContractLibErrors { MakerNotWhitelisted = 'MAKER_NOT_WHITELISTED', TakerNotWhitelisted = 'TAKER_NOT_WHITELISTED', AssetProxyDoesNotExist = 'ASSET_PROXY_DOES_NOT_EXIST', + LibBytesGreaterThanZeroLengthRequired = 'GREATER_THAN_ZERO_LENGTH_REQUIRED', + LibBytesGreaterOrEqualTo4LengthRequired = 'GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED', + LibBytesGreaterOrEqualTo20LengthRequired = 'GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED', + LibBytesGreaterOrEqualTo32LengthRequired = 'GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED', + LibBytesGreaterOrEqualToNestedBytesLengthRequired = 'GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED', + LibBytesGreaterOrEqualToSourceBytesLengthRequired = 'GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED', + Erc20InsufficientBalance = 'ERC20_INSUFFICIENT_BALANCE', + Erc20InsufficientAllowance = 'ERC20_INSUFFICIENT_ALLOWANCE', } -- cgit v1.2.3 From 7a216901bec6f7f3640d41fb90dcc49ed673cc6f Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Fri, 22 Jun 2018 18:41:55 +0200 Subject: Remove revert reason 'Exchange' prefix --- packages/contracts/src/utils/types.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index f65a9858b..b3da040c3 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -173,8 +173,8 @@ export enum ContractLibErrors { AssetProxyMismatch = 'ASSET_PROXY_MISMATCH', AssetProxyIdMismatch = 'ASSET_PROXY_ID_MISMATCH', LengthGreaterThan0Required = 'LENGTH_GREATER_THAN_0_REQUIRED', - ExchangeLength0Required = 'LENGTH_0_REQUIRED', - ExchangeLength65Required = 'LENGTH_65_REQUIRED', + Length0Required = 'LENGTH_0_REQUIRED', + Length65Required = 'LENGTH_65_REQUIRED', InvalidAmount = 'INVALID_AMOUNT', TransferFailed = 'TRANSFER_FAILED', SenderNotAuthorized = 'SENDER_NOT_AUTHORIZED', -- cgit v1.2.3 From 4409f11b24324e23ee2f53436c0226028820e96d Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Fri, 22 Jun 2018 18:45:45 +0200 Subject: Rename ContractLibErrors to RevertReasons --- packages/contracts/src/utils/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index b3da040c3..03eb5c9b2 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -152,7 +152,7 @@ export interface MatchOrder { rightSignature: string; } -export enum ContractLibErrors { +export enum RevertReasons { OrderUnfillable = 'ORDER_UNFILLABLE', InvalidMaker = 'INVALID_MAKER', InvalidTaker = 'INVALID_TAKER', -- cgit v1.2.3