aboutsummaryrefslogtreecommitdiffstats
path: root/packages
diff options
context:
space:
mode:
authorAmir Bandeali <abandeali1@gmail.com>2018-07-11 03:46:53 +0800
committerGitHub <noreply@github.com>2018-07-11 03:46:53 +0800
commitaf7e02de2789efd3e73f60bf92b700ccec8ba520 (patch)
tree892cc61220dd4bbc769ef0e43c034b9a780103b0 /packages
parent2625cbbfed087dae55c62950a9260145cf5ca8e6 (diff)
parent092ca6bcf5de2d94ff503f11233c87d428eedcb8 (diff)
downloaddexon-sol-tools-af7e02de2789efd3e73f60bf92b700ccec8ba520.tar
dexon-sol-tools-af7e02de2789efd3e73f60bf92b700ccec8ba520.tar.gz
dexon-sol-tools-af7e02de2789efd3e73f60bf92b700ccec8ba520.tar.bz2
dexon-sol-tools-af7e02de2789efd3e73f60bf92b700ccec8ba520.tar.lz
dexon-sol-tools-af7e02de2789efd3e73f60bf92b700ccec8ba520.tar.xz
dexon-sol-tools-af7e02de2789efd3e73f60bf92b700ccec8ba520.tar.zst
dexon-sol-tools-af7e02de2789efd3e73f60bf92b700ccec8ba520.zip
Merge pull request #837 from 0xProject/fix/contracts/assetProxyOwnerVersion
Use 0.4.10 in AssetProxyOwner
Diffstat (limited to 'packages')
-rw-r--r--packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol27
-rw-r--r--packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol23
-rw-r--r--packages/contracts/src/2.0.0/utils/LibBytes/LibBytes.sol3
-rw-r--r--packages/contracts/test/asset_proxy/proxies.ts6
-rw-r--r--packages/contracts/test/multisig/asset_proxy_owner.ts24
-rw-r--r--packages/contracts/test/utils/constants.ts3
-rw-r--r--packages/contracts/test/utils/multi_sig_wrapper.ts7
7 files changed, 76 insertions, 17 deletions
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/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/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;