diff options
Diffstat (limited to 'packages')
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 { |