From d27c254181e4749beb904d002a5a0918e4545088 Mon Sep 17 00:00:00 2001 From: Brian Weickmann Date: Sun, 29 Jul 2018 12:25:43 -0700 Subject: [PATCH 1/2] Formatting in ERC20Wrapper and making some functions external. Linked ERC20Wrapper to transferProxy. --- contracts/lib/ERC20Wrapper.sol | 104 ++++++++++++++++++++++----------- utils/coreWrapper.ts | 9 +++ 2 files changed, 79 insertions(+), 34 deletions(-) diff --git a/contracts/lib/ERC20Wrapper.sol b/contracts/lib/ERC20Wrapper.sol index f67bcf12e..9d64eda84 100644 --- a/contracts/lib/ERC20Wrapper.sol +++ b/contracts/lib/ERC20Wrapper.sol @@ -30,84 +30,120 @@ import { IERC20 } from "./IERC20.sol"; */ library ERC20Wrapper { - // ============ Constants ============ - - string constant INVALID_RETURN_TRANSFER = "Transferred token does not return null or true on successful transfer."; - /* solium-disable-next-line max-len */ - string constant INVALID_RETURN_TRANSFERFROM = "Transferred token does not return null or true on successful transferFrom."; - string constant INVALID_RETURN_APPROVE = "Approved token does not return null or true on successful approve."; - // ============ Internal Functions ============ + /** + * Check balance owner's balance of ERC20 token + * + * @param _token The address of the ERC20 token + * @param _owner The owner who's balance is being checked + * @return uint256 The _owner's amount of tokens + */ function balanceOf( - address _tokenAddress, - address _ownerAddress + address _token, + address _owner ) - internal + external view returns (uint256) { - return IERC20(_tokenAddress).balanceOf(_ownerAddress); + return IERC20(_token).balanceOf(_owner); } + /** + * Checks spender's allowance to use token's on owner's behalf. + * + * @param _token The address of the ERC20 token + * @param _owner The token owner address + * @param _spender The address the allowance is being checked on + * @return uint256 The spender's allowance on behalf of owner + */ function allowance( - address _tokenAddress, - address _tokenOwner, + address _token, + address _owner, address _spender ) internal view returns (uint256) { - return IERC20(_tokenAddress).allowance(_tokenOwner, _spender); + return IERC20(_token).allowance(_owner, _spender); } + /** + * Transfers tokens from an address. Handle's tokens that return true or null. + * If other value returned, reverts. + * + * @param _token The address of the ERC20 token + * @param _to The address to transfer to + * @param _quantity The amount of tokens to transfer + */ function transfer( - address _tokenAddress, + address _token, address _to, uint256 _quantity ) - internal + external { - IERC20(_tokenAddress).transfer(_to, _quantity); + IERC20(_token).transfer(_to, _quantity); - require( - checkSuccess(), - INVALID_RETURN_TRANSFER - ); + // Check that transfer returns true or null + require(checkSuccess()); } + /** + * Transfers tokens from an address (that has set allowance on the proxy). + * Handle's tokens that return true or null. If other value returned, reverts. + * + * @param _token The address of the ERC20 token + * @param _from The address to transfer from + * @param _to The address to transfer to + * @param _quantity The number of tokens to transfer + */ function transferFrom( - address _tokenAddress, + address _token, address _from, address _to, uint256 _quantity ) - internal + external { - IERC20(_tokenAddress).transferFrom(_from, _to, _quantity); + IERC20(_token).transferFrom(_from, _to, _quantity); - require( - checkSuccess(), - INVALID_RETURN_TRANSFERFROM - ); + // Check that transferFrom returns true or null + require(checkSuccess()); } + /** + * Grants spender ability to spend on owner's behalf. + * Handle's tokens that return true or null. If other value returned, reverts. + * + * @param _token The address of the ERC20 token + * @param _spender The address to approve for transfer + * @param _quantity The amount of tokens to approve spender for + */ function approve( - address _tokenAddress, + address _token, address _spender, uint256 _quantity ) internal { - IERC20(_tokenAddress).approve(_spender, _quantity); + IERC20(_token).approve(_spender, _quantity); - require( - checkSuccess(), - INVALID_RETURN_APPROVE - ); + // Check that approve returns true or null + require(checkSuccess()); } + /** + * Ensure's the owner has granted enough allowance for system to + * transfer tokens. + * + * @param _token The address of the ERC20 token + * @param _owner The address of the token owner + * @param _spender The address to grant/check allowance for + * @param _quantity The amount to see if allowed for + */ function ensureAllowance( address _token, address _owner, diff --git a/utils/coreWrapper.ts b/utils/coreWrapper.ts index 71447327d..ad6ad6c14 100644 --- a/utils/coreWrapper.ts +++ b/utils/coreWrapper.ts @@ -18,6 +18,7 @@ import { extractNewSetTokenAddressFromLogs } from './contract_logs/core'; const Authorizable = artifacts.require('Authorizable'); const Core = artifacts.require('Core'); +const ERC20Wrapper = artifacts.require('ERC20Wrapper'); const OrderLibrary = artifacts.require('OrderLibrary'); const OrderLibraryMock = artifacts.require('OrderLibraryMock'); const TransferProxy = artifacts.require('TransferProxy'); @@ -30,6 +31,7 @@ export class CoreWrapper { private _tokenOwnerAddress: Address; private _contractOwnerAddress: Address; private _truffleOrderLibrary: any; + private _truffleERC20Wrapper: any; constructor(tokenOwnerAddress: Address, contractOwnerAddress: Address) { this._tokenOwnerAddress = tokenOwnerAddress; @@ -41,6 +43,13 @@ export class CoreWrapper { public async deployTransferProxyAsync( from: Address = this._tokenOwnerAddress ): Promise { + if (!this._truffleERC20Wrapper) { + this._truffleERC20Wrapper = await ERC20Wrapper.new( + { from: this._tokenOwnerAddress }, + ); + } + + await TransferProxy.link('ERC20Wrapper', this._truffleERC20Wrapper.address); const truffleTransferProxy = await TransferProxy.new( { from, gas: DEFAULT_GAS }, ); From 9c742e1fa80ec2e9200eaa5df3a712424449d29b Mon Sep 17 00:00:00 2001 From: Brian Weickmann Date: Sun, 29 Jul 2018 13:14:42 -0700 Subject: [PATCH 2/2] Linked ERC20Wrapper to other contracts using it. Removed unused dependencies in exchangeWrapper util. --- utils/coreWrapper.ts | 7 +++++++ utils/exchangeWrapper.ts | 22 ++++++++++++++-------- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/utils/coreWrapper.ts b/utils/coreWrapper.ts index ad6ad6c14..90b3cbe10 100644 --- a/utils/coreWrapper.ts +++ b/utils/coreWrapper.ts @@ -65,6 +65,13 @@ export class CoreWrapper { public async deployVaultAsync( from: Address = this._tokenOwnerAddress ): Promise { + if (!this._truffleERC20Wrapper) { + this._truffleERC20Wrapper = await ERC20Wrapper.new( + { from: this._tokenOwnerAddress }, + ); + } + + await Vault.link('ERC20Wrapper', this._truffleERC20Wrapper.address); const truffleVault = await Vault.new( { from }, ); diff --git a/utils/exchangeWrapper.ts b/utils/exchangeWrapper.ts index bd00e71ad..fb36ad8e9 100644 --- a/utils/exchangeWrapper.ts +++ b/utils/exchangeWrapper.ts @@ -1,17 +1,16 @@ -import * as _ from "lodash"; +import { TakerWalletWrapperContract } from '../types/generated/taker_wallet_wrapper'; +import { TransferProxyContract } from '../types/generated/transfer_proxy'; -import { TakerWalletWrapperContract } from "../types/generated/taker_wallet_wrapper"; -import { TransferProxyContract } from "../types/generated/transfer_proxy"; +import { Address } from '../types/common.js'; +import { DEFAULT_GAS } from './constants'; -import { BigNumber } from "bignumber.js"; -import { Address } from "../types/common.js"; -import { DEFAULT_GAS } from "./constants"; - -const TakerWalletWrapper = artifacts.require("TakerWalletWrapper"); +const ERC20Wrapper = artifacts.require('ERC20Wrapper'); +const TakerWalletWrapper = artifacts.require('TakerWalletWrapper'); export class ExchangeWrapper { private _contractOwnerAddress: Address; + private _truffleERC20Wrapper: any; constructor(contractOwnerAddress: Address) { this._contractOwnerAddress = contractOwnerAddress; @@ -23,6 +22,13 @@ export class ExchangeWrapper { transferProxy: TransferProxyContract, from: Address = this._contractOwnerAddress ): Promise { + if (!this._truffleERC20Wrapper) { + this._truffleERC20Wrapper = await ERC20Wrapper.new( + { from }, + ); + } + + await TakerWalletWrapper.link('ERC20Wrapper', this._truffleERC20Wrapper.address); const takerWalletWrapperInstance = await TakerWalletWrapper.new( transferProxy.address, { from, gas: DEFAULT_GAS },