diff options
Diffstat (limited to 'packages/contracts')
11 files changed, 94 insertions, 25 deletions
diff --git a/packages/contracts/src/1.0.0/MultiSigWalletWithTImeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol b/packages/contracts/src/1.0.0/MultiSigWalletWithTImeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol index 241e02d4a..aee722c53 100644 --- a/packages/contracts/src/1.0.0/MultiSigWalletWithTImeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol +++ b/packages/contracts/src/1.0.0/MultiSigWalletWithTImeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol @@ -18,7 +18,7 @@ pragma solidity ^0.4.10; -import "../../current/multisig/MultiSigWalletWithTimeLock.sol"; +import "../../2.0.0/multisig/MultiSigWalletWithTimeLock.sol"; contract MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress is MultiSigWalletWithTimeLock { @@ -79,4 +79,4 @@ contract MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress is MultiSigWall } return true; } -}
\ No newline at end of file +} diff --git a/packages/contracts/src/2.0.0/forwarder/Forwarder.sol b/packages/contracts/src/2.0.0/forwarder/Forwarder.sol index fc17a4c72..546e7f22c 100644 --- a/packages/contracts/src/2.0.0/forwarder/Forwarder.sol +++ b/packages/contracts/src/2.0.0/forwarder/Forwarder.sol @@ -40,7 +40,6 @@ contract Forwarder is address _exchange, address _etherToken, address _zrxToken, - bytes4 _erc20AssetProxyId, bytes memory _zrxAssetData, bytes memory _wethAssetData ) 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 e7cf4ab5c..8b7333646 100644 --- a/packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol +++ b/packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol @@ -16,16 +16,14 @@ */ -pragma solidity ^0.4.10; +pragma solidity 0.4.10; import "../../multisig/MultiSigWalletWithTimeLock.sol"; -import "../../utils/LibBytes/LibBytes.sol"; contract AssetProxyOwner is MultiSigWalletWithTimeLock { - using LibBytes for bytes; event AssetProxyRegistration(address assetProxyContract, bool isRegistered); @@ -40,7 +38,7 @@ contract AssetProxyOwner is modifier validRemoveAuthorizedAddressAtIndexTx(uint256 transactionId) { Transaction storage tx = transactions[transactionId]; require(isAssetProxyRegistered[tx.destination]); - require(tx.data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR); + require(readBytes4(tx.data, 0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR); _; } @@ -97,4 +95,25 @@ contract AssetProxyOwner is 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); + assembly { + result := mload(add(b, 32)) + // 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/test/TestAssetProxyOwner/TestAssetProxyOwner.sol b/packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol index d6b6b29f2..75e782d43 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.24; +pragma solidity 0.4.10; import "../../protocol/AssetProxyOwner/AssetProxyOwner.sol"; @@ -26,7 +26,7 @@ contract TestAssetProxyOwner is AssetProxyOwner { - constructor( + function TestAssetProxyOwner( address[] memory _owners, address[] memory _assetProxyContracts, uint256 _required, @@ -38,7 +38,6 @@ contract TestAssetProxyOwner is function testValidRemoveAuthorizedAddressAtIndexTx(uint256 id) public - view validRemoveAuthorizedAddressAtIndexTx(id) returns (bool) { @@ -51,9 +50,23 @@ contract TestAssetProxyOwner is /// @return Successful if data is a call to `removeAuthorizedAddressAtIndex`. function isFunctionRemoveAuthorizedAddressAtIndex(bytes memory data) public - pure returns (bool) { - return data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR; + 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; } } diff --git a/packages/contracts/src/2.0.0/utils/LibBytes/LibBytes.sol b/packages/contracts/src/2.0.0/utils/LibBytes/LibBytes.sol index 01d34fa8f..504e950a8 100644 --- a/packages/contracts/src/2.0.0/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/2.0.0/utils/LibBytes/LibBytes.sol @@ -457,7 +457,8 @@ library LibBytes { /// @return bytes4 value from byte array. function readBytes4( bytes memory b, - uint256 index) + uint256 index + ) internal pure returns (bytes4 result) diff --git a/packages/contracts/src/2.0.0/utils/SafeMath/SafeMath.sol b/packages/contracts/src/2.0.0/utils/SafeMath/SafeMath.sol index 4413244db..190989181 100644 --- a/packages/contracts/src/2.0.0/utils/SafeMath/SafeMath.sol +++ b/packages/contracts/src/2.0.0/utils/SafeMath/SafeMath.sol @@ -7,8 +7,14 @@ contract SafeMath { pure returns (uint256) { + if (a == 0) { + return 0; + } uint256 c = a * b; - assert(a == 0 || c / a == b); + require( + c / a == b, + "UINT256_OVERFLOW" + ); return c; } @@ -26,7 +32,10 @@ contract SafeMath { pure returns (uint256) { - assert(b <= a); + require( + b <= a, + "UINT256_OVERFLOW" + ); return a - b; } @@ -36,7 +45,10 @@ contract SafeMath { returns (uint256) { uint256 c = a + b; - assert(c >= a); + require( + c >= a, + "UINT256_OVERFLOW" + ); return c; } diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts index e1167b156..36e0800d6 100644 --- a/packages/contracts/test/asset_proxy/proxies.ts +++ b/packages/contracts/test/asset_proxy/proxies.ts @@ -272,7 +272,7 @@ describe('Asset Transfer Proxies', () => { to: erc721Proxy.address, data, from: exchangeAddress, - gas: constants.TRANSFER_FROM_GAS, + gas: constants.MAX_TRANSFER_FROM_GAS, }), ); // Verify that no log was emitted by erc721 receiver @@ -311,7 +311,7 @@ describe('Asset Transfer Proxies', () => { to: erc721Proxy.address, data, from: exchangeAddress, - gas: constants.TRANSFER_FROM_GAS, + gas: constants.MAX_TRANSFER_FROM_GAS, }), ); // Validate log emitted by erc721 receiver @@ -349,7 +349,7 @@ describe('Asset Transfer Proxies', () => { to: erc721Proxy.address, data, from: exchangeAddress, - gas: constants.TRANSFER_FROM_GAS, + gas: constants.MAX_TRANSFER_FROM_GAS, }), RevertReason.TransferFailed, ); diff --git a/packages/contracts/test/forwarder/forwarder.ts b/packages/contracts/test/forwarder/forwarder.ts index b4555d417..f0bf6ac03 100644 --- a/packages/contracts/test/forwarder/forwarder.ts +++ b/packages/contracts/test/forwarder/forwarder.ts @@ -1,6 +1,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { assetProxyUtils } from '@0xproject/order-utils'; -import { AssetProxyId, RevertReason, SignedOrder } from '@0xproject/types'; +import { RevertReason, SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; import * as chai from 'chai'; @@ -131,7 +131,6 @@ describe(ContractName.Forwarder, () => { exchangeInstance.address, wethContract.address, zrxToken.address, - AssetProxyId.ERC20, zrxAssetData, wethAssetData, ); diff --git a/packages/contracts/test/multisig/asset_proxy_owner.ts b/packages/contracts/test/multisig/asset_proxy_owner.ts index 10fed6815..6a9843153 100644 --- a/packages/contracts/test/multisig/asset_proxy_owner.ts +++ b/packages/contracts/test/multisig/asset_proxy_owner.ts @@ -148,6 +148,25 @@ 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; @@ -238,8 +257,6 @@ describe('AssetProxyOwner', () => { const registerAssetProxySubmitLog = registerAssetProxySubmitRes.logs[0] as LogWithDecodedArgs< AssetProxyOwnerSubmissionEventArgs >; - const registerAssetProxyTxId = registerAssetProxySubmitLog.args.transactionId; - await multiSigWrapper.confirmTransactionAsync(registerAssetProxyTxId, owners[1]); const addAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData(authorized); const erc20AddAuthorizedAddressSubmitRes = await multiSigWrapper.submitTransactionAsync( @@ -257,9 +274,12 @@ describe('AssetProxyOwner', () => { >; const erc721AddAuthorizedAddressSubmitLog = erc721AddAuthorizedAddressSubmitRes .logs[0] as LogWithDecodedArgs<AssetProxyOwnerSubmissionEventArgs>; + + const registerAssetProxyTxId = registerAssetProxySubmitLog.args.transactionId; const erc20AddAuthorizedAddressTxId = erc20AddAuthorizedAddressSubmitLog.args.transactionId; const erc721AddAuthorizedAddressTxId = erc721AddAuthorizedAddressSubmitLog.args.transactionId; + await multiSigWrapper.confirmTransactionAsync(registerAssetProxyTxId, owners[1]); await multiSigWrapper.confirmTransactionAsync(erc20AddAuthorizedAddressTxId, owners[1]); await multiSigWrapper.confirmTransactionAsync(erc721AddAuthorizedAddressTxId, owners[1]); await increaseTimeAndMineBlockAsync(SECONDS_TIME_LOCKED.toNumber()); diff --git a/packages/contracts/test/utils/constants.ts b/packages/contracts/test/utils/constants.ts index 7f3ad62e1..e8995f9d6 100644 --- a/packages/contracts/test/utils/constants.ts +++ b/packages/contracts/test/utils/constants.ts @@ -24,9 +24,10 @@ export const constants = { // ensure we always use the minimum interval. AWAIT_TRANSACTION_MINED_MS: 0, MAX_ETHERTOKEN_WITHDRAW_GAS: 43000, + MAX_EXECUTE_TRANSACTION_GAS: 1000000, MAX_TOKEN_TRANSFERFROM_GAS: 80000, MAX_TOKEN_APPROVE_GAS: 60000, - TRANSFER_FROM_GAS: 150000, + MAX_TRANSFER_FROM_GAS: 150000, DUMMY_TOKEN_NAME: '', DUMMY_TOKEN_SYMBOL: '', DUMMY_TOKEN_DECIMALS: new BigNumber(18), diff --git a/packages/contracts/test/utils/multi_sig_wrapper.ts b/packages/contracts/test/utils/multi_sig_wrapper.ts index 6e7746dfc..2024c177d 100644 --- a/packages/contracts/test/utils/multi_sig_wrapper.ts +++ b/packages/contracts/test/utils/multi_sig_wrapper.ts @@ -6,6 +6,7 @@ import * as _ from 'lodash'; import { AssetProxyOwnerContract } from '../../generated_contract_wrappers/asset_proxy_owner'; import { MultiSigWalletContract } from '../../generated_contract_wrappers/multi_sig_wallet'; +import { constants } from './constants'; import { LogDecoder } from './log_decoder'; export class MultiSigWrapper { @@ -36,7 +37,10 @@ export class MultiSigWrapper { return tx; } public async executeTransactionAsync(txId: BigNumber, from: string): Promise<TransactionReceiptWithDecodedLogs> { - const txHash = await this._multiSig.executeTransaction.sendTransactionAsync(txId, { from }); + const txHash = await this._multiSig.executeTransaction.sendTransactionAsync(txId, { + from, + gas: constants.MAX_EXECUTE_TRANSACTION_GAS, + }); const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } @@ -48,6 +52,7 @@ export class MultiSigWrapper { const txHash = await (this ._multiSig as AssetProxyOwnerContract).executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(txId, { from, + gas: constants.MAX_EXECUTE_TRANSACTION_GAS, }); const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; |