crowetic/protocol
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Don't use variables for revert reasons

Browse Source
This commit is contained in:
Amir Bandeali 2018-06-25 16:01:34 -07:00
parent 0163984ea4
commit a89908540f
14 changed files with 86 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol
View File

@ -19,12 +19,10 @@
pragma solidity ^0.4.24;
pragma experimental ABIEncoderV2;
import "./libs/LibAssetProxyErrors.sol";
import "../../utils/Ownable/Ownable.sol";
import "./mixins/MAuthorizable.sol";
contract MixinAuthorizable is
LibAssetProxyErrors,
Ownable,
MAuthorizable
{
@ -33,7 +31,7 @@ contract MixinAuthorizable is
modifier onlyAuthorized {
require(
authorized[msg.sender],
SENDER_NOT_AUTHORIZED
"SENDER_NOT_AUTHORIZED"
);
_;
}
@ -49,7 +47,7 @@ contract MixinAuthorizable is
{
require(
!authorized[target],
TARGET_ALREADY_AUTHORIZED
"TARGET_ALREADY_AUTHORIZED"
);
authorized[target] = true;
@ -65,7 +63,7 @@ contract MixinAuthorizable is
{
require(
authorized[target],
TARGET_NOT_AUTHORIZED
"TARGET_NOT_AUTHORIZED"
);
delete authorized[target];
@ -91,15 +89,15 @@ contract MixinAuthorizable is
{
require(
authorized[target],
TARGET_NOT_AUTHORIZED
"TARGET_NOT_AUTHORIZED"
);
require(
index < authorities.length,
INDEX_OUT_OF_BOUNDS
"INDEX_OUT_OF_BOUNDS"
);
require(
authorities[index] == target,
AUTHORIZED_ADDRESS_MISMATCH
"AUTHORIZED_ADDRESS_MISMATCH"
);
delete authorized[target];

8
packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol
View File

@ -21,11 +21,9 @@ pragma experimental ABIEncoderV2;
import "../../utils/LibBytes/LibBytes.sol";
import "../../tokens/ERC20Token/IERC20Token.sol";
import "./libs/LibTransferErrors.sol";
contract MixinERC20Transfer is
LibTransferErrors
{
contract MixinERC20Transfer {
using LibBytes for bytes;
/// @dev Internal version of `transferFrom`.
@ -109,7 +107,7 @@ contract MixinERC20Transfer is
}
require(
success,
TRANSFER_FAILED
"TRANSFER_FAILED"
);
}
}

3
packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol
View File

@ -18,7 +18,10 @@
pragma solidity ^0.4.24;
/// @dev This contract documents the revert reasons used in the AssetProxy contracts.
/// This contract is intended to serve as a reference, but is not actually used for efficiency reasons.
contract LibAssetProxyErrors {
/// Authorizable errors ///
string constant SENDER_NOT_AUTHORIZED = "SENDER_NOT_AUTHORIZED"; // Sender not authorized to call this method.
string constant TARGET_NOT_AUTHORIZED = "TARGET_NOT_AUTHORIZED"; // Target address not authorized to call this method.

3
packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibTransferErrors.sol
View File

@ -18,7 +18,10 @@
pragma solidity ^0.4.24;
/// @dev This contract documents the revert reasons used in the `transferFrom` methods of different AssetProxy contracts.
/// This contract is intended to serve as a reference, but is not actually used for efficiency reasons.
contract LibTransferErrors {
/// Transfer errors ///
string constant INVALID_AMOUNT = "INVALID_AMOUNT"; // Transfer amount must equal 1.
string constant TRANSFER_FAILED = "TRANSFER_FAILED"; // Transfer failed.

22
packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
View File

@ -23,7 +23,6 @@ import "./libs/LibConstants.sol";
import "./libs/LibFillResults.sol";
import "./libs/LibOrder.sol";
import "./libs/LibMath.sol";
import "./libs/LibExchangeErrors.sol";
import "./mixins/MExchangeCore.sol";
import "./mixins/MSignatureValidator.sol";
import "./mixins/MTransactions.sol";
@ -34,7 +33,6 @@ contract MixinExchangeCore is
LibMath,
LibOrder,
LibFillResults,
LibExchangeErrors,
MAssetProxyDispatcher,
MExchangeCore,
MSignatureValidator,
@ -70,7 +68,7 @@ contract MixinExchangeCore is
// Ensure orderEpoch is monotonically increasing
require(
newOrderEpoch > oldOrderEpoch,
INVALID_NEW_ORDER_EPOCH
"INVALID_NEW_ORDER_EPOCH"
);
// Update orderEpoch
@ -282,20 +280,20 @@ contract MixinExchangeCore is
// An order can only be filled if its status is FILLABLE.
require(
orderInfo.orderStatus == uint8(OrderStatus.FILLABLE),
ORDER_UNFILLABLE
"ORDER_UNFILLABLE"
);
// Revert if fill amount is invalid
require(
takerAssetFillAmount != 0,
INVALID_TAKER_AMOUNT
"INVALID_TAKER_AMOUNT"
);
// Validate sender is allowed to fill this order
if (order.senderAddress != address(0)) {
require(
order.senderAddress == msg.sender,
INVALID_SENDER
"INVALID_SENDER"
);
}
@ -303,7 +301,7 @@ contract MixinExchangeCore is
if (order.takerAddress != address(0)) {
require(
order.takerAddress == takerAddress,
INVALID_TAKER
"INVALID_TAKER"
);
}
@ -315,7 +313,7 @@ contract MixinExchangeCore is
order.makerAddress,
signature
),
INVALID_ORDER_SIGNATURE
"INVALID_ORDER_SIGNATURE"
);
}
@ -326,7 +324,7 @@ contract MixinExchangeCore is
order.takerAssetAmount,
order.makerAssetAmount
),
ROUNDING_ERROR
"ROUNDING_ERROR"
);
}
@ -344,14 +342,14 @@ contract MixinExchangeCore is
// An order can only be cancelled if its status is FILLABLE.
require(
orderInfo.orderStatus == uint8(OrderStatus.FILLABLE),
ORDER_UNFILLABLE
"ORDER_UNFILLABLE"
);
// Validate sender is allowed to cancel this order
if (order.senderAddress != address(0)) {
require(
order.senderAddress == msg.sender,
INVALID_SENDER
"INVALID_SENDER"
);
}
@ -359,7 +357,7 @@ contract MixinExchangeCore is
address makerAddress = getCurrentContextAddress();
require(
order.makerAddress == makerAddress,
INVALID_MAKER
"INVALID_MAKER"
);
}

4
packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
View File

@ -18,7 +18,6 @@ import "./libs/LibConstants.sol";
import "./libs/LibMath.sol";
import "./libs/LibOrder.sol";
import "./libs/LibFillResults.sol";
import "./libs/LibExchangeErrors.sol";
import "./mixins/MExchangeCore.sol";
import "./mixins/MMatchOrders.sol";
import "./mixins/MTransactions.sol";
@ -27,7 +26,6 @@ import "./mixins/MAssetProxyDispatcher.sol";
contract MixinMatchOrders is
LibConstants,
LibMath,
LibExchangeErrors,
MAssetProxyDispatcher,
MExchangeCore,
MMatchOrders,
@ -141,7 +139,7 @@ contract MixinMatchOrders is
require(
safeMul(leftOrder.makerAssetAmount, rightOrder.makerAssetAmount) >=
safeMul(leftOrder.takerAssetAmount, rightOrder.takerAssetAmount),
NEGATIVE_SPREAD_REQUIRED
"NEGATIVE_SPREAD_REQUIRED"
);
}

36
packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
View File

@ -19,23 +19,17 @@
pragma solidity ^0.4.24;
import "../../utils/LibBytes/LibBytes.sol";
import "./libs/LibExchangeErrors.sol";
import "./mixins/MSignatureValidator.sol";
import "./mixins/MTransactions.sol";
import "./interfaces/IWallet.sol";
import "./interfaces/IValidator.sol";
contract MixinSignatureValidator is
LibExchangeErrors,
MSignatureValidator,
MTransactions
{
using LibBytes for bytes;
// Personal message headers
string constant ETH_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n32";
string constant TREZOR_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n\x20";
// Mapping of hash => signer => signed
mapping (bytes32 => mapping (address => bool)) public preSigned;
@ -59,7 +53,7 @@ contract MixinSignatureValidator is
signerAddress,
signature
),
INVALID_SIGNATURE
"INVALID_SIGNATURE"
);
preSigned[hash][signerAddress] = true;
}
@ -99,14 +93,14 @@ contract MixinSignatureValidator is
// TODO: Domain separation: make hash depend on role. (Taker sig should not be valid as maker sig, etc.)
require(
signature.length > 0,
LENGTH_GREATER_THAN_0_REQUIRED
"LENGTH_GREATER_THAN_0_REQUIRED"
);
// Ensure signature is supported
uint8 signatureTypeRaw = uint8(signature.popLastByte());
require(
signatureTypeRaw < uint8(SignatureType.NSignatureTypes),
SIGNATURE_UNSUPPORTED
"SIGNATURE_UNSUPPORTED"
);
// Pop last byte off of signature byte array.
@ -124,7 +118,7 @@ contract MixinSignatureValidator is
// it an explicit option. This aids testing and analysis. It is
// also the initialization value for the enum type.
if (signatureType == SignatureType.Illegal) {
revert(SIGNATURE_ILLEGAL);
revert("SIGNATURE_ILLEGAL");
// Always invalid signature.
// Like Illegal, this is always implicitly available and therefore
@ -133,7 +127,7 @@ contract MixinSignatureValidator is
} else if (signatureType == SignatureType.Invalid) {
require(
signature.length == 0,
LENGTH_0_REQUIRED
"LENGTH_0_REQUIRED"
);
isValid = false;
return isValid;
@ -142,7 +136,7 @@ contract MixinSignatureValidator is
} else if (signatureType == SignatureType.EIP712) {
require(
signature.length == 65,
LENGTH_65_REQUIRED
"LENGTH_65_REQUIRED"
);
v = uint8(signature[0]);
r = signature.readBytes32(1);
@ -155,13 +149,16 @@ contract MixinSignatureValidator is
} else if (signatureType == SignatureType.EthSign) {
require(
signature.length == 65,
LENGTH_65_REQUIRED
"LENGTH_65_REQUIRED"
);
v = uint8(signature[0]);
r = signature.readBytes32(1);
s = signature.readBytes32(33);
recovered = ecrecover(
keccak256(abi.encodePacked(ETH_PERSONAL_MESSAGE, hash)),
keccak256(abi.encodePacked(
"\x19Ethereum Signed Message:\n32",
hash
)),
v,
r,
s
@ -180,7 +177,7 @@ contract MixinSignatureValidator is
} else if (signatureType == SignatureType.Caller) {
require(
signature.length == 0,
LENGTH_0_REQUIRED
"LENGTH_0_REQUIRED"
);
isValid = signerAddress == msg.sender;
return isValid;
@ -230,13 +227,16 @@ contract MixinSignatureValidator is
} else if (signatureType == SignatureType.Trezor) {
require(
signature.length == 65,
LENGTH_65_REQUIRED
"LENGTH_65_REQUIRED"
);
v = uint8(signature[0]);
r = signature.readBytes32(1);
s = signature.readBytes32(33);
recovered = ecrecover(
keccak256(abi.encodePacked(TREZOR_PERSONAL_MESSAGE, hash)),
keccak256(abi.encodePacked(
"\x19Ethereum Signed Message:\n\x20",
hash
)),
v,
r,
s
@ -250,6 +250,6 @@ contract MixinSignatureValidator is
// that we currently support. In this case returning false
// may lead the caller to incorrectly believe that the
// signature was invalid.)
revert(SIGNATURE_UNSUPPORTED);
revert("SIGNATURE_UNSUPPORTED");
}
}

10
packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
View File

@ -20,12 +20,10 @@ pragma solidity ^0.4.24;
import "./libs/LibExchangeErrors.sol";
import "./mixins/MSignatureValidator.sol";
import "./mixins/MTransactions.sol";
import "./libs/LibExchangeErrors.sol";
import "./libs/LibEIP712.sol";
contract MixinTransactions is
LibEIP712,
LibExchangeErrors,
MSignatureValidator,
MTransactions
{
@ -90,7 +88,7 @@ contract MixinTransactions is
// Prevent reentrancy
require(
currentContextAddress == address(0),
REENTRANCY_ILLEGAL
"REENTRANCY_ILLEGAL"
);
bytes32 transactionHash = hashEIP712Message(hashZeroExTransaction(
@ -102,7 +100,7 @@ contract MixinTransactions is
// Validate transaction has not been executed
require(
!transactions[transactionHash],
INVALID_TX_HASH
"INVALID_TX_HASH"
);
// Transaction always valid if signer is sender of transaction
@ -114,7 +112,7 @@ contract MixinTransactions is
signerAddress,
signature
),
INVALID_TX_SIGNATURE
"INVALID_TX_SIGNATURE"
);
// Set the current transaction signer
@ -125,7 +123,7 @@ contract MixinTransactions is
transactions[transactionHash] = true;
require(
address(this).delegatecall(data),
FAILED_EXECUTION
"FAILED_EXECUTION"
);
// Reset current transaction signer

4
packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
View File

@ -22,13 +22,11 @@ pragma experimental ABIEncoderV2;
import "./libs/LibMath.sol";
import "./libs/LibOrder.sol";
import "./libs/LibFillResults.sol";
import "./libs/LibExchangeErrors.sol";
import "./mixins/MExchangeCore.sol";
contract MixinWrapperFunctions is
LibMath,
LibFillResults,
LibExchangeErrors,
MExchangeCore
{
/// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled.
@ -50,7 +48,7 @@ contract MixinWrapperFunctions is
);
require(
fillResults.takerAssetFilledAmount == takerAssetFillAmount,
COMPLETE_FILL_FAILED
"COMPLETE_FILL_FAILED"
);
return fillResults;
}

5
packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
View File

@ -18,7 +18,10 @@
pragma solidity ^0.4.24;
/// @dev This contract documents the revert reasons used in the Exchange contract.
/// This contract is intended to serve as a reference, but is not actually used for efficiency reasons.
contract LibExchangeErrors {
/// Order validation errors ///
string constant ORDER_UNFILLABLE = "ORDER_UNFILLABLE"; // Order cannot be filled.
string constant INVALID_MAKER = "INVALID_MAKER"; // Invalid makerAddress.
@ -56,9 +59,11 @@ contract LibExchangeErrors {
/// dispatchTransferFrom errors ///
string constant ASSET_PROXY_DOES_NOT_EXIST = "ASSET_PROXY_DOES_NOT_EXIST"; // No assetProxy registered at given id.
string constant TRANSFER_FAILED = "TRANSFER_FAILED"; // Asset transfer unsuccesful.
/// Length validation errors ///
string constant LENGTH_GREATER_THAN_0_REQUIRED = "LENGTH_GREATER_THAN_0_REQUIRED"; // Byte array must have a length greater than 0.
string constant LENGTH_GREATER_THAN_3_REQUIRED = "LENGTH_GREATER_THAN_3_REQUIRED"; // Byte array must have a length greater than 3.
string constant LENGTH_0_REQUIRED = "LENGTH_0_REQUIRED"; // Byte array must have a length of 0.
string constant LENGTH_65_REQUIRED = "LENGTH_65_REQUIRED"; // Byte array must have a length of 65.
}

1
packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol
View File

@ -23,7 +23,6 @@ import "../../../utils/SafeMath/SafeMath.sol";
contract LibMath is
SafeMath
{
string constant ROUNDING_ERROR_ON_PARTIAL_AMOUNT = "A rounding error occurred when calculating partial transfer amounts.";
/// @dev Calculates partial value given a numerator and denominator.
/// @param numerator Numerator.

51
packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
View File

@ -19,17 +19,8 @@
pragma solidity ^0.4.24;
library LibBytes {
using LibBytes for bytes;
// Revert reasons
string constant GREATER_THAN_ZERO_LENGTH_REQUIRED = "GREATER_THAN_ZERO_LENGTH_REQUIRED";
string constant GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED";
string constant GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED";
string constant GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED";
string constant GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED";
string constant GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED";
string constant FROM_LESS_THAN_TO_REQUIRED = "FROM_LESS_THAN_TO_REQUIRED";
string constant TO_LESS_THAN_LENGTH_REQUIRED = "TO_LESS_THAN_LENGTH_REQUIRED";
using LibBytes for bytes;
/// @dev Gets the memory address for a byte array.
/// @param input Byte array to lookup.
@ -176,8 +167,14 @@ library LibBytes {
pure
returns (bytes memory result)
{
require(from <= to, FROM_LESS_THAN_TO_REQUIRED);
require(to < b.length, TO_LESS_THAN_LENGTH_REQUIRED);
require(
from <= to,
"FROM_LESS_THAN_TO_REQUIRED"
);
require(
to < b.length,
"TO_LESS_THAN_LENGTH_REQUIRED"
);
// Create a new bytes structure and copy contents
result = new bytes(to - from);
@ -199,8 +196,14 @@ library LibBytes {
pure
returns (bytes memory result)
{
require(from <= to, FROM_LESS_THAN_TO_REQUIRED);
require(to < b.length, TO_LESS_THAN_LENGTH_REQUIRED);
require(
from <= to,
"FROM_LESS_THAN_TO_REQUIRED"
);
require(
to < b.length,
"TO_LESS_THAN_LENGTH_REQUIRED"
);
// Create a new bytes structure around [from, to) in-place.
assembly {
@ -220,7 +223,7 @@ library LibBytes {
{
require(
b.length > 0,
GREATER_THAN_ZERO_LENGTH_REQUIRED
"GREATER_THAN_ZERO_LENGTH_REQUIRED"
);
// Store last byte.
@ -244,7 +247,7 @@ library LibBytes {
{
require(
b.length >= 20,
GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED
"GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED"
);
// Store last 20 bytes.
@ -290,7 +293,7 @@ library LibBytes {
{
require(
b.length >= index + 20, // 20 is length of address
GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED
"GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED"
);
// Add offset to index:
@ -322,7 +325,7 @@ library LibBytes {
{
require(
b.length >= index + 20, // 20 is length of address
GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED
"GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED"
);
// Add offset to index:
@ -365,7 +368,7 @@ library LibBytes {
{
require(
b.length >= index + 32,
GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED
"GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED"
);
// Arrays are prefixed by a 256 bit length parameter
@ -392,7 +395,7 @@ library LibBytes {
{
require(
b.length >= index + 32,
GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED
"GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED"
);
// Arrays are prefixed by a 256 bit length parameter
@ -447,7 +450,7 @@ library LibBytes {
{
require(
b.length >= index + 4,
GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED
"GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED"
);
assembly {
result := mload(add(b, 32))
@ -480,7 +483,7 @@ library LibBytes {
// length of nested bytes
require(
b.length >= index + nestedBytesLength,
GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED
"GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED"
);
// Return a pointer to the byte array as it exists inside `b`
@ -506,7 +509,7 @@ library LibBytes {
// length of input
require(
b.length >= index + 32 /* 32 bytes to store length */ + input.length,
GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED
"GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED"
);
// Copy <input> into <b>
@ -531,7 +534,7 @@ library LibBytes {
// Dest length must be >= source length, or some bytes would not be copied.
require(
dest.length >= sourceLen,
GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED
"GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED"
);
memCopy(
dest.contentAddress(),

5
packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol
View File

@ -13,9 +13,6 @@ import "./IOwnable.sol";
contract Ownable is IOwnable {
address public owner;
// Revert reasons
string constant ONLY_CONTRACT_OWNER = "ONLY_CONTRACT_OWNER";
constructor ()
public
{
@ -25,7 +22,7 @@ contract Ownable is IOwnable {
modifier onlyOwner() {
require(
msg.sender == owner,
ONLY_CONTRACT_OWNER
"ONLY_CONTRACT_OWNER"
);
_;
}

6
packages/contracts/test/asset_proxy/proxies.ts
View File

@ -236,7 +236,8 @@ describe('Asset Transfer Proxies', () => {
it('should have an id of 0xf47261b0', async () => {
const proxyId = await erc20Proxy.getProxyId.callAsync();
expect(proxyId).to.equal('0xf47261b0');
const expectedProxyId = '0xf47261b0';
expect(proxyId).to.equal(expectedProxyId);
});
});
@ -484,7 +485,8 @@ describe('Asset Transfer Proxies', () => {
it('should have an id of 0x08e937fa', async () => {
const proxyId = await erc721Proxy.getProxyId.callAsync();
expect(proxyId).to.equal('0x08e937fa');
const expectedProxyId = '0x08e937fa';
expect(proxyId).to.equal(expectedProxyId);
});
});
});