diff --git a/.solcover.js b/.solcover.js index b6220a4a9..f2f8491bf 100644 --- a/.solcover.js +++ b/.solcover.js @@ -5,7 +5,6 @@ module.exports = { 'Migrations.sol', 'mocks', 'external', - 'core/lib/ERC20Wrapper.sol', // TODO: Find a cleaner way to test state mutating functions, we will skip 'core/exchange-wrappers/ZeroExExchangeWrapper.sol' // TODO: test this ], }; diff --git a/contracts/core/TransferProxy.sol b/contracts/core/TransferProxy.sol index 28c7c89f3..b64d76761 100644 --- a/contracts/core/TransferProxy.sol +++ b/contracts/core/TransferProxy.sol @@ -19,7 +19,7 @@ pragma solidity 0.4.24; import { Authorizable } from "../lib/Authorizable.sol"; import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol"; -import { ERC20Wrapper } from "./lib/ERC20Wrapper.sol"; +import { ERC20Wrapper } from "../lib/ERC20Wrapper.sol"; /** diff --git a/contracts/core/Vault.sol b/contracts/core/Vault.sol index 3c96b66b5..2e9d95d47 100644 --- a/contracts/core/Vault.sol +++ b/contracts/core/Vault.sol @@ -19,7 +19,7 @@ pragma solidity 0.4.24; import { Authorizable } from "../lib/Authorizable.sol"; import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol"; -import { ERC20Wrapper } from "./lib/ERC20Wrapper.sol"; +import { ERC20Wrapper } from "../lib/ERC20Wrapper.sol"; /** diff --git a/contracts/core/exchange-wrappers/ZeroExExchangeWrapper.sol b/contracts/core/exchange-wrappers/ZeroExExchangeWrapper.sol index 5c1812dfe..6128d0f6c 100644 --- a/contracts/core/exchange-wrappers/ZeroExExchangeWrapper.sol +++ b/contracts/core/exchange-wrappers/ZeroExExchangeWrapper.sol @@ -23,7 +23,7 @@ import { LibBytes } from "../../external/0x/LibBytes.sol"; import { LibOrder as ZeroExOrder } from "../../external/0x/Exchange/libs/LibOrder.sol"; import { LibFillResults as ZeroExFillResults } from "../../external/0x/Exchange/libs/LibFillResults.sol"; import { IExchange as ZeroExExchange } from "../../external/0x/Exchange/interfaces/IExchange.sol"; -import { ERC20Wrapper as ERC20 } from "../../core/lib/ERC20Wrapper.sol"; +import { ERC20Wrapper as ERC20 } from "../../lib/ERC20Wrapper.sol"; diff --git a/contracts/core/lib/ERC20Wrapper.sol b/contracts/lib/ERC20Wrapper.sol similarity index 97% rename from contracts/core/lib/ERC20Wrapper.sol rename to contracts/lib/ERC20Wrapper.sol index 41557ab58..1687589dc 100644 --- a/contracts/core/lib/ERC20Wrapper.sol +++ b/contracts/lib/ERC20Wrapper.sol @@ -16,8 +16,8 @@ pragma solidity 0.4.24; -import { IERC20 } from "../../lib/IERC20.sol"; -import { CommonMath } from "../../lib/CommonMath.sol"; +import { CommonMath } from "./CommonMath.sol"; +import { IERC20 } from "./IERC20.sol"; /** diff --git a/contracts/lib/IERC20.sol b/contracts/lib/IERC20.sol index d4696c26f..9a073a8a6 100644 --- a/contracts/lib/IERC20.sol +++ b/contracts/lib/IERC20.sol @@ -57,5 +57,6 @@ interface IERC20 { address _spender, uint256 _quantity ) - external; + external + returns (bool); } diff --git a/contracts/mocks/lib/ERC20WrapperMock.sol b/contracts/mocks/lib/ERC20WrapperMock.sol new file mode 100644 index 000000000..056a07acb --- /dev/null +++ b/contracts/mocks/lib/ERC20WrapperMock.sol @@ -0,0 +1,17 @@ +pragma solidity 0.4.24; +pragma experimental "ABIEncoderV2"; + +import { ERC20Wrapper } from "../../lib/ERC20Wrapper.sol"; + +// Mock contract implementation of OrderLibrary functions +contract ERC20WrapperMock { + function approve( + address _token, + address _spender, + uint256 _quantity + ) + public + { + ERC20Wrapper.approve(_token, _spender, _quantity); + } +} diff --git a/test/contracts/lib/commonMath.spec.ts b/test/contracts/lib/commonMath.spec.ts index 1e6c76b26..39dead3e4 100644 --- a/test/contracts/lib/commonMath.spec.ts +++ b/test/contracts/lib/commonMath.spec.ts @@ -10,8 +10,8 @@ import { Address } from "../../../types/common.js"; // Contract types import { CommonMathMockContract } from "../../../types/generated/common_math_mock"; -// Artifacts -const CommonMathMock = artifacts.require("CommonMathMock"); +// Wrappers +import { LibraryMockWrapper } from "../../utils/libraryMockWrapper"; // Testing Set up import { BigNumberSetup } from "../../utils/bigNumberSetup"; @@ -20,8 +20,10 @@ BigNumberSetup.configure(); ChaiSetup.configure(); const { expect, assert } = chai; + contract("CommonMathMock", (accounts) => { const [ownerAccount] = accounts; + const libraryMockWrapper = new LibraryMockWrapper(ownerAccount); let commonMathLibrary: CommonMathMockContract; @@ -29,14 +31,7 @@ contract("CommonMathMock", (accounts) => { let caller: Address = ownerAccount; beforeEach(async () => { - const truffleCommonMathLibrary = await CommonMathMock.new( - { from: ownerAccount }, - ); - - commonMathLibrary = new CommonMathMockContract( - web3.eth.contract(truffleCommonMathLibrary.abi).at(truffleCommonMathLibrary.address), - { from: ownerAccount }, - ); + commonMathLibrary = await libraryMockWrapper.deployCommonMathLibraryAsync() }); async function subject(): Promise { diff --git a/test/contracts/lib/erc20Wrapper.spec.ts b/test/contracts/lib/erc20Wrapper.spec.ts new file mode 100644 index 000000000..4d6acd13b --- /dev/null +++ b/test/contracts/lib/erc20Wrapper.spec.ts @@ -0,0 +1,75 @@ +import * as chai from "chai"; +import * as _ from "lodash"; + +import * as ABIDecoder from "abi-decoder"; +import { BigNumber } from "bignumber.js"; + +// Types +import { Address } from "../../../types/common.js"; + +// Contract types +import { ERC20WrapperMockContract } from "../../../types/generated/e_r_c20_wrapper_mock"; +import { StandardTokenMockContract } from "../../../types/generated/standard_token_mock"; + +// Wrappers +import { LibraryMockWrapper } from "../../utils/libraryMockWrapper"; +import { ERC20Wrapper } from "../../utils/erc20Wrapper"; + +// Testing Set up +import { BigNumberSetup } from "../../utils/bigNumberSetup"; +import ChaiSetup from "../../utils/chaiSetup"; +BigNumberSetup.configure(); +ChaiSetup.configure(); +const { expect, assert } = chai; + +import { UNLIMITED_ALLOWANCE_IN_BASE_UNITS } from "../../utils/constants"; + + +contract("ERC20WrapperMock", (accounts) => { + const [ + deployerAccount, + ownerAccount, + spenderAccount + ] = accounts; + + let erc20WrapperLibrary: ERC20WrapperMockContract; + + const erc20Wrapper = new ERC20Wrapper(deployerAccount); + const libraryMockWrapper = new LibraryMockWrapper(deployerAccount); + + describe.only("#approve", async () => { + const subjectTransferAllowance: BigNumber = UNLIMITED_ALLOWANCE_IN_BASE_UNITS; + let token: StandardTokenMockContract; + let caller: Address = ownerAccount; + + beforeEach(async () => { + token = await erc20Wrapper.deployTokenAsync(ownerAccount); + erc20WrapperLibrary = await libraryMockWrapper.deployERC20WrapperLibraryAsync(); + }); + + async function subject(): Promise { + return erc20WrapperLibrary.approve.sendTransactionAsync( + token.address, + spenderAccount, + subjectTransferAllowance, + { from: ownerAccount }, + ); + } + + it("approves the spender on behalf of the calling contract", async () => { + const existingAllowance = await token.allowance.callAsync( + erc20WrapperLibrary.address, + spenderAccount + ); + + await subject(); + + const expectedNewAllowance = existingAllowance.add(subjectTransferAllowance); + const newSpenderAllowance = await token.allowance.callAsync( + erc20WrapperLibrary.address, + spenderAccount + ); + expect(newSpenderAllowance).to.bignumber.equal(subjectTransferAllowance); + }); + }); +}); diff --git a/test/utils/libraryMockWrapper.ts b/test/utils/libraryMockWrapper.ts new file mode 100644 index 000000000..38ec0c204 --- /dev/null +++ b/test/utils/libraryMockWrapper.ts @@ -0,0 +1,48 @@ +import * as _ from "lodash"; + +import { CommonMathMockContract } from "../../types/generated/common_math_mock"; +import { ERC20WrapperMockContract } from "../../types/generated/e_r_c20_wrapper_mock"; + +import { BigNumber } from "bignumber.js"; +import { Address } from "../../types/common.js"; +import { DEFAULT_GAS } from "../utils/constants"; + +const ERC20WrapperMock = artifacts.require("ERC20WrapperMock"); +const CommonMathMock = artifacts.require("CommonMathMock"); + + +export class LibraryMockWrapper { + private _contractOwnerAddress: Address; + + constructor(contractOwnerAddress: Address) { + this._contractOwnerAddress = contractOwnerAddress; + } + + /* ============ Deployment ============ */ + + public async deployCommonMathLibraryAsync( + from: Address = this._contractOwnerAddress + ): Promise { + const truffleCommonMathLibrary = await CommonMathMock.new( + { from }, + ); + + return new CommonMathMockContract( + web3.eth.contract(truffleCommonMathLibrary.abi).at(truffleCommonMathLibrary.address), + { from }, + ); + } + + public async deployERC20WrapperLibraryAsync( + from: Address = this._contractOwnerAddress + ): Promise { + const erc20WrapperMockContract = await ERC20WrapperMock.new( + { from }, + ); + + return new ERC20WrapperMockContract( + web3.eth.contract(erc20WrapperMockContract.abi).at(erc20WrapperMockContract.address), + { from }, + ); + } +}