Use 0.4.10 in AssetProxyOwner, add readBytes4 to contract and remove LibBytes

This commit is contained in:
Amir Bandeali 2018-07-06 14:12:53 -07:00
parent 2625cbbfed
commit 092ca6bcf5
7 changed files with 76 additions and 17 deletions

View File

@ -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;
}
}

View File

@ -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;
}
}

View File

@ -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)

View File

@ -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,
);

View File

@ -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());

View File

@ -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),

View File

@ -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;