update some RichErrors and @return directives per comments

This commit is contained in:
Michael Zhu
2019-08-20 16:22:11 -07:00
parent 3c169388e2
commit 9f4fe259f9
8 changed files with 134 additions and 77 deletions

View File

@@ -109,13 +109,13 @@ contract MixinAssets is
// Transfer tokens.
// We do a raw call so we can check the success separate
// from the return data.
(bool success,) = token.call(abi.encodeWithSelector(
(bool success, bytes memory returnData) = token.call(abi.encodeWithSelector(
ERC20_TRANSFER_SELECTOR,
msg.sender,
amount
));
if (!success) {
LibRichErrors.rrevert(LibForwarderRichErrors.TransferFailedError());
LibRichErrors.rrevert(LibForwarderRichErrors.TransferFailedError(returnData));
}
// Check return data.
@@ -135,7 +135,7 @@ contract MixinAssets is
}
}
if (!success) {
LibRichErrors.rrevert(LibForwarderRichErrors.TransferFailedError());
LibRichErrors.rrevert(LibForwarderRichErrors.TransferFailedError(returnData));
}
}
@@ -149,7 +149,7 @@ contract MixinAssets is
internal
{
if (amount != 1) {
LibRichErrors.rrevert(LibForwarderRichErrors.InvalidErc721AmountError(
LibRichErrors.rrevert(LibForwarderRichErrors.Erc721AmountMustEqualOneError(
amount
));
}

View File

@@ -85,7 +85,8 @@ contract MixinExchangeWrapper is
/// @param order A single order specification.
/// @param signature Signature for the given order.
/// @param remainingTakerAssetFillAmount Remaining amount of WETH to sell.
/// @return Amounts of WETH spent and makerAsset acquired by the taker for this order.
/// @return wethSpentAmount Amount of WETH spent on the given order.
/// @return makerAssetAcquiredAmount Amount of maker asset acquired from the given order.
function _marketSellSingleOrder(
LibOrder.Order memory order,
bytes memory signature,
@@ -100,7 +101,7 @@ contract MixinExchangeWrapper is
// No fee or percentage fee
if (order.takerFee == 0 || order.makerAssetData.equals(order.takerFeeAssetData)) {
// Attempt to sell the remaining amount of WETH
singleFillResults = _fillOrderNoThrow(
LibFillResults.FillResults memory singleFillResults = _fillOrderNoThrow(
order,
remainingTakerAssetFillAmount,
signature
@@ -122,7 +123,7 @@ contract MixinExchangeWrapper is
remainingTakerAssetFillAmount
);
singleFillResults = _fillOrderNoThrow(
LibFillResults.FillResults memory singleFillResults = _fillOrderNoThrow(
order,
takerAssetFillAmount,
signature
@@ -144,7 +145,8 @@ contract MixinExchangeWrapper is
/// @param orders Array of order specifications.
/// @param wethSellAmount Desired amount of WETH to sell.
/// @param signatures Proofs that orders have been signed by makers.
/// @return Amounts of WETH spent and makerAsset acquired by the taker.
/// @return totalWethSpentAmount Total amount of WETH spent on the given orders.
/// @return totalMakerAssetAcquiredAmount Total amount of maker asset acquired from the given orders.
function _marketSellWeth(
LibOrder.Order[] memory orders,
uint256 wethSellAmount,
@@ -172,7 +174,7 @@ contract MixinExchangeWrapper is
}
// The remaining amount of WETH to sell
uint256 remainingTakerAssetFillAmount = wethSellAmount.safeSub(wethSpentAmount);
uint256 remainingTakerAssetFillAmount = wethSellAmount.safeSub(totalWethSpentAmount);
(
uint256 wethSpentAmount,
@@ -198,7 +200,8 @@ contract MixinExchangeWrapper is
/// @param order A single order specification.
/// @param signature Signature for the given order.
/// @param remainingMakerAssetFillAmount Remaining amount of maker asset to buy.
/// @return Amounts of WETH spent and makerAsset acquired by the taker for this order.
/// @return wethSpentAmount Amount of WETH spent on the given order.
/// @return makerAssetAcquiredAmount Amount of maker asset acquired from the given order.
function _marketBuySingleOrder(
LibOrder.Order memory order,
bytes memory signature,
@@ -210,9 +213,6 @@ contract MixinExchangeWrapper is
uint256 makerAssetAcquiredAmount
)
{
// The remaining amount of maker asset to buy
uint256 remainingMakerAssetFillAmount = makerAssetBuyAmount.safeSub(totalMakerAssetAcquiredAmount);
// No fee or WETH fee
if (order.takerFee == 0 || order.takerFeeAssetData.equals(order.takerAssetData)) {
// Calculate the remaining amount of takerAsset to sell
@@ -223,7 +223,7 @@ contract MixinExchangeWrapper is
);
// Attempt to sell the remaining amount of takerAsset
singleFillResults = _fillOrderNoThrow(
LibFillResults.FillResults memory singleFillResults = _fillOrderNoThrow(
order,
remainingTakerAssetFillAmount,
signature
@@ -245,7 +245,7 @@ contract MixinExchangeWrapper is
);
// Attempt to sell the remaining amount of takerAsset
singleFillResults = _fillOrderNoThrow(
LibFillResults.FillResults memory singleFillResults = _fillOrderNoThrow(
order,
remainingTakerAssetFillAmount,
signature
@@ -270,7 +270,8 @@ contract MixinExchangeWrapper is
/// @param orders Array of order specifications.
/// @param makerAssetBuyAmount Desired amount of makerAsset to fill.
/// @param signatures Proofs that orders have been signed by makers.
/// @return Amounts of WETH spent and makerAsset acquired by the taker.
/// @return totalWethSpentAmount Total amount of WETH spent on the given orders.
/// @return totalMakerAssetAcquiredAmount Total amount of maker asset acquired from the given orders.
function _marketBuyExactAmountWithWeth(
LibOrder.Order[] memory orders,
uint256 makerAssetBuyAmount,
@@ -317,7 +318,10 @@ contract MixinExchangeWrapper is
}
if (totalMakerAssetAcquiredAmount < makerAssetBuyAmount) {
LibRichErrors.rrevert(LibForwarderRichErrors.CompleteFillFailedError());
LibRichErrors.rrevert(LibForwarderRichErrors.CompleteBuyFailedError(
makerAssetBuyAmount,
totalMakerAssetAcquiredAmount
));
}
}
}

View File

@@ -61,7 +61,9 @@ contract MixinForwarderCore is
/// @param signatures Proofs that orders have been created by makers.
/// @param feePercentage Percentage of WETH sold that will payed as fee to forwarding contract feeRecipient.
/// @param feeRecipient Address that will receive ETH when orders are filled.
/// @return Amounts of WETH spent, makerAsset acquired, and forwarder fees paid for the given set of orders.
/// @return wethSpentAmount Amount of WETH spent on the given set of orders.
/// @return makerAssetAcquiredAmount Amount of maker asset acquired from the given set of orders.
/// @return ethFeePaid Amount of ETH spent on the given forwarder fee.
function marketSellOrdersWithEth(
LibOrder.Order[] memory orders,
bytes[] memory signatures,
@@ -120,7 +122,9 @@ contract MixinForwarderCore is
/// @param signatures Proofs that orders have been created by makers.
/// @param feePercentage Percentage of WETH sold that will payed as fee to forwarding contract feeRecipient.
/// @param feeRecipient Address that will receive ETH when orders are filled.
/// @return Amounts of WETH spent, makerAsset acquired, and forwarder fees paid for the given set of orders.
/// @return wethSpentAmount Amount of WETH spent on the given set of orders.
/// @return makerAssetAcquiredAmount Amount of maker asset acquired from the given set of orders.
/// @return ethFeePaid Amount of ETH spent on the given forwarder fee.
function marketBuyOrdersWithEth(
LibOrder.Order[] memory orders,
uint256 makerAssetBuyAmount,

View File

@@ -47,7 +47,7 @@ contract MixinWeth is
internal
{
if (msg.value == 0) {
LibRichErrors.rrevert(LibForwarderRichErrors.InvalidMsgValueError());
LibRichErrors.rrevert(LibForwarderRichErrors.MsgValueCantEqualZeroError());
}
ETHER_TOKEN.deposit.value(msg.value)();
}
@@ -57,7 +57,7 @@ contract MixinWeth is
/// @param wethSold Amount of WETH sold when filling primary orders.
/// @param feePercentage Percentage of WETH sold that will payed as fee to forwarding contract feeRecipient.
/// @param feeRecipient Address that will receive ETH when orders are filled.
/// @return Amount paid to feeRecipient as a percentage fee on the total WETH sold.
/// @return ethFee Amount paid to feeRecipient as a percentage fee on the total WETH sold.
function _transferEthFeeAndRefund(
uint256 wethSold,
uint256 feePercentage,
@@ -93,7 +93,10 @@ contract MixinWeth is
// Ensure fee is less than amount of WETH remaining.
if (ethFee > wethRemaining) {
LibRichErrors.rrevert(LibForwarderRichErrors.InsufficientEthForFeeError());
LibRichErrors.rrevert(LibForwarderRichErrors.InsufficientEthForFeeError(
ethFee,
wethRemaining
));
}
// Do nothing if no WETH remaining

View File

@@ -31,7 +31,9 @@ contract IForwarderCore {
/// @param signatures Proofs that orders have been created by makers.
/// @param feePercentage Percentage of WETH sold that will payed as fee to forwarding contract feeRecipient.
/// @param feeRecipient Address that will receive ETH when orders are filled.
/// @return Amounts of WETH spent, makerAsset acquired, and forwarder fees paid for the given set of orders.
/// @return wethSpentAmount Amount of WETH spent on the given set of orders.
/// @return makerAssetAcquiredAmount Amount of maker asset acquired from the given set of orders.
/// @return ethFeePaid Amount of ETH spent on the given forwarder fee.
function marketSellOrdersWithEth(
LibOrder.Order[] memory orders,
bytes[] memory signatures,
@@ -55,7 +57,9 @@ contract IForwarderCore {
/// @param signatures Proofs that orders have been created by makers.
/// @param feePercentage Percentage of WETH sold that will payed as fee to forwarding contract feeRecipient.
/// @param feeRecipient Address that will receive ETH when orders are filled.
/// @return Amounts of WETH spent, makerAsset acquired, and forwarder fees paid for the given set of orders.
/// @return wethSpentAmount Amount of WETH spent on the given set of orders.
/// @return makerAssetAcquiredAmount Amount of maker asset acquired from the given set of orders.
/// @return ethFeePaid Amount of ETH spent on the given forwarder fee.
function marketBuyOrdersWithEth(
LibOrder.Order[] memory orders,
uint256 makerAssetBuyAmount,

View File

@@ -31,9 +31,9 @@ library LibForwarderRichErrors {
bytes4 internal constant UNSUPPORTED_ASSET_PROXY_ERROR_SELECTOR =
0x7996a271;
// bytes4(keccak256("CompleteFillFailedError()"))
bytes internal constant COMPLETE_FILL_FAILED_ERROR =
hex"4bcb2058";
// bytes4(keccak256("CompleteBuyFailedError(uint256,uint256)"))
bytes4 internal constant COMPLETE_BUY_FAILED_ERROR_SELECTOR =
0x91353a0c;
// bytes4(keccak256("MakerAssetMismatchError(bytes,bytes)"))
bytes4 internal constant MAKER_ASSET_MISMATCH_ERROR_SELECTOR =
@@ -47,33 +47,29 @@ library LibForwarderRichErrors {
bytes4 internal constant FEE_PERCENTAGE_TOO_LARGE_ERROR_SELECTOR =
0x1174fb80;
// bytes4(keccak256("InsufficientEthForFeeError()"))
bytes internal constant INSUFFICIENT_ETH_FOR_FEE_ERROR =
hex"54a53d9e";
// bytes4(keccak256("InsufficientEthForFeeError(uint256,uint256)"))
bytes4 internal constant INSUFFICIENT_ETH_FOR_FEE_ERROR_SELECTOR =
0xecf40fd9;
// bytes4(keccak256("OversoldWethError(uint256,uint256)"))
bytes4 internal constant OVERSOLD_WETH_ERROR_SELECTOR =
0x5cc555c8;
// bytes4(keccak256("TransferFailedError()"))
bytes internal constant TRANSFER_FAILED_ERROR =
hex"570f1df4";
// bytes4(keccak256("TransferFailedError(bytes)"))
bytes4 internal constant TRANSFER_FAILED_ERROR_SELECTOR =
0x5e7eb60f;
// bytes4(keccak256("DefaultFunctionWethContractOnlyError(address)"))
bytes4 internal constant DEFAULT_FUNCTION_WETH_CONTRACT_ONLY_ERROR_SELECTOR =
0x08b18698;
// bytes4(keccak256("InvalidMsgValueError()"))
bytes4 internal constant INVALID_MSG_VALUE_ERROR_SELECTOR =
0xb0658a43;
// bytes4(keccak256("MsgValueCantEqualZeroError()"))
bytes internal constant MSG_VALUE_CANT_EQUAL_ZERO_ERROR =
hex"1213e1d6";
// bytes4(keccak256("InvalidMsgValueError()"))
bytes internal constant INVALID_MSG_VALUE_ERROR =
hex"b0658a43";
// bytes4(keccak256("InvalidErc721AmountError(uint256)"))
bytes4 internal constant INVALID_ERC721_AMOUNT_ERROR_SELECTOR =
0x27ed87bf;
// bytes4(keccak256("Erc721AmountMustEqualOneError(uint256)"))
bytes4 internal constant ERC721_AMOUNT_MUST_EQUAL_ONE_ERROR_SELECTOR =
0xbaffa474;
// solhint-disable func-name-mixedcase
function UnregisteredAssetProxyError()
@@ -97,12 +93,19 @@ library LibForwarderRichErrors {
);
}
function CompleteFillFailedError()
function CompleteBuyFailedError(
uint256 expectedAssetBuyAmount,
uint256 actualAssetBuyAmount
)
internal
pure
returns (bytes memory)
{
return COMPLETE_FILL_FAILED_ERROR;
return abi.encodeWithSelector(
COMPLETE_BUY_FAILED_ERROR_SELECTOR,
expectedAssetBuyAmount,
actualAssetBuyAmount
);
}
function MakerAssetMismatchError(
@@ -146,12 +149,19 @@ library LibForwarderRichErrors {
);
}
function InsufficientEthForFeeError()
function InsufficientEthForFeeError(
uint256 ethFeeRequired,
uint256 ethAvailable
)
internal
pure
returns (bytes memory)
{
return INSUFFICIENT_ETH_FOR_FEE_ERROR;
return abi.encodeWithSelector(
INSUFFICIENT_ETH_FOR_FEE_ERROR_SELECTOR,
ethFeeRequired,
ethAvailable
);
}
function OversoldWethError(
@@ -169,16 +179,21 @@ library LibForwarderRichErrors {
);
}
function TransferFailedError()
function TransferFailedError(
bytes memory errorData
)
internal
pure
returns (bytes memory)
{
return TRANSFER_FAILED_ERROR;
return abi.encodeWithSelector(
TRANSFER_FAILED_ERROR_SELECTOR,
errorData
);
}
function DefaultFunctionWethContractOnlyError(
address callerAddress
address senderAddress
)
internal
pure
@@ -186,19 +201,19 @@ library LibForwarderRichErrors {
{
return abi.encodeWithSelector(
DEFAULT_FUNCTION_WETH_CONTRACT_ONLY_ERROR_SELECTOR,
callerAddress
senderAddress
);
}
function InvalidMsgValueError()
function MsgValueCantEqualZeroError()
internal
pure
returns (bytes memory)
{
return INVALID_MSG_VALUE_ERROR;
return MSG_VALUE_CANT_EQUAL_ZERO_ERROR;
}
function InvalidErc721AmountError(
function Erc721AmountMustEqualOneError(
uint256 amount
)
internal
@@ -206,7 +221,7 @@ library LibForwarderRichErrors {
returns (bytes memory)
{
return abi.encodeWithSelector(
INVALID_ERC721_AMOUNT_ERROR_SELECTOR,
ERC721_AMOUNT_MUST_EQUAL_ONE_ERROR_SELECTOR,
amount
);
}

View File

@@ -468,8 +468,13 @@ describe(ContractName.Forwarder, () => {
});
it('should revert if the amount of ETH sent is too low to fill the makerAssetAmount', async () => {
const order = await orderFactory.newSignedOrderAsync();
const revertError = new ForwarderRevertErrors.CompleteFillFailedError();
await forwarderTestFactory.marketBuyTestAsync([order], new BigNumber(0.5), erc20Token, {
const fillFraction = new BigNumber(0.5);
const revertError = new ForwarderRevertErrors.CompleteBuyFailedError(
order.makerAssetAmount.times(fillFraction),
constants.ZERO_AMOUNT,
);
await forwarderTestFactory.marketBuyTestAsync([order], fillFraction, erc20Token, {
ethValueAdjustment: new BigNumber(-2),
revertError,
});
@@ -708,10 +713,22 @@ describe(ContractName.Forwarder, () => {
});
it('should fail if there is not enough ETH remaining to pay the fee', async () => {
const order = await orderFactory.newSignedOrderAsync();
const revertError = new ForwarderRevertErrors.InsufficientEthForFeeError();
const forwarderFeePercentage = new BigNumber(2);
const fillFraction = new BigNumber(0.5);
const ethFee = ForwarderTestFactory.getPercentageOfValue(
order.takerAssetAmount.times(fillFraction),
forwarderFeePercentage,
);
const revertError = new ForwarderRevertErrors.InsufficientEthForFeeError(
ethFee,
ethFee.minus(1),
);
// -2 to compensate for the extra 1 wei added in ForwarderTestFactory to account for rounding
await forwarderTestFactory.marketBuyTestAsync([order], new BigNumber(0.5), erc20Token, {
ethValueAdjustment: new BigNumber(-2),
forwarderFeePercentage: new BigNumber(2),
forwarderFeePercentage,
revertError,
});
});

View File

@@ -1,5 +1,4 @@
import { BigNumber, RevertError } from '@0x/utils';
import * as _ from 'lodash';
// tslint:disable:max-classes-per-file
@@ -15,9 +14,16 @@ export class UnsupportedAssetProxyError extends RevertError {
}
}
export class CompleteFillFailedError extends RevertError {
constructor() {
super('CompleteFillFailedError', 'CompleteFillFailedError()', {});
export class CompleteBuyFailedError extends RevertError {
constructor(
expectedAssetBuyAmount?: BigNumber | number | string,
actualAssetBuyAmount?: BigNumber | number | string,
) {
super(
'CompleteBuyFailedError',
'CompleteBuyFailedError(uint256 expectedAssetBuyAmount, uint256 actualAssetBuyAmount)',
{ expectedAssetBuyAmount, actualAssetBuyAmount },
);
}
}
@@ -49,8 +55,12 @@ export class FeePercentageTooLargeError extends RevertError {
}
export class InsufficientEthForFeeError extends RevertError {
constructor() {
super('InsufficientEthForFeeError', 'InsufficientEthForFeeError()', {});
constructor(ethFeeRequired?: BigNumber | number | string, ethAvailable?: BigNumber | number | string) {
super(
'InsufficientEthForFeeError',
'InsufficientEthForFeeError(uint256 ethFeeRequired, uint256 ethAvailable)',
{ ethFeeRequired, ethAvailable },
);
}
}
@@ -64,28 +74,28 @@ export class OversoldWethError extends RevertError {
}
export class TransferFailedError extends RevertError {
constructor() {
super('TransferFailedError', 'TransferFailedError()', {});
constructor(errorData?: string) {
super('TransferFailedError', 'TransferFailedError(bytes errorData)', { errorData });
}
}
export class DefaultFunctionWethContractOnlyError extends RevertError {
constructor(callerAddress?: string) {
super('DefaultFunctionWethContractOnlyError', 'DefaultFunctionWethContractOnlyError(address callerAddress)', {
callerAddress,
constructor(senderAddress?: string) {
super('DefaultFunctionWethContractOnlyError', 'DefaultFunctionWethContractOnlyError(address senderAddress)', {
senderAddress,
});
}
}
export class InvalidMsgValueError extends RevertError {
export class MsgValueCantEqualZeroError extends RevertError {
constructor() {
super('InvalidMsgValueError', 'InvalidMsgValueError()', {});
super('MsgValueCantEqualZeroError', 'MsgValueCantEqualZeroError()', {});
}
}
export class InvalidErc721AmountError extends RevertError {
export class Erc721AmountMustEqualOneError extends RevertError {
constructor(amount?: BigNumber | number | string) {
super('InvalidErc721AmountError', 'InvalidErc721AmountError(uint256 amount)', {
super('Erc721AmountMustEqualOneError', 'Erc721AmountMustEqualOneError(uint256 amount)', {
amount,
});
}
@@ -94,7 +104,7 @@ export class InvalidErc721AmountError extends RevertError {
const types = [
UnregisteredAssetProxyError,
UnsupportedAssetProxyError,
CompleteFillFailedError,
CompleteBuyFailedError,
MakerAssetMismatchError,
UnsupportedFeeError,
FeePercentageTooLargeError,
@@ -102,8 +112,8 @@ const types = [
OversoldWethError,
TransferFailedError,
DefaultFunctionWethContractOnlyError,
InvalidMsgValueError,
InvalidErc721AmountError,
MsgValueCantEqualZeroError,
Erc721AmountMustEqualOneError,
];
// Register the types we've defined.