From 23df5cc201b3494c39389ac4cb4de346d7cbbd00 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Fri, 4 May 2018 14:30:49 -0700 Subject: Update MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress to use a mapping of registered proxies --- ...etWithTimeLockExceptRemoveAuthorizedAddress.sol | 66 ++++++++++++++++------ 1 file changed, 48 insertions(+), 18 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/contracts/current/multisig/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol b/packages/contracts/src/contracts/current/multisig/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol index 3d44e4c07..cc5808813 100644 --- a/packages/contracts/src/contracts/current/multisig/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol +++ b/packages/contracts/src/contracts/current/multisig/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol @@ -18,15 +18,19 @@ pragma solidity ^0.4.10; -import { MultiSigWalletWithTimeLock } from "../MultiSigWalletWithTimeLock/MultiSigWalletWithTimeLock.sol"; +import "../MultiSigWalletWithTimeLock/MultiSigWalletWithTimeLock.sol"; -contract MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress is MultiSigWalletWithTimeLock { +contract MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress is + MultiSigWalletWithTimeLock +{ + event AssetProxyRegistration(address assetProxyContract, bool isRegistered); - address public TOKEN_TRANSFER_PROXY_CONTRACT; + // Mapping of AssetProxy contract address => approved to execute removeAuthorizedAddress without time lock. + mapping (address => bool) public isAssetProxyRegistered; - modifier validRemoveAuthorizedAddressTx(uint transactionId) { + modifier validRemoveAuthorizedAddressTx(uint256 transactionId) { Transaction storage tx = transactions[transactionId]; - require(tx.destination == TOKEN_TRANSFER_PROXY_CONTRACT); + require(isAssetProxyRegistered[tx.destination]); require(isFunctionRemoveAuthorizedAddress(tx.data)); _; } @@ -35,21 +39,36 @@ contract MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress is MultiSigWall /// @param _owners List of initial owners. /// @param _required Number of required confirmations. /// @param _secondsTimeLocked Duration needed after a transaction is confirmed and before it becomes executable, in seconds. - /// @param _tokenTransferProxy Address of TokenTransferProxy contract. + /// @param _assetProxyContracts Array of AssetProxy contract addresses. function MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress( - address[] _owners, - uint _required, - uint _secondsTimeLocked, - address _tokenTransferProxy) + address[] memory _owners, + uint256 _required, + uint256 _secondsTimeLocked, + address[] memory _assetProxyContracts) public MultiSigWalletWithTimeLock(_owners, _required, _secondsTimeLocked) { - TOKEN_TRANSFER_PROXY_CONTRACT = _tokenTransferProxy; + for (uint256 i = 0; i < _assetProxyContracts.length; i++) { + require(_assetProxyContracts[i] != address(0)); + isAssetProxyRegistered[_assetProxyContracts[i]] = true; + } + } + + /// @dev Sets approval for calling removeAuthorizedAddress on an AssetProxy contract without a timelock. + /// @param assetProxyContract Address of AssetProxy contract. + /// @param isRegistered Status of approval for AssetProxy contract. + function registerAssetProxy(address assetProxyContract, bool isRegistered) + public + onlyWallet + notNull(assetProxyContract) + { + isAssetProxyRegistered[assetProxyContract] = isRegistered; + AssetProxyRegistration(assetProxyContract, isRegistered); } /// @dev Allows execution of removeAuthorizedAddress without time lock. /// @param transactionId Transaction ID. - function executeRemoveAuthorizedAddress(uint transactionId) + function executeRemoveAuthorizedAddress(uint256 transactionId) public notExecuted(transactionId) fullyConfirmed(transactionId) @@ -68,15 +87,26 @@ contract MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress is MultiSigWall /// @dev Compares first 4 bytes of byte array to removeAuthorizedAddress function signature. /// @param data Transaction data. /// @return Successful if data is a call to removeAuthorizedAddress. - function isFunctionRemoveAuthorizedAddress(bytes data) + function isFunctionRemoveAuthorizedAddress(bytes memory data) public - constant + pure returns (bool) { - bytes4 removeAuthorizedAddressSignature = bytes4(sha3("removeAuthorizedAddress(address)")); - for (uint i = 0; i < 4; i++) { - require(data[i] == removeAuthorizedAddressSignature[i]); - } + bytes4 removeAuthorizedAddressSelector = bytes4(keccak256("removeAuthorizedAddress(address)")); + bytes4 first4Bytes = readFirst4(data); + require(removeAuthorizedAddressSelector == first4Bytes); return true; } + + function readFirst4(bytes memory data) + public + pure + returns (bytes4 result) + { + require(data.length >= 4); + assembly { + result := mload(add(data, 32)) + } + return result; + } } -- cgit v1.2.3 From cc44f5f75d8a1152be9fd49491a9c194cfa2b285 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 20 May 2018 20:10:04 -0700 Subject: Update multisig tests and utils --- packages/contracts/src/utils/multi_sig_wrapper.ts | 68 ++-- packages/contracts/test/exchange/core.ts | 5 - .../contracts/test/multi_sig_with_time_lock.ts | 150 +++----- ...i_sig_with_time_lock_except_remove_auth_addr.ts | 385 ++++++++++++++------- 4 files changed, 355 insertions(+), 253 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/utils/multi_sig_wrapper.ts b/packages/contracts/src/utils/multi_sig_wrapper.ts index 41a1dd8d9..9a2b26d77 100644 --- a/packages/contracts/src/utils/multi_sig_wrapper.ts +++ b/packages/contracts/src/utils/multi_sig_wrapper.ts @@ -1,43 +1,61 @@ -import { AbiDefinition, MethodAbi } from '@0xproject/types'; +import { TransactionReceiptWithDecodedLogs, ZeroEx } from '0x.js'; import { BigNumber } from '@0xproject/utils'; -import ABI = require('ethereumjs-abi'); -import ethUtil = require('ethereumjs-util'); import * as _ from 'lodash'; import * as Web3 from 'web3'; import { MultiSigWalletContract } from '../contract_wrappers/generated/multi_sig_wallet'; +import { MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract } from '../contract_wrappers/generated/multi_sig_wallet_with_time_lock_except_remove_authorized_address'; -import { TransactionDataParams } from './types'; +import { constants } from './constants'; +import { LogDecoder } from './log_decoder'; export class MultiSigWrapper { private _multiSig: MultiSigWalletContract; - public static encodeFnArgs(name: string, abi: AbiDefinition[], args: any[]): string { - const abiEntity = _.find(abi, { name }) as MethodAbi; - if (_.isUndefined(abiEntity)) { - throw new Error(`Did not find abi entry for name: ${name}`); - } - const types = _.map(abiEntity.inputs, input => input.type); - const funcSig = ethUtil.bufferToHex(ABI.methodID(name, types)); - const argsData = _.map(args, arg => { - const target = _.isBoolean(arg) ? +arg : arg; - const targetBuff = ethUtil.toBuffer(target); - return ethUtil.setLengthLeft(targetBuff, 32).toString('hex'); - }); - return funcSig + argsData.join(''); - } - constructor(multiSigContract: MultiSigWalletContract) { + private _logDecoder: LogDecoder = new LogDecoder(constants.TESTRPC_NETWORK_ID); + private _zeroEx: ZeroEx; + constructor(multiSigContract: MultiSigWalletContract, zeroEx: ZeroEx) { this._multiSig = multiSigContract; + this._zeroEx = zeroEx; } public async submitTransactionAsync( destination: string, + data: string, from: string, - dataParams: TransactionDataParams, - value: BigNumber = new BigNumber(0), - ): Promise { - const { name, abi, args = [] } = dataParams; - const encoded = MultiSigWrapper.encodeFnArgs(name, abi, args); - return this._multiSig.submitTransaction.sendTransactionAsync(destination, value, encoded, { + opts: { value?: BigNumber } = {}, + ): Promise { + const value = _.isUndefined(opts.value) ? new BigNumber(0) : opts.value; + const txHash = await this._multiSig.submitTransaction.sendTransactionAsync(destination, value, data, { from, }); + const tx = await this._getTxWithDecodedMultiSigLogs(txHash); + return tx; + } + public async confirmTransactionAsync(txId: BigNumber, from: string): Promise { + const txHash = await this._multiSig.confirmTransaction.sendTransactionAsync(txId, { from }); + const tx = await this._getTxWithDecodedMultiSigLogs(txHash); + return tx; + } + public async executeTransactionAsync(txId: BigNumber, from: string): Promise { + const txHash = await this._multiSig.executeTransaction.sendTransactionAsync(txId, { from }); + const tx = await this._getTxWithDecodedMultiSigLogs(txHash); + return tx; + } + public async executeRemoveAuthorizedAddressAsync( + txId: BigNumber, + from: string, + ): Promise { + const txHash = await (this + ._multiSig as MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract).executeRemoveAuthorizedAddress.sendTransactionAsync( + txId, + { from }, + ); + const tx = await this._getTxWithDecodedMultiSigLogs(txHash); + return tx; + } + private async _getTxWithDecodedMultiSigLogs(txHash: string) { + const tx = await this._zeroEx.awaitTransactionMinedAsync(txHash); + tx.logs = _.filter(tx.logs, log => log.address === this._multiSig.address); + tx.logs = _.map(tx.logs, log => this._logDecoder.decodeLogOrThrow(log)); + return tx; } } diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index 4776216ca..84f25361c 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -127,11 +127,6 @@ describe('Exchange core', () => { afterEach(async () => { await blockchainLifecycle.revertAsync(); }); - describe('internal functions', () => { - it('should include transferViaTokenTransferProxy', () => { - expect((exchange as any).transferViaTokenTransferProxy).to.be.undefined(); - }); - }); describe('fillOrder', () => { beforeEach(async () => { diff --git a/packages/contracts/test/multi_sig_with_time_lock.ts b/packages/contracts/test/multi_sig_with_time_lock.ts index 732077df2..c369ca63a 100644 --- a/packages/contracts/test/multi_sig_with_time_lock.ts +++ b/packages/contracts/test/multi_sig_with_time_lock.ts @@ -1,40 +1,39 @@ import { BlockchainLifecycle, web3Factory } from '@0xproject/dev-utils'; import { LogWithDecodedArgs } from '@0xproject/types'; -import { AbiDecoder, BigNumber } from '@0xproject/utils'; +import { BigNumber } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; import * as chai from 'chai'; +import * as _ from 'lodash'; import 'make-promises-safe'; import * as Web3 from 'web3'; -import * as multiSigWalletJSON from '../../build/contracts/MultiSigWalletWithTimeLock.json'; -import { MultiSigWalletContract } from '../src/contract_wrappers/generated/multi_sig_wallet'; -import { MultiSigWalletWithTimeLockContract } from '../src/contract_wrappers/generated/multi_sig_wallet_with_time_lock'; +import { + MultiSigWalletWithTimeLockContract, + SubmissionContractEventArgs, +} from '../src/contract_wrappers/generated/multi_sig_wallet_with_time_lock'; import { artifacts } from '../src/utils/artifacts'; import { chaiSetup } from '../src/utils/chai_setup'; import { constants } from '../src/utils/constants'; import { MultiSigWrapper } from '../src/utils/multi_sig_wrapper'; -import { SubmissionContractEventArgs } from '../src/utils/types'; import { provider, txDefaults, web3Wrapper } from '../src/utils/web3_wrapper'; -const MULTI_SIG_ABI = artifacts.MultiSigWalletWithTimeLock.compilerOutput.abi; chaiSetup.configure(); const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); -const abiDecoder = new AbiDecoder([MULTI_SIG_ABI]); describe('MultiSigWalletWithTimeLock', () => { let owners: string[]; + const requiredApprovals = new BigNumber(2); + const SECONDS_TIME_LOCKED = new BigNumber(1000000); + before(async () => { const accounts = await web3Wrapper.getAvailableAddressesAsync(); owners = [accounts[0], accounts[1]]; }); - const SIGNATURES_REQUIRED = new BigNumber(2); - const SECONDS_TIME_LOCKED = new BigNumber(10000); let multiSig: MultiSigWalletWithTimeLockContract; let multiSigWrapper: MultiSigWrapper; - let txId: BigNumber; - let initialSecondsTimeLocked: number; + beforeEach(async () => { await blockchainLifecycle.startAsync(); }); @@ -51,19 +50,18 @@ describe('MultiSigWalletWithTimeLock', () => { await blockchainLifecycle.revertAsync(); }); before('deploy a wallet', async () => { + const secondsTimeLocked = new BigNumber(0); multiSig = await MultiSigWalletWithTimeLockContract.deployFrom0xArtifactAsync( artifacts.MultiSigWalletWithTimeLock, provider, txDefaults, owners, - SIGNATURES_REQUIRED, - new BigNumber(0), + requiredApprovals, + secondsTimeLocked, ); - multiSigWrapper = new MultiSigWrapper((multiSig as any) as MultiSigWalletContract); - - const secondsTimeLocked = await multiSig.secondsTimeLocked.callAsync(); - initialSecondsTimeLocked = secondsTimeLocked.toNumber(); + multiSigWrapper = new MultiSigWrapper(multiSig, provider); }); + it('should throw when not called by wallet', async () => { return expect( multiSig.changeTimeLock.sendTransactionAsync(SECONDS_TIME_LOCKED, { from: owners[0] }), @@ -72,22 +70,11 @@ describe('MultiSigWalletWithTimeLock', () => { it('should throw without enough confirmations', async () => { const destination = multiSig.address; - const from = owners[0]; - const dataParams = { - name: 'changeTimeLock', - abi: MULTI_SIG_ABI, - args: [SECONDS_TIME_LOCKED.toNumber()], - }; - const txHash = await multiSigWrapper.submitTransactionAsync(destination, from, dataParams); - const txReceipt = await web3Wrapper.awaitTransactionSuccessAsync( - txHash, - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const log = abiDecoder.tryToDecodeLogOrNoop(txReceipt.logs[0]) as LogWithDecodedArgs< - SubmissionContractEventArgs - >; + const changeTimeLockData = multiSig.changeTimeLock.getABIEncodedTransactionData(SECONDS_TIME_LOCKED); + const res = await multiSigWrapper.submitTransactionAsync(destination, changeTimeLockData, owners[0]); + const log = res.logs[0] as LogWithDecodedArgs; + const txId = log.args.transactionId; - txId = log.args.transactionId; return expect( multiSig.executeTransaction.sendTransactionAsync(txId, { from: owners[0] }), ).to.be.rejectedWith(constants.REVERT); @@ -95,28 +82,13 @@ describe('MultiSigWalletWithTimeLock', () => { it('should set confirmation time with enough confirmations', async () => { const destination = multiSig.address; - const from = owners[0]; - const dataParams = { - name: 'changeTimeLock', - abi: MULTI_SIG_ABI, - args: [SECONDS_TIME_LOCKED.toNumber()], - }; - let txHash = await multiSigWrapper.submitTransactionAsync(destination, from, dataParams); - const txReceipt = await web3Wrapper.awaitTransactionSuccessAsync( - txHash, - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const log = abiDecoder.tryToDecodeLogOrNoop(txReceipt.logs[0]) as LogWithDecodedArgs< - SubmissionContractEventArgs - >; + const changeTimeLockData = multiSig.changeTimeLock.getABIEncodedTransactionData(SECONDS_TIME_LOCKED); + const subRes = await multiSigWrapper.submitTransactionAsync(destination, changeTimeLockData, owners[0]); + const subLog = subRes.logs[0] as LogWithDecodedArgs; + const txId = subLog.args.transactionId; - txId = log.args.transactionId; - txHash = await multiSig.confirmTransaction.sendTransactionAsync(txId, { from: owners[1] }); - const res = await web3Wrapper.awaitTransactionSuccessAsync( - txHash, - constants.AWAIT_TRANSACTION_MINED_MS, - ); - expect(res.logs).to.have.length(2); + const confirmRes = await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); + expect(confirmRes.logs).to.have.length(2); const blockNum = await web3Wrapper.getBlockNumberAsync(); const blockInfo = await web3Wrapper.getBlockAsync(blockNum); @@ -128,33 +100,13 @@ describe('MultiSigWalletWithTimeLock', () => { it('should be executable with enough confirmations and secondsTimeLocked of 0', async () => { const destination = multiSig.address; - const from = owners[0]; - const dataParams = { - name: 'changeTimeLock', - abi: MULTI_SIG_ABI, - args: [SECONDS_TIME_LOCKED.toNumber()], - }; - let txHash = await multiSigWrapper.submitTransactionAsync(destination, from, dataParams); - const txReceipt = await web3Wrapper.awaitTransactionSuccessAsync( - txHash, - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const log = abiDecoder.tryToDecodeLogOrNoop(txReceipt.logs[0]) as LogWithDecodedArgs< - SubmissionContractEventArgs - >; - - txId = log.args.transactionId; - txHash = await multiSig.confirmTransaction.sendTransactionAsync(txId, { from: owners[1] }); - await web3Wrapper.awaitTransactionSuccessAsync(txHash, constants.AWAIT_TRANSACTION_MINED_MS); - - expect(initialSecondsTimeLocked).to.be.equal(0); + const changeTimeLockData = multiSig.changeTimeLock.getABIEncodedTransactionData(SECONDS_TIME_LOCKED); + const subRes = await multiSigWrapper.submitTransactionAsync(destination, changeTimeLockData, owners[0]); + const subLog = subRes.logs[0] as LogWithDecodedArgs; + const txId = subLog.args.transactionId; - txHash = await multiSig.executeTransaction.sendTransactionAsync(txId, { from: owners[0] }); - const res = await web3Wrapper.awaitTransactionSuccessAsync( - txHash, - constants.AWAIT_TRANSACTION_MINED_MS, - ); - expect(res.logs).to.have.length(2); + await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); + await multiSigWrapper.executeTransactionAsync(txId, owners[1]); const secondsTimeLocked = new BigNumber(await multiSig.secondsTimeLocked.callAsync()); expect(secondsTimeLocked).to.be.bignumber.equal(SECONDS_TIME_LOCKED); @@ -167,45 +119,29 @@ describe('MultiSigWalletWithTimeLock', () => { after(async () => { await blockchainLifecycle.revertAsync(); }); - before('deploy a wallet', async () => { + let txId: BigNumber; + const newSecondsTimeLocked = new BigNumber(0); + before('deploy a wallet, submit transaction to change timelock, and confirm the transaction', async () => { multiSig = await MultiSigWalletWithTimeLockContract.deployFrom0xArtifactAsync( artifacts.MultiSigWalletWithTimeLock, provider, txDefaults, owners, - SIGNATURES_REQUIRED, + requiredApprovals, SECONDS_TIME_LOCKED, ); - multiSigWrapper = new MultiSigWrapper((multiSig as any) as MultiSigWalletContract); + multiSigWrapper = new MultiSigWrapper(multiSig, provider); - const secondsTimeLocked = await multiSig.secondsTimeLocked.callAsync(); - initialSecondsTimeLocked = secondsTimeLocked.toNumber(); - const destination = multiSig.address; - const from = owners[0]; - const dataParams = { - name: 'changeTimeLock', - abi: MULTI_SIG_ABI, - args: [newSecondsTimeLocked], - }; - let txHash = await multiSigWrapper.submitTransactionAsync(destination, from, dataParams); - let txReceipt = await web3Wrapper.awaitTransactionSuccessAsync( - txHash, - constants.AWAIT_TRANSACTION_MINED_MS, + const changeTimeLockData = multiSig.changeTimeLock.getABIEncodedTransactionData(newSecondsTimeLocked); + const res = await multiSigWrapper.submitTransactionAsync( + multiSig.address, + changeTimeLockData, + owners[0], ); - const log = abiDecoder.tryToDecodeLogOrNoop(txReceipt.logs[0]) as LogWithDecodedArgs< - SubmissionContractEventArgs - >; + const log = res.logs[0] as LogWithDecodedArgs; txId = log.args.transactionId; - txHash = await multiSig.confirmTransaction.sendTransactionAsync(txId, { - from: owners[1], - }); - txReceipt = await web3Wrapper.awaitTransactionSuccessAsync( - txHash, - constants.AWAIT_TRANSACTION_MINED_MS, - ); - expect(txReceipt.logs).to.have.length(2); + await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); }); - const newSecondsTimeLocked = 0; it('should throw if it has enough confirmations but is not past the time lock', async () => { return expect( multiSig.executeTransaction.sendTransactionAsync(txId, { from: owners[0] }), diff --git a/packages/contracts/test/multi_sig_with_time_lock_except_remove_auth_addr.ts b/packages/contracts/test/multi_sig_with_time_lock_except_remove_auth_addr.ts index dcd206b1c..e384e093b 100644 --- a/packages/contracts/test/multi_sig_with_time_lock_except_remove_auth_addr.ts +++ b/packages/contracts/test/multi_sig_with_time_lock_except_remove_auth_addr.ts @@ -1,67 +1,52 @@ -/* - * - * @TODO: Before deploying, the MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress contract must be updated - * to have a mapping of all approved addresses. These tests must be updated appropriately. - * For now, these tests have been commented out by @hysz (greg@0xproject.com). - * import { LogWithDecodedArgs, ZeroEx } from '0x.js'; -import { BlockchainLifecycle, devConstants, web3Factory } from '@0xproject/dev-utils'; -import { AbiDecoder, BigNumber } from '@0xproject/utils'; -import { Web3Wrapper } from '@0xproject/web3-wrapper'; +import { BlockchainLifecycle } from '@0xproject/dev-utils'; +import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; +import * as _ from 'lodash'; import 'make-promises-safe'; import * as Web3 from 'web3'; -import { MultiSigWalletContract } from '../src/contract_wrappers/generated/multi_sig_wallet'; -import { MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract } from '../src/contract_wrappers/generated/multi_sig_wallet_with_time_lock_except_remove_authorized_address'; -import { TokenTransferProxyContract } from '../src/contract_wrappers/generated/token_transfer_proxy'; +import { AuthorizableContract } from '../src/contract_wrappers/generated/authorizable'; +import { + AssetProxyRegistrationContractEventArgs, + ExecutionContractEventArgs, + ExecutionFailureContractEventArgs, + MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract, + SubmissionContractEventArgs, +} from '../src/contract_wrappers/generated/multi_sig_wallet_with_time_lock_except_remove_authorized_address'; import { artifacts } from '../src/utils/artifacts'; +import { chaiSetup } from '../src/utils/chai_setup'; import { constants } from '../src/utils/constants'; -import { crypto } from '../src/utils/crypto'; import { MultiSigWrapper } from '../src/utils/multi_sig_wrapper'; -import { ContractName, SubmissionContractEventArgs, TransactionDataParams } from '../src/utils/types'; - -import { chaiSetup } from './utils/chai_setup'; - -import { provider, txDefaults, web3Wrapper } from './utils/web3_wrapper'; - -const PROXY_ABI = artifacts.TokenTransferProxy.compilerOutput.abi; -const MUTISIG_WALLET_WITH_TIME_LOCK_EXCEPT_REMOVE_AUTHORIZED_ADDRESS_ABI = - artifacts.MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.compilerOutput.abi; +import { provider, txDefaults, web3Wrapper } from '../src/utils/web3_wrapper'; chaiSetup.configure(); const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); -const abiDecoder = new AbiDecoder([MUTISIG_WALLET_WITH_TIME_LOCK_EXCEPT_REMOVE_AUTHORIZED_ADDRESS_ABI]); +const zeroEx = new ZeroEx(provider, { networkId: constants.TESTRPC_NETWORK_ID }); describe('MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress', () => { - const zeroEx = new ZeroEx(provider, { networkId: constants.TESTRPC_NETWORK_ID }); let owners: string[]; + let authorized: string; const requiredApprovals = new BigNumber(2); const SECONDS_TIME_LOCKED = new BigNumber(1000000); - // initialize fake addresses - let authorizedAddress: string; - let unauthorizedAddress: string; - - let tokenTransferProxy: TokenTransferProxyContract; + let erc20Proxy: AuthorizableContract; + let erc721Proxy: AuthorizableContract; let multiSig: MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract; let multiSigWrapper: MultiSigWrapper; - let validDestination: string; before(async () => { const accounts = await web3Wrapper.getAvailableAddressesAsync(); owners = [accounts[0], accounts[1]]; - [authorizedAddress, unauthorizedAddress] = accounts; - const initialOwner = accounts[0]; - tokenTransferProxy = await TokenTransferProxyContract.deployFrom0xArtifactAsync( - artifacts.TokenTransferProxy, + const initialOwner = (authorized = accounts[0]); + erc20Proxy = await AuthorizableContract.deployFrom0xArtifactAsync(artifacts.Authorizable, provider, txDefaults); + erc721Proxy = await AuthorizableContract.deployFrom0xArtifactAsync( + artifacts.Authorizable, provider, txDefaults, ); - await tokenTransferProxy.addAuthorizedAddress.sendTransactionAsync(authorizedAddress, { - from: initialOwner, - }); + const defaultAssetProxyContractAddresses: string[] = []; multiSig = await MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract.deployFrom0xArtifactAsync( artifacts.MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress, provider, @@ -69,13 +54,11 @@ describe('MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress', () => { owners, requiredApprovals, SECONDS_TIME_LOCKED, - tokenTransferProxy.address, + defaultAssetProxyContractAddresses, ); - await tokenTransferProxy.transferOwnership.sendTransactionAsync(multiSig.address, { - from: initialOwner, - }); - multiSigWrapper = new MultiSigWrapper((multiSig as any) as MultiSigWalletContract); - validDestination = tokenTransferProxy.address; + multiSigWrapper = new MultiSigWrapper(multiSig, zeroEx); + await erc20Proxy.transferOwnership.sendTransactionAsync(multiSig.address, { from: initialOwner }); + await erc721Proxy.transferOwnership.sendTransactionAsync(multiSig.address, { from: initialOwner }); }); beforeEach(async () => { await blockchainLifecycle.startAsync(); @@ -84,31 +67,195 @@ describe('MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress', () => { await blockchainLifecycle.revertAsync(); }); + describe('constructor', () => { + it('should register passed in assetProxyContracts', async () => { + const assetProxyContractAddresses = [erc20Proxy.address, erc721Proxy.address]; + const newMultiSig = await MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract.deployFrom0xArtifactAsync( + artifacts.MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress, + provider, + txDefaults, + owners, + requiredApprovals, + SECONDS_TIME_LOCKED, + assetProxyContractAddresses, + ); + const isErc20ProxyRegistered = await newMultiSig.isAssetProxyRegistered.callAsync(erc20Proxy.address); + const isErc721ProxyRegistered = await newMultiSig.isAssetProxyRegistered.callAsync(erc721Proxy.address); + expect(isErc20ProxyRegistered).to.equal(true); + expect(isErc721ProxyRegistered).to.equal(true); + }); + it('should throw if a null address is included in assetProxyContracts', async () => { + const assetProxyContractAddresses = [erc20Proxy.address, ZeroEx.NULL_ADDRESS]; + expect( + MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract.deployFrom0xArtifactAsync( + artifacts.MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress, + provider, + txDefaults, + owners, + requiredApprovals, + SECONDS_TIME_LOCKED, + assetProxyContractAddresses, + ), + ).to.be.rejectedWith(constants.REVERT); + }); + }); + describe('readFirst4', () => { + it('should return the first 4 bytes of a byte array of arbitrary length', async () => { + const addAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData(owners[0]); + const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( + owners[0], + ); + const expectedAddAuthorizedAddressSelector = addAuthorizedAddressData.slice(0, 10); + const expectedRemoveAuthorizedAddressSelector = removeAuthorizedAddressData.slice(0, 10); + const [addAuthorizedAddressSelector, removeAuthorizedAddressSelector] = await Promise.all([ + multiSig.readFirst4.callAsync(addAuthorizedAddressData), + multiSig.readFirst4.callAsync(removeAuthorizedAddressData), + ]); + expect(expectedAddAuthorizedAddressSelector).to.equal(addAuthorizedAddressSelector); + expect(expectedRemoveAuthorizedAddressSelector).to.equal(removeAuthorizedAddressSelector); + }); + }); + describe('isFunctionRemoveAuthorizedAddress', () => { it('should throw if data is not for removeAuthorizedAddress', async () => { - const data = MultiSigWrapper.encodeFnArgs('addAuthorizedAddress', PROXY_ABI, [owners[0]]); - return expect(multiSig.isFunctionRemoveAuthorizedAddress.callAsync(data)).to.be.rejectedWith( - constants.REVERT, + const notRemoveAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData( + owners[0], ); + return expect( + multiSig.isFunctionRemoveAuthorizedAddress.callAsync(notRemoveAuthorizedAddressData), + ).to.be.rejectedWith(constants.REVERT); }); it('should return true if data is for removeAuthorizedAddress', async () => { - const data = MultiSigWrapper.encodeFnArgs('removeAuthorizedAddress', PROXY_ABI, [owners[0]]); - const isFunctionRemoveAuthorizedAddress = await multiSig.isFunctionRemoveAuthorizedAddress.callAsync(data); + const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( + owners[0], + ); + const isFunctionRemoveAuthorizedAddress = await multiSig.isFunctionRemoveAuthorizedAddress.callAsync( + removeAuthorizedAddressData, + ); expect(isFunctionRemoveAuthorizedAddress).to.be.true(); }); }); + describe('registerAssetProxy', () => { + it('should throw if not called by multisig', async () => { + const isRegistered = true; + expect( + multiSig.registerAssetProxy.sendTransactionAsync(erc20Proxy.address, isRegistered, { from: owners[0] }), + ).to.be.rejectedWith(constants.REVERT); + }); + + it('should register an address if called by multisig after timelock', async () => { + const addressToRegister = erc20Proxy.address; + const isRegistered = true; + const registerAssetProxyData = multiSig.registerAssetProxy.getABIEncodedTransactionData( + addressToRegister, + isRegistered, + ); + const submitTxRes = await multiSigWrapper.submitTransactionAsync( + multiSig.address, + registerAssetProxyData, + owners[0], + ); + const log = submitTxRes.logs[0] as LogWithDecodedArgs; + const txId = log.args.transactionId; + + await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); + await web3Wrapper.increaseTimeAsync(SECONDS_TIME_LOCKED.toNumber()); + + const executeTxRes = await multiSigWrapper.executeTransactionAsync(txId, owners[0]); + const registerLog = executeTxRes.logs[0] as LogWithDecodedArgs; + expect(registerLog.args.assetProxyContract).to.equal(addressToRegister); + expect(registerLog.args.isRegistered).to.equal(isRegistered); + + const isAssetProxyRegistered = await multiSig.isAssetProxyRegistered.callAsync(addressToRegister); + expect(isAssetProxyRegistered).to.equal(isRegistered); + }); + + it('should fail if registering a null address', async () => { + const addressToRegister = ZeroEx.NULL_ADDRESS; + const isRegistered = true; + const registerAssetProxyData = multiSig.registerAssetProxy.getABIEncodedTransactionData( + addressToRegister, + isRegistered, + ); + const submitTxRes = await multiSigWrapper.submitTransactionAsync( + multiSig.address, + registerAssetProxyData, + owners[0], + ); + const log = submitTxRes.logs[0] as LogWithDecodedArgs; + const txId = log.args.transactionId; + + await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); + await web3Wrapper.increaseTimeAsync(SECONDS_TIME_LOCKED.toNumber()); + + const executeTxRes = await multiSigWrapper.executeTransactionAsync(txId, owners[0]); + const failureLog = executeTxRes.logs[0] as LogWithDecodedArgs; + expect(failureLog.args.transactionId).to.be.bignumber.equal(txId); + + const isAssetProxyRegistered = await multiSig.isAssetProxyRegistered.callAsync(addressToRegister); + expect(isAssetProxyRegistered).to.equal(false); + }); + }); + describe('executeRemoveAuthorizedAddress', () => { + before('authorize both proxies and register erc20 proxy', async () => { + // Only register ERC20 proxy + const addressToRegister = erc20Proxy.address; + const isRegistered = true; + const registerAssetProxyData = multiSig.registerAssetProxy.getABIEncodedTransactionData( + addressToRegister, + isRegistered, + ); + const registerAssetProxySubmitRes = await multiSigWrapper.submitTransactionAsync( + multiSig.address, + registerAssetProxyData, + owners[0], + ); + const registerAssetProxySubmitLog = registerAssetProxySubmitRes.logs[0] as LogWithDecodedArgs< + SubmissionContractEventArgs + >; + const registerAssetProxyTxId = registerAssetProxySubmitLog.args.transactionId; + await multiSigWrapper.confirmTransactionAsync(registerAssetProxyTxId, owners[1]); + + const addAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData(authorized); + const erc20AddAuthorizedAddressSubmitRes = await multiSigWrapper.submitTransactionAsync( + erc20Proxy.address, + addAuthorizedAddressData, + owners[0], + ); + const erc721AddAuthorizedAddressSubmitRes = await multiSigWrapper.submitTransactionAsync( + erc721Proxy.address, + addAuthorizedAddressData, + owners[0], + ); + const erc20AddAuthorizedAddressSubmitLog = erc20AddAuthorizedAddressSubmitRes.logs[0] as LogWithDecodedArgs< + SubmissionContractEventArgs + >; + const erc721AddAuthorizedAddressSubmitLog = erc721AddAuthorizedAddressSubmitRes + .logs[0] as LogWithDecodedArgs; + const erc20AddAuthorizedAddressTxId = erc20AddAuthorizedAddressSubmitLog.args.transactionId; + const erc721AddAuthorizedAddressTxId = erc721AddAuthorizedAddressSubmitLog.args.transactionId; + + await multiSigWrapper.confirmTransactionAsync(erc20AddAuthorizedAddressTxId, owners[1]); + await multiSigWrapper.confirmTransactionAsync(erc721AddAuthorizedAddressTxId, owners[1]); + await web3Wrapper.increaseTimeAsync(SECONDS_TIME_LOCKED.toNumber()); + await multiSigWrapper.executeTransactionAsync(registerAssetProxyTxId, owners[0]); + await multiSigWrapper.executeTransactionAsync(erc20AddAuthorizedAddressTxId, owners[0]); + await multiSigWrapper.executeTransactionAsync(erc721AddAuthorizedAddressTxId, owners[0]); + }); + it('should throw without the required confirmations', async () => { - const dataParams: TransactionDataParams = { - name: 'removeAuthorizedAddress', - abi: PROXY_ABI, - args: [authorizedAddress], - }; - const txHash = await multiSigWrapper.submitTransactionAsync(validDestination, owners[0], dataParams); - const res = await zeroEx.awaitTransactionMinedAsync(txHash); - const log = abiDecoder.tryToDecodeLogOrNoop(res.logs[0]) as LogWithDecodedArgs; + const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( + authorized, + ); + const res = await multiSigWrapper.submitTransactionAsync( + erc20Proxy.address, + removeAuthorizedAddressData, + owners[0], + ); + const log = res.logs[0] as LogWithDecodedArgs; const txId = log.args.transactionId; return expect( @@ -116,25 +263,19 @@ describe('MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress', () => { ).to.be.rejectedWith(constants.REVERT); }); - it('should throw if tx destination is not the tokenTransferProxy', async () => { - const invalidTokenTransferProxy = await TokenTransferProxyContract.deployFrom0xArtifactAsync( - artifacts.TokenTransferProxy, - provider, - txDefaults, + it('should throw if tx destination is not registered', async () => { + const removeAuthorizedAddressData = erc721Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( + authorized, ); - const invalidDestination = invalidTokenTransferProxy.address; - const dataParams: TransactionDataParams = { - name: 'removeAuthorizedAddress', - abi: PROXY_ABI, - args: [authorizedAddress], - }; - const txHash = await multiSigWrapper.submitTransactionAsync(invalidDestination, owners[0], dataParams); - const res = await zeroEx.awaitTransactionMinedAsync(txHash); - const log = abiDecoder.tryToDecodeLogOrNoop(res.logs[0]) as LogWithDecodedArgs; + const res = await multiSigWrapper.submitTransactionAsync( + erc721Proxy.address, + removeAuthorizedAddressData, + owners[0], + ); + const log = res.logs[0] as LogWithDecodedArgs; const txId = log.args.transactionId; - await multiSig.confirmTransaction.sendTransactionAsync(txId, { from: owners[1] }); - const isConfirmed = await multiSig.isConfirmed.callAsync(txId); - expect(isConfirmed).to.be.true(); + + await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); return expect( multiSig.executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from: owners[1] }), @@ -142,64 +283,76 @@ describe('MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress', () => { }); it('should throw if tx data is not for removeAuthorizedAddress', async () => { - const dataParams: TransactionDataParams = { - name: 'addAuthorizedAddress', - abi: PROXY_ABI, - args: [unauthorizedAddress], - }; - const txHash = await multiSigWrapper.submitTransactionAsync(validDestination, owners[0], dataParams); - const res = await zeroEx.awaitTransactionMinedAsync(txHash); - const log = abiDecoder.tryToDecodeLogOrNoop(res.logs[0]) as LogWithDecodedArgs; + const newAuthorized = owners[1]; + const addAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData( + newAuthorized, + ); + const res = await multiSigWrapper.submitTransactionAsync( + erc20Proxy.address, + addAuthorizedAddressData, + owners[0], + ); + const log = res.logs[0] as LogWithDecodedArgs; const txId = log.args.transactionId; - await multiSig.confirmTransaction.sendTransactionAsync(txId, { from: owners[1] }); - const isConfirmed = await multiSig.isConfirmed.callAsync(txId); - expect(isConfirmed).to.be.true(); + + await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); return expect( multiSig.executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from: owners[1] }), ).to.be.rejectedWith(constants.REVERT); }); - it('should execute removeAuthorizedAddress for valid tokenTransferProxy if fully confirmed', async () => { - const dataParams: TransactionDataParams = { - name: 'removeAuthorizedAddress', - abi: PROXY_ABI, - args: [authorizedAddress], - }; - const txHash = await multiSigWrapper.submitTransactionAsync(validDestination, owners[0], dataParams); - const res = await zeroEx.awaitTransactionMinedAsync(txHash); - const log = abiDecoder.tryToDecodeLogOrNoop(res.logs[0]) as LogWithDecodedArgs; - const txId = log.args.transactionId; - await multiSig.confirmTransaction.sendTransactionAsync(txId, { from: owners[1] }); - const isConfirmed = await multiSig.isConfirmed.callAsync(txId); - expect(isConfirmed).to.be.true(); - await multiSig.executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from: owners[1] }); - const isAuthorized = await tokenTransferProxy.authorized.callAsync(authorizedAddress); - expect(isAuthorized).to.be.false(); + it('should execute removeAuthorizedAddress for registered address if fully confirmed', async () => { + const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( + authorized, + ); + const submitRes = await multiSigWrapper.submitTransactionAsync( + erc20Proxy.address, + removeAuthorizedAddressData, + owners[0], + ); + const submitLog = submitRes.logs[0] as LogWithDecodedArgs; + const txId = submitLog.args.transactionId; + + await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); + + const execRes = await multiSigWrapper.executeRemoveAuthorizedAddressAsync(txId, owners[0]); + const execLog = execRes.logs[0] as LogWithDecodedArgs; + expect(execLog.args.transactionId).to.be.bignumber.equal(txId); + + const tx = await multiSig.transactions.callAsync(txId); + const isExecuted = tx[3]; + expect(isExecuted).to.equal(true); + + const isAuthorized = await erc20Proxy.authorized.callAsync(authorized); + expect(isAuthorized).to.equal(false); }); it('should throw if already executed', async () => { - const dataParams: TransactionDataParams = { - name: 'removeAuthorizedAddress', - abi: PROXY_ABI, - args: [authorizedAddress], - }; - const txHash = await multiSigWrapper.submitTransactionAsync(validDestination, owners[0], dataParams); - const res = await zeroEx.awaitTransactionMinedAsync(txHash); - const log = abiDecoder.tryToDecodeLogOrNoop(res.logs[0]) as LogWithDecodedArgs; - const txId = log.args.transactionId; - await multiSig.confirmTransaction.sendTransactionAsync(txId, { from: owners[1] }); - const isConfirmed = await multiSig.isConfirmed.callAsync(txId); - expect(isConfirmed).to.be.true(); - await multiSig.executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from: owners[1] }); + const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( + authorized, + ); + const submitRes = await multiSigWrapper.submitTransactionAsync( + erc20Proxy.address, + removeAuthorizedAddressData, + owners[0], + ); + const submitLog = submitRes.logs[0] as LogWithDecodedArgs; + const txId = submitLog.args.transactionId; + + await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); + + const execRes = await multiSigWrapper.executeRemoveAuthorizedAddressAsync(txId, owners[0]); + const execLog = execRes.logs[0] as LogWithDecodedArgs; + expect(execLog.args.transactionId).to.be.bignumber.equal(txId); + const tx = await multiSig.transactions.callAsync(txId); const isExecuted = tx[3]; - expect(isExecuted).to.be.true(); + expect(isExecuted).to.equal(true); + return expect( multiSig.executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from: owners[1] }), ).to.be.rejectedWith(constants.REVERT); }); }); }); - -*/ -- cgit v1.2.3 From 22ad9e1e1a0c6d2fa5c1b1249d02ef119ef4517e Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Fri, 4 May 2018 19:09:01 -0700 Subject: Address feedback, rename contract to AssetProxyOwner --- packages/contracts/package.json | 2 +- ...etWithTimeLockExceptRemoveAuthorizedAddress.sol | 112 ------- .../protocol/AssetProxyOwner/AssetProxyOwner.sol | 119 +++++++ packages/contracts/src/utils/artifacts.ts | 4 +- packages/contracts/src/utils/multi_sig_wrapper.ts | 7 +- packages/contracts/src/utils/types.ts | 2 +- packages/contracts/test/asset_proxy_owner.ts | 358 +++++++++++++++++++++ ...i_sig_with_time_lock_except_remove_auth_addr.ts | 358 --------------------- 8 files changed, 483 insertions(+), 479 deletions(-) delete mode 100644 packages/contracts/src/contracts/current/multisig/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol create mode 100644 packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol create mode 100644 packages/contracts/test/asset_proxy_owner.ts delete mode 100644 packages/contracts/test/multi_sig_with_time_lock_except_remove_auth_addr.ts (limited to 'packages/contracts') diff --git a/packages/contracts/package.json b/packages/contracts/package.json index c36d991e1..e4e5b131d 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -28,7 +28,7 @@ "test:circleci": "yarn test" }, "config": { - "abis": "../migrations/artifacts/2.0.0/@(DummyERC20Token|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress|TestAssetProxyDispatcher|TestLibBytes|TestLibs|TestSignatureValidator|TokenRegistry|WETH9|ZRXToken).json" + "abis": "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock||TestAssetProxyDispatcher|TestLibBytes|TestLibs|TestSignatureValidator|TokenRegistry|WETH9|ZRXToken).json" }, "repository": { "type": "git", diff --git a/packages/contracts/src/contracts/current/multisig/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol b/packages/contracts/src/contracts/current/multisig/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol deleted file mode 100644 index cc5808813..000000000 --- a/packages/contracts/src/contracts/current/multisig/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol +++ /dev/null @@ -1,112 +0,0 @@ -/* - - Copyright 2018 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity ^0.4.10; - -import "../MultiSigWalletWithTimeLock/MultiSigWalletWithTimeLock.sol"; - -contract MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress is - MultiSigWalletWithTimeLock -{ - event AssetProxyRegistration(address assetProxyContract, bool isRegistered); - - // Mapping of AssetProxy contract address => approved to execute removeAuthorizedAddress without time lock. - mapping (address => bool) public isAssetProxyRegistered; - - modifier validRemoveAuthorizedAddressTx(uint256 transactionId) { - Transaction storage tx = transactions[transactionId]; - require(isAssetProxyRegistered[tx.destination]); - require(isFunctionRemoveAuthorizedAddress(tx.data)); - _; - } - - /// @dev Contract constructor sets initial owners, required number of confirmations, time lock, and tokenTransferProxy address. - /// @param _owners List of initial owners. - /// @param _required Number of required confirmations. - /// @param _secondsTimeLocked Duration needed after a transaction is confirmed and before it becomes executable, in seconds. - /// @param _assetProxyContracts Array of AssetProxy contract addresses. - function MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress( - address[] memory _owners, - uint256 _required, - uint256 _secondsTimeLocked, - address[] memory _assetProxyContracts) - public - MultiSigWalletWithTimeLock(_owners, _required, _secondsTimeLocked) - { - for (uint256 i = 0; i < _assetProxyContracts.length; i++) { - require(_assetProxyContracts[i] != address(0)); - isAssetProxyRegistered[_assetProxyContracts[i]] = true; - } - } - - /// @dev Sets approval for calling removeAuthorizedAddress on an AssetProxy contract without a timelock. - /// @param assetProxyContract Address of AssetProxy contract. - /// @param isRegistered Status of approval for AssetProxy contract. - function registerAssetProxy(address assetProxyContract, bool isRegistered) - public - onlyWallet - notNull(assetProxyContract) - { - isAssetProxyRegistered[assetProxyContract] = isRegistered; - AssetProxyRegistration(assetProxyContract, isRegistered); - } - - /// @dev Allows execution of removeAuthorizedAddress without time lock. - /// @param transactionId Transaction ID. - function executeRemoveAuthorizedAddress(uint256 transactionId) - public - notExecuted(transactionId) - fullyConfirmed(transactionId) - validRemoveAuthorizedAddressTx(transactionId) - { - Transaction storage tx = transactions[transactionId]; - tx.executed = true; - if (tx.destination.call.value(tx.value)(tx.data)) - Execution(transactionId); - else { - ExecutionFailure(transactionId); - tx.executed = false; - } - } - - /// @dev Compares first 4 bytes of byte array to removeAuthorizedAddress function signature. - /// @param data Transaction data. - /// @return Successful if data is a call to removeAuthorizedAddress. - function isFunctionRemoveAuthorizedAddress(bytes memory data) - public - pure - returns (bool) - { - bytes4 removeAuthorizedAddressSelector = bytes4(keccak256("removeAuthorizedAddress(address)")); - bytes4 first4Bytes = readFirst4(data); - require(removeAuthorizedAddressSelector == first4Bytes); - return true; - } - - function readFirst4(bytes memory data) - public - pure - returns (bytes4 result) - { - require(data.length >= 4); - assembly { - result := mload(add(data, 32)) - } - return result; - } -} diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol new file mode 100644 index 000000000..1b0f9c34f --- /dev/null +++ b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol @@ -0,0 +1,119 @@ +/* + + Copyright 2018 ZeroEx Intl. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +*/ + +pragma solidity ^0.4.10; + +import "../../multisig/MultiSigWalletWithTimeLock/MultiSigWalletWithTimeLock.sol"; + +contract AssetProxyOwner is + MultiSigWalletWithTimeLock +{ + + event AssetProxyRegistration(address assetProxyContract, bool isRegistered); + + // Mapping of AssetProxy contract address => approved to execute removeAuthorizedAddress without time lock. + mapping (address => bool) public isAssetProxyRegistered; + + bytes4 constant REMOVE_AUTHORIZED_ADDRESS_SELECTOR = bytes4(keccak256("removeAuthorizedAddress(address)")); + + modifier validRemoveAuthorizedAddressTx(uint256 transactionId) { + Transaction storage tx = transactions[transactionId]; + require(isAssetProxyRegistered[tx.destination]); + require(isFunctionRemoveAuthorizedAddress(tx.data)); + _; + } + + /// @dev Contract constructor sets initial owners, required number of confirmations, + /// time lock, and list of AssetProxy addresses. + /// @param _owners List of initial owners. + /// @param _required Number of required confirmations. + /// @param _secondsTimeLocked Duration needed after a transaction is confirmed and before it becomes executable, in seconds. + /// @param _assetProxyContracts Array of AssetProxy contract addresses. + function AssetProxyOwner( + address[] memory _owners, + uint256 _required, + uint256 _secondsTimeLocked, + address[] memory _assetProxyContracts) + public + MultiSigWalletWithTimeLock(_owners, _required, _secondsTimeLocked) + { + for (uint256 i = 0; i < _assetProxyContracts.length; i++) { + require(_assetProxyContracts[i] != address(0)); + isAssetProxyRegistered[_assetProxyContracts[i]] = true; + } + } + + /// @dev Registers or deregisters an AssetProxy to be able to execute + /// removeAuthorizedAddress without a timelock. + /// @param assetProxyContract Address of AssetProxy contract. + /// @param isRegistered Status of approval for AssetProxy contract. + function registerAssetProxy(address assetProxyContract, bool isRegistered) + public + onlyWallet + notNull(assetProxyContract) + { + isAssetProxyRegistered[assetProxyContract] = isRegistered; + AssetProxyRegistration(assetProxyContract, isRegistered); + } + + /// @dev Allows execution of removeAuthorizedAddress without time lock. + /// @param transactionId Transaction ID. + function executeRemoveAuthorizedAddress(uint256 transactionId) + public + notExecuted(transactionId) + fullyConfirmed(transactionId) + validRemoveAuthorizedAddressTx(transactionId) + { + Transaction storage tx = transactions[transactionId]; + tx.executed = true; + if (tx.destination.call.value(tx.value)(tx.data)) + Execution(transactionId); + else { + ExecutionFailure(transactionId); + tx.executed = false; + } + } + + /// @dev Compares first 4 bytes of byte array to removeAuthorizedAddress function signature. + /// @param data Transaction data. + /// @return Successful if data is a call to removeAuthorizedAddress. + function isFunctionRemoveAuthorizedAddress(bytes memory data) + public + pure + returns (bool) + { + bytes4 first4Bytes = readFirst4(data); + require(REMOVE_AUTHORIZED_ADDRESS_SELECTOR == first4Bytes); + return true; + } + + /// @dev Reads the first 4 bytes from a byte array of arbitrary length. + /// @param data Byte array to read first 4 bytes from. + /// @return First 4 bytes of data. + function readFirst4(bytes memory data) + public + pure + returns (bytes4 result) + { + require(data.length >= 4); + assembly { + result := mload(add(data, 32)) + } + return result; + } +} diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index caf5b94fd..fe74ea072 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -1,5 +1,6 @@ import { ContractArtifact } from '@0xproject/sol-compiler'; +import * as AssetProxyOwner from '../artifacts/AssetProxyOwner.json'; import * as DummyERC20Token from '../artifacts/DummyERC20Token.json'; import * as DummyERC721Token from '../artifacts/DummyERC721Token.json'; import * as ERC20Proxy from '../artifacts/ERC20Proxy.json'; @@ -8,7 +9,6 @@ import * as Exchange from '../artifacts/Exchange.json'; import * as MixinAuthorizable from '../artifacts/MixinAuthorizable.json'; import * as MultiSigWallet from '../artifacts/MultiSigWallet.json'; import * as MultiSigWalletWithTimeLock from '../artifacts/MultiSigWalletWithTimeLock.json'; -import * as MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress from '../artifacts/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.json'; import * as TestAssetProxyDispatcher from '../artifacts/TestAssetProxyDispatcher.json'; import * as TestLibBytes from '../artifacts/TestLibBytes.json'; import * as TestLibs from '../artifacts/TestLibs.json'; @@ -18,6 +18,7 @@ import * as EtherToken from '../artifacts/WETH9.json'; import * as ZRX from '../artifacts/ZRXToken.json'; export const artifacts = { + AssetProxyOwner: (AssetProxyOwner as any) as ContractArtifact, DummyERC20Token: (DummyERC20Token as any) as ContractArtifact, DummyERC721Token: (DummyERC721Token as any) as ContractArtifact, ERC20Proxy: (ERC20Proxy as any) as ContractArtifact, @@ -27,7 +28,6 @@ export const artifacts = { MixinAuthorizable: (MixinAuthorizable as any) as ContractArtifact, MultiSigWallet: (MultiSigWallet as any) as ContractArtifact, MultiSigWalletWithTimeLock: (MultiSigWalletWithTimeLock as any) as ContractArtifact, - MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress: (MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress as any) as ContractArtifact, TestAssetProxyDispatcher: (TestAssetProxyDispatcher as any) as ContractArtifact, TestLibBytes: (TestLibBytes as any) as ContractArtifact, TestLibs: (TestLibs as any) as ContractArtifact, diff --git a/packages/contracts/src/utils/multi_sig_wrapper.ts b/packages/contracts/src/utils/multi_sig_wrapper.ts index 9a2b26d77..c33e7bb47 100644 --- a/packages/contracts/src/utils/multi_sig_wrapper.ts +++ b/packages/contracts/src/utils/multi_sig_wrapper.ts @@ -3,8 +3,8 @@ import { BigNumber } from '@0xproject/utils'; import * as _ from 'lodash'; import * as Web3 from 'web3'; +import { AssetProxyOwnerContract } from '../contract_wrappers/generated/asset_proxy_owner'; import { MultiSigWalletContract } from '../contract_wrappers/generated/multi_sig_wallet'; -import { MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract } from '../contract_wrappers/generated/multi_sig_wallet_with_time_lock_except_remove_authorized_address'; import { constants } from './constants'; import { LogDecoder } from './log_decoder'; @@ -45,10 +45,7 @@ export class MultiSigWrapper { from: string, ): Promise { const txHash = await (this - ._multiSig as MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract).executeRemoveAuthorizedAddress.sendTransactionAsync( - txId, - { from }, - ); + ._multiSig as AssetProxyOwnerContract).executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from }); const tx = await this._getTxWithDecodedMultiSigLogs(txHash); return tx; } diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index ef86b4f38..05cd7ec16 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -99,7 +99,7 @@ export enum ContractName { ZRXToken = 'ZRXToken', DummyERC20Token = 'DummyERC20Token', EtherToken = 'WETH9', - MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress = 'MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress', + AssetProxyOwner = 'AssetProxyOwner', AccountLevels = 'AccountLevels', EtherDelta = 'EtherDelta', Arbitrage = 'Arbitrage', diff --git a/packages/contracts/test/asset_proxy_owner.ts b/packages/contracts/test/asset_proxy_owner.ts new file mode 100644 index 000000000..7b470e82e --- /dev/null +++ b/packages/contracts/test/asset_proxy_owner.ts @@ -0,0 +1,358 @@ +import { LogWithDecodedArgs, ZeroEx } from '0x.js'; +import { BlockchainLifecycle } from '@0xproject/dev-utils'; +import { BigNumber } from '@0xproject/utils'; +import * as chai from 'chai'; +import * as _ from 'lodash'; +import 'make-promises-safe'; +import * as Web3 from 'web3'; + +import { + AssetProxyOwnerContract, + AssetProxyRegistrationContractEventArgs, + ExecutionContractEventArgs, + ExecutionFailureContractEventArgs, + SubmissionContractEventArgs, +} from '../src/contract_wrappers/generated/asset_proxy_owner'; +import { MixinAuthorizableContract } from '../src/contract_wrappers/generated/mixin_authorizable'; +import { artifacts } from '../src/utils/artifacts'; +import { chaiSetup } from '../src/utils/chai_setup'; +import { constants } from '../src/utils/constants'; +import { MultiSigWrapper } from '../src/utils/multi_sig_wrapper'; +import { provider, txDefaults, web3Wrapper } from '../src/utils/web3_wrapper'; + +chaiSetup.configure(); +const expect = chai.expect; +const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); +const zeroEx = new ZeroEx(provider, { networkId: constants.TESTRPC_NETWORK_ID }); + +describe('AssetProxyOwner', () => { + let owners: string[]; + let authorized: string; + const requiredApprovals = new BigNumber(2); + const SECONDS_TIME_LOCKED = new BigNumber(1000000); + + let erc20Proxy: MixinAuthorizableContract; + let erc721Proxy: MixinAuthorizableContract; + let multiSig: AssetProxyOwnerContract; + let multiSigWrapper: MultiSigWrapper; + + before(async () => { + const accounts = await web3Wrapper.getAvailableAddressesAsync(); + owners = [accounts[0], accounts[1]]; + const initialOwner = (authorized = accounts[0]); + erc20Proxy = await MixinAuthorizableContract.deployFrom0xArtifactAsync(artifacts.MixinAuthorizable, provider, txDefaults); + erc721Proxy = await MixinAuthorizableContract.deployFrom0xArtifactAsync( + artifacts.MixinAuthorizable, + provider, + txDefaults, + ); + const defaultAssetProxyContractAddresses: string[] = []; + multiSig = await AssetProxyOwnerContract.deployFrom0xArtifactAsync( + artifacts.AssetProxyOwner, + provider, + txDefaults, + owners, + requiredApprovals, + SECONDS_TIME_LOCKED, + defaultAssetProxyContractAddresses, + ]); + multiSigWrapper = new MultiSigWrapper(multiSig, zeroEx); + await erc20Proxy.transferOwnership.sendTransactionAsync(multiSig.address, { from: initialOwner }); + await erc721Proxy.transferOwnership.sendTransactionAsync(multiSig.address, { from: initialOwner }); + }); + beforeEach(async () => { + await blockchainLifecycle.startAsync(); + }); + afterEach(async () => { + await blockchainLifecycle.revertAsync(); + }); + + describe('constructor', () => { + it('should register passed in assetProxyContracts', async () => { + const assetProxyContractAddresses = [erc20Proxy.address, erc721Proxy.address]; + const newMultiSig = await AssetProxyOwnerContract.deployFrom0xArtifactAsync( + artifacts.AssetProxyOwner, + provider, + txDefaults, + owners, + requiredApprovals, + SECONDS_TIME_LOCKED, + assetProxyContractAddresses, + ); + const isErc20ProxyRegistered = await newMultiSig.isAssetProxyRegistered.callAsync(erc20Proxy.address); + const isErc721ProxyRegistered = await newMultiSig.isAssetProxyRegistered.callAsync(erc721Proxy.address); + expect(isErc20ProxyRegistered).to.equal(true); + expect(isErc721ProxyRegistered).to.equal(true); + }); + it('should throw if a null address is included in assetProxyContracts', async () => { + const assetProxyContractAddresses = [erc20Proxy.address, ZeroEx.NULL_ADDRESS]; + expect( + AssetProxyOwnerContract.deployFrom0xArtifactAsync( + artifacts.AssetProxyOwner, + provider, + txDefaults, + owners, + requiredApprovals, + SECONDS_TIME_LOCKED, + assetProxyContractAddresses, + ), + ).to.be.rejectedWith(constants.REVERT); + }); + }); + describe('readFirst4', () => { + it('should return the first 4 bytes of a byte array of arbitrary length', async () => { + const addAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData(owners[0]); + const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( + owners[0], + ); + const expectedAddAuthorizedAddressSelector = addAuthorizedAddressData.slice(0, 10); + const expectedRemoveAuthorizedAddressSelector = removeAuthorizedAddressData.slice(0, 10); + const [addAuthorizedAddressSelector, removeAuthorizedAddressSelector] = await Promise.all([ + multiSig.readFirst4.callAsync(addAuthorizedAddressData), + multiSig.readFirst4.callAsync(removeAuthorizedAddressData), + ]); + expect(expectedAddAuthorizedAddressSelector).to.equal(addAuthorizedAddressSelector); + expect(expectedRemoveAuthorizedAddressSelector).to.equal(removeAuthorizedAddressSelector); + }); + }); + + describe('isFunctionRemoveAuthorizedAddress', () => { + it('should throw if data is not for removeAuthorizedAddress', async () => { + const notRemoveAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData( + owners[0], + ); + return expect( + multiSig.isFunctionRemoveAuthorizedAddress.callAsync(notRemoveAuthorizedAddressData), + ).to.be.rejectedWith(constants.REVERT); + }); + + it('should return true if data is for removeAuthorizedAddress', async () => { + const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( + owners[0], + ); + const isFunctionRemoveAuthorizedAddress = await multiSig.isFunctionRemoveAuthorizedAddress.callAsync( + removeAuthorizedAddressData, + ); + expect(isFunctionRemoveAuthorizedAddress).to.be.true(); + }); + }); + + describe('registerAssetProxy', () => { + it('should throw if not called by multisig', async () => { + const isRegistered = true; + expect( + multiSig.registerAssetProxy.sendTransactionAsync(erc20Proxy.address, isRegistered, { from: owners[0] }), + ).to.be.rejectedWith(constants.REVERT); + }); + + it('should register an address if called by multisig after timelock', async () => { + const addressToRegister = erc20Proxy.address; + const isRegistered = true; + const registerAssetProxyData = multiSig.registerAssetProxy.getABIEncodedTransactionData( + addressToRegister, + isRegistered, + ); + const submitTxRes = await multiSigWrapper.submitTransactionAsync( + multiSig.address, + registerAssetProxyData, + owners[0], + ); + const log = submitTxRes.logs[0] as LogWithDecodedArgs; + const txId = log.args.transactionId; + + await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); + await web3Wrapper.increaseTimeAsync(SECONDS_TIME_LOCKED.toNumber()); + + const executeTxRes = await multiSigWrapper.executeTransactionAsync(txId, owners[0]); + const registerLog = executeTxRes.logs[0] as LogWithDecodedArgs; + expect(registerLog.args.assetProxyContract).to.equal(addressToRegister); + expect(registerLog.args.isRegistered).to.equal(isRegistered); + + const isAssetProxyRegistered = await multiSig.isAssetProxyRegistered.callAsync(addressToRegister); + expect(isAssetProxyRegistered).to.equal(isRegistered); + }); + + it('should fail if registering a null address', async () => { + const addressToRegister = ZeroEx.NULL_ADDRESS; + const isRegistered = true; + const registerAssetProxyData = multiSig.registerAssetProxy.getABIEncodedTransactionData( + addressToRegister, + isRegistered, + ); + const submitTxRes = await multiSigWrapper.submitTransactionAsync( + multiSig.address, + registerAssetProxyData, + owners[0], + ); + const log = submitTxRes.logs[0] as LogWithDecodedArgs; + const txId = log.args.transactionId; + + await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); + await web3Wrapper.increaseTimeAsync(SECONDS_TIME_LOCKED.toNumber()); + + const executeTxRes = await multiSigWrapper.executeTransactionAsync(txId, owners[0]); + const failureLog = executeTxRes.logs[0] as LogWithDecodedArgs; + expect(failureLog.args.transactionId).to.be.bignumber.equal(txId); + + const isAssetProxyRegistered = await multiSig.isAssetProxyRegistered.callAsync(addressToRegister); + expect(isAssetProxyRegistered).to.equal(false); + }); + }); + + describe('executeRemoveAuthorizedAddress', () => { + before('authorize both proxies and register erc20 proxy', async () => { + // Only register ERC20 proxy + const addressToRegister = erc20Proxy.address; + const isRegistered = true; + const registerAssetProxyData = multiSig.registerAssetProxy.getABIEncodedTransactionData( + addressToRegister, + isRegistered, + ); + const registerAssetProxySubmitRes = await multiSigWrapper.submitTransactionAsync( + multiSig.address, + registerAssetProxyData, + owners[0], + ); + const registerAssetProxySubmitLog = registerAssetProxySubmitRes.logs[0] as LogWithDecodedArgs< + SubmissionContractEventArgs + >; + const registerAssetProxyTxId = registerAssetProxySubmitLog.args.transactionId; + await multiSigWrapper.confirmTransactionAsync(registerAssetProxyTxId, owners[1]); + + const addAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData(authorized); + const erc20AddAuthorizedAddressSubmitRes = await multiSigWrapper.submitTransactionAsync( + erc20Proxy.address, + addAuthorizedAddressData, + owners[0], + ); + const erc721AddAuthorizedAddressSubmitRes = await multiSigWrapper.submitTransactionAsync( + erc721Proxy.address, + addAuthorizedAddressData, + owners[0], + ); + const erc20AddAuthorizedAddressSubmitLog = erc20AddAuthorizedAddressSubmitRes.logs[0] as LogWithDecodedArgs< + SubmissionContractEventArgs + >; + const erc721AddAuthorizedAddressSubmitLog = erc721AddAuthorizedAddressSubmitRes + .logs[0] as LogWithDecodedArgs; + const erc20AddAuthorizedAddressTxId = erc20AddAuthorizedAddressSubmitLog.args.transactionId; + const erc721AddAuthorizedAddressTxId = erc721AddAuthorizedAddressSubmitLog.args.transactionId; + + await multiSigWrapper.confirmTransactionAsync(erc20AddAuthorizedAddressTxId, owners[1]); + await multiSigWrapper.confirmTransactionAsync(erc721AddAuthorizedAddressTxId, owners[1]); + await web3Wrapper.increaseTimeAsync(SECONDS_TIME_LOCKED.toNumber()); + await multiSigWrapper.executeTransactionAsync(registerAssetProxyTxId, owners[0]); + await multiSigWrapper.executeTransactionAsync(erc20AddAuthorizedAddressTxId, owners[0]); + await multiSigWrapper.executeTransactionAsync(erc721AddAuthorizedAddressTxId, owners[0]); + }); + + it('should throw without the required confirmations', async () => { + const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( + authorized, + ); + const res = await multiSigWrapper.submitTransactionAsync( + erc20Proxy.address, + removeAuthorizedAddressData, + owners[0], + ); + const log = res.logs[0] as LogWithDecodedArgs; + const txId = log.args.transactionId; + + return expect( + multiSig.executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from: owners[1] }), + ).to.be.rejectedWith(constants.REVERT); + }); + + it('should throw if tx destination is not registered', async () => { + const removeAuthorizedAddressData = erc721Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( + authorized, + ); + const res = await multiSigWrapper.submitTransactionAsync( + erc721Proxy.address, + removeAuthorizedAddressData, + owners[0], + ); + const log = res.logs[0] as LogWithDecodedArgs; + const txId = log.args.transactionId; + + await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); + + return expect( + multiSig.executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from: owners[1] }), + ).to.be.rejectedWith(constants.REVERT); + }); + + it('should throw if tx data is not for removeAuthorizedAddress', async () => { + const newAuthorized = owners[1]; + const addAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData( + newAuthorized, + ); + const res = await multiSigWrapper.submitTransactionAsync( + erc20Proxy.address, + addAuthorizedAddressData, + owners[0], + ); + const log = res.logs[0] as LogWithDecodedArgs; + const txId = log.args.transactionId; + + await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); + + return expect( + multiSig.executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from: owners[1] }), + ).to.be.rejectedWith(constants.REVERT); + }); + + it('should execute removeAuthorizedAddress for registered address if fully confirmed', async () => { + const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( + authorized, + ); + const submitRes = await multiSigWrapper.submitTransactionAsync( + erc20Proxy.address, + removeAuthorizedAddressData, + owners[0], + ); + const submitLog = submitRes.logs[0] as LogWithDecodedArgs; + const txId = submitLog.args.transactionId; + + await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); + + const execRes = await multiSigWrapper.executeRemoveAuthorizedAddressAsync(txId, owners[0]); + const execLog = execRes.logs[0] as LogWithDecodedArgs; + expect(execLog.args.transactionId).to.be.bignumber.equal(txId); + + const tx = await multiSig.transactions.callAsync(txId); + const isExecuted = tx[3]; + expect(isExecuted).to.equal(true); + + const isAuthorized = await erc20Proxy.authorized.callAsync(authorized); + expect(isAuthorized).to.equal(false); + }); + + it('should throw if already executed', async () => { + const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( + authorized, + ); + const submitRes = await multiSigWrapper.submitTransactionAsync( + erc20Proxy.address, + removeAuthorizedAddressData, + owners[0], + ); + const submitLog = submitRes.logs[0] as LogWithDecodedArgs; + const txId = submitLog.args.transactionId; + + await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); + + const execRes = await multiSigWrapper.executeRemoveAuthorizedAddressAsync(txId, owners[0]); + const execLog = execRes.logs[0] as LogWithDecodedArgs; + expect(execLog.args.transactionId).to.be.bignumber.equal(txId); + + const tx = await multiSig.transactions.callAsync(txId); + const isExecuted = tx[3]; + expect(isExecuted).to.equal(true); + + return expect( + multiSig.executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from: owners[1] }), + ).to.be.rejectedWith(constants.REVERT); + }); + }); +}); diff --git a/packages/contracts/test/multi_sig_with_time_lock_except_remove_auth_addr.ts b/packages/contracts/test/multi_sig_with_time_lock_except_remove_auth_addr.ts deleted file mode 100644 index e384e093b..000000000 --- a/packages/contracts/test/multi_sig_with_time_lock_except_remove_auth_addr.ts +++ /dev/null @@ -1,358 +0,0 @@ -import { LogWithDecodedArgs, ZeroEx } from '0x.js'; -import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { BigNumber } from '@0xproject/utils'; -import * as chai from 'chai'; -import * as _ from 'lodash'; -import 'make-promises-safe'; -import * as Web3 from 'web3'; - -import { AuthorizableContract } from '../src/contract_wrappers/generated/authorizable'; -import { - AssetProxyRegistrationContractEventArgs, - ExecutionContractEventArgs, - ExecutionFailureContractEventArgs, - MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract, - SubmissionContractEventArgs, -} from '../src/contract_wrappers/generated/multi_sig_wallet_with_time_lock_except_remove_authorized_address'; -import { artifacts } from '../src/utils/artifacts'; -import { chaiSetup } from '../src/utils/chai_setup'; -import { constants } from '../src/utils/constants'; -import { MultiSigWrapper } from '../src/utils/multi_sig_wrapper'; -import { provider, txDefaults, web3Wrapper } from '../src/utils/web3_wrapper'; - -chaiSetup.configure(); -const expect = chai.expect; -const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); -const zeroEx = new ZeroEx(provider, { networkId: constants.TESTRPC_NETWORK_ID }); - -describe('MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress', () => { - let owners: string[]; - let authorized: string; - const requiredApprovals = new BigNumber(2); - const SECONDS_TIME_LOCKED = new BigNumber(1000000); - - let erc20Proxy: AuthorizableContract; - let erc721Proxy: AuthorizableContract; - let multiSig: MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract; - let multiSigWrapper: MultiSigWrapper; - - before(async () => { - const accounts = await web3Wrapper.getAvailableAddressesAsync(); - owners = [accounts[0], accounts[1]]; - const initialOwner = (authorized = accounts[0]); - erc20Proxy = await AuthorizableContract.deployFrom0xArtifactAsync(artifacts.Authorizable, provider, txDefaults); - erc721Proxy = await AuthorizableContract.deployFrom0xArtifactAsync( - artifacts.Authorizable, - provider, - txDefaults, - ); - const defaultAssetProxyContractAddresses: string[] = []; - multiSig = await MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract.deployFrom0xArtifactAsync( - artifacts.MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress, - provider, - txDefaults, - owners, - requiredApprovals, - SECONDS_TIME_LOCKED, - defaultAssetProxyContractAddresses, - ); - multiSigWrapper = new MultiSigWrapper(multiSig, zeroEx); - await erc20Proxy.transferOwnership.sendTransactionAsync(multiSig.address, { from: initialOwner }); - await erc721Proxy.transferOwnership.sendTransactionAsync(multiSig.address, { from: initialOwner }); - }); - beforeEach(async () => { - await blockchainLifecycle.startAsync(); - }); - afterEach(async () => { - await blockchainLifecycle.revertAsync(); - }); - - describe('constructor', () => { - it('should register passed in assetProxyContracts', async () => { - const assetProxyContractAddresses = [erc20Proxy.address, erc721Proxy.address]; - const newMultiSig = await MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract.deployFrom0xArtifactAsync( - artifacts.MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress, - provider, - txDefaults, - owners, - requiredApprovals, - SECONDS_TIME_LOCKED, - assetProxyContractAddresses, - ); - const isErc20ProxyRegistered = await newMultiSig.isAssetProxyRegistered.callAsync(erc20Proxy.address); - const isErc721ProxyRegistered = await newMultiSig.isAssetProxyRegistered.callAsync(erc721Proxy.address); - expect(isErc20ProxyRegistered).to.equal(true); - expect(isErc721ProxyRegistered).to.equal(true); - }); - it('should throw if a null address is included in assetProxyContracts', async () => { - const assetProxyContractAddresses = [erc20Proxy.address, ZeroEx.NULL_ADDRESS]; - expect( - MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddressContract.deployFrom0xArtifactAsync( - artifacts.MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress, - provider, - txDefaults, - owners, - requiredApprovals, - SECONDS_TIME_LOCKED, - assetProxyContractAddresses, - ), - ).to.be.rejectedWith(constants.REVERT); - }); - }); - describe('readFirst4', () => { - it('should return the first 4 bytes of a byte array of arbitrary length', async () => { - const addAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData(owners[0]); - const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( - owners[0], - ); - const expectedAddAuthorizedAddressSelector = addAuthorizedAddressData.slice(0, 10); - const expectedRemoveAuthorizedAddressSelector = removeAuthorizedAddressData.slice(0, 10); - const [addAuthorizedAddressSelector, removeAuthorizedAddressSelector] = await Promise.all([ - multiSig.readFirst4.callAsync(addAuthorizedAddressData), - multiSig.readFirst4.callAsync(removeAuthorizedAddressData), - ]); - expect(expectedAddAuthorizedAddressSelector).to.equal(addAuthorizedAddressSelector); - expect(expectedRemoveAuthorizedAddressSelector).to.equal(removeAuthorizedAddressSelector); - }); - }); - - describe('isFunctionRemoveAuthorizedAddress', () => { - it('should throw if data is not for removeAuthorizedAddress', async () => { - const notRemoveAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData( - owners[0], - ); - return expect( - multiSig.isFunctionRemoveAuthorizedAddress.callAsync(notRemoveAuthorizedAddressData), - ).to.be.rejectedWith(constants.REVERT); - }); - - it('should return true if data is for removeAuthorizedAddress', async () => { - const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( - owners[0], - ); - const isFunctionRemoveAuthorizedAddress = await multiSig.isFunctionRemoveAuthorizedAddress.callAsync( - removeAuthorizedAddressData, - ); - expect(isFunctionRemoveAuthorizedAddress).to.be.true(); - }); - }); - - describe('registerAssetProxy', () => { - it('should throw if not called by multisig', async () => { - const isRegistered = true; - expect( - multiSig.registerAssetProxy.sendTransactionAsync(erc20Proxy.address, isRegistered, { from: owners[0] }), - ).to.be.rejectedWith(constants.REVERT); - }); - - it('should register an address if called by multisig after timelock', async () => { - const addressToRegister = erc20Proxy.address; - const isRegistered = true; - const registerAssetProxyData = multiSig.registerAssetProxy.getABIEncodedTransactionData( - addressToRegister, - isRegistered, - ); - const submitTxRes = await multiSigWrapper.submitTransactionAsync( - multiSig.address, - registerAssetProxyData, - owners[0], - ); - const log = submitTxRes.logs[0] as LogWithDecodedArgs; - const txId = log.args.transactionId; - - await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); - await web3Wrapper.increaseTimeAsync(SECONDS_TIME_LOCKED.toNumber()); - - const executeTxRes = await multiSigWrapper.executeTransactionAsync(txId, owners[0]); - const registerLog = executeTxRes.logs[0] as LogWithDecodedArgs; - expect(registerLog.args.assetProxyContract).to.equal(addressToRegister); - expect(registerLog.args.isRegistered).to.equal(isRegistered); - - const isAssetProxyRegistered = await multiSig.isAssetProxyRegistered.callAsync(addressToRegister); - expect(isAssetProxyRegistered).to.equal(isRegistered); - }); - - it('should fail if registering a null address', async () => { - const addressToRegister = ZeroEx.NULL_ADDRESS; - const isRegistered = true; - const registerAssetProxyData = multiSig.registerAssetProxy.getABIEncodedTransactionData( - addressToRegister, - isRegistered, - ); - const submitTxRes = await multiSigWrapper.submitTransactionAsync( - multiSig.address, - registerAssetProxyData, - owners[0], - ); - const log = submitTxRes.logs[0] as LogWithDecodedArgs; - const txId = log.args.transactionId; - - await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); - await web3Wrapper.increaseTimeAsync(SECONDS_TIME_LOCKED.toNumber()); - - const executeTxRes = await multiSigWrapper.executeTransactionAsync(txId, owners[0]); - const failureLog = executeTxRes.logs[0] as LogWithDecodedArgs; - expect(failureLog.args.transactionId).to.be.bignumber.equal(txId); - - const isAssetProxyRegistered = await multiSig.isAssetProxyRegistered.callAsync(addressToRegister); - expect(isAssetProxyRegistered).to.equal(false); - }); - }); - - describe('executeRemoveAuthorizedAddress', () => { - before('authorize both proxies and register erc20 proxy', async () => { - // Only register ERC20 proxy - const addressToRegister = erc20Proxy.address; - const isRegistered = true; - const registerAssetProxyData = multiSig.registerAssetProxy.getABIEncodedTransactionData( - addressToRegister, - isRegistered, - ); - const registerAssetProxySubmitRes = await multiSigWrapper.submitTransactionAsync( - multiSig.address, - registerAssetProxyData, - owners[0], - ); - const registerAssetProxySubmitLog = registerAssetProxySubmitRes.logs[0] as LogWithDecodedArgs< - SubmissionContractEventArgs - >; - const registerAssetProxyTxId = registerAssetProxySubmitLog.args.transactionId; - await multiSigWrapper.confirmTransactionAsync(registerAssetProxyTxId, owners[1]); - - const addAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData(authorized); - const erc20AddAuthorizedAddressSubmitRes = await multiSigWrapper.submitTransactionAsync( - erc20Proxy.address, - addAuthorizedAddressData, - owners[0], - ); - const erc721AddAuthorizedAddressSubmitRes = await multiSigWrapper.submitTransactionAsync( - erc721Proxy.address, - addAuthorizedAddressData, - owners[0], - ); - const erc20AddAuthorizedAddressSubmitLog = erc20AddAuthorizedAddressSubmitRes.logs[0] as LogWithDecodedArgs< - SubmissionContractEventArgs - >; - const erc721AddAuthorizedAddressSubmitLog = erc721AddAuthorizedAddressSubmitRes - .logs[0] as LogWithDecodedArgs; - const erc20AddAuthorizedAddressTxId = erc20AddAuthorizedAddressSubmitLog.args.transactionId; - const erc721AddAuthorizedAddressTxId = erc721AddAuthorizedAddressSubmitLog.args.transactionId; - - await multiSigWrapper.confirmTransactionAsync(erc20AddAuthorizedAddressTxId, owners[1]); - await multiSigWrapper.confirmTransactionAsync(erc721AddAuthorizedAddressTxId, owners[1]); - await web3Wrapper.increaseTimeAsync(SECONDS_TIME_LOCKED.toNumber()); - await multiSigWrapper.executeTransactionAsync(registerAssetProxyTxId, owners[0]); - await multiSigWrapper.executeTransactionAsync(erc20AddAuthorizedAddressTxId, owners[0]); - await multiSigWrapper.executeTransactionAsync(erc721AddAuthorizedAddressTxId, owners[0]); - }); - - it('should throw without the required confirmations', async () => { - const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( - authorized, - ); - const res = await multiSigWrapper.submitTransactionAsync( - erc20Proxy.address, - removeAuthorizedAddressData, - owners[0], - ); - const log = res.logs[0] as LogWithDecodedArgs; - const txId = log.args.transactionId; - - return expect( - multiSig.executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from: owners[1] }), - ).to.be.rejectedWith(constants.REVERT); - }); - - it('should throw if tx destination is not registered', async () => { - const removeAuthorizedAddressData = erc721Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( - authorized, - ); - const res = await multiSigWrapper.submitTransactionAsync( - erc721Proxy.address, - removeAuthorizedAddressData, - owners[0], - ); - const log = res.logs[0] as LogWithDecodedArgs; - const txId = log.args.transactionId; - - await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); - - return expect( - multiSig.executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from: owners[1] }), - ).to.be.rejectedWith(constants.REVERT); - }); - - it('should throw if tx data is not for removeAuthorizedAddress', async () => { - const newAuthorized = owners[1]; - const addAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData( - newAuthorized, - ); - const res = await multiSigWrapper.submitTransactionAsync( - erc20Proxy.address, - addAuthorizedAddressData, - owners[0], - ); - const log = res.logs[0] as LogWithDecodedArgs; - const txId = log.args.transactionId; - - await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); - - return expect( - multiSig.executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from: owners[1] }), - ).to.be.rejectedWith(constants.REVERT); - }); - - it('should execute removeAuthorizedAddress for registered address if fully confirmed', async () => { - const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( - authorized, - ); - const submitRes = await multiSigWrapper.submitTransactionAsync( - erc20Proxy.address, - removeAuthorizedAddressData, - owners[0], - ); - const submitLog = submitRes.logs[0] as LogWithDecodedArgs; - const txId = submitLog.args.transactionId; - - await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); - - const execRes = await multiSigWrapper.executeRemoveAuthorizedAddressAsync(txId, owners[0]); - const execLog = execRes.logs[0] as LogWithDecodedArgs; - expect(execLog.args.transactionId).to.be.bignumber.equal(txId); - - const tx = await multiSig.transactions.callAsync(txId); - const isExecuted = tx[3]; - expect(isExecuted).to.equal(true); - - const isAuthorized = await erc20Proxy.authorized.callAsync(authorized); - expect(isAuthorized).to.equal(false); - }); - - it('should throw if already executed', async () => { - const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( - authorized, - ); - const submitRes = await multiSigWrapper.submitTransactionAsync( - erc20Proxy.address, - removeAuthorizedAddressData, - owners[0], - ); - const submitLog = submitRes.logs[0] as LogWithDecodedArgs; - const txId = submitLog.args.transactionId; - - await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); - - const execRes = await multiSigWrapper.executeRemoveAuthorizedAddressAsync(txId, owners[0]); - const execLog = execRes.logs[0] as LogWithDecodedArgs; - expect(execLog.args.transactionId).to.be.bignumber.equal(txId); - - const tx = await multiSig.transactions.callAsync(txId); - const isExecuted = tx[3]; - expect(isExecuted).to.equal(true); - - return expect( - multiSig.executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from: owners[1] }), - ).to.be.rejectedWith(constants.REVERT); - }); - }); -}); -- cgit v1.2.3 From 84257dac2be92192d5b99dd31617aaebeddb285b Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sat, 19 May 2018 10:00:26 -0700 Subject: Fix build --- packages/contracts/compiler.json | 2 +- packages/contracts/package.json | 3 +- ...etWithTimeLockExceptRemoveAuthorizedAddress.sol | 82 ++++++++++++++++++++++ packages/contracts/test/asset_proxy_owner.ts | 8 ++- 4 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 packages/contracts/src/contracts/previous/MultiSigWalletWithTImeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol (limited to 'packages/contracts') diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json index bf6cc2376..7d469d2c3 100644 --- a/packages/contracts/compiler.json +++ b/packages/contracts/compiler.json @@ -19,6 +19,7 @@ } }, "contracts": [ + "AssetProxyOwner", "DummyERC20Token", "DummyERC721Token", "ERC20Proxy", @@ -27,7 +28,6 @@ "MixinAuthorizable", "MultiSigWallet", "MultiSigWalletWithTimeLock", - "MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress", "TestAssetProxyDispatcher", "TestLibBytes", "TestLibs", diff --git a/packages/contracts/package.json b/packages/contracts/package.json index e4e5b131d..180b8058a 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -20,7 +20,8 @@ "run_mocha": "mocha 'lib/test/**/*.js' --timeout 100000 --bail --exit", "compile": "sol-compiler", "clean": "shx rm -rf lib src/contract_wrappers/generated", - "generate_contract_wrappers": "abi-gen --abis ${npm_package_config_abis} --template ../contract_templates/contract.handlebars --partials '../contract_templates/partials/**/*.handlebars' --output src/contract_wrappers/generated --backend ethers && prettier --write 'src/contract_wrappers/generated/**.ts'", + "generate_contract_wrappers": + "abi-gen --abis ${npm_package_config_abis} --template ../contract_templates/contract.handlebars --partials '../contract_templates/partials/**/*.handlebars' --output src/contract_wrappers/generated --backend ethers && prettier --write 'src/contract_wrappers/generated/**.ts'", "lint": "tslint --project .", "coverage:report:text": "istanbul report text", "coverage:report:html": "istanbul report html && open coverage/index.html", diff --git a/packages/contracts/src/contracts/previous/MultiSigWalletWithTImeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol b/packages/contracts/src/contracts/previous/MultiSigWalletWithTImeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol new file mode 100644 index 000000000..8d7d64a75 --- /dev/null +++ b/packages/contracts/src/contracts/previous/MultiSigWalletWithTImeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol @@ -0,0 +1,82 @@ +/* + + Copyright 2017 ZeroEx Intl. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + +*/ + +pragma solidity ^0.4.10; + +import "../../current/multisig/MultiSigWalletWithTimeLock/MultiSigWalletWithTimeLock.sol"; + +contract MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress is MultiSigWalletWithTimeLock { + + address public TOKEN_TRANSFER_PROXY_CONTRACT; + + modifier validRemoveAuthorizedAddressTx(uint transactionId) { + Transaction storage tx = transactions[transactionId]; + require(tx.destination == TOKEN_TRANSFER_PROXY_CONTRACT); + require(isFunctionRemoveAuthorizedAddress(tx.data)); + _; + } + + /// @dev Contract constructor sets initial owners, required number of confirmations, time lock, and tokenTransferProxy address. + /// @param _owners List of initial owners. + /// @param _required Number of required confirmations. + /// @param _secondsTimeLocked Duration needed after a transaction is confirmed and before it becomes executable, in seconds. + /// @param _tokenTransferProxy Address of TokenTransferProxy contract. + function MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress( + address[] _owners, + uint _required, + uint _secondsTimeLocked, + address _tokenTransferProxy) + public + MultiSigWalletWithTimeLock(_owners, _required, _secondsTimeLocked) + { + TOKEN_TRANSFER_PROXY_CONTRACT = _tokenTransferProxy; + } + + /// @dev Allows execution of removeAuthorizedAddress without time lock. + /// @param transactionId Transaction ID. + function executeRemoveAuthorizedAddress(uint transactionId) + public + notExecuted(transactionId) + fullyConfirmed(transactionId) + validRemoveAuthorizedAddressTx(transactionId) + { + Transaction storage tx = transactions[transactionId]; + tx.executed = true; + if (tx.destination.call.value(tx.value)(tx.data)) + Execution(transactionId); + else { + ExecutionFailure(transactionId); + tx.executed = false; + } + } + + /// @dev Compares first 4 bytes of byte array to removeAuthorizedAddress function signature. + /// @param data Transaction data. + /// @return Successful if data is a call to removeAuthorizedAddress. + function isFunctionRemoveAuthorizedAddress(bytes data) + public + constant + returns (bool) + { + bytes4 removeAuthorizedAddressSignature = bytes4(sha3("removeAuthorizedAddress(address)")); + for (uint i = 0; i < 4; i++) { + require(data[i] == removeAuthorizedAddressSignature[i]); + } + return true; + } +} \ No newline at end of file diff --git a/packages/contracts/test/asset_proxy_owner.ts b/packages/contracts/test/asset_proxy_owner.ts index 7b470e82e..2e8e9373a 100644 --- a/packages/contracts/test/asset_proxy_owner.ts +++ b/packages/contracts/test/asset_proxy_owner.ts @@ -40,7 +40,11 @@ describe('AssetProxyOwner', () => { const accounts = await web3Wrapper.getAvailableAddressesAsync(); owners = [accounts[0], accounts[1]]; const initialOwner = (authorized = accounts[0]); - erc20Proxy = await MixinAuthorizableContract.deployFrom0xArtifactAsync(artifacts.MixinAuthorizable, provider, txDefaults); + erc20Proxy = await MixinAuthorizableContract.deployFrom0xArtifactAsync( + artifacts.MixinAuthorizable, + provider, + txDefaults, + ); erc721Proxy = await MixinAuthorizableContract.deployFrom0xArtifactAsync( artifacts.MixinAuthorizable, provider, @@ -55,7 +59,7 @@ describe('AssetProxyOwner', () => { requiredApprovals, SECONDS_TIME_LOCKED, defaultAssetProxyContractAddresses, - ]); + ); multiSigWrapper = new MultiSigWrapper(multiSig, zeroEx); await erc20Proxy.transferOwnership.sendTransactionAsync(multiSig.address, { from: initialOwner }); await erc721Proxy.transferOwnership.sendTransactionAsync(multiSig.address, { from: initialOwner }); -- cgit v1.2.3 From 326a566db29031edf21320923e67dde309573efd Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 20 May 2018 19:14:06 -0700 Subject: Add old MultiSig to previous contracts, cleanup file structure --- .../contracts/current/multisig/MultiSigWallet.sol | 365 +++++++++++++++++++++ .../multisig/MultiSigWallet/MultiSigWallet.sol | 365 --------------------- .../multisig/MultiSigWalletWithTimeLock.sol | 132 ++++++++ .../MultiSigWalletWithTimeLock.sol | 132 -------- .../protocol/AssetProxyOwner/AssetProxyOwner.sol | 2 +- ...etWithTimeLockExceptRemoveAuthorizedAddress.sol | 4 +- packages/contracts/src/utils/multi_sig_wrapper.ts | 10 +- packages/contracts/test/asset_proxy_owner.ts | 8 +- .../contracts/test/multi_sig_with_time_lock.ts | 6 +- 9 files changed, 512 insertions(+), 512 deletions(-) create mode 100644 packages/contracts/src/contracts/current/multisig/MultiSigWallet.sol delete mode 100644 packages/contracts/src/contracts/current/multisig/MultiSigWallet/MultiSigWallet.sol create mode 100644 packages/contracts/src/contracts/current/multisig/MultiSigWalletWithTimeLock.sol delete mode 100644 packages/contracts/src/contracts/current/multisig/MultiSigWalletWithTimeLock/MultiSigWalletWithTimeLock.sol (limited to 'packages/contracts') diff --git a/packages/contracts/src/contracts/current/multisig/MultiSigWallet.sol b/packages/contracts/src/contracts/current/multisig/MultiSigWallet.sol new file mode 100644 index 000000000..79fd92029 --- /dev/null +++ b/packages/contracts/src/contracts/current/multisig/MultiSigWallet.sol @@ -0,0 +1,365 @@ +pragma solidity ^0.4.10; + +/// @title Multisignature wallet - Allows multiple parties to agree on transactions before execution. +/// @author Stefan George - +contract MultiSigWallet { + + uint constant public MAX_OWNER_COUNT = 50; + + event Confirmation(address indexed sender, uint indexed transactionId); + event Revocation(address indexed sender, uint indexed transactionId); + event Submission(uint indexed transactionId); + event Execution(uint indexed transactionId); + event ExecutionFailure(uint indexed transactionId); + event Deposit(address indexed sender, uint value); + event OwnerAddition(address indexed owner); + event OwnerRemoval(address indexed owner); + event RequirementChange(uint required); + + mapping (uint => Transaction) public transactions; + mapping (uint => mapping (address => bool)) public confirmations; + mapping (address => bool) public isOwner; + address[] public owners; + uint public required; + uint public transactionCount; + + struct Transaction { + address destination; + uint value; + bytes data; + bool executed; + } + + modifier onlyWallet() { + if (msg.sender != address(this)) + throw; + _; + } + + modifier ownerDoesNotExist(address owner) { + if (isOwner[owner]) + throw; + _; + } + + modifier ownerExists(address owner) { + if (!isOwner[owner]) + throw; + _; + } + + modifier transactionExists(uint transactionId) { + if (transactions[transactionId].destination == 0) + throw; + _; + } + + modifier confirmed(uint transactionId, address owner) { + if (!confirmations[transactionId][owner]) + throw; + _; + } + + modifier notConfirmed(uint transactionId, address owner) { + if (confirmations[transactionId][owner]) + throw; + _; + } + + modifier notExecuted(uint transactionId) { + if (transactions[transactionId].executed) + throw; + _; + } + + modifier notNull(address _address) { + if (_address == 0) + throw; + _; + } + + modifier validRequirement(uint ownerCount, uint _required) { + if ( ownerCount > MAX_OWNER_COUNT + || _required > ownerCount + || _required == 0 + || ownerCount == 0) + throw; + _; + } + + /// @dev Fallback function allows to deposit ether. + function() + payable + { + if (msg.value > 0) + Deposit(msg.sender, msg.value); + } + + /* + * Public functions + */ + /// @dev Contract constructor sets initial owners and required number of confirmations. + /// @param _owners List of initial owners. + /// @param _required Number of required confirmations. + function MultiSigWallet(address[] _owners, uint _required) + public + validRequirement(_owners.length, _required) + { + for (uint i=0; i<_owners.length; i++) { + if (isOwner[_owners[i]] || _owners[i] == 0) + throw; + isOwner[_owners[i]] = true; + } + owners = _owners; + required = _required; + } + + /// @dev Allows to add a new owner. Transaction has to be sent by wallet. + /// @param owner Address of new owner. + function addOwner(address owner) + public + onlyWallet + ownerDoesNotExist(owner) + notNull(owner) + validRequirement(owners.length + 1, required) + { + isOwner[owner] = true; + owners.push(owner); + OwnerAddition(owner); + } + + /// @dev Allows to remove an owner. Transaction has to be sent by wallet. + /// @param owner Address of owner. + function removeOwner(address owner) + public + onlyWallet + ownerExists(owner) + { + isOwner[owner] = false; + for (uint i=0; i owners.length) + changeRequirement(owners.length); + OwnerRemoval(owner); + } + + /// @dev Allows to replace an owner with a new owner. Transaction has to be sent by wallet. + /// @param owner Address of owner to be replaced. + /// @param owner Address of new owner. + function replaceOwner(address owner, address newOwner) + public + onlyWallet + ownerExists(owner) + ownerDoesNotExist(newOwner) + { + for (uint i=0; i -contract MultiSigWallet { - - uint constant public MAX_OWNER_COUNT = 50; - - event Confirmation(address indexed sender, uint indexed transactionId); - event Revocation(address indexed sender, uint indexed transactionId); - event Submission(uint indexed transactionId); - event Execution(uint indexed transactionId); - event ExecutionFailure(uint indexed transactionId); - event Deposit(address indexed sender, uint value); - event OwnerAddition(address indexed owner); - event OwnerRemoval(address indexed owner); - event RequirementChange(uint required); - - mapping (uint => Transaction) public transactions; - mapping (uint => mapping (address => bool)) public confirmations; - mapping (address => bool) public isOwner; - address[] public owners; - uint public required; - uint public transactionCount; - - struct Transaction { - address destination; - uint value; - bytes data; - bool executed; - } - - modifier onlyWallet() { - if (msg.sender != address(this)) - throw; - _; - } - - modifier ownerDoesNotExist(address owner) { - if (isOwner[owner]) - throw; - _; - } - - modifier ownerExists(address owner) { - if (!isOwner[owner]) - throw; - _; - } - - modifier transactionExists(uint transactionId) { - if (transactions[transactionId].destination == 0) - throw; - _; - } - - modifier confirmed(uint transactionId, address owner) { - if (!confirmations[transactionId][owner]) - throw; - _; - } - - modifier notConfirmed(uint transactionId, address owner) { - if (confirmations[transactionId][owner]) - throw; - _; - } - - modifier notExecuted(uint transactionId) { - if (transactions[transactionId].executed) - throw; - _; - } - - modifier notNull(address _address) { - if (_address == 0) - throw; - _; - } - - modifier validRequirement(uint ownerCount, uint _required) { - if ( ownerCount > MAX_OWNER_COUNT - || _required > ownerCount - || _required == 0 - || ownerCount == 0) - throw; - _; - } - - /// @dev Fallback function allows to deposit ether. - function() - payable - { - if (msg.value > 0) - Deposit(msg.sender, msg.value); - } - - /* - * Public functions - */ - /// @dev Contract constructor sets initial owners and required number of confirmations. - /// @param _owners List of initial owners. - /// @param _required Number of required confirmations. - function MultiSigWallet(address[] _owners, uint _required) - public - validRequirement(_owners.length, _required) - { - for (uint i=0; i<_owners.length; i++) { - if (isOwner[_owners[i]] || _owners[i] == 0) - throw; - isOwner[_owners[i]] = true; - } - owners = _owners; - required = _required; - } - - /// @dev Allows to add a new owner. Transaction has to be sent by wallet. - /// @param owner Address of new owner. - function addOwner(address owner) - public - onlyWallet - ownerDoesNotExist(owner) - notNull(owner) - validRequirement(owners.length + 1, required) - { - isOwner[owner] = true; - owners.push(owner); - OwnerAddition(owner); - } - - /// @dev Allows to remove an owner. Transaction has to be sent by wallet. - /// @param owner Address of owner. - function removeOwner(address owner) - public - onlyWallet - ownerExists(owner) - { - isOwner[owner] = false; - for (uint i=0; i owners.length) - changeRequirement(owners.length); - OwnerRemoval(owner); - } - - /// @dev Allows to replace an owner with a new owner. Transaction has to be sent by wallet. - /// @param owner Address of owner to be replaced. - /// @param owner Address of new owner. - function replaceOwner(address owner, address newOwner) - public - onlyWallet - ownerExists(owner) - ownerDoesNotExist(newOwner) - { - for (uint i=0; i +contract MultiSigWalletWithTimeLock is MultiSigWallet { + + event ConfirmationTimeSet(uint indexed transactionId, uint confirmationTime); + event TimeLockChange(uint secondsTimeLocked); + + uint public secondsTimeLocked; + + mapping (uint => uint) public confirmationTimes; + + modifier notFullyConfirmed(uint transactionId) { + require(!isConfirmed(transactionId)); + _; + } + + modifier fullyConfirmed(uint transactionId) { + require(isConfirmed(transactionId)); + _; + } + + modifier pastTimeLock(uint transactionId) { + require(block.timestamp >= confirmationTimes[transactionId] + secondsTimeLocked); + _; + } + + /* + * Public functions + */ + + /// @dev Contract constructor sets initial owners, required number of confirmations, and time lock. + /// @param _owners List of initial owners. + /// @param _required Number of required confirmations. + /// @param _secondsTimeLocked Duration needed after a transaction is confirmed and before it becomes executable, in seconds. + function MultiSigWalletWithTimeLock(address[] _owners, uint _required, uint _secondsTimeLocked) + public + MultiSigWallet(_owners, _required) + { + secondsTimeLocked = _secondsTimeLocked; + } + + /// @dev Changes the duration of the time lock for transactions. + /// @param _secondsTimeLocked Duration needed after a transaction is confirmed and before it becomes executable, in seconds. + function changeTimeLock(uint _secondsTimeLocked) + public + onlyWallet + { + secondsTimeLocked = _secondsTimeLocked; + TimeLockChange(_secondsTimeLocked); + } + + /// @dev Allows an owner to confirm a transaction. + /// @param transactionId Transaction ID. + function confirmTransaction(uint transactionId) + public + ownerExists(msg.sender) + transactionExists(transactionId) + notConfirmed(transactionId, msg.sender) + notFullyConfirmed(transactionId) + { + confirmations[transactionId][msg.sender] = true; + Confirmation(msg.sender, transactionId); + if (isConfirmed(transactionId)) { + setConfirmationTime(transactionId, block.timestamp); + } + } + + /// @dev Allows an owner to revoke a confirmation for a transaction. + /// @param transactionId Transaction ID. + function revokeConfirmation(uint transactionId) + public + ownerExists(msg.sender) + confirmed(transactionId, msg.sender) + notExecuted(transactionId) + notFullyConfirmed(transactionId) + { + confirmations[transactionId][msg.sender] = false; + Revocation(msg.sender, transactionId); + } + + /// @dev Allows anyone to execute a confirmed transaction. + /// @param transactionId Transaction ID. + function executeTransaction(uint transactionId) + public + notExecuted(transactionId) + fullyConfirmed(transactionId) + pastTimeLock(transactionId) + { + Transaction storage tx = transactions[transactionId]; + tx.executed = true; + if (tx.destination.call.value(tx.value)(tx.data)) + Execution(transactionId); + else { + ExecutionFailure(transactionId); + tx.executed = false; + } + } + + /* + * Internal functions + */ + + /// @dev Sets the time of when a submission first passed. + function setConfirmationTime(uint transactionId, uint confirmationTime) + internal + { + confirmationTimes[transactionId] = confirmationTime; + ConfirmationTimeSet(transactionId, confirmationTime); + } +} diff --git a/packages/contracts/src/contracts/current/multisig/MultiSigWalletWithTimeLock/MultiSigWalletWithTimeLock.sol b/packages/contracts/src/contracts/current/multisig/MultiSigWalletWithTimeLock/MultiSigWalletWithTimeLock.sol deleted file mode 100644 index e393b565f..000000000 --- a/packages/contracts/src/contracts/current/multisig/MultiSigWalletWithTimeLock/MultiSigWalletWithTimeLock.sol +++ /dev/null @@ -1,132 +0,0 @@ -/* - - Copyright 2018 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity ^0.4.10; - -import { MultiSigWallet } from "../MultiSigWallet/MultiSigWallet.sol"; - -/// @title Multisignature wallet with time lock- Allows multiple parties to execute a transaction after a time lock has passed. -/// @author Amir Bandeali - -contract MultiSigWalletWithTimeLock is MultiSigWallet { - - event ConfirmationTimeSet(uint indexed transactionId, uint confirmationTime); - event TimeLockChange(uint secondsTimeLocked); - - uint public secondsTimeLocked; - - mapping (uint => uint) public confirmationTimes; - - modifier notFullyConfirmed(uint transactionId) { - require(!isConfirmed(transactionId)); - _; - } - - modifier fullyConfirmed(uint transactionId) { - require(isConfirmed(transactionId)); - _; - } - - modifier pastTimeLock(uint transactionId) { - require(block.timestamp >= confirmationTimes[transactionId] + secondsTimeLocked); - _; - } - - /* - * Public functions - */ - - /// @dev Contract constructor sets initial owners, required number of confirmations, and time lock. - /// @param _owners List of initial owners. - /// @param _required Number of required confirmations. - /// @param _secondsTimeLocked Duration needed after a transaction is confirmed and before it becomes executable, in seconds. - function MultiSigWalletWithTimeLock(address[] _owners, uint _required, uint _secondsTimeLocked) - public - MultiSigWallet(_owners, _required) - { - secondsTimeLocked = _secondsTimeLocked; - } - - /// @dev Changes the duration of the time lock for transactions. - /// @param _secondsTimeLocked Duration needed after a transaction is confirmed and before it becomes executable, in seconds. - function changeTimeLock(uint _secondsTimeLocked) - public - onlyWallet - { - secondsTimeLocked = _secondsTimeLocked; - TimeLockChange(_secondsTimeLocked); - } - - /// @dev Allows an owner to confirm a transaction. - /// @param transactionId Transaction ID. - function confirmTransaction(uint transactionId) - public - ownerExists(msg.sender) - transactionExists(transactionId) - notConfirmed(transactionId, msg.sender) - notFullyConfirmed(transactionId) - { - confirmations[transactionId][msg.sender] = true; - Confirmation(msg.sender, transactionId); - if (isConfirmed(transactionId)) { - setConfirmationTime(transactionId, block.timestamp); - } - } - - /// @dev Allows an owner to revoke a confirmation for a transaction. - /// @param transactionId Transaction ID. - function revokeConfirmation(uint transactionId) - public - ownerExists(msg.sender) - confirmed(transactionId, msg.sender) - notExecuted(transactionId) - notFullyConfirmed(transactionId) - { - confirmations[transactionId][msg.sender] = false; - Revocation(msg.sender, transactionId); - } - - /// @dev Allows anyone to execute a confirmed transaction. - /// @param transactionId Transaction ID. - function executeTransaction(uint transactionId) - public - notExecuted(transactionId) - fullyConfirmed(transactionId) - pastTimeLock(transactionId) - { - Transaction storage tx = transactions[transactionId]; - tx.executed = true; - if (tx.destination.call.value(tx.value)(tx.data)) - Execution(transactionId); - else { - ExecutionFailure(transactionId); - tx.executed = false; - } - } - - /* - * Internal functions - */ - - /// @dev Sets the time of when a submission first passed. - function setConfirmationTime(uint transactionId, uint confirmationTime) - internal - { - confirmationTimes[transactionId] = confirmationTime; - ConfirmationTimeSet(transactionId, confirmationTime); - } -} diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol index 1b0f9c34f..1061d0b9a 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol @@ -18,7 +18,7 @@ pragma solidity ^0.4.10; -import "../../multisig/MultiSigWalletWithTimeLock/MultiSigWalletWithTimeLock.sol"; +import "../../multisig/MultiSigWalletWithTimeLock.sol"; contract AssetProxyOwner is MultiSigWalletWithTimeLock diff --git a/packages/contracts/src/contracts/previous/MultiSigWalletWithTImeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol b/packages/contracts/src/contracts/previous/MultiSigWalletWithTImeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol index 8d7d64a75..241e02d4a 100644 --- a/packages/contracts/src/contracts/previous/MultiSigWalletWithTImeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol +++ b/packages/contracts/src/contracts/previous/MultiSigWalletWithTImeLockExceptRemoveAuthorizedAddress/MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress.sol @@ -1,6 +1,6 @@ /* - Copyright 2017 ZeroEx Intl. + Copyright 2018 ZeroEx Intl. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -18,7 +18,7 @@ pragma solidity ^0.4.10; -import "../../current/multisig/MultiSigWalletWithTimeLock/MultiSigWalletWithTimeLock.sol"; +import "../../current/multisig/MultiSigWalletWithTimeLock.sol"; contract MultiSigWalletWithTimeLockExceptRemoveAuthorizedAddress is MultiSigWalletWithTimeLock { diff --git a/packages/contracts/src/utils/multi_sig_wrapper.ts b/packages/contracts/src/utils/multi_sig_wrapper.ts index c33e7bb47..730cdcbef 100644 --- a/packages/contracts/src/utils/multi_sig_wrapper.ts +++ b/packages/contracts/src/utils/multi_sig_wrapper.ts @@ -27,17 +27,17 @@ export class MultiSigWrapper { const txHash = await this._multiSig.submitTransaction.sendTransactionAsync(destination, value, data, { from, }); - const tx = await this._getTxWithDecodedMultiSigLogs(txHash); + const tx = await this._getTxWithDecodedMultiSigLogsAsync(txHash); return tx; } public async confirmTransactionAsync(txId: BigNumber, from: string): Promise { const txHash = await this._multiSig.confirmTransaction.sendTransactionAsync(txId, { from }); - const tx = await this._getTxWithDecodedMultiSigLogs(txHash); + const tx = await this._getTxWithDecodedMultiSigLogsAsync(txHash); return tx; } public async executeTransactionAsync(txId: BigNumber, from: string): Promise { const txHash = await this._multiSig.executeTransaction.sendTransactionAsync(txId, { from }); - const tx = await this._getTxWithDecodedMultiSigLogs(txHash); + const tx = await this._getTxWithDecodedMultiSigLogsAsync(txHash); return tx; } public async executeRemoveAuthorizedAddressAsync( @@ -46,10 +46,10 @@ export class MultiSigWrapper { ): Promise { const txHash = await (this ._multiSig as AssetProxyOwnerContract).executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from }); - const tx = await this._getTxWithDecodedMultiSigLogs(txHash); + const tx = await this._getTxWithDecodedMultiSigLogsAsync(txHash); return tx; } - private async _getTxWithDecodedMultiSigLogs(txHash: string) { + private async _getTxWithDecodedMultiSigLogsAsync(txHash: string): Promise { const tx = await this._zeroEx.awaitTransactionMinedAsync(txHash); tx.logs = _.filter(tx.logs, log => log.address === this._multiSig.address); tx.logs = _.map(tx.logs, log => this._logDecoder.decodeLogOrThrow(log)); diff --git a/packages/contracts/test/asset_proxy_owner.ts b/packages/contracts/test/asset_proxy_owner.ts index 2e8e9373a..6e999dd99 100644 --- a/packages/contracts/test/asset_proxy_owner.ts +++ b/packages/contracts/test/asset_proxy_owner.ts @@ -28,7 +28,7 @@ const zeroEx = new ZeroEx(provider, { networkId: constants.TESTRPC_NETWORK_ID }) describe('AssetProxyOwner', () => { let owners: string[]; let authorized: string; - const requiredApprovals = new BigNumber(2); + const REQUIRED_APPROVALS = new BigNumber(2); const SECONDS_TIME_LOCKED = new BigNumber(1000000); let erc20Proxy: MixinAuthorizableContract; @@ -56,7 +56,7 @@ describe('AssetProxyOwner', () => { provider, txDefaults, owners, - requiredApprovals, + REQUIRED_APPROVALS, SECONDS_TIME_LOCKED, defaultAssetProxyContractAddresses, ); @@ -79,7 +79,7 @@ describe('AssetProxyOwner', () => { provider, txDefaults, owners, - requiredApprovals, + REQUIRED_APPROVALS, SECONDS_TIME_LOCKED, assetProxyContractAddresses, ); @@ -96,7 +96,7 @@ describe('AssetProxyOwner', () => { provider, txDefaults, owners, - requiredApprovals, + REQUIRED_APPROVALS, SECONDS_TIME_LOCKED, assetProxyContractAddresses, ), diff --git a/packages/contracts/test/multi_sig_with_time_lock.ts b/packages/contracts/test/multi_sig_with_time_lock.ts index c369ca63a..ace0f0045 100644 --- a/packages/contracts/test/multi_sig_with_time_lock.ts +++ b/packages/contracts/test/multi_sig_with_time_lock.ts @@ -23,7 +23,7 @@ const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); describe('MultiSigWalletWithTimeLock', () => { let owners: string[]; - const requiredApprovals = new BigNumber(2); + const REQUIRED_APPROVALS = new BigNumber(2); const SECONDS_TIME_LOCKED = new BigNumber(1000000); before(async () => { @@ -56,7 +56,7 @@ describe('MultiSigWalletWithTimeLock', () => { provider, txDefaults, owners, - requiredApprovals, + REQUIRED_APPROVALS, secondsTimeLocked, ); multiSigWrapper = new MultiSigWrapper(multiSig, provider); @@ -127,7 +127,7 @@ describe('MultiSigWalletWithTimeLock', () => { provider, txDefaults, owners, - requiredApprovals, + REQUIRED_APPROVALS, SECONDS_TIME_LOCKED, ); multiSigWrapper = new MultiSigWrapper(multiSig, provider); -- cgit v1.2.3 From d4aacd218a4fdb1876ac656e04fcccdb892a9395 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 22 May 2018 07:39:21 -0700 Subject: Move readFirst4 to LibBytes --- .../protocol/AssetProxyOwner/AssetProxyOwner.sol | 31 ++++++++-------------- .../current/test/TestLibBytes/TestLibBytes.sol | 12 +++++++++ .../contracts/current/utils/LibBytes/LibBytes.sol | 19 +++++++++++++ packages/contracts/test/asset_proxy_owner.ts | 22 +++------------ packages/contracts/test/libraries/lib_bytes.ts | 8 ++++++ 5 files changed, 53 insertions(+), 39 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol index 1061d0b9a..1b74dfc44 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol @@ -19,8 +19,10 @@ pragma solidity ^0.4.10; import "../../multisig/MultiSigWalletWithTimeLock.sol"; +import "../../utils/LibBytes/LibBytes.sol"; contract AssetProxyOwner is + LibBytes, MultiSigWalletWithTimeLock { @@ -31,6 +33,8 @@ contract AssetProxyOwner is bytes4 constant REMOVE_AUTHORIZED_ADDRESS_SELECTOR = bytes4(keccak256("removeAuthorizedAddress(address)")); + /// @dev Function will revert if the transaction does not call `removeAuthorizedAddress` + /// on an approved AssetProxy contract. modifier validRemoveAuthorizedAddressTx(uint256 transactionId) { Transaction storage tx = transactions[transactionId]; require(isAssetProxyRegistered[tx.destination]); @@ -41,20 +45,22 @@ contract AssetProxyOwner is /// @dev Contract constructor sets initial owners, required number of confirmations, /// time lock, and list of AssetProxy addresses. /// @param _owners List of initial owners. + /// @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. - /// @param _assetProxyContracts Array of AssetProxy contract addresses. function AssetProxyOwner( address[] memory _owners, + address[] memory _assetProxyContracts, uint256 _required, - uint256 _secondsTimeLocked, - address[] memory _assetProxyContracts) + uint256 _secondsTimeLocked + ) public MultiSigWalletWithTimeLock(_owners, _required, _secondsTimeLocked) { for (uint256 i = 0; i < _assetProxyContracts.length; i++) { - require(_assetProxyContracts[i] != address(0)); - isAssetProxyRegistered[_assetProxyContracts[i]] = true; + address assetProxy = _assetProxyContracts[i]; + require(assetProxy != address(0)); + isAssetProxyRegistered[assetProxy] = true; } } @@ -101,19 +107,4 @@ contract AssetProxyOwner is require(REMOVE_AUTHORIZED_ADDRESS_SELECTOR == first4Bytes); return true; } - - /// @dev Reads the first 4 bytes from a byte array of arbitrary length. - /// @param data Byte array to read first 4 bytes from. - /// @return First 4 bytes of data. - function readFirst4(bytes memory data) - public - pure - returns (bytes4 result) - { - require(data.length >= 4); - assembly { - result := mload(add(data, 32)) - } - return result; - } } diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index ac4602933..5a6801262 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -130,4 +130,16 @@ contract TestLibBytes is writeUint256(b, index, input); return b; } + + /// @dev Reads the first 4 bytes from a byte array of arbitrary length. + /// @param b Byte array to read first 4 bytes from. + /// @return First 4 bytes of data. + function publicReadFirst4(bytes memory b) + public + pure + returns (bytes4 result) + { + result = readFirst4(b); + return result; + } } diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 2c5d9e756..975565773 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -21,6 +21,7 @@ pragma solidity ^0.4.24; contract LibBytes { // Revert reasons + string constant GTE_4_LENGTH_REQUIRED = "Length must be greater than or equal to 4."; string constant GTE_20_LENGTH_REQUIRED = "Length must be greater than or equal to 20."; string constant GTE_32_LENGTH_REQUIRED = "Length must be greater than or equal to 32."; @@ -203,4 +204,22 @@ contract LibBytes { { writeBytes32(b, index, bytes32(input)); } + + /// @dev Reads the first 4 bytes from a byte array of arbitrary length. + /// @param b Byte array to read first 4 bytes from. + /// @return First 4 bytes of data. + function readFirst4(bytes memory b) + internal + pure + returns (bytes4 result) + { + require( + b.length >= 4, + GTE_4_LENGTH_REQUIRED + ); + assembly { + result := mload(add(b, 32)) + } + return result; + } } diff --git a/packages/contracts/test/asset_proxy_owner.ts b/packages/contracts/test/asset_proxy_owner.ts index 6e999dd99..e3c6a5324 100644 --- a/packages/contracts/test/asset_proxy_owner.ts +++ b/packages/contracts/test/asset_proxy_owner.ts @@ -56,9 +56,9 @@ describe('AssetProxyOwner', () => { provider, txDefaults, owners, + defaultAssetProxyContractAddresses, REQUIRED_APPROVALS, SECONDS_TIME_LOCKED, - defaultAssetProxyContractAddresses, ); multiSigWrapper = new MultiSigWrapper(multiSig, zeroEx); await erc20Proxy.transferOwnership.sendTransactionAsync(multiSig.address, { from: initialOwner }); @@ -79,9 +79,9 @@ describe('AssetProxyOwner', () => { provider, txDefaults, owners, + assetProxyContractAddresses, REQUIRED_APPROVALS, SECONDS_TIME_LOCKED, - assetProxyContractAddresses, ); const isErc20ProxyRegistered = await newMultiSig.isAssetProxyRegistered.callAsync(erc20Proxy.address); const isErc721ProxyRegistered = await newMultiSig.isAssetProxyRegistered.callAsync(erc721Proxy.address); @@ -96,29 +96,13 @@ describe('AssetProxyOwner', () => { provider, txDefaults, owners, + assetProxyContractAddresses, REQUIRED_APPROVALS, SECONDS_TIME_LOCKED, - assetProxyContractAddresses, ), ).to.be.rejectedWith(constants.REVERT); }); }); - describe('readFirst4', () => { - it('should return the first 4 bytes of a byte array of arbitrary length', async () => { - const addAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData(owners[0]); - const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( - owners[0], - ); - const expectedAddAuthorizedAddressSelector = addAuthorizedAddressData.slice(0, 10); - const expectedRemoveAuthorizedAddressSelector = removeAuthorizedAddressData.slice(0, 10); - const [addAuthorizedAddressSelector, removeAuthorizedAddressSelector] = await Promise.all([ - multiSig.readFirst4.callAsync(addAuthorizedAddressData), - multiSig.readFirst4.callAsync(removeAuthorizedAddressData), - ]); - expect(expectedAddAuthorizedAddressSelector).to.equal(addAuthorizedAddressSelector); - expect(expectedRemoveAuthorizedAddressSelector).to.equal(removeAuthorizedAddressSelector); - }); - }); describe('isFunctionRemoveAuthorizedAddress', () => { it('should throw if data is not for removeAuthorizedAddress', async () => { diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts index ce3adbdae..fc28c363b 100644 --- a/packages/contracts/test/libraries/lib_bytes.ts +++ b/packages/contracts/test/libraries/lib_bytes.ts @@ -248,4 +248,12 @@ 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 () => {}); }); */ + + describe('readFirst4', () => { + it('should return the first 4 bytes of a byte array of arbitrary length', async () => { + const first4Bytes = libBytes.publicReadFirst4.callAsync(byteArrayLongerThan32Bytes); + const expectedFirst4Bytes = byteArrayLongerThan32Bytes.slice(0, 10); + expect(first4Bytes).to.equal(expectedFirst4Bytes); + }); + }); }); -- cgit v1.2.3 From fdea260e41111f3d27c05ab2d2523d2473c1ac05 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 22 May 2018 12:53:37 -0700 Subject: Cleanup tests --- packages/contracts/src/utils/multi_sig_wrapper.ts | 2 +- packages/contracts/test/asset_proxy_owner.ts | 4 ++-- packages/contracts/test/exchange/core.ts | 4 ++-- packages/contracts/test/libraries/lib_bytes.ts | 8 +++++++- 4 files changed, 12 insertions(+), 6 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/utils/multi_sig_wrapper.ts b/packages/contracts/src/utils/multi_sig_wrapper.ts index 730cdcbef..3e6c96d84 100644 --- a/packages/contracts/src/utils/multi_sig_wrapper.ts +++ b/packages/contracts/src/utils/multi_sig_wrapper.ts @@ -50,7 +50,7 @@ export class MultiSigWrapper { return tx; } private async _getTxWithDecodedMultiSigLogsAsync(txHash: string): Promise { - const tx = await this._zeroEx.awaitTransactionMinedAsync(txHash); + const tx = await this._zeroEx.awaitTransactionMinedAsync(txHash, constants.AWAIT_TRANSACTION_MINED_MS); tx.logs = _.filter(tx.logs, log => log.address === this._multiSig.address); tx.logs = _.map(tx.logs, log => this._logDecoder.decodeLogOrThrow(log)); return tx; diff --git a/packages/contracts/test/asset_proxy_owner.ts b/packages/contracts/test/asset_proxy_owner.ts index e3c6a5324..7a520d0ad 100644 --- a/packages/contracts/test/asset_proxy_owner.ts +++ b/packages/contracts/test/asset_proxy_owner.ts @@ -90,7 +90,7 @@ describe('AssetProxyOwner', () => { }); it('should throw if a null address is included in assetProxyContracts', async () => { const assetProxyContractAddresses = [erc20Proxy.address, ZeroEx.NULL_ADDRESS]; - expect( + return expect( AssetProxyOwnerContract.deployFrom0xArtifactAsync( artifacts.AssetProxyOwner, provider, @@ -128,7 +128,7 @@ describe('AssetProxyOwner', () => { describe('registerAssetProxy', () => { it('should throw if not called by multisig', async () => { const isRegistered = true; - expect( + return expect( multiSig.registerAssetProxy.sendTransactionAsync(erc20Proxy.address, isRegistered, { from: owners[0] }), ).to.be.rejectedWith(constants.REVERT); }); diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index 84f25361c..29b19e920 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -703,7 +703,7 @@ describe('Exchange core', () => { // Create 3 orders with makerEpoch values: 0,1,2,3 // Since we cancelled with makerEpoch=1, orders with makerEpoch<=1 will not be processed erc20Balances = await erc20Wrapper.getBalancesAsync(); - const signedOrders = await Promise.all([ + const signedOrders = [ orderFactory.newSignedOrder({ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(9), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(9), 18), @@ -724,7 +724,7 @@ describe('Exchange core', () => { takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(7979), 18), salt: new BigNumber(3), }), - ]); + ]; await exchangeWrapper.batchFillOrdersNoThrowAsync(signedOrders, takerAddress); const newBalances = await erc20Wrapper.getBalancesAsync(); diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts index fc28c363b..968bac300 100644 --- a/packages/contracts/test/libraries/lib_bytes.ts +++ b/packages/contracts/test/libraries/lib_bytes.ts @@ -250,8 +250,14 @@ describe('LibBytes', () => { */ describe('readFirst4', () => { + it('should revert if byte array has a length < 4', async () => { + const byteArrayLessThan4Bytes = '0x010101'; + return expect(libBytes.publicReadFirst4.callAsync(byteArrayLessThan4Bytes)).to.be.rejectedWith( + constants.REVERT, + ); + }); it('should return the first 4 bytes of a byte array of arbitrary length', async () => { - const first4Bytes = libBytes.publicReadFirst4.callAsync(byteArrayLongerThan32Bytes); + const first4Bytes = await libBytes.publicReadFirst4.callAsync(byteArrayLongerThan32Bytes); const expectedFirst4Bytes = byteArrayLongerThan32Bytes.slice(0, 10); expect(first4Bytes).to.equal(expectedFirst4Bytes); }); -- cgit v1.2.3 From 237ebb07161c46fa85ca778a1c2ff60f63611bd7 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Thu, 24 May 2018 15:33:56 -0700 Subject: Use web3-wrapper instead of 0x.js, update logDecoder --- .../protocol/AssetProxyOwner/AssetProxyOwner.sol | 5 ++- packages/contracts/src/utils/erc20_wrapper.ts | 6 ++- packages/contracts/src/utils/erc721_wrapper.ts | 6 ++- packages/contracts/src/utils/exchange_wrapper.ts | 6 +-- packages/contracts/src/utils/log_decoder.ts | 48 ++++++++++------------ packages/contracts/src/utils/multi_sig_wrapper.ts | 17 ++++---- packages/contracts/test/asset_proxy_owner.ts | 9 ++-- 7 files changed, 48 insertions(+), 49 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol index 1b74dfc44..7f5f056b5 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol @@ -28,7 +28,8 @@ contract AssetProxyOwner is event AssetProxyRegistration(address assetProxyContract, bool isRegistered); - // Mapping of AssetProxy contract address => approved to execute removeAuthorizedAddress without time lock. + // Mapping of AssetProxy contract address => + // if this contract is allowed to call the AssetProxy's removeAuthorizedAddress method without a time lock. mapping (address => bool) public isAssetProxyRegistered; bytes4 constant REMOVE_AUTHORIZED_ADDRESS_SELECTOR = bytes4(keccak256("removeAuthorizedAddress(address)")); @@ -95,7 +96,7 @@ contract AssetProxyOwner is } } - /// @dev Compares first 4 bytes of byte array to removeAuthorizedAddress function signature. + /// @dev Compares first 4 bytes of byte array to removeAuthorizedAddress function selector. /// @param data Transaction data. /// @return Successful if data is a call to removeAuthorizedAddress. function isFunctionRemoveAuthorizedAddress(bytes memory data) diff --git a/packages/contracts/src/utils/erc20_wrapper.ts b/packages/contracts/src/utils/erc20_wrapper.ts index 0303649a5..92cf01cc2 100644 --- a/packages/contracts/src/utils/erc20_wrapper.ts +++ b/packages/contracts/src/utils/erc20_wrapper.ts @@ -1,5 +1,6 @@ import { Provider } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; +import { Web3Wrapper } from '@0xproject/web3-wrapper'; import * as _ from 'lodash'; import { DummyERC20TokenContract } from '../contract_wrappers/generated/dummy_e_r_c20_token'; @@ -13,10 +14,12 @@ import { txDefaults } from './web3_wrapper'; export class ERC20Wrapper { private _tokenOwnerAddresses: string[]; private _contractOwnerAddress: string; + private _web3Wrapper: Web3Wrapper; private _provider: Provider; private _dummyTokenContracts?: DummyERC20TokenContract[]; private _proxyContract?: ERC20ProxyContract; constructor(provider: Provider, tokenOwnerAddresses: string[], contractOwnerAddress: string) { + this._web3Wrapper = new Web3Wrapper(provider); this._provider = provider; this._tokenOwnerAddresses = tokenOwnerAddresses; this._contractOwnerAddress = contractOwnerAddress; @@ -68,7 +71,8 @@ export class ERC20Wrapper { ); }); }); - await Promise.all([...setBalancePromises, ...setAllowancePromises]); + const txHashes = await Promise.all([...setBalancePromises, ...setAllowancePromises]); + await Promise.all(_.map(txHashes, async txHash => this._web3Wrapper.awaitTransactionSuccessAsync(txHash))); } public async getBalancesAsync(): Promise { this._validateDummyTokenContractsExistOrThrow(); diff --git a/packages/contracts/src/utils/erc721_wrapper.ts b/packages/contracts/src/utils/erc721_wrapper.ts index aee796e4b..bea801f3d 100644 --- a/packages/contracts/src/utils/erc721_wrapper.ts +++ b/packages/contracts/src/utils/erc721_wrapper.ts @@ -1,6 +1,7 @@ import { generatePseudoRandomSalt } from '@0xproject/order-utils'; import { Provider } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; +import { Web3Wrapper } from '@0xproject/web3-wrapper'; import * as _ from 'lodash'; import { DummyERC721TokenContract } from '../contract_wrappers/generated/dummy_e_r_c721_token'; @@ -14,11 +15,13 @@ import { txDefaults } from './web3_wrapper'; export class ERC721Wrapper { private _tokenOwnerAddresses: string[]; private _contractOwnerAddress: string; + private _web3Wrapper: Web3Wrapper; private _provider: Provider; private _dummyTokenContracts?: DummyERC721TokenContract[]; private _proxyContract?: ERC721ProxyContract; private _initialTokenIdsByOwner: ERC721TokenIdsByOwner = {}; constructor(provider: Provider, tokenOwnerAddresses: string[], contractOwnerAddress: string) { + this._web3Wrapper = new Web3Wrapper(provider); this._provider = provider; this._tokenOwnerAddresses = tokenOwnerAddresses; this._contractOwnerAddress = contractOwnerAddress; @@ -80,7 +83,8 @@ export class ERC721Wrapper { ); }); }); - await Promise.all([...setBalancePromises, ...setAllowancePromises]); + const txHashes = await Promise.all([...setBalancePromises, ...setAllowancePromises]); + await Promise.all(_.map(txHashes, async txHash => this._web3Wrapper.awaitTransactionSuccessAsync(txHash))); } public async getBalancesAsync(): Promise { this._validateDummyTokenContractsExistOrThrow(); diff --git a/packages/contracts/src/utils/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts index c353442f3..5ed696272 100644 --- a/packages/contracts/src/utils/exchange_wrapper.ts +++ b/packages/contracts/src/utils/exchange_wrapper.ts @@ -2,20 +2,18 @@ import { Provider, TransactionReceiptWithDecodedLogs } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; import * as _ from 'lodash'; -import * as Web3 from 'web3'; import { ExchangeContract } from '../contract_wrappers/generated/exchange'; import { constants } from './constants'; import { formatters } from './formatters'; -import { LogDecoder } from './log_decoder'; +import { logDecoder } from './log_decoder'; import { orderUtils } from './order_utils'; import { AssetProxyId, OrderInfo, SignedOrder, SignedTransaction } from './types'; export class ExchangeWrapper { private _exchange: ExchangeContract; private _web3Wrapper: Web3Wrapper; - private _logDecoder: LogDecoder = new LogDecoder(constants.TESTRPC_NETWORK_ID); constructor(exchangeContract: ExchangeContract, provider: Provider) { this._exchange = exchangeContract; this._web3Wrapper = new Web3Wrapper(provider); @@ -249,7 +247,7 @@ export class ExchangeWrapper { private async _getTxWithDecodedExchangeLogsAsync(txHash: string): Promise { const tx = await this._web3Wrapper.awaitTransactionSuccessAsync(txHash, constants.AWAIT_TRANSACTION_MINED_MS); tx.logs = _.filter(tx.logs, log => log.address === this._exchange.address); - tx.logs = _.map(tx.logs, log => this._logDecoder.decodeLogOrThrow(log)); + tx.logs = _.map(tx.logs, log => logDecoder.decodeLogOrThrow(log)); return tx; } } diff --git a/packages/contracts/src/utils/log_decoder.ts b/packages/contracts/src/utils/log_decoder.ts index 747c7644d..d2e65d176 100644 --- a/packages/contracts/src/utils/log_decoder.ts +++ b/packages/contracts/src/utils/log_decoder.ts @@ -5,35 +5,29 @@ import * as _ from 'lodash'; import { artifacts } from './artifacts'; -export class LogDecoder { - private _abiDecoder: AbiDecoder; - constructor(networkIdIfExists?: number) { - if (_.isUndefined(networkIdIfExists)) { - throw new Error('networkId not specified'); +const abiArrays: AbiDefinition[][] = []; +_.forEach(artifacts, (artifact: ContractArtifact) => { + const compilerOutput = artifact.compilerOutput; + abiArrays.push(compilerOutput.abi); +}); +const abiDecoder = new AbiDecoder(abiArrays); + +export const logDecoder = { + wrapLogBigNumbers(log: any): any { + const argNames = _.keys(log.args); + for (const argName of argNames) { + const isWeb3BigNumber = _.startsWith(log.args[argName].constructor.toString(), 'function BigNumber('); + if (isWeb3BigNumber) { + log.args[argName] = new BigNumber(log.args[argName]); + } } - const abiArrays: AbiDefinition[][] = []; - _.forEach(artifacts, (artifact: ContractArtifact) => { - const compilerOutput = artifact.compilerOutput; - abiArrays.push(compilerOutput.abi); - }); - this._abiDecoder = new AbiDecoder(abiArrays); - } - public decodeLogOrThrow(log: LogEntry): LogWithDecodedArgs | RawLog { - const logWithDecodedArgsOrLog = this._abiDecoder.tryToDecodeLogOrNoop(log); + }, + decodeLogOrThrow(log: LogEntry): LogWithDecodedArgs | RawLog { + const logWithDecodedArgsOrLog = abiDecoder.tryToDecodeLogOrNoop(log); if (_.isUndefined((logWithDecodedArgsOrLog as LogWithDecodedArgs).args)) { throw new Error(`Unable to decode log: ${JSON.stringify(log)}`); } - wrapLogBigNumbers(logWithDecodedArgsOrLog); + logDecoder.wrapLogBigNumbers(logWithDecodedArgsOrLog); return logWithDecodedArgsOrLog; - } -} - -function wrapLogBigNumbers(log: any): any { - const argNames = _.keys(log.args); - for (const argName of argNames) { - const isWeb3BigNumber = _.startsWith(log.args[argName].constructor.toString(), 'function BigNumber('); - if (isWeb3BigNumber) { - log.args[argName] = new BigNumber(log.args[argName]); - } - } -} + }, +}; diff --git a/packages/contracts/src/utils/multi_sig_wrapper.ts b/packages/contracts/src/utils/multi_sig_wrapper.ts index 3e6c96d84..5c73cdf5a 100644 --- a/packages/contracts/src/utils/multi_sig_wrapper.ts +++ b/packages/contracts/src/utils/multi_sig_wrapper.ts @@ -1,21 +1,20 @@ -import { TransactionReceiptWithDecodedLogs, ZeroEx } from '0x.js'; +import { Provider, TransactionReceiptWithDecodedLogs } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; +import { Web3Wrapper } from '@0xproject/web3-wrapper'; import * as _ from 'lodash'; -import * as Web3 from 'web3'; import { AssetProxyOwnerContract } from '../contract_wrappers/generated/asset_proxy_owner'; import { MultiSigWalletContract } from '../contract_wrappers/generated/multi_sig_wallet'; import { constants } from './constants'; -import { LogDecoder } from './log_decoder'; +import { logDecoder } from './log_decoder'; export class MultiSigWrapper { private _multiSig: MultiSigWalletContract; - private _logDecoder: LogDecoder = new LogDecoder(constants.TESTRPC_NETWORK_ID); - private _zeroEx: ZeroEx; - constructor(multiSigContract: MultiSigWalletContract, zeroEx: ZeroEx) { + private _web3Wrapper: Web3Wrapper; + constructor(multiSigContract: MultiSigWalletContract, provider: Provider) { this._multiSig = multiSigContract; - this._zeroEx = zeroEx; + this._web3Wrapper = new Web3Wrapper(provider); } public async submitTransactionAsync( destination: string, @@ -50,9 +49,9 @@ export class MultiSigWrapper { return tx; } private async _getTxWithDecodedMultiSigLogsAsync(txHash: string): Promise { - const tx = await this._zeroEx.awaitTransactionMinedAsync(txHash, constants.AWAIT_TRANSACTION_MINED_MS); + const tx = await this._web3Wrapper.awaitTransactionMinedAsync(txHash, constants.AWAIT_TRANSACTION_MINED_MS); tx.logs = _.filter(tx.logs, log => log.address === this._multiSig.address); - tx.logs = _.map(tx.logs, log => this._logDecoder.decodeLogOrThrow(log)); + tx.logs = _.map(tx.logs, log => logDecoder.decodeLogOrThrow(log)); return tx; } } diff --git a/packages/contracts/test/asset_proxy_owner.ts b/packages/contracts/test/asset_proxy_owner.ts index 7a520d0ad..db68b5678 100644 --- a/packages/contracts/test/asset_proxy_owner.ts +++ b/packages/contracts/test/asset_proxy_owner.ts @@ -1,5 +1,5 @@ -import { LogWithDecodedArgs, ZeroEx } from '0x.js'; import { BlockchainLifecycle } from '@0xproject/dev-utils'; +import { LogWithDecodedArgs } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; import * as _ from 'lodash'; @@ -23,7 +23,6 @@ import { provider, txDefaults, web3Wrapper } from '../src/utils/web3_wrapper'; chaiSetup.configure(); const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); -const zeroEx = new ZeroEx(provider, { networkId: constants.TESTRPC_NETWORK_ID }); describe('AssetProxyOwner', () => { let owners: string[]; @@ -60,7 +59,7 @@ describe('AssetProxyOwner', () => { REQUIRED_APPROVALS, SECONDS_TIME_LOCKED, ); - multiSigWrapper = new MultiSigWrapper(multiSig, zeroEx); + multiSigWrapper = new MultiSigWrapper(multiSig, provider); await erc20Proxy.transferOwnership.sendTransactionAsync(multiSig.address, { from: initialOwner }); await erc721Proxy.transferOwnership.sendTransactionAsync(multiSig.address, { from: initialOwner }); }); @@ -89,7 +88,7 @@ describe('AssetProxyOwner', () => { expect(isErc721ProxyRegistered).to.equal(true); }); it('should throw if a null address is included in assetProxyContracts', async () => { - const assetProxyContractAddresses = [erc20Proxy.address, ZeroEx.NULL_ADDRESS]; + const assetProxyContractAddresses = [erc20Proxy.address, constants.NULL_ADDRESS]; return expect( AssetProxyOwnerContract.deployFrom0xArtifactAsync( artifacts.AssetProxyOwner, @@ -161,7 +160,7 @@ describe('AssetProxyOwner', () => { }); it('should fail if registering a null address', async () => { - const addressToRegister = ZeroEx.NULL_ADDRESS; + const addressToRegister = constants.NULL_ADDRESS; const isRegistered = true; const registerAssetProxyData = multiSig.registerAssetProxy.getABIEncodedTransactionData( addressToRegister, -- cgit v1.2.3