diff --git a/contracts/broker/contracts/src/Broker.sol b/contracts/broker/contracts/src/Broker.sol index 2c8c91f44b..e60b1cf36f 100644 --- a/contracts/broker/contracts/src/Broker.sol +++ b/contracts/broker/contracts/src/Broker.sol @@ -24,10 +24,12 @@ import "@0x/contracts-exchange-forwarder/contracts/src/libs/LibAssetDataTransfer import "@0x/contracts-exchange-libs/contracts/src/LibFillResults.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol"; import "@0x/contracts-utils/contracts/src/LibBytes.sol"; +import "@0x/contracts-utils/contracts/src/LibRichErrors.sol"; import "@0x/contracts-utils/contracts/src/LibSafeMath.sol"; import "@0x/contracts-utils/contracts/src/Refundable.sol"; import "./interfaces/IBroker.sol"; import "./interfaces/IPropertyValidator.sol"; +import "./libs/LibBrokerRichErrors.sol"; // solhint-disable space-after-comma @@ -67,10 +69,22 @@ contract Broker is ) external { - require(from == address(this), "INVALID_FROM_ADDRESS"); - require(amounts.length == 1, "MUST_PROVIDE_ONE_AMOUNT"); + if (from != address(this)) { + LibRichErrors.rrevert( + LibBrokerRichErrors.InvalidFromAddressError(from) + ); + } + if (amounts.length != 1) { + LibRichErrors.rrevert( + LibBrokerRichErrors.AmountsLengthMustEqualOneError(amounts.length) + ); + } uint256 remainingAmount = amounts[0]; - require(_cachedAssetData.length.safeSub(_cacheIndex) >= remainingAmount, "TOO_FEW_BROKERED_ASSETS_PROVIDED"); + if (_cachedAssetData.length.safeSub(_cacheIndex) < remainingAmount) { + LibRichErrors.rrevert( + LibBrokerRichErrors.TooFewBrokerAssetsProvidedError(_cachedAssetData.length) + ); + } while (remainingAmount != 0) { bytes memory assetToTransfer = _cachedAssetData[_cacheIndex]; @@ -125,11 +139,13 @@ contract Broker is _sender = msg.sender; // Sanity-check the provided function selector - require( - fillFunctionSelector == IExchange(address(0)).fillOrder.selector || - fillFunctionSelector == IExchange(address(0)).fillOrKillOrder.selector, - "UNRECOGNIZED_FUNCTION_SELECTOR" - ); + if ( + fillFunctionSelector != IExchange(address(0)).fillOrder.selector && + fillFunctionSelector != IExchange(address(0)).fillOrKillOrder.selector + ) { + LibBrokerRichErrors.InvalidFunctionSelectorError(fillFunctionSelector); + } + // Perform the fill bytes memory fillCalldata = abi.encodeWithSelector( @@ -177,12 +193,13 @@ contract Broker is _sender = msg.sender; // Sanity-check the provided function selector - require( - batchFillFunctionSelector == IExchange(address(0)).batchFillOrders.selector || - batchFillFunctionSelector == IExchange(address(0)).batchFillOrKillOrders.selector || - batchFillFunctionSelector == IExchange(address(0)).batchFillOrdersNoThrow.selector, - "UNRECOGNIZED_FUNCTION_SELECTOR" - ); + if ( + batchFillFunctionSelector != IExchange(address(0)).batchFillOrders.selector && + batchFillFunctionSelector != IExchange(address(0)).batchFillOrKillOrders.selector && + batchFillFunctionSelector != IExchange(address(0)).batchFillOrdersNoThrow.selector + ) { + LibBrokerRichErrors.InvalidFunctionSelectorError(batchFillFunctionSelector); + } // Perform the batch fill bytes memory batchFillCalldata = abi.encodeWithSelector( diff --git a/contracts/broker/contracts/src/libs/LibBrokerRichErrors.sol b/contracts/broker/contracts/src/libs/LibBrokerRichErrors.sol new file mode 100644 index 0000000000..fa2c9322d3 --- /dev/null +++ b/contracts/broker/contracts/src/libs/LibBrokerRichErrors.sol @@ -0,0 +1,93 @@ +/* + + Copyright 2019 ZeroEx Intl. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +*/ + +pragma solidity ^0.5.9; + + +library LibBrokerRichErrors { + + + // bytes4(keccak256("InvalidFromAddressError(address)")) + bytes4 internal constant INVALID_FROM_ADDRESS_ERROR_SELECTOR = + 0x906bfb3c; + + // bytes4(keccak256("AmountsLengthMustEqualOneError(uint256)")) + bytes4 internal constant AMOUNTS_LENGTH_MUST_EQUAL_ONE_ERROR_SELECTOR = + 0xba9be200; + + // bytes4(keccak256("TooFewBrokerAssetsProvidedError(uint256)")) + bytes4 internal constant TOO_FEW_BROKER_ASSETS_PROVIDED_ERROR_SELECTOR = + 0x55272586; + + // bytes4(keccak256("InvalidFunctionSelectorError(bytes4)")) + bytes4 internal constant INVALID_FUNCTION_SELECTOR_ERROR_SELECTOR = + 0x540943f1; + + // solhint-disable func-name-mixedcase + function InvalidFromAddressError( + address from + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + INVALID_FROM_ADDRESS_ERROR_SELECTOR, + from + ); + } + + function AmountsLengthMustEqualOneError( + uint256 amountsLength + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + AMOUNTS_LENGTH_MUST_EQUAL_ONE_ERROR_SELECTOR, + amountsLength + ); + } + + function TooFewBrokerAssetsProvidedError( + uint256 numBrokeredAssets + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + TOO_FEW_BROKER_ASSETS_PROVIDED_ERROR_SELECTOR, + numBrokeredAssets + ); + } + + function InvalidFunctionSelectorError( + bytes4 selector + ) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector( + INVALID_FUNCTION_SELECTOR_ERROR_SELECTOR, + selector + ); + } +} diff --git a/contracts/broker/package.json b/contracts/broker/package.json index c6767bc77c..90dcd43171 100644 --- a/contracts/broker/package.json +++ b/contracts/broker/package.json @@ -39,7 +39,7 @@ }, "config": { "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually.", - "abis": "./test/generated-artifacts/@(Broker|GodsUnchainedValidator|IBroker|IGodsUnchained|IPropertyValidator|TestGodsUnchained).json" + "abis": "./test/generated-artifacts/@(Broker|GodsUnchainedValidator|IBroker|IGodsUnchained|IPropertyValidator|LibBrokerRichErrors|TestGodsUnchained).json" }, "repository": { "type": "git", diff --git a/contracts/broker/src/artifacts.ts b/contracts/broker/src/artifacts.ts index 2df8d4bdaa..3bb32b942c 100644 --- a/contracts/broker/src/artifacts.ts +++ b/contracts/broker/src/artifacts.ts @@ -10,12 +10,14 @@ import * as GodsUnchainedValidator from '../generated-artifacts/GodsUnchainedVal import * as IBroker from '../generated-artifacts/IBroker.json'; import * as IGodsUnchained from '../generated-artifacts/IGodsUnchained.json'; import * as IPropertyValidator from '../generated-artifacts/IPropertyValidator.json'; +import * as LibBrokerRichErrors from '../generated-artifacts/LibBrokerRichErrors.json'; import * as TestGodsUnchained from '../generated-artifacts/TestGodsUnchained.json'; export const artifacts = { Broker: Broker as ContractArtifact, IBroker: IBroker as ContractArtifact, IGodsUnchained: IGodsUnchained as ContractArtifact, IPropertyValidator: IPropertyValidator as ContractArtifact, + LibBrokerRichErrors: LibBrokerRichErrors as ContractArtifact, GodsUnchainedValidator: GodsUnchainedValidator as ContractArtifact, TestGodsUnchained: TestGodsUnchained as ContractArtifact, }; diff --git a/contracts/broker/src/index.ts b/contracts/broker/src/index.ts index fa32cb41c0..74006467cc 100644 --- a/contracts/broker/src/index.ts +++ b/contracts/broker/src/index.ts @@ -1,3 +1,32 @@ export { artifacts } from './artifacts'; export { BrokerContract, GodsUnchainedValidatorContract, TestGodsUnchainedContract } from './wrappers'; export { godsUnchainedUtils } from './gods_unchained_utils'; +export { BrokerRevertErrors } from '@0x/utils'; +export { + ContractArtifact, + ContractChains, + CompilerOpts, + StandardContractOutput, + CompilerSettings, + ContractChainData, + ContractAbi, + DevdocOutput, + EvmOutput, + CompilerSettingsMetadata, + OptimizerSettings, + OutputField, + ParamDescription, + EvmBytecodeOutput, + AbiDefinition, + FunctionAbi, + EventAbi, + RevertErrorAbi, + EventParameter, + DataItem, + MethodAbi, + ConstructorAbi, + FallbackAbi, + ConstructorStateMutability, + TupleDataItem, + StateMutability, +} from 'ethereum-types'; diff --git a/contracts/broker/src/wrappers.ts b/contracts/broker/src/wrappers.ts index 0ece6fecb4..c231b37fbf 100644 --- a/contracts/broker/src/wrappers.ts +++ b/contracts/broker/src/wrappers.ts @@ -8,4 +8,5 @@ export * from '../generated-wrappers/gods_unchained_validator'; export * from '../generated-wrappers/i_broker'; export * from '../generated-wrappers/i_gods_unchained'; export * from '../generated-wrappers/i_property_validator'; +export * from '../generated-wrappers/lib_broker_rich_errors'; export * from '../generated-wrappers/test_gods_unchained'; diff --git a/contracts/broker/test/artifacts.ts b/contracts/broker/test/artifacts.ts index 9d1b49d8e0..6db14b6c9a 100644 --- a/contracts/broker/test/artifacts.ts +++ b/contracts/broker/test/artifacts.ts @@ -10,12 +10,14 @@ import * as GodsUnchainedValidator from '../test/generated-artifacts/GodsUnchain import * as IBroker from '../test/generated-artifacts/IBroker.json'; import * as IGodsUnchained from '../test/generated-artifacts/IGodsUnchained.json'; import * as IPropertyValidator from '../test/generated-artifacts/IPropertyValidator.json'; +import * as LibBrokerRichErrors from '../test/generated-artifacts/LibBrokerRichErrors.json'; import * as TestGodsUnchained from '../test/generated-artifacts/TestGodsUnchained.json'; export const artifacts = { Broker: Broker as ContractArtifact, IBroker: IBroker as ContractArtifact, IGodsUnchained: IGodsUnchained as ContractArtifact, IPropertyValidator: IPropertyValidator as ContractArtifact, + LibBrokerRichErrors: LibBrokerRichErrors as ContractArtifact, GodsUnchainedValidator: GodsUnchainedValidator as ContractArtifact, TestGodsUnchained: TestGodsUnchained as ContractArtifact, }; diff --git a/contracts/broker/test/wrappers.ts b/contracts/broker/test/wrappers.ts index 56e4f9b8cf..a05b726463 100644 --- a/contracts/broker/test/wrappers.ts +++ b/contracts/broker/test/wrappers.ts @@ -8,4 +8,5 @@ export * from '../test/generated-wrappers/gods_unchained_validator'; export * from '../test/generated-wrappers/i_broker'; export * from '../test/generated-wrappers/i_gods_unchained'; export * from '../test/generated-wrappers/i_property_validator'; +export * from '../test/generated-wrappers/lib_broker_rich_errors'; export * from '../test/generated-wrappers/test_gods_unchained'; diff --git a/contracts/broker/tsconfig.json b/contracts/broker/tsconfig.json index 6b6d69640c..849374d53a 100644 --- a/contracts/broker/tsconfig.json +++ b/contracts/broker/tsconfig.json @@ -8,12 +8,14 @@ "generated-artifacts/IBroker.json", "generated-artifacts/IGodsUnchained.json", "generated-artifacts/IPropertyValidator.json", + "generated-artifacts/LibBrokerRichErrors.json", "generated-artifacts/TestGodsUnchained.json", "test/generated-artifacts/Broker.json", "test/generated-artifacts/GodsUnchainedValidator.json", "test/generated-artifacts/IBroker.json", "test/generated-artifacts/IGodsUnchained.json", "test/generated-artifacts/IPropertyValidator.json", + "test/generated-artifacts/LibBrokerRichErrors.json", "test/generated-artifacts/TestGodsUnchained.json" ], "exclude": ["./deploy/solc/solc_bin"] diff --git a/packages/utils/CHANGELOG.json b/packages/utils/CHANGELOG.json index 0947fac776..7e9aa09d5d 100644 --- a/packages/utils/CHANGELOG.json +++ b/packages/utils/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "5.3.0", + "changes": [ + { + "note": "Added Broker revert errors", + "pr": 2455 + } + ] + }, { "version": "5.2.0", "changes": [ diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index fedcacbdd7..e3cc79e78f 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -27,6 +27,7 @@ export { AnyRevertError, } from './revert_error'; +export import BrokerRevertErrors = require('./revert_errors/broker/revert_errors'); export import CoordinatorRevertErrors = require('./revert_errors/coordinator/revert_errors'); export import ExchangeForwarderRevertErrors = require('./revert_errors/exchange-forwarder/revert_errors'); export import LibMathRevertErrors = require('./revert_errors/exchange-libs/lib_math_revert_errors'); diff --git a/packages/utils/src/revert_errors/broker/revert_errors.ts b/packages/utils/src/revert_errors/broker/revert_errors.ts new file mode 100644 index 0000000000..e551454858 --- /dev/null +++ b/packages/utils/src/revert_errors/broker/revert_errors.ts @@ -0,0 +1,44 @@ +import { BigNumber } from '../../configured_bignumber'; +import { RevertError } from '../../revert_error'; + +// tslint:disable:max-classes-per-file + +export class InvalidFromAddressError extends RevertError { + constructor(from?: string) { + super('InvalidFromAddressError', 'InvalidFromAddressError(address from)', { from }); + } +} + +export class AmountsLengthMustEqualOneError extends RevertError { + constructor(amountsLength?: BigNumber | number | string) { + super('AmountsLengthMustEqualOneError', 'AmountsLengthMustEqualOneError(uint256 amountsLength)', { + amountsLength, + }); + } +} + +export class TooFewBrokerAssetsProvidedError extends RevertError { + constructor(numBrokeredAssets?: BigNumber | number | string) { + super('TooFewBrokerAssetsProvidedError', 'TooFewBrokerAssetsProvidedError(uint256 numBrokeredAssets)', { + numBrokeredAssets, + }); + } +} + +export class InvalidFunctionSelectorError extends RevertError { + constructor(selector?: string) { + super('InvalidFunctionSelectorError', 'InvalidFunctionSelectorError(bytes4 selector)', { selector }); + } +} + +const types = [ + InvalidFromAddressError, + AmountsLengthMustEqualOneError, + TooFewBrokerAssetsProvidedError, + InvalidFunctionSelectorError, +]; + +// Register the types we've defined. +for (const type of types) { + RevertError.registerType(type); +}