aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAmir Bandeali <abandeali1@gmail.com>2018-08-30 06:57:53 +0800
committerGitHub <noreply@github.com>2018-08-30 06:57:53 +0800
commiteb4517d7379128b18c830b704742154c8b2f3c8d (patch)
treeb35903f1951a683ee50db1908dcc1fc46b81c165
parentf87420a7768cb6342ad53f0d05734466e039f231 (diff)
parentd80701c2779512324c7a9bef2a87b6f63239d281 (diff)
downloaddexon-sol-tools-eb4517d7379128b18c830b704742154c8b2f3c8d.tar
dexon-sol-tools-eb4517d7379128b18c830b704742154c8b2f3c8d.tar.gz
dexon-sol-tools-eb4517d7379128b18c830b704742154c8b2f3c8d.tar.bz2
dexon-sol-tools-eb4517d7379128b18c830b704742154c8b2f3c8d.tar.lz
dexon-sol-tools-eb4517d7379128b18c830b704742154c8b2f3c8d.tar.xz
dexon-sol-tools-eb4517d7379128b18c830b704742154c8b2f3c8d.tar.zst
dexon-sol-tools-eb4517d7379128b18c830b704742154c8b2f3c8d.zip
Merge pull request #1041 from 0xProject/refactor/contracts/assetProxyOwnerVersion
Update AssetProxyOwner to 0.4.24, remove redundant code
-rw-r--r--packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol1
-rw-r--r--packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol1
-rw-r--r--packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol54
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol4
-rw-r--r--packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol23
-rw-r--r--packages/contracts/test/exchange/signature_validator.ts14
-rw-r--r--packages/contracts/test/libraries/lib_bytes.ts48
-rw-r--r--packages/contracts/test/multisig/asset_proxy_owner.ts45
-rw-r--r--packages/contracts/test/tokens/unlimited_allowance_token.ts8
-rw-r--r--packages/contracts/test/utils/assertions.ts17
-rw-r--r--packages/types/CHANGELOG.json9
-rw-r--r--packages/types/src/index.ts3
12 files changed, 105 insertions, 122 deletions
diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol
index 785f3aba9..258443bca 100644
--- a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol
+++ b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol
@@ -18,7 +18,6 @@
pragma solidity 0.4.24;
-import "../../utils/LibBytes/LibBytes.sol";
import "./MixinAuthorizable.sol";
diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol
index caf05b0dd..65b664b8b 100644
--- a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol
+++ b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol
@@ -18,7 +18,6 @@
pragma solidity 0.4.24;
-import "../../utils/LibBytes/LibBytes.sol";
import "./MixinAuthorizable.sol";
diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol b/packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol
index 7da3d87e2..4d00e92d3 100644
--- a/packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol
+++ b/packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol
@@ -16,15 +16,18 @@
*/
-pragma solidity 0.4.10;
+pragma solidity 0.4.24;
import "../../multisig/MultiSigWalletWithTimeLock.sol";
+import "../../utils/LibBytes/LibBytes.sol";
contract AssetProxyOwner is
MultiSigWalletWithTimeLock
{
+ using LibBytes for bytes;
+
event AssetProxyRegistration(address assetProxyContract, bool isRegistered);
// Mapping of AssetProxy contract address =>
@@ -37,8 +40,14 @@ contract AssetProxyOwner is
/// on an approved AssetProxy contract.
modifier validRemoveAuthorizedAddressAtIndexTx(uint256 transactionId) {
Transaction storage tx = transactions[transactionId];
- require(isAssetProxyRegistered[tx.destination]);
- require(readBytes4(tx.data, 0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR);
+ require(
+ isAssetProxyRegistered[tx.destination],
+ "UNREGISTERED_ASSET_PROXY"
+ );
+ require(
+ tx.data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR,
+ "INVALID_FUNCTION_SELECTOR"
+ );
_;
}
@@ -48,7 +57,7 @@ contract AssetProxyOwner is
/// @param _assetProxyContracts Array of AssetProxy contract addresses.
/// @param _required Number of required confirmations.
/// @param _secondsTimeLocked Duration needed after a transaction is confirmed and before it becomes executable, in seconds.
- function AssetProxyOwner(
+ constructor (
address[] memory _owners,
address[] memory _assetProxyContracts,
uint256 _required,
@@ -59,7 +68,10 @@ contract AssetProxyOwner is
{
for (uint256 i = 0; i < _assetProxyContracts.length; i++) {
address assetProxy = _assetProxyContracts[i];
- require(assetProxy != address(0));
+ require(
+ assetProxy != address(0),
+ "INVALID_ASSET_PROXY"
+ );
isAssetProxyRegistered[assetProxy] = true;
}
}
@@ -74,7 +86,7 @@ contract AssetProxyOwner is
notNull(assetProxyContract)
{
isAssetProxyRegistered[assetProxyContract] = isRegistered;
- AssetProxyRegistration(assetProxyContract, isRegistered);
+ emit AssetProxyRegistration(assetProxyContract, isRegistered);
}
/// @dev Allows execution of `removeAuthorizedAddressAtIndex` without time lock.
@@ -89,36 +101,10 @@ contract AssetProxyOwner is
tx.executed = true;
// solhint-disable-next-line avoid-call-value
if (tx.destination.call.value(tx.value)(tx.data))
- Execution(transactionId);
+ emit Execution(transactionId);
else {
- ExecutionFailure(transactionId);
+ emit ExecutionFailure(transactionId);
tx.executed = false;
}
}
-
- /// @dev Reads an unpadded bytes4 value from a position in a byte array.
- /// @param b Byte array containing a bytes4 value.
- /// @param index Index in byte array of bytes4 value.
- /// @return bytes4 value from byte array.
- function readBytes4(
- bytes memory b,
- uint256 index
- )
- internal
- returns (bytes4 result)
- {
- require(b.length >= index + 4);
-
- // Arrays are prefixed by a 32 byte length field
- index += 32;
-
- // Read the bytes4 from array memory
- assembly {
- result := mload(add(b, index))
- // Solidity does not require us to clean the trailing bytes.
- // We do it anyway
- result := and(result, 0xFFFFFFFF00000000000000000000000000000000000000000000000000000000)
- }
- return result;
- }
}
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol
index 80475e6e3..e90b62f19 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol
@@ -19,7 +19,6 @@
pragma solidity 0.4.24;
import "../../utils/Ownable/Ownable.sol";
-import "../../utils/LibBytes/LibBytes.sol";
import "./mixins/MAssetProxyDispatcher.sol";
import "../AssetProxy/interfaces/IAssetProxy.sol";
@@ -28,7 +27,6 @@ contract MixinAssetProxyDispatcher is
Ownable,
MAssetProxyDispatcher
{
- using LibBytes for bytes;
// Mapping from Asset Proxy Id's to their respective Asset Proxy
mapping (bytes4 => IAssetProxy) public assetProxies;
@@ -90,7 +88,7 @@ contract MixinAssetProxyDispatcher is
"LENGTH_GREATER_THAN_3_REQUIRED"
);
- // Lookup assetProxy
+ // Lookup assetProxy. We do not use `LibBytes.readBytes4` for gas efficiency reasons.
bytes4 assetProxyId;
assembly {
assetProxyId := and(mload(
diff --git a/packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol b/packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol
index 75e782d43..38ec42a72 100644
--- a/packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol
+++ b/packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol
@@ -16,7 +16,7 @@
*/
-pragma solidity 0.4.10;
+pragma solidity 0.4.24;
import "../../protocol/AssetProxyOwner/AssetProxyOwner.sol";
@@ -26,7 +26,7 @@ contract TestAssetProxyOwner is
AssetProxyOwner
{
- function TestAssetProxyOwner(
+ constructor (
address[] memory _owners,
address[] memory _assetProxyContracts,
uint256 _required,
@@ -38,6 +38,7 @@ contract TestAssetProxyOwner is
function testValidRemoveAuthorizedAddressAtIndexTx(uint256 id)
public
+ view
validRemoveAuthorizedAddressAtIndexTx(id)
returns (bool)
{
@@ -50,23 +51,9 @@ contract TestAssetProxyOwner is
/// @return Successful if data is a call to `removeAuthorizedAddressAtIndex`.
function isFunctionRemoveAuthorizedAddressAtIndex(bytes memory data)
public
+ pure
returns (bool)
{
- return readBytes4(data, 0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR;
- }
-
- /// @dev Reads an unpadded bytes4 value from a position in a byte array.
- /// @param b Byte array containing a bytes4 value.
- /// @param index Index in byte array of bytes4 value.
- /// @return bytes4 value from byte array.
- function publicReadBytes4(
- bytes memory b,
- uint256 index
- )
- public
- returns (bytes4 result)
- {
- result = readBytes4(b, index);
- return result;
+ return data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR;
}
}
diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts
index da2febfd8..b25483c4b 100644
--- a/packages/contracts/test/exchange/signature_validator.ts
+++ b/packages/contracts/test/exchange/signature_validator.ts
@@ -14,7 +14,7 @@ import { ValidatorContract } from '../../generated_contract_wrappers/validator';
import { WalletContract } from '../../generated_contract_wrappers/wallet';
import { addressUtils } from '../utils/address_utils';
import { artifacts } from '../utils/artifacts';
-import { expectContractCallFailed, expectContractCallFailedWithoutReasonAsync } from '../utils/assertions';
+import { expectContractCallFailedAsync, expectContractCallFailedWithoutReasonAsync } from '../utils/assertions';
import { chaiSetup } from '../utils/chai_setup';
import { constants } from '../utils/constants';
import { LogDecoder } from '../utils/log_decoder';
@@ -119,7 +119,7 @@ describe('MixinSignatureValidator', () => {
it('should revert when signature is empty', async () => {
const emptySignature = '0x';
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
signedOrder.makerAddress,
@@ -133,7 +133,7 @@ describe('MixinSignatureValidator', () => {
const unsupportedSignatureType = SignatureType.NSignatureTypes;
const unsupportedSignatureHex = '0x' + Buffer.from([unsupportedSignatureType]).toString('hex');
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
signedOrder.makerAddress,
@@ -146,7 +146,7 @@ describe('MixinSignatureValidator', () => {
it('should revert when SignatureType=Illegal', async () => {
const unsupportedSignatureHex = '0x' + Buffer.from([SignatureType.Illegal]).toString('hex');
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
signedOrder.makerAddress,
@@ -173,7 +173,7 @@ describe('MixinSignatureValidator', () => {
const signatureBuffer = Buffer.concat([fillerData, signatureType]);
const signatureHex = ethUtil.bufferToHex(signatureBuffer);
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
signedOrder.makerAddress,
@@ -339,7 +339,7 @@ describe('MixinSignatureValidator', () => {
ethUtil.toBuffer(`0x${SignatureType.Wallet}`),
]);
const signatureHex = ethUtil.bufferToHex(signature);
- await expectContractCallFailed(
+ await expectContractCallFailedAsync(
signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
maliciousWallet.address,
@@ -385,7 +385,7 @@ describe('MixinSignatureValidator', () => {
const signature = Buffer.concat([validatorAddress, signatureType]);
const signatureHex = ethUtil.bufferToHex(signature);
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
- await expectContractCallFailed(
+ await expectContractCallFailedAsync(
signatureValidator.publicIsValidSignature.callAsync(orderHashHex, signerAddress, signatureHex),
RevertReason.ValidatorError,
);
diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts
index efdfa13a2..13640a761 100644
--- a/packages/contracts/test/libraries/lib_bytes.ts
+++ b/packages/contracts/test/libraries/lib_bytes.ts
@@ -9,7 +9,7 @@ import * as _ from 'lodash';
import { TestLibBytesContract } from '../../generated_contract_wrappers/test_lib_bytes';
import { artifacts } from '../utils/artifacts';
-import { expectContractCallFailed } from '../utils/assertions';
+import { expectContractCallFailedAsync } from '../utils/assertions';
import { chaiSetup } from '../utils/chai_setup';
import { constants } from '../utils/constants';
import { typeEncodingUtils } from '../utils/type_encoding_utils';
@@ -103,7 +103,7 @@ describe('LibBytes', () => {
describe('popLastByte', () => {
it('should revert if length is 0', async () => {
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicPopLastByte.callAsync(constants.NULL_BYTES),
RevertReason.LibBytesGreaterThanZeroLengthRequired,
);
@@ -125,7 +125,7 @@ describe('LibBytes', () => {
describe('popLast20Bytes', () => {
it('should revert if length is less than 20', async () => {
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicPopLast20Bytes.callAsync(byteArrayShorterThan20Bytes),
RevertReason.LibBytesGreaterOrEqualTo20LengthRequired,
);
@@ -200,7 +200,7 @@ describe('LibBytes', () => {
describe('deepCopyBytes', () => {
it('should revert if dest is shorter than source', async () => {
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicDeepCopyBytes.callAsync(byteArrayShorterThan32Bytes, byteArrayLongerThan32Bytes),
RevertReason.LibBytesGreaterOrEqualToSourceBytesLengthRequired,
);
@@ -253,7 +253,7 @@ describe('LibBytes', () => {
it('should fail if the byte array is too short to hold an address', async () => {
const shortByteArray = '0xabcdef';
const offset = new BigNumber(0);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadAddress.callAsync(shortByteArray, offset),
RevertReason.LibBytesGreaterOrEqualTo20LengthRequired,
);
@@ -261,7 +261,7 @@ describe('LibBytes', () => {
it('should fail if the length between the offset and end of the byte array is too short to hold an address', async () => {
const byteArray = testAddress;
const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadAddress.callAsync(byteArray, badOffset),
RevertReason.LibBytesGreaterOrEqualTo20LengthRequired,
);
@@ -297,7 +297,7 @@ describe('LibBytes', () => {
});
it('should fail if the byte array is too short to hold an address', async () => {
const offset = new BigNumber(0);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicWriteAddress.callAsync(byteArrayShorterThan20Bytes, offset, testAddress),
RevertReason.LibBytesGreaterOrEqualTo20LengthRequired,
);
@@ -305,7 +305,7 @@ describe('LibBytes', () => {
it('should fail if the length between the offset and end of the byte array is too short to hold an address', async () => {
const byteArray = byteArrayLongerThan32Bytes;
const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicWriteAddress.callAsync(byteArray, badOffset, testAddress),
RevertReason.LibBytesGreaterOrEqualTo20LengthRequired,
);
@@ -329,14 +329,14 @@ describe('LibBytes', () => {
});
it('should fail if the byte array is too short to hold a bytes32', async () => {
const offset = new BigNumber(0);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadBytes32.callAsync(byteArrayShorterThan32Bytes, offset),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
});
it('should fail if the length between the offset and end of the byte array is too short to hold a bytes32', async () => {
const badOffset = new BigNumber(ethUtil.toBuffer(testBytes32).byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadBytes32.callAsync(testBytes32, badOffset),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
@@ -372,7 +372,7 @@ describe('LibBytes', () => {
});
it('should fail if the byte array is too short to hold a bytes32', async () => {
const offset = new BigNumber(0);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicWriteBytes32.callAsync(byteArrayShorterThan32Bytes, offset, testBytes32),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
@@ -380,7 +380,7 @@ describe('LibBytes', () => {
it('should fail if the length between the offset and end of the byte array is too short to hold a bytes32', async () => {
const byteArray = byteArrayLongerThan32Bytes;
const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicWriteBytes32.callAsync(byteArray, badOffset, testBytes32),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
@@ -408,7 +408,7 @@ describe('LibBytes', () => {
});
it('should fail if the byte array is too short to hold a uint256', async () => {
const offset = new BigNumber(0);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadUint256.callAsync(byteArrayShorterThan32Bytes, offset),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
@@ -418,7 +418,7 @@ describe('LibBytes', () => {
const testUint256AsBuffer = ethUtil.toBuffer(formattedTestUint256);
const byteArray = ethUtil.bufferToHex(testUint256AsBuffer);
const badOffset = new BigNumber(testUint256AsBuffer.byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadUint256.callAsync(byteArray, badOffset),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
@@ -458,7 +458,7 @@ describe('LibBytes', () => {
});
it('should fail if the byte array is too short to hold a uint256', async () => {
const offset = new BigNumber(0);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicWriteUint256.callAsync(byteArrayShorterThan32Bytes, offset, testUint256),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
@@ -466,7 +466,7 @@ describe('LibBytes', () => {
it('should fail if the length between the offset and end of the byte array is too short to hold a uint256', async () => {
const byteArray = byteArrayLongerThan32Bytes;
const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicWriteUint256.callAsync(byteArray, badOffset, testUint256),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
@@ -478,7 +478,7 @@ describe('LibBytes', () => {
it('should revert if byte array has a length < 4', async () => {
const byteArrayLessThan4Bytes = '0x010101';
const offset = new BigNumber(0);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadBytes4.callAsync(byteArrayLessThan4Bytes, offset),
RevertReason.LibBytesGreaterOrEqualTo4LengthRequired,
);
@@ -504,7 +504,7 @@ describe('LibBytes', () => {
});
it('should fail if the length between the offset and end of the byte array is too short to hold a bytes4', async () => {
const badOffset = new BigNumber(ethUtil.toBuffer(testBytes4).byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadBytes4.callAsync(testBytes4, badOffset),
RevertReason.LibBytesGreaterOrEqualTo4LengthRequired,
);
@@ -554,28 +554,28 @@ describe('LibBytes', () => {
it('should fail if the byte array is too short to hold the length of a nested byte array', async () => {
// The length of the nested array is 32 bytes. By storing less than 32 bytes, a length cannot be read.
const offset = new BigNumber(0);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadBytesWithLength.callAsync(byteArrayShorterThan32Bytes, offset),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
});
it('should fail if we store a nested byte array length, without a nested byte array', async () => {
const offset = new BigNumber(0);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadBytesWithLength.callAsync(testBytes32, offset),
RevertReason.LibBytesGreaterOrEqualToNestedBytesLengthRequired,
);
});
it('should fail if the length between the offset and end of the byte array is too short to hold the length of a nested byte array', async () => {
const badOffset = new BigNumber(ethUtil.toBuffer(byteArrayShorterThan32Bytes).byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadBytesWithLength.callAsync(byteArrayShorterThan32Bytes, badOffset),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
});
it('should fail if the length between the offset and end of the byte array is too short to hold the nested byte array', async () => {
const badOffset = new BigNumber(ethUtil.toBuffer(testBytes32).byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadBytesWithLength.callAsync(testBytes32, badOffset),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
@@ -687,7 +687,7 @@ describe('LibBytes', () => {
it('should fail if the byte array is too short to hold the length of a nested byte array', async () => {
const offset = new BigNumber(0);
const emptyByteArray = ethUtil.bufferToHex(new Buffer(1));
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, offset, longData),
RevertReason.LibBytesGreaterOrEqualToNestedBytesLengthRequired,
);
@@ -695,7 +695,7 @@ describe('LibBytes', () => {
it('should fail if the length between the offset and end of the byte array is too short to hold the length of a nested byte array', async () => {
const emptyByteArray = ethUtil.bufferToHex(new Buffer(shortTestBytesAsBuffer.byteLength));
const badOffset = new BigNumber(ethUtil.toBuffer(shortTestBytesAsBuffer).byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, badOffset, shortData),
RevertReason.LibBytesGreaterOrEqualToNestedBytesLengthRequired,
);
diff --git a/packages/contracts/test/multisig/asset_proxy_owner.ts b/packages/contracts/test/multisig/asset_proxy_owner.ts
index 9515941ff..bb2b3b1a3 100644
--- a/packages/contracts/test/multisig/asset_proxy_owner.ts
+++ b/packages/contracts/test/multisig/asset_proxy_owner.ts
@@ -1,4 +1,5 @@
import { BlockchainLifecycle } from '@0xproject/dev-utils';
+import { RevertReason } from '@0xproject/types';
import { BigNumber } from '@0xproject/utils';
import * as chai from 'chai';
import { LogWithDecodedArgs } from 'ethereum-types';
@@ -14,9 +15,11 @@ import { MixinAuthorizableContract } from '../../generated_contract_wrappers/mix
import { TestAssetProxyOwnerContract } from '../../generated_contract_wrappers/test_asset_proxy_owner';
import { artifacts } from '../utils/artifacts';
import {
- expectContractCallFailedWithoutReasonAsync,
- expectContractCreationFailedWithoutReason,
+ expectContractCallFailedAsync,
+ expectContractCreationFailedAsync,
+ expectTransactionFailedAsync,
expectTransactionFailedWithoutReasonAsync,
+ sendTransactionResult,
} from '../utils/assertions';
import { increaseTimeAndMineBlockAsync } from '../utils/block_timestamp';
import { chaiSetup } from '../utils/chai_setup';
@@ -109,8 +112,8 @@ describe('AssetProxyOwner', () => {
});
it('should throw if a null address is included in assetProxyContracts', async () => {
const assetProxyContractAddresses = [erc20Proxy.address, constants.NULL_ADDRESS];
- return expectContractCreationFailedWithoutReason(
- AssetProxyOwnerContract.deployFrom0xArtifactAsync(
+ return expectContractCreationFailedAsync(
+ (AssetProxyOwnerContract.deployFrom0xArtifactAsync(
artifacts.AssetProxyOwner,
provider,
txDefaults,
@@ -118,7 +121,8 @@ describe('AssetProxyOwner', () => {
assetProxyContractAddresses,
REQUIRED_APPROVALS,
SECONDS_TIME_LOCKED,
- ),
+ ) as any) as sendTransactionResult,
+ RevertReason.InvalidAssetProxy,
);
});
});
@@ -148,25 +152,6 @@ describe('AssetProxyOwner', () => {
});
});
- describe('readBytes4', () => {
- it('should revert if byte array has a length < 4', async () => {
- const byteArrayLessThan4Bytes = '0x010101';
- return expectContractCallFailedWithoutReasonAsync(
- testAssetProxyOwner.publicReadBytes4.callAsync(byteArrayLessThan4Bytes, new BigNumber(0)),
- );
- });
- it('should return the first 4 bytes of a byte array of arbitrary length', async () => {
- const byteArrayLongerThan32Bytes =
- '0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef';
- const first4Bytes = await testAssetProxyOwner.publicReadBytes4.callAsync(
- byteArrayLongerThan32Bytes,
- new BigNumber(0),
- );
- const expectedFirst4Bytes = byteArrayLongerThan32Bytes.slice(0, 10);
- expect(first4Bytes).to.equal(expectedFirst4Bytes);
- });
- });
-
describe('registerAssetProxy', () => {
it('should throw if not called by multisig', async () => {
const isRegistered = true;
@@ -300,8 +285,9 @@ describe('AssetProxyOwner', () => {
);
const log = submitTxRes.logs[0] as LogWithDecodedArgs<AssetProxyOwnerSubmissionEventArgs>;
const txId = log.args.transactionId;
- return expectContractCallFailedWithoutReasonAsync(
+ return expectContractCallFailedAsync(
testAssetProxyOwner.testValidRemoveAuthorizedAddressAtIndexTx.callAsync(txId),
+ RevertReason.InvalidFunctionSelector,
);
});
@@ -335,8 +321,9 @@ describe('AssetProxyOwner', () => {
);
const log = submitTxRes.logs[0] as LogWithDecodedArgs<AssetProxyOwnerSubmissionEventArgs>;
const txId = log.args.transactionId;
- return expectContractCallFailedWithoutReasonAsync(
+ return expectContractCallFailedAsync(
testAssetProxyOwner.testValidRemoveAuthorizedAddressAtIndexTx.callAsync(txId),
+ RevertReason.UnregisteredAssetProxy,
);
});
});
@@ -377,10 +364,11 @@ describe('AssetProxyOwner', () => {
await multiSigWrapper.confirmTransactionAsync(txId, owners[1]);
- return expectTransactionFailedWithoutReasonAsync(
+ return expectTransactionFailedAsync(
testAssetProxyOwner.executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(txId, {
from: owners[1],
}),
+ RevertReason.UnregisteredAssetProxy,
);
});
@@ -399,10 +387,11 @@ describe('AssetProxyOwner', () => {
await multiSigWrapper.confirmTransactionAsync(txId, owners[1]);
- return expectTransactionFailedWithoutReasonAsync(
+ return expectTransactionFailedAsync(
testAssetProxyOwner.executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(txId, {
from: owners[1],
}),
+ RevertReason.InvalidFunctionSelector,
);
});
diff --git a/packages/contracts/test/tokens/unlimited_allowance_token.ts b/packages/contracts/test/tokens/unlimited_allowance_token.ts
index f2725b408..63680fe9b 100644
--- a/packages/contracts/test/tokens/unlimited_allowance_token.ts
+++ b/packages/contracts/test/tokens/unlimited_allowance_token.ts
@@ -5,7 +5,7 @@ import * as chai from 'chai';
import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token';
import { artifacts } from '../utils/artifacts';
-import { expectContractCallFailed } from '../utils/assertions';
+import { expectContractCallFailedAsync } from '../utils/assertions';
import { chaiSetup } from '../utils/chai_setup';
import { constants } from '../utils/constants';
import { provider, txDefaults, web3Wrapper } from '../utils/web3_wrapper';
@@ -54,7 +54,7 @@ describe('UnlimitedAllowanceToken', () => {
it('should throw if owner has insufficient balance', async () => {
const ownerBalance = await token.balanceOf.callAsync(owner);
const amountToTransfer = ownerBalance.plus(1);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
token.transfer.callAsync(spender, amountToTransfer, { from: owner }),
RevertReason.Erc20InsufficientBalance,
);
@@ -93,7 +93,7 @@ describe('UnlimitedAllowanceToken', () => {
await token.approve.sendTransactionAsync(spender, amountToTransfer, { from: owner }),
constants.AWAIT_TRANSACTION_MINED_MS,
);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
token.transferFrom.callAsync(owner, spender, amountToTransfer, {
from: spender,
}),
@@ -109,7 +109,7 @@ describe('UnlimitedAllowanceToken', () => {
const isSpenderAllowanceInsufficient = spenderAllowance.cmp(amountToTransfer) < 0;
expect(isSpenderAllowanceInsufficient).to.be.true();
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
token.transferFrom.callAsync(owner, spender, amountToTransfer, {
from: spender,
}),
diff --git a/packages/contracts/test/utils/assertions.ts b/packages/contracts/test/utils/assertions.ts
index 61df800c8..3361a751a 100644
--- a/packages/contracts/test/utils/assertions.ts
+++ b/packages/contracts/test/utils/assertions.ts
@@ -159,7 +159,7 @@ export async function expectTransactionFailedWithoutReasonAsync(p: sendTransacti
* @returns a new Promise which will reject if the conditions are not met and
* otherwise resolve with no value.
*/
-export async function expectContractCallFailed<T>(p: Promise<T>, reason: RevertReason): Promise<void> {
+export async function expectContractCallFailedAsync<T>(p: Promise<T>, reason: RevertReason): Promise<void> {
return expect(p).to.be.rejectedWith(reason);
}
@@ -180,7 +180,20 @@ export async function expectContractCallFailedWithoutReasonAsync<T>(p: Promise<T
* @returns a new Promise which will reject if the conditions are not met and
* otherwise resolve with no value.
*/
-export async function expectContractCreationFailedWithoutReason<T>(p: Promise<T>): Promise<void> {
+export async function expectContractCreationFailedAsync<T>(
+ p: sendTransactionResult,
+ reason: RevertReason,
+): Promise<void> {
+ return expectTransactionFailedAsync(p, reason);
+}
+
+/**
+ * Resolves if the contract creation/deployment fails without a revert reason.
+ * @param p a Promise resulting from a contract creation/deployment
+ * @returns a new Promise which will reject if the conditions are not met and
+ * otherwise resolve with no value.
+ */
+export async function expectContractCreationFailedWithoutReasonAsync<T>(p: Promise<T>): Promise<void> {
const errMessage = await _getTransactionFailedErrorMessageAsync();
return expect(p).to.be.rejectedWith(errMessage);
}
diff --git a/packages/types/CHANGELOG.json b/packages/types/CHANGELOG.json
index e96d2a742..0bd9e3b37 100644
--- a/packages/types/CHANGELOG.json
+++ b/packages/types/CHANGELOG.json
@@ -1,5 +1,14 @@
[
{
+ "version": "1.0.1-rc.7",
+ "changes": [
+ {
+ "note": "Add AssetProxyOwner revert reasons",
+ "pr": 1041
+ }
+ ]
+ },
+ {
"version": "1.0.1-rc.6",
"changes": [
{
diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts
index d8bffccf9..3011dd87f 100644
--- a/packages/types/src/index.ts
+++ b/packages/types/src/index.ts
@@ -224,6 +224,9 @@ export enum RevertReason {
Erc721InvalidSelector = 'ERC721_INVALID_SELECTOR',
WalletError = 'WALLET_ERROR',
ValidatorError = 'VALIDATOR_ERROR',
+ InvalidFunctionSelector = 'INVALID_FUNCTION_SELECTOR',
+ InvalidAssetProxy = 'INVALID_ASSET_PROXY',
+ UnregisteredAssetProxy = 'UNREGISTERED_ASSET_PROXY',
}
export enum StatusCodes {