From 6b76e51ba7b7b80f16a4eec957f98a874b2fb1d4 Mon Sep 17 00:00:00 2001 From: bweick Date: Fri, 29 Jun 2018 14:05:37 -0700 Subject: [PATCH 1/8] Adding lib bytes and removing coverage of it. --- .solcover.js | 1 + contracts/external/LibBytes.sol | 47 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 contracts/external/LibBytes.sol diff --git a/.solcover.js b/.solcover.js index aed5cfdb7..c5f5cbc8a 100644 --- a/.solcover.js +++ b/.solcover.js @@ -4,5 +4,6 @@ module.exports = { skipFiles: [ 'Migrations.sol', 'test', + 'external/LibBytes.sol' ], }; diff --git a/contracts/external/LibBytes.sol b/contracts/external/LibBytes.sol new file mode 100644 index 000000000..1db0d7b63 --- /dev/null +++ b/contracts/external/LibBytes.sol @@ -0,0 +1,47 @@ +/* + 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.24; + +library LibBytes { + + using LibBytes for bytes; + + + /// @dev Reads a bytes32 value from a position in a byte array. + /// @param b Byte array containing a bytes32 value. + /// @param index Index in byte array of bytes32 value. + /// @return bytes32 value from byte array. + function readBytes32( + bytes memory b, + uint256 index + ) + internal + pure + returns (bytes32 result) + { + require( + b.length >= index + 32, + "GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED" + ); + + // Arrays are prefixed by a 256 bit length parameter + index += 32; + + // Read the bytes32 from array memory + assembly { + result := mload(add(b, index)) + } + return result; + } +} \ No newline at end of file From a80af4e458877c728ddfc98899cbfff06561805b Mon Sep 17 00:00:00 2001 From: bweick Date: Sun, 1 Jul 2018 16:52:00 -0700 Subject: [PATCH 2/8] Added fuller fillOrder function as well as signature checking and hashing abilities. Created basic tools for hashing and signing msgs in tests. --- contracts/core/extensions/CoreIssuance.sol | 2 +- .../core/extensions/CoreIssuanceOrder.sol | 90 +++++++++- package.json | 4 +- .../core/extensions/coreIssuanceOrder.spec.ts | 160 ++++++++++++++++-- test/utils/constants.ts | 19 +++ test/utils/orderWrapper.ts | 67 +++++++- types/common.ts | 19 +++ 7 files changed, 336 insertions(+), 25 deletions(-) diff --git a/contracts/core/extensions/CoreIssuance.sol b/contracts/core/extensions/CoreIssuance.sol index 3c2977a50..eb587465a 100644 --- a/contracts/core/extensions/CoreIssuance.sol +++ b/contracts/core/extensions/CoreIssuance.sol @@ -141,7 +141,7 @@ contract CoreIssuance is /** * Exchanges components for Set Tokens, accepting any owner * - * @param _owner Address to issue set to + * @param _owner Address to issue set to * @param _setAddress Address of set to issue * @param _quantity Quantity of set to issue */ diff --git a/contracts/core/extensions/CoreIssuanceOrder.sol b/contracts/core/extensions/CoreIssuanceOrder.sol index 22c622a3c..9ef07f594 100644 --- a/contracts/core/extensions/CoreIssuanceOrder.sol +++ b/contracts/core/extensions/CoreIssuanceOrder.sol @@ -18,8 +18,9 @@ pragma solidity 0.4.24; import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol"; -import { ICoreIssuance } from "../interfaces/ICoreIssuance.sol"; import { CoreModifiers } from "../lib/CoreSharedModifiers.sol"; +import { ICoreIssuance } from "../interfaces/ICoreIssuance.sol"; +import { LibBytes } from "../../external/LibBytes.sol"; /** @@ -35,18 +36,89 @@ contract CoreIssuanceOrder is ICoreIssuance { using SafeMath for uint256; + using LibBytes for bytes; + + string constant INVALID_SIGNATURE = "Invalid order signature."; function fillOrder( - address _maker, - address _setAddress, - uint _quantity + address[4] _addresses, + uint[5] _values, + uint _fillQuantity, + uint8 _v, + bytes32 _r, + bytes32 _s ) public - isValidSet(_setAddress) - isPositiveQuantity(_quantity) - isNaturalUnitMultiple(_quantity, _setAddress) + // isValidSet(_setAddress) + // isPositiveQuantity(_quantity) + // isNaturalUnitMultiple(_quantity, _setAddress) { + //Create order hash + bytes32 orderHash = generateOrderHash( + _addresses, + _values + ); + + // Verify signature is authentic + require(validateSignature( + orderHash, + _addresses[1], + _v, + _r, + _s + ), + INVALID_SIGNATURE + ); + //Issue Set - issueInternal(_maker, _setAddress, _quantity); + issueInternal( + _addresses[1], + _addresses[0], + _fillQuantity + ); + } + + function generateOrderHash( + address[4] _addresses, + uint[5] _values + ) + public + returns(bytes32) + { + return keccak256(abi.encodePacked( + _addresses[0], //setAddress + _addresses[1], //makerAddress + _addresses[2], //makerToken + _addresses[3], //relayerToken + _values[0], //quantity + _values[1], //makerTokenAmount + _values[2], //expiration + _values[3], //relayerTokenAmount + _values[4] //salt + )); + } + + function validateSignature( + bytes32 _orderHash, + address _signerAddress, + uint8 _v, + bytes32 _r, + bytes32 _s + ) + public + returns(bool) + { + address recAddress; + + bytes memory msgPrefix = "\x19Ethereum Signed Message:\n32"; + + recAddress = ecrecover( + keccak256(abi.encodePacked(msgPrefix, _orderHash)), + _v, + _r, + _s + ); + + return recAddress == _signerAddress; } -} \ No newline at end of file +} diff --git a/package.json b/package.json index 047aa0c23..67f709998 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "dist": "bash scripts/prepare_dist.sh", "clean": "rm -rf build; rm -rf transpiled; rm -rf types/generated", "compile": "truffle compile", - "prepare-test": "yarn clean && truffle compile --all && yarn run generate-typings && yarn run transpile", + "prepare-test": "yarn clean && truffle compile --all && yarn run generate-typings && yarn run transpile", "test": "yarn prepare-test && truffle test `find ./transpiled/test -name '*.spec.js'`", "transpile": "tsc", "generate-typings": "abi-gen --abis './build/contracts/*.json' --out './types/generated' --template './types/contract_templates/contract.mustache' --partials './types/contract_templates/partials/*.mustache' && yarn run rename-generated-abi", @@ -30,7 +30,7 @@ "lint-ts": "tslint --fix test/*.ts", "lint-sol": "solium -d contracts/", "lint": "yarn run lint-sol && yarn run lint-ts", - "chain": "ganache-cli --networkId 70 --accounts 20 -l 10000000 -e 1000000", + "chain": "ganache-cli --networkId 70 --accounts 20 -l 10000000 -e 1000000 -m 'set protocol to the moon'", "coverage-setup": "cp -r transpiled/test/* test/. && cp -r transpiled/types/* types/.", "coverage-cleanup": "find test -name \\*.js* -type f -delete && find types -name \\*.js* -type f -delete", "coverage": "yarn prepare-test && yarn coverage-setup && ./node_modules/.bin/solidity-coverage && yarn coverage-cleanup" diff --git a/test/core/extensions/coreIssuanceOrder.spec.ts b/test/core/extensions/coreIssuanceOrder.spec.ts index c44cc5d73..1157d8ed2 100644 --- a/test/core/extensions/coreIssuanceOrder.spec.ts +++ b/test/core/extensions/coreIssuanceOrder.spec.ts @@ -6,7 +6,7 @@ import { BigNumber } from "bignumber.js"; import { ether } from "../../utils/units"; // Types -import { Address } from "../../../types/common.js"; +import { Address, IssuanceOrder } from "../../../types/common.js"; // Contract types import { CoreContract } from "../../../types/generated/core"; @@ -22,6 +22,12 @@ const Core = artifacts.require("Core"); // Core wrapper import { CoreWrapper } from "../../utils/coreWrapper"; import { ERC20Wrapper } from "../../utils/erc20Wrapper"; +import { + generateSalt, + generateTimeStamp, + hashOrderHex, + signMessage +} from "../../utils/orderWrapper"; // Testing Set up import { BigNumberSetup } from "../../config/bigNumberSetup"; @@ -41,6 +47,7 @@ import { import { DEPLOYED_TOKEN_QUANTITY, + PRIVATE_KEYS, } from "../../utils/constants"; contract("CoreIssuance", (accounts) => { @@ -48,6 +55,9 @@ contract("CoreIssuance", (accounts) => { ownerAccount, takerAccount, makerAccount, + signerAccount, + mockSetTokenAccount, + mockTokenAccount ] = accounts; let core: CoreContract; @@ -74,19 +84,25 @@ contract("CoreIssuance", (accounts) => { await coreWrapper.setDefaultStateAndAuthorizationsAsync(core, vault, transferProxy, setTokenFactory); }); - describe("#fillOrder", async () => { + describe.only("#fillOrder", async () => { let subjectCaller: Address; let subjectQuantityToIssue: BigNumber; - let subjectSetToIssue: Address; const naturalUnit: BigNumber = ether(2); let components: StandardTokenMockContract[] = []; let componentUnits: BigNumber[]; let setToken: SetTokenContract; + let signerAddress: Address; + + let addresses: Address[]; + let values: BigNumber[]; + let signature: any; beforeEach(async () => { - components = await erc20Wrapper.deployTokensAsync(2, makerAccount); - await erc20Wrapper.approveTransfersAsync(components, transferProxy.address, makerAccount); + signerAddress = signerAccount; + + components = await erc20Wrapper.deployTokensAsync(2, signerAddress); //For current purposes issue to maker/signer + await erc20Wrapper.approveTransfersAsync(components, transferProxy.address, signerAddress); const componentAddresses = _.map(components, (token) => token.address); componentUnits = _.map(components, () => ether(4)); // Multiple of naturalUnit @@ -100,14 +116,34 @@ contract("CoreIssuance", (accounts) => { subjectCaller = takerAccount; subjectQuantityToIssue = ether(2); - subjectSetToIssue = setToken.address; + + const order = { + setAddress: setToken.address, + quantity: ether(4), + makerAddress: signerAddress, + makerToken: componentAddresses[0], + makerTokenAmount: ether(10), + expiration: generateTimeStamp(), + relayerToken: componentAddresses[0], + relayerTokenAmount: ether(1), + salt: generateSalt() + } as IssuanceOrder; + + addresses = [order.setAddress, order.makerAddress, order.makerToken, order.relayerToken]; + values = [order.quantity, order.makerTokenAmount, order.expiration, order.relayerTokenAmount, order.salt]; + + const orderHash = hashOrderHex(order); + signature = await signMessage(orderHash, signerAddress); }); async function subject(): Promise { return core.fillOrder.sendTransactionAsync( - makerAccount, - subjectSetToIssue, + addresses, + values, subjectQuantityToIssue, + signature.v, + signature.r, + signature.s, { from: subjectCaller }, ); } @@ -116,14 +152,114 @@ contract("CoreIssuance", (accounts) => { const component: StandardTokenMockContract = _.first(components); const unit: BigNumber = _.first(componentUnits); - const existingBalance = await component.balanceOf.callAsync(makerAccount); - assertTokenBalance(component, DEPLOYED_TOKEN_QUANTITY, makerAccount); - + const existingBalance = await component.balanceOf.callAsync(signerAddress); + assertTokenBalance(component, DEPLOYED_TOKEN_QUANTITY, signerAddress); await subject(); - const newBalance = await component.balanceOf.callAsync(makerAccount); + const newBalance = await component.balanceOf.callAsync(signerAddress); const expectedNewBalance = existingBalance.sub(subjectQuantityToIssue.div(naturalUnit).mul(unit)); expect(newBalance).to.be.bignumber.equal(expectedNewBalance); }); }); + describe("#validateSignature", async () => { + let subjectCaller: Address; + let subjectMaker: Address; + let signerAddress: Address; + + let orderHash: string; + let signature: any; + + beforeEach(async () => { + + subjectCaller = takerAccount; + subjectMaker = signerAccount; + signerAddress = signerAccount; + + const order = { + setAddress: mockSetTokenAccount, + quantity: ether(2), + makerAddress: subjectMaker, + makerToken: mockTokenAccount, + makerTokenAmount: ether(10), + expiration: generateTimeStamp(), + relayerToken: mockTokenAccount, + relayerTokenAmount: ether(1), + salt: generateSalt() + } as IssuanceOrder; + + orderHash = hashOrderHex(order); + signature = await signMessage(orderHash, signerAddress); + }); + + async function subject(): Promise { + return core.validateSignature.callAsync( + orderHash, + subjectMaker, + signature.v, + signature.r, + signature.s, + { from: subjectCaller }, + ); + } + + it("should return true", async () => { + const validSig = await subject(); + + expect(validSig).to.equal(true); + }); + describe("when the message is not signed by the maker", async () => { + beforeEach(async () => { + subjectMaker = makerAccount; + }); + + it("should return false", async () => { + const validSig = await subject(); + + expect(validSig).to.equal(false); + }); + }); + }); + describe("#generateOrderHash", async () => { + let subjectCaller: Address; + + let addresses: Address[]; + let values: BigNumber[]; + let orderHash: string; + + beforeEach(async () => { + + subjectCaller = takerAccount; + + const order = { + setAddress: mockSetTokenAccount, + quantity: ether(2), + makerAddress: makerAccount, + makerToken: mockTokenAccount, + makerTokenAmount: ether(10), + expiration: generateTimeStamp(), + relayerToken: mockTokenAccount, + relayerTokenAmount: ether(1), + salt: generateSalt(), + } as IssuanceOrder; + + orderHash = hashOrderHex(order); + + addresses = [order.setAddress, order.makerAddress, order.makerToken, order.relayerToken]; + values = [order.quantity, order.makerTokenAmount, order.expiration, order.relayerTokenAmount, order.salt]; + }); + + async function subject(): Promise { + return core.generateOrderHash.callAsync( + addresses, + values, + { from: subjectCaller }, + ); + } + + it("should return true", async () => { + const contractOrderHash = await subject(); + + expect(contractOrderHash).to.equal(orderHash); + }); + }); }); diff --git a/test/utils/constants.ts b/test/utils/constants.ts index be7b19e17..a06bb6149 100644 --- a/test/utils/constants.ts +++ b/test/utils/constants.ts @@ -11,6 +11,7 @@ export const STANDARD_QUANTITY_ISSUED: BigNumber = ether(10); export const STANDARD_NATURAL_UNIT = ether(1); export const STANDARD_COMPONENT_UNIT = ether(1); export const UNLIMITED_ALLOWANCE_IN_BASE_UNITS = new BigNumber(2).pow(256).minus(1); +export const MAX_DIGITS_IN_UNSIGNED_256_INT = 78; export const ZERO: BigNumber = new BigNumber(0); export const ONE: BigNumber = new BigNumber(1); @@ -20,3 +21,21 @@ export const ORDER_TYPE = { KYBER: "kyber", MAKER_WALLET: "makerWallet", } + +export const PRIVATE_KEYS = [ + "767df558efc63b6ba9a9257e68509c38f5c48d5938a41ab191a9a073ebff7c4f", + "6dc5d3331608e4b99b68dd8b9dee89973ebc6feee1cb0ba9a2ec814a99c680c1", + "2b73a8e22d40615e0144bee14c5f80668c449029d9f60eba71b800f0979337af", + "95fab20a86f7aa81c47854e3a0d265014359d557027c3e07c64dbac9f66b0930", + "b8400424887469943ca6e4448596a709c64ad768600e9c24538fda6c8f5d1599", + "737392faafc4b9fa8861ec0dfb1e3a01e383e270331faa25f28fb40866740046", + "92c0c7a1aba1e59f1f97af9a482649a4196b2b766f794b85bebcefeac8b80e30", + "05973025898f0c1c1c723545630ecd251c42297d8db665aec245526022d82a98", + "84f5e4921ea1ee2e53e2713bd5170877635c168d0fdab8044f15b835020f1f6c", + "b17ee27cba4f656d8cd13165ba9fdfa79abf5308253c75654030d360f1e65329", + "55d6cb34052149b6a93624bbfd1a277a37e352f34e16bc405054adf42b6eab18", + "cc87c4b4d1665092048511f0318755884771d498f79d29aa78b341a3f058f4c6", + "8884500103a7809924cbb5b6e7c1f8b9e27e7b6da839935f221406e02a954293", + "0a44845c2b09e9f942578f7dd960653595c152e558dbf7fb40bd85e918dd565f", + "843445407853ed9455b0b3511b50dc11a5c329746abbed08c95582b895c450a9", +] diff --git a/test/utils/orderWrapper.ts b/test/utils/orderWrapper.ts index 3a9952b72..0e13d7b8e 100644 --- a/test/utils/orderWrapper.ts +++ b/test/utils/orderWrapper.ts @@ -1,12 +1,31 @@ import * as _ from "lodash"; import * as ethUtil from "ethereumjs-util"; +import * as ethABI from 'ethereumjs-abi'; import { BigNumber } from "bignumber.js"; -import { Address, Bytes32, UInt } from "../../types/common.js"; +import BN = require('bn.js'); + +import { Address, Bytes32, UInt, IssuanceOrder, SolidityTypes } from "../../types/common.js"; import { ORDER_TYPE, + MAX_DIGITS_IN_UNSIGNED_256_INT, } from "../utils/constants"; +function bigNumberToBN(value: BigNumber) { + return new BN(value.toString(), 10); +} + +function parseSigHexAsRSV(sigHex: string): any { + const { v,r,s } = ethUtil.fromRpcSig(sigHex); + + const ecSig = { + v, + r: ethUtil.bufferToHex(r), + s: ethUtil.bufferToHex(s), + }; + return ecSig +} + export function generateExchangeOrdersHeader( orderCount: UInt ): Bytes32 { @@ -47,3 +66,49 @@ export function generateExchangeOrdersData( return ethUtil.bufferToHex(buffer); } + +export function generateSalt(): BigNumber { + const randomNumber = BigNumber.random(MAX_DIGITS_IN_UNSIGNED_256_INT); + const factor = new BigNumber(10).pow(MAX_DIGITS_IN_UNSIGNED_256_INT-1); + const salt = randomNumber.times(factor).round(); + return salt; +} + +export function generateTimeStamp(): BigNumber { + const tenMinutes = 10 * 60 * 1000; + const expiration = new BigNumber(Date.now() + tenMinutes); + return expiration; +} + +export function hashOrderHex( + order: IssuanceOrder, +): string { + const orderParts = [ + {value: order.setAddress, type: SolidityTypes.Address}, + {value: order.makerAddress, type: SolidityTypes.Address}, + {value: order.makerToken, type: SolidityTypes.Address}, + {value: order.relayerToken, type: SolidityTypes.Address}, + {value: bigNumberToBN(order.quantity), type: SolidityTypes.Uint256}, + {value: bigNumberToBN(order.makerTokenAmount), type: SolidityTypes.Uint256}, + {value: bigNumberToBN(order.expiration), type: SolidityTypes.Uint256}, + {value: bigNumberToBN(order.relayerTokenAmount), type: SolidityTypes.Uint256}, + {value: bigNumberToBN(order.salt), type: SolidityTypes.Uint256} + ] + + const types = _.map(orderParts, o => o.type); + const values = _.map(orderParts, o => o.value); + const hashBuff = ethABI.soliditySHA3(types, values); + const hashHex = ethUtil.bufferToHex(hashBuff); + return hashHex; +} + +export async function signMessage( + msg: string, + address: Address +): Promise { + const normalSigner = String(address).toLowerCase(); + + const sig = web3.eth.sign(address, msg); + const ecSig = parseSigHexAsRSV(sig); + return ecSig; +} diff --git a/types/common.ts b/types/common.ts index 27c6d42c6..b01c8ab01 100644 --- a/types/common.ts +++ b/types/common.ts @@ -29,6 +29,18 @@ export interface TxDataPayable extends TxData { value?: BigNumber; } +export interface IssuanceOrder { + setAddress: Address, + quantity: BigNumber, + makerAddress: Address, + makerToken: Address, + makerTokenAmount: BigNumber, + expiration: BigNumber, + relayerToken: Address, + relayerTokenAmount: BigNumber, + salt: BigNumber +} + export interface Log { event: string; address: Address; @@ -38,3 +50,10 @@ export interface Log { export type Address = string; export type UInt = number | BigNumber; export type Bytes32 = string; + +export enum SolidityTypes { + Address = 'address', + Uint256 = 'uint256', + Uint8 = 'uint8', + Uint = 'uint', +} From a51155c00a575e8691685c468785110869ae271d Mon Sep 17 00:00:00 2001 From: bweick Date: Mon, 2 Jul 2018 08:44:21 -0700 Subject: [PATCH 3/8] Added validation logic and cleaned up tests. --- .../core/extensions/CoreIssuanceOrder.sol | 98 ++++++++--- .../core/extensions/coreIssuanceOrder.spec.ts | 159 ++++++++++-------- test/utils/orderWrapper.ts | 44 ++++- 3 files changed, 197 insertions(+), 104 deletions(-) diff --git a/contracts/core/extensions/CoreIssuanceOrder.sol b/contracts/core/extensions/CoreIssuanceOrder.sol index 9ef07f594..3fe0ff506 100644 --- a/contracts/core/extensions/CoreIssuanceOrder.sol +++ b/contracts/core/extensions/CoreIssuanceOrder.sol @@ -40,6 +40,19 @@ contract CoreIssuanceOrder is string constant INVALID_SIGNATURE = "Invalid order signature."; + struct IssuanceOrder { + address setAddress; + uint256 quantity; + address makerAddress; + address makerToken; + uint256 makerTokenAmount; + uint256 expiration; + address relayerToken; + uint256 relayerTokenAmount; + uint256 salt; + bytes32 orderHash; + } + function fillOrder( address[4] _addresses, uint[5] _values, @@ -49,31 +62,49 @@ contract CoreIssuanceOrder is bytes32 _s ) public - // isValidSet(_setAddress) - // isPositiveQuantity(_quantity) - // isNaturalUnitMultiple(_quantity, _setAddress) + isValidSet(_addresses[0]) + isPositiveQuantity(_fillQuantity) + isNaturalUnitMultiple(_fillQuantity, _addresses[0]) { - //Create order hash - bytes32 orderHash = generateOrderHash( - _addresses, - _values + + IssuanceOrder memory order = IssuanceOrder({ + setAddress: _addresses[0], + quantity: _values[0], + makerAddress: _addresses[1], + makerToken: _addresses[2], + makerTokenAmount: _values[1], + expiration: _values[2], + relayerToken: _addresses[3], + relayerTokenAmount: _values[3], + salt: _values[4], + orderHash: generateOrderHash( + _addresses, + _values + ) + }); + + // Verify order is valid + validateOrder( + order, + _fillQuantity ); // Verify signature is authentic - require(validateSignature( - orderHash, - _addresses[1], - _v, - _r, - _s - ), + require( + validateSignature( + order.orderHash, + order.makerAddress, + _v, + _r, + _s + ), INVALID_SIGNATURE ); //Issue Set issueInternal( - _addresses[1], - _addresses[0], + order.makerAddress, + order.setAddress, _fillQuantity ); } @@ -85,17 +116,30 @@ contract CoreIssuanceOrder is public returns(bytes32) { - return keccak256(abi.encodePacked( - _addresses[0], //setAddress - _addresses[1], //makerAddress - _addresses[2], //makerToken - _addresses[3], //relayerToken - _values[0], //quantity - _values[1], //makerTokenAmount - _values[2], //expiration - _values[3], //relayerTokenAmount - _values[4] //salt - )); + return keccak256( + abi.encodePacked( + _addresses[0], //setAddress + _addresses[1], //makerAddress + _addresses[2], //makerToken + _addresses[3], //relayerToken + _values[0], //quantity + _values[1], //makerTokenAmount + _values[2], //expiration + _values[3], //relayerTokenAmount + _values[4] //salt + ) + ); + } + + function validateOrder( + IssuanceOrder _order, + uint _fillQuantity + ) + internal + { + require(_order.makerTokenAmount > 0 && _order.quantity > 0); + require(block.timestamp <= _order.expiration); + // Check to see if filled } function validateSignature( diff --git a/test/core/extensions/coreIssuanceOrder.spec.ts b/test/core/extensions/coreIssuanceOrder.spec.ts index 1157d8ed2..a129afd85 100644 --- a/test/core/extensions/coreIssuanceOrder.spec.ts +++ b/test/core/extensions/coreIssuanceOrder.spec.ts @@ -23,10 +23,7 @@ const Core = artifacts.require("Core"); import { CoreWrapper } from "../../utils/coreWrapper"; import { ERC20Wrapper } from "../../utils/erc20Wrapper"; import { - generateSalt, - generateTimeStamp, - hashOrderHex, - signMessage + generateFillOrderParameters, } from "../../utils/orderWrapper"; // Testing Set up @@ -48,9 +45,11 @@ import { import { DEPLOYED_TOKEN_QUANTITY, PRIVATE_KEYS, + ZERO, + NULL_ADDRESS, } from "../../utils/constants"; -contract("CoreIssuance", (accounts) => { +contract("CoreIssuanceOrder", (accounts) => { const [ ownerAccount, takerAccount, @@ -93,10 +92,9 @@ contract("CoreIssuance", (accounts) => { let componentUnits: BigNumber[]; let setToken: SetTokenContract; let signerAddress: Address; + let componentAddresses: Address[]; - let addresses: Address[]; - let values: BigNumber[]; - let signature: any; + let parameters: any; beforeEach(async () => { signerAddress = signerAccount; @@ -104,7 +102,7 @@ contract("CoreIssuance", (accounts) => { components = await erc20Wrapper.deployTokensAsync(2, signerAddress); //For current purposes issue to maker/signer await erc20Wrapper.approveTransfersAsync(components, transferProxy.address, signerAddress); - const componentAddresses = _.map(components, (token) => token.address); + componentAddresses = _.map(components, (token) => token.address); componentUnits = _.map(components, () => ether(4)); // Multiple of naturalUnit setToken = await coreWrapper.createSetTokenAsync( core, @@ -117,33 +115,17 @@ contract("CoreIssuance", (accounts) => { subjectCaller = takerAccount; subjectQuantityToIssue = ether(2); - const order = { - setAddress: setToken.address, - quantity: ether(4), - makerAddress: signerAddress, - makerToken: componentAddresses[0], - makerTokenAmount: ether(10), - expiration: generateTimeStamp(), - relayerToken: componentAddresses[0], - relayerTokenAmount: ether(1), - salt: generateSalt() - } as IssuanceOrder; - - addresses = [order.setAddress, order.makerAddress, order.makerToken, order.relayerToken]; - values = [order.quantity, order.makerTokenAmount, order.expiration, order.relayerTokenAmount, order.salt]; - - const orderHash = hashOrderHex(order); - signature = await signMessage(orderHash, signerAddress); + parameters = await generateFillOrderParameters(setToken.address, signerAddress, componentAddresses[0]) }); async function subject(): Promise { return core.fillOrder.sendTransactionAsync( - addresses, - values, + parameters.addresses, + parameters.values, subjectQuantityToIssue, - signature.v, - signature.r, - signature.s, + parameters.signature.v, + parameters.signature.r, + parameters.signature.s, { from: subjectCaller }, ); } @@ -160,14 +142,74 @@ contract("CoreIssuance", (accounts) => { const expectedNewBalance = existingBalance.sub(subjectQuantityToIssue.div(naturalUnit).mul(unit)); expect(newBalance).to.be.bignumber.equal(expectedNewBalance); }); + it("mints the correct quantity of the set for the user", async () => { + const existingBalance = await setToken.balanceOf.callAsync(signerAddress); + + await subject(); + + assertTokenBalance(setToken, existingBalance.add(subjectQuantityToIssue), signerAddress); + }); + describe("when the quantity to issue is not positive", async () => { + beforeEach(async () => { + subjectQuantityToIssue = ZERO; + }); + + it("should revert", async () => { + await expectRevertError(subject()); + }); + }); + describe("when the set was not created through core", async () => { + beforeEach(async () => { + parameters = await generateFillOrderParameters(NULL_ADDRESS, signerAddress, componentAddresses[0]) + }); + + it("should revert", async () => { + await expectRevertError(subject()); + }); + }); + describe("when the quantity is not a multiple of the natural unit of the set", async () => { + beforeEach(async () => { + subjectQuantityToIssue = ether(3); + }); + + it("should revert", async () => { + await expectRevertError(subject()); + }); + }); + describe("when the order has expired", async () => { + beforeEach(async () => { + parameters = await generateFillOrderParameters(NULL_ADDRESS, signerAddress, componentAddresses[0], undefined, undefined, -1) + }); + + it("should revert", async () => { + await expectRevertError(subject()); + }); + }); + describe("when invalid Set Token quantity in Issuance Order", async () => { + beforeEach(async () => { + parameters = await generateFillOrderParameters(NULL_ADDRESS, signerAddress, componentAddresses[0], ZERO) + }); + + it("should revert", async () => { + await expectRevertError(subject()); + }); + }); + describe("when invalid makerTokenAmount in Issuance Order", async () => { + beforeEach(async () => { + parameters = await generateFillOrderParameters(NULL_ADDRESS, signerAddress, componentAddresses[0], undefined, ZERO) + }); + + it("should revert", async () => { + await expectRevertError(subject()); + }); + }); }); describe("#validateSignature", async () => { let subjectCaller: Address; let subjectMaker: Address; let signerAddress: Address; - let orderHash: string; - let signature: any; + let parameters: any; beforeEach(async () => { @@ -175,29 +217,16 @@ contract("CoreIssuance", (accounts) => { subjectMaker = signerAccount; signerAddress = signerAccount; - const order = { - setAddress: mockSetTokenAccount, - quantity: ether(2), - makerAddress: subjectMaker, - makerToken: mockTokenAccount, - makerTokenAmount: ether(10), - expiration: generateTimeStamp(), - relayerToken: mockTokenAccount, - relayerTokenAmount: ether(1), - salt: generateSalt() - } as IssuanceOrder; - - orderHash = hashOrderHex(order); - signature = await signMessage(orderHash, signerAddress); + parameters = await generateFillOrderParameters(mockSetTokenAccount, signerAddress, mockTokenAccount); }); async function subject(): Promise { return core.validateSignature.callAsync( - orderHash, + parameters.orderHash, subjectMaker, - signature.v, - signature.r, - signature.s, + parameters.signature.v, + parameters.signature.r, + parameters.signature.s, { from: subjectCaller }, ); } @@ -222,36 +251,18 @@ contract("CoreIssuance", (accounts) => { describe("#generateOrderHash", async () => { let subjectCaller: Address; - let addresses: Address[]; - let values: BigNumber[]; - let orderHash: string; + let parameters: any; beforeEach(async () => { - subjectCaller = takerAccount; - const order = { - setAddress: mockSetTokenAccount, - quantity: ether(2), - makerAddress: makerAccount, - makerToken: mockTokenAccount, - makerTokenAmount: ether(10), - expiration: generateTimeStamp(), - relayerToken: mockTokenAccount, - relayerTokenAmount: ether(1), - salt: generateSalt(), - } as IssuanceOrder; - - orderHash = hashOrderHex(order); - - addresses = [order.setAddress, order.makerAddress, order.makerToken, order.relayerToken]; - values = [order.quantity, order.makerTokenAmount, order.expiration, order.relayerTokenAmount, order.salt]; + parameters = await generateFillOrderParameters(mockSetTokenAccount, makerAccount, mockTokenAccount); }); async function subject(): Promise { return core.generateOrderHash.callAsync( - addresses, - values, + parameters.addresses, + parameters.values, { from: subjectCaller }, ); } @@ -259,7 +270,7 @@ contract("CoreIssuance", (accounts) => { it("should return true", async () => { const contractOrderHash = await subject(); - expect(contractOrderHash).to.equal(orderHash); + expect(contractOrderHash).to.equal(parameters.orderHash); }); }); }); diff --git a/test/utils/orderWrapper.ts b/test/utils/orderWrapper.ts index 0e13d7b8e..b1b00a8fa 100644 --- a/test/utils/orderWrapper.ts +++ b/test/utils/orderWrapper.ts @@ -11,6 +11,8 @@ import { MAX_DIGITS_IN_UNSIGNED_256_INT, } from "../utils/constants"; +import { ether } from "./units"; + function bigNumberToBN(value: BigNumber) { return new BN(value.toString(), 10); } @@ -74,9 +76,11 @@ export function generateSalt(): BigNumber { return salt; } -export function generateTimeStamp(): BigNumber { - const tenMinutes = 10 * 60 * 1000; - const expiration = new BigNumber(Date.now() + tenMinutes); +export function generateTimeStamp( + min: number, +): BigNumber { + const timeToExpiration = min * 60 * 1000; + const expiration = new BigNumber(Date.now() + timeToExpiration); return expiration; } @@ -112,3 +116,37 @@ export async function signMessage( const ecSig = parseSigHexAsRSV(sig); return ecSig; } + +export async function generateFillOrderParameters( + setAddress: Address, + signerAddress: Address, + componentAddress: Address, + quantity: BigNumber = ether(4), + makerTokenAmount: BigNumber = ether(10), + timeToExpiration: number = 10, + +): Promise { + const order = { + setAddress, + quantity, + makerAddress: signerAddress, + makerToken: componentAddress, + makerTokenAmount, + expiration: generateTimeStamp(timeToExpiration), + relayerToken: componentAddress, + relayerTokenAmount: ether(1), + salt: generateSalt() + } as IssuanceOrder; + + const addresses = [order.setAddress, order.makerAddress, order.makerToken, order.relayerToken]; + const values = [order.quantity, order.makerTokenAmount, order.expiration, order.relayerTokenAmount, order.salt]; + + const orderHash = hashOrderHex(order); + const signature = await signMessage(orderHash, signerAddress); + return { + addresses, + values, + orderHash, + signature, + }; +} From 0de51628a4153381e9daf421fbb8991fc362ae5a Mon Sep 17 00:00:00 2001 From: bweick Date: Mon, 2 Jul 2018 13:22:14 -0700 Subject: [PATCH 4/8] Removed LibBytes from branch. --- .../core/extensions/CoreIssuanceOrder.sol | 2 - contracts/external/LibBytes.sol | 47 ------------------- .../core/extensions/coreIssuanceOrder.spec.ts | 4 +- 3 files changed, 2 insertions(+), 51 deletions(-) delete mode 100644 contracts/external/LibBytes.sol diff --git a/contracts/core/extensions/CoreIssuanceOrder.sol b/contracts/core/extensions/CoreIssuanceOrder.sol index 3fe0ff506..d927e399d 100644 --- a/contracts/core/extensions/CoreIssuanceOrder.sol +++ b/contracts/core/extensions/CoreIssuanceOrder.sol @@ -20,7 +20,6 @@ pragma solidity 0.4.24; import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol"; import { CoreModifiers } from "../lib/CoreSharedModifiers.sol"; import { ICoreIssuance } from "../interfaces/ICoreIssuance.sol"; -import { LibBytes } from "../../external/LibBytes.sol"; /** @@ -36,7 +35,6 @@ contract CoreIssuanceOrder is ICoreIssuance { using SafeMath for uint256; - using LibBytes for bytes; string constant INVALID_SIGNATURE = "Invalid order signature."; diff --git a/contracts/external/LibBytes.sol b/contracts/external/LibBytes.sol deleted file mode 100644 index 1db0d7b63..000000000 --- a/contracts/external/LibBytes.sol +++ /dev/null @@ -1,47 +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.24; - -library LibBytes { - - using LibBytes for bytes; - - - /// @dev Reads a bytes32 value from a position in a byte array. - /// @param b Byte array containing a bytes32 value. - /// @param index Index in byte array of bytes32 value. - /// @return bytes32 value from byte array. - function readBytes32( - bytes memory b, - uint256 index - ) - internal - pure - returns (bytes32 result) - { - require( - b.length >= index + 32, - "GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED" - ); - - // Arrays are prefixed by a 256 bit length parameter - index += 32; - - // Read the bytes32 from array memory - assembly { - result := mload(add(b, index)) - } - return result; - } -} \ No newline at end of file diff --git a/test/core/extensions/coreIssuanceOrder.spec.ts b/test/core/extensions/coreIssuanceOrder.spec.ts index a129afd85..a292b4a19 100644 --- a/test/core/extensions/coreIssuanceOrder.spec.ts +++ b/test/core/extensions/coreIssuanceOrder.spec.ts @@ -83,7 +83,7 @@ contract("CoreIssuanceOrder", (accounts) => { await coreWrapper.setDefaultStateAndAuthorizationsAsync(core, vault, transferProxy, setTokenFactory); }); - describe.only("#fillOrder", async () => { + describe("#fillOrder", async () => { let subjectCaller: Address; let subjectQuantityToIssue: BigNumber; @@ -115,7 +115,7 @@ contract("CoreIssuanceOrder", (accounts) => { subjectCaller = takerAccount; subjectQuantityToIssue = ether(2); - parameters = await generateFillOrderParameters(setToken.address, signerAddress, componentAddresses[0]) + parameters = await generateFillOrderParameters(setToken.address, signerAddress, componentAddresses[0]); }); async function subject(): Promise { From 38d7961a9adbfc23cbdc7afa1c8842bd8ff04251 Mon Sep 17 00:00:00 2001 From: bweick Date: Mon, 2 Jul 2018 15:26:47 -0700 Subject: [PATCH 5/8] Added docs/comments and revert error messages --- .../core/extensions/CoreIssuanceOrder.sol | 87 ++++++++++++++----- 1 file changed, 66 insertions(+), 21 deletions(-) diff --git a/contracts/core/extensions/CoreIssuanceOrder.sol b/contracts/core/extensions/CoreIssuanceOrder.sol index d927e399d..06f192da6 100644 --- a/contracts/core/extensions/CoreIssuanceOrder.sol +++ b/contracts/core/extensions/CoreIssuanceOrder.sol @@ -37,20 +37,32 @@ contract CoreIssuanceOrder is using SafeMath for uint256; string constant INVALID_SIGNATURE = "Invalid order signature."; + string constant INVALID_TOKEN_AMOUNTS = "Quantity and makerTokenAmount should be greater than 0."; + string constant ORDER_EXPIRED = "This order has expired."; struct IssuanceOrder { - address setAddress; - uint256 quantity; - address makerAddress; - address makerToken; - uint256 makerTokenAmount; - uint256 expiration; - address relayerToken; - uint256 relayerTokenAmount; - uint256 salt; + address setAddress; // _addresses[0] + uint256 quantity; // _values[0] + address makerAddress; // _addresses[1] + address makerToken; // _addresses[2] + uint256 makerTokenAmount; // _values[1] + uint256 expiration; // _values[2] + address relayerToken; // _addresses[3] + uint256 relayerTokenAmount; // _values[3] + uint256 salt; // _values[4] bytes32 orderHash; } + /** + * Fill an issuance order + * + * @param _addresses [setAddress, makerAddress, makerToken, relayerToken] + * @param _values [quantity, makerTokenAmount, expiration, relayerTokenAmount, salt] + * @param _fillQuantity Quantity of set to be filled + * @param _v v element of ECDSA signature + * @param _r r element of ECDSA signature + * @param _s s element of ECDSA signature + */ function fillOrder( address[4] _addresses, uint[5] _values, @@ -107,6 +119,12 @@ contract CoreIssuanceOrder is ); } + /** + * Create hash of order parameters + * + * @param _addresses [setAddress, makerAddress, makerToken, relayerToken] + * @param _values [quantity, makerTokenAmount, expiration, relayerTokenAmount, salt] + */ function generateOrderHash( address[4] _addresses, uint[5] _values @@ -114,32 +132,56 @@ contract CoreIssuanceOrder is public returns(bytes32) { + // Hash the order parameters return keccak256( abi.encodePacked( - _addresses[0], //setAddress - _addresses[1], //makerAddress - _addresses[2], //makerToken - _addresses[3], //relayerToken - _values[0], //quantity - _values[1], //makerTokenAmount - _values[2], //expiration - _values[3], //relayerTokenAmount - _values[4] //salt + _addresses[0], // setAddress + _addresses[1], // makerAddress + _addresses[2], // makerToken + _addresses[3], // relayerToken + _values[0], // quantity + _values[1], // makerTokenAmount + _values[2], // expiration + _values[3], // relayerTokenAmount + _values[4] // salt ) ); } + /** + * Validate order params are still valid + * + * @param _order IssuanceOrder object containing order params + * @param _fillQuantity Quantity of Set to be filled + */ function validateOrder( IssuanceOrder _order, uint _fillQuantity ) internal { - require(_order.makerTokenAmount > 0 && _order.quantity > 0); - require(block.timestamp <= _order.expiration); - // Check to see if filled + // Make sure makerTokenAmount and Set Token to issue is greater than 0. + require( + _order.makerTokenAmount > 0 && _order.quantity > 0, + INVALID_TOKEN_AMOUNTS + ); + // Make sure the order hasn't expired + require( + block.timestamp <= _order.expiration, + ORDER_EXPIRED + ); + // TO DO: Check to see if filled } + /** + * Validate order signature + * + * @param _orderHash Hash of issuance order + * @param _signerAddress Address of Issuance Order signer + * @param _v v element of ECDSA signature + * @param _r r element of ECDSA signature + * @param _s s element of ECDSA signature + */ function validateSignature( bytes32 _orderHash, address _signerAddress, @@ -150,10 +192,13 @@ contract CoreIssuanceOrder is public returns(bool) { + // Public address returned by ecrecover function address recAddress; + // Ethereum msg prefix bytes memory msgPrefix = "\x19Ethereum Signed Message:\n32"; + // Find what address signed the order recAddress = ecrecover( keccak256(abi.encodePacked(msgPrefix, _orderHash)), _v, From e40ea2921cb994b255fcb3c816d8729edfb38769 Mon Sep 17 00:00:00 2001 From: bweick Date: Mon, 2 Jul 2018 17:43:06 -0700 Subject: [PATCH 6/8] Refactored CoreIssuanceOrder to make OrderLibnrary and associated tests. --- .../core/extensions/CoreIssuanceOrder.sol | 92 ++----------- contracts/core/lib/OrderLibrary.sol | 114 ++++++++++++++++ contracts/test/lib/MockOrderLibrary.sol | 42 ++++++ package.json | 2 +- .../core/extensions/coreIssuanceOrder.spec.ts | 70 ---------- test/core/lib/orderLibrary.spec.ts | 126 ++++++++++++++++++ test/utils/coreWrapper.ts | 15 +++ 7 files changed, 309 insertions(+), 152 deletions(-) create mode 100644 contracts/core/lib/OrderLibrary.sol create mode 100644 contracts/test/lib/MockOrderLibrary.sol create mode 100644 test/core/lib/orderLibrary.spec.ts diff --git a/contracts/core/extensions/CoreIssuanceOrder.sol b/contracts/core/extensions/CoreIssuanceOrder.sol index 06f192da6..81dc36b2c 100644 --- a/contracts/core/extensions/CoreIssuanceOrder.sol +++ b/contracts/core/extensions/CoreIssuanceOrder.sol @@ -20,6 +20,7 @@ pragma solidity 0.4.24; import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol"; import { CoreModifiers } from "../lib/CoreSharedModifiers.sol"; import { ICoreIssuance } from "../interfaces/ICoreIssuance.sol"; +import { OrderLibrary } from "../lib/OrderLibrary.sol"; /** @@ -36,22 +37,13 @@ contract CoreIssuanceOrder is { using SafeMath for uint256; + /* ============ Constants ============ */ + string constant INVALID_SIGNATURE = "Invalid order signature."; string constant INVALID_TOKEN_AMOUNTS = "Quantity and makerTokenAmount should be greater than 0."; string constant ORDER_EXPIRED = "This order has expired."; - struct IssuanceOrder { - address setAddress; // _addresses[0] - uint256 quantity; // _values[0] - address makerAddress; // _addresses[1] - address makerToken; // _addresses[2] - uint256 makerTokenAmount; // _values[1] - uint256 expiration; // _values[2] - address relayerToken; // _addresses[3] - uint256 relayerTokenAmount; // _values[3] - uint256 salt; // _values[4] - bytes32 orderHash; - } + /* ============ External Functions ============ */ /** * Fill an issuance order @@ -71,13 +63,13 @@ contract CoreIssuanceOrder is bytes32 _r, bytes32 _s ) - public + external isValidSet(_addresses[0]) isPositiveQuantity(_fillQuantity) isNaturalUnitMultiple(_fillQuantity, _addresses[0]) { - IssuanceOrder memory order = IssuanceOrder({ + OrderLibrary.IssuanceOrder memory order = OrderLibrary.IssuanceOrder({ setAddress: _addresses[0], quantity: _values[0], makerAddress: _addresses[1], @@ -87,7 +79,7 @@ contract CoreIssuanceOrder is relayerToken: _addresses[3], relayerTokenAmount: _values[3], salt: _values[4], - orderHash: generateOrderHash( + orderHash: OrderLibrary.generateOrderHash( _addresses, _values ) @@ -101,7 +93,7 @@ contract CoreIssuanceOrder is // Verify signature is authentic require( - validateSignature( + OrderLibrary.validateSignature( order.orderHash, order.makerAddress, _v, @@ -119,34 +111,7 @@ contract CoreIssuanceOrder is ); } - /** - * Create hash of order parameters - * - * @param _addresses [setAddress, makerAddress, makerToken, relayerToken] - * @param _values [quantity, makerTokenAmount, expiration, relayerTokenAmount, salt] - */ - function generateOrderHash( - address[4] _addresses, - uint[5] _values - ) - public - returns(bytes32) - { - // Hash the order parameters - return keccak256( - abi.encodePacked( - _addresses[0], // setAddress - _addresses[1], // makerAddress - _addresses[2], // makerToken - _addresses[3], // relayerToken - _values[0], // quantity - _values[1], // makerTokenAmount - _values[2], // expiration - _values[3], // relayerTokenAmount - _values[4] // salt - ) - ); - } + /* ============ Internal Functions ============ */ /** * Validate order params are still valid @@ -155,10 +120,11 @@ contract CoreIssuanceOrder is * @param _fillQuantity Quantity of Set to be filled */ function validateOrder( - IssuanceOrder _order, + OrderLibrary.IssuanceOrder _order, uint _fillQuantity ) internal + view { // Make sure makerTokenAmount and Set Token to issue is greater than 0. require( @@ -172,40 +138,4 @@ contract CoreIssuanceOrder is ); // TO DO: Check to see if filled } - - /** - * Validate order signature - * - * @param _orderHash Hash of issuance order - * @param _signerAddress Address of Issuance Order signer - * @param _v v element of ECDSA signature - * @param _r r element of ECDSA signature - * @param _s s element of ECDSA signature - */ - function validateSignature( - bytes32 _orderHash, - address _signerAddress, - uint8 _v, - bytes32 _r, - bytes32 _s - ) - public - returns(bool) - { - // Public address returned by ecrecover function - address recAddress; - - // Ethereum msg prefix - bytes memory msgPrefix = "\x19Ethereum Signed Message:\n32"; - - // Find what address signed the order - recAddress = ecrecover( - keccak256(abi.encodePacked(msgPrefix, _orderHash)), - _v, - _r, - _s - ); - - return recAddress == _signerAddress; - } } diff --git a/contracts/core/lib/OrderLibrary.sol b/contracts/core/lib/OrderLibrary.sol new file mode 100644 index 000000000..d08723e32 --- /dev/null +++ b/contracts/core/lib/OrderLibrary.sol @@ -0,0 +1,114 @@ +/* + Copyright 2018 Set Labs Inc. + + 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.24; + + +/** + * @title OrderLibrary + * @author Set Protocol + * + * The Order Library contains functions for checking validation and hashing of Orders. + * + */ + +library OrderLibrary { + + /* ============ Structs ============ */ + + struct IssuanceOrder { + address setAddress; // _addresses[0] + uint256 quantity; // _values[0] + address makerAddress; // _addresses[1] + address makerToken; // _addresses[2] + uint256 makerTokenAmount; // _values[1] + uint256 expiration; // _values[2] + address relayerToken; // _addresses[3] + uint256 relayerTokenAmount; // _values[3] + uint256 salt; // _values[4] + bytes32 orderHash; + } + + /* ============ Internal Functions ============ */ + + /** + * Create hash of order parameters + * + * @param _addresses [setAddress, makerAddress, makerToken, relayerToken] + * @param _values [quantity, makerTokenAmount, expiration, relayerTokenAmount, salt] + */ + function generateOrderHash( + address[4] _addresses, + uint[5] _values + ) + internal + pure + returns(bytes32) + { + // Hash the order parameters + return keccak256( + abi.encodePacked( + _addresses[0], // setAddress + _addresses[1], // makerAddress + _addresses[2], // makerToken + _addresses[3], // relayerToken + _values[0], // quantity + _values[1], // makerTokenAmount + _values[2], // expiration + _values[3], // relayerTokenAmount + _values[4] // salt + ) + ); + } + + /** + * Validate order signature + * + * @param _orderHash Hash of issuance order + * @param _signerAddress Address of Issuance Order signer + * @param _v v element of ECDSA signature + * @param _r r element of ECDSA signature + * @param _s s element of ECDSA signature + */ + function validateSignature( + bytes32 _orderHash, + address _signerAddress, + uint8 _v, + bytes32 _r, + bytes32 _s + ) + internal + pure + returns(bool) + { + // Public address returned by ecrecover function + address recAddress; + + // Ethereum msg prefix + bytes memory msgPrefix = "\x19Ethereum Signed Message:\n32"; + + // Find what address signed the order + recAddress = ecrecover( + keccak256(abi.encodePacked(msgPrefix, _orderHash)), + _v, + _r, + _s + ); + + return recAddress == _signerAddress; + } + +} diff --git a/contracts/test/lib/MockOrderLibrary.sol b/contracts/test/lib/MockOrderLibrary.sol new file mode 100644 index 000000000..e4aa74c40 --- /dev/null +++ b/contracts/test/lib/MockOrderLibrary.sol @@ -0,0 +1,42 @@ +pragma solidity 0.4.24; +pragma experimental "ABIEncoderV2"; + +import { OrderLibrary } from "../../core/lib/OrderLibrary.sol"; + +// Mock contract implementation of OrderLibrary functions +contract MockOrderLibrary { + function testGenerateOrderHash( + address[4] _addresses, + uint[5] _values + ) + public + pure + returns(bytes32) + { + return OrderLibrary.generateOrderHash( + _addresses, + _values + ); + } + + function testValidateSignature( + bytes32 _orderHash, + address _signerAddress, + uint8 _v, + bytes32 _r, + bytes32 _s + ) + public + pure + returns(bool) + { + return OrderLibrary.validateSignature( + _orderHash, + _signerAddress, + _v, + _r, + _s + ); + } +} + diff --git a/package.json b/package.json index 67f709998..0d97ae794 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ "lint-ts": "tslint --fix test/*.ts", "lint-sol": "solium -d contracts/", "lint": "yarn run lint-sol && yarn run lint-ts", - "chain": "ganache-cli --networkId 70 --accounts 20 -l 10000000 -e 1000000 -m 'set protocol to the moon'", + "chain": "ganache-cli --networkId 70 --accounts 20 -l 10000000 -e 1000000", "coverage-setup": "cp -r transpiled/test/* test/. && cp -r transpiled/types/* types/.", "coverage-cleanup": "find test -name \\*.js* -type f -delete && find types -name \\*.js* -type f -delete", "coverage": "yarn prepare-test && yarn coverage-setup && ./node_modules/.bin/solidity-coverage && yarn coverage-cleanup" diff --git a/test/core/extensions/coreIssuanceOrder.spec.ts b/test/core/extensions/coreIssuanceOrder.spec.ts index a292b4a19..9eb8b42fa 100644 --- a/test/core/extensions/coreIssuanceOrder.spec.ts +++ b/test/core/extensions/coreIssuanceOrder.spec.ts @@ -44,7 +44,6 @@ import { import { DEPLOYED_TOKEN_QUANTITY, - PRIVATE_KEYS, ZERO, NULL_ADDRESS, } from "../../utils/constants"; @@ -204,73 +203,4 @@ contract("CoreIssuanceOrder", (accounts) => { }); }); }); - describe("#validateSignature", async () => { - let subjectCaller: Address; - let subjectMaker: Address; - let signerAddress: Address; - - let parameters: any; - - beforeEach(async () => { - - subjectCaller = takerAccount; - subjectMaker = signerAccount; - signerAddress = signerAccount; - - parameters = await generateFillOrderParameters(mockSetTokenAccount, signerAddress, mockTokenAccount); - }); - - async function subject(): Promise { - return core.validateSignature.callAsync( - parameters.orderHash, - subjectMaker, - parameters.signature.v, - parameters.signature.r, - parameters.signature.s, - { from: subjectCaller }, - ); - } - - it("should return true", async () => { - const validSig = await subject(); - - expect(validSig).to.equal(true); - }); - describe("when the message is not signed by the maker", async () => { - beforeEach(async () => { - subjectMaker = makerAccount; - }); - - it("should return false", async () => { - const validSig = await subject(); - - expect(validSig).to.equal(false); - }); - }); - }); - describe("#generateOrderHash", async () => { - let subjectCaller: Address; - - let parameters: any; - - beforeEach(async () => { - subjectCaller = takerAccount; - - parameters = await generateFillOrderParameters(mockSetTokenAccount, makerAccount, mockTokenAccount); - }); - - async function subject(): Promise { - return core.generateOrderHash.callAsync( - parameters.addresses, - parameters.values, - { from: subjectCaller }, - ); - } - - it("should return true", async () => { - const contractOrderHash = await subject(); - - expect(contractOrderHash).to.equal(parameters.orderHash); - }); - }); }); diff --git a/test/core/lib/orderLibrary.spec.ts b/test/core/lib/orderLibrary.spec.ts new file mode 100644 index 000000000..e4e558a70 --- /dev/null +++ b/test/core/lib/orderLibrary.spec.ts @@ -0,0 +1,126 @@ +import * as chai from "chai"; +import * as _ from "lodash"; + +import * as ABIDecoder from "abi-decoder"; +import { BigNumber } from "bignumber.js"; +import { ether } from "../../utils/units"; + +// Types +import { Address, IssuanceOrder } from "../../../types/common.js"; + +// Contract types +import { MockOrderLibraryContract } from "../../../types/generated/mock_order_library"; + +// Artifacts +const MockOrderLibrary = artifacts.require("MockOrderLibrary"); + +// Core wrapper +import { CoreWrapper } from "../../utils/coreWrapper"; +import { + generateFillOrderParameters, +} from "../../utils/orderWrapper"; + +// Testing Set up +import { BigNumberSetup } from "../../config/bigNumberSetup"; +import ChaiSetup from "../../config/chaiSetup"; +BigNumberSetup.configure(); +ChaiSetup.configure(); +const { expect, assert } = chai; + +import { + expectRevertError, +} from "../../utils/tokenAssertions"; + +import { + ZERO, + NULL_ADDRESS, +} from "../../utils/constants"; + +contract("OrderLibrary", (accounts) => { + const [ + ownerAccount, + takerAccount, + makerAccount, + signerAccount, + mockSetTokenAccount, + mockTokenAccount + ] = accounts; + + let orderLib: MockOrderLibraryContract; + + const coreWrapper = new CoreWrapper(ownerAccount, ownerAccount); + + beforeEach(async () => { + orderLib = await coreWrapper.deployMockOrderLibAsync(); + }); + + describe("#validateSignature", async () => { + let subjectCaller: Address; + let subjectMaker: Address; + let signerAddress: Address; + + let parameters: any; + + beforeEach(async () => { + + subjectCaller = takerAccount; + subjectMaker = signerAccount; + signerAddress = signerAccount; + + parameters = await generateFillOrderParameters(mockSetTokenAccount, signerAddress, mockTokenAccount); + }); + + async function subject(): Promise { + return orderLib.testValidateSignature.callAsync( + parameters.orderHash, + subjectMaker, + parameters.signature.v, + parameters.signature.r, + parameters.signature.s, + { from: subjectCaller }, + ); + } + + it("should return true", async () => { + const validSig = await subject(); + + expect(validSig).to.equal(true); + }); + describe("when the message is not signed by the maker", async () => { + beforeEach(async () => { + subjectMaker = makerAccount; + }); + + it("should return false", async () => { + const validSig = await subject(); + + expect(validSig).to.equal(false); + }); + }); + }); + describe("#generateOrderHash", async () => { + let subjectCaller: Address; + + let parameters: any; + + beforeEach(async () => { + subjectCaller = takerAccount; + + parameters = await generateFillOrderParameters(mockSetTokenAccount, makerAccount, mockTokenAccount); + }); + + async function subject(): Promise { + return orderLib.testGenerateOrderHash.callAsync( + parameters.addresses, + parameters.values, + { from: subjectCaller }, + ); + } + + it("should return true", async () => { + const contractOrderHash = await subject(); + + expect(contractOrderHash).to.equal(parameters.orderHash); + }); + }); +}); \ No newline at end of file diff --git a/test/utils/coreWrapper.ts b/test/utils/coreWrapper.ts index 275f8484c..0306f7f6e 100644 --- a/test/utils/coreWrapper.ts +++ b/test/utils/coreWrapper.ts @@ -2,6 +2,7 @@ import * as _ from "lodash"; import { AuthorizableContract } from "../../types/generated/authorizable"; import { CoreContract } from "../../types/generated/core"; +import { MockOrderLibraryContract } from "../../types/generated/mock_order_library"; import { SetTokenContract } from "../../types/generated/set_token"; import { SetTokenFactoryContract } from "../../types/generated/set_token_factory"; import { StandardTokenMockContract } from "../../types/generated/standard_token_mock"; @@ -16,6 +17,7 @@ import { extractNewSetTokenAddressFromLogs } from "../logs/contracts/core"; const Authorizable = artifacts.require("Authorizable"); const Core = artifacts.require("Core"); +const MockOrderLibrary = artifacts.require("MockOrderLibrary"); const TransferProxy = artifacts.require("TransferProxy"); const SetTokenFactory = artifacts.require("SetTokenFactory"); const Vault = artifacts.require("Vault"); @@ -92,6 +94,19 @@ export class CoreWrapper { ); } + public async deployMockOrderLibAsync( + from: Address = this._tokenOwnerAddress + ): Promise { + const truffleMockOrderLibrary = await MockOrderLibrary.new( + { from }, + ); + + return new MockOrderLibraryContract( + web3.eth.contract(truffleMockOrderLibrary.abi).at(truffleMockOrderLibrary.address), + { from, gas: DEFAULT_GAS }, + ); + } + public async deploySetTokenAsync( factory: Address, componentAddresses: Address[], From afa3be52fc1bf145aaa7d185bfaa697df49dd360 Mon Sep 17 00:00:00 2001 From: bweick Date: Mon, 2 Jul 2018 18:42:20 -0700 Subject: [PATCH 7/8] Added validateSignature test to CoreIssuanceOrder. Fixed erring tests, changed parameter name. --- .../core/extensions/CoreIssuanceOrder.sol | 1 + .../core/extensions/coreIssuanceOrder.spec.ts | 33 ++++++++++++------- test/core/lib/orderLibrary.spec.ts | 22 ++++++------- test/utils/orderWrapper.ts | 5 +-- 4 files changed, 36 insertions(+), 25 deletions(-) diff --git a/contracts/core/extensions/CoreIssuanceOrder.sol b/contracts/core/extensions/CoreIssuanceOrder.sol index 81dc36b2c..bc5e5e06c 100644 --- a/contracts/core/extensions/CoreIssuanceOrder.sol +++ b/contracts/core/extensions/CoreIssuanceOrder.sol @@ -136,6 +136,7 @@ contract CoreIssuanceOrder is block.timestamp <= _order.expiration, ORDER_EXPIRED ); + // TO DO: Check to make sure quantity is multiple of natural unit // TO DO: Check to see if filled } } diff --git a/test/core/extensions/coreIssuanceOrder.spec.ts b/test/core/extensions/coreIssuanceOrder.spec.ts index 9eb8b42fa..7f26748f3 100644 --- a/test/core/extensions/coreIssuanceOrder.spec.ts +++ b/test/core/extensions/coreIssuanceOrder.spec.ts @@ -82,7 +82,7 @@ contract("CoreIssuanceOrder", (accounts) => { await coreWrapper.setDefaultStateAndAuthorizationsAsync(core, vault, transferProxy, setTokenFactory); }); - describe("#fillOrder", async () => { + describe.only("#fillOrder", async () => { let subjectCaller: Address; let subjectQuantityToIssue: BigNumber; @@ -93,7 +93,7 @@ contract("CoreIssuanceOrder", (accounts) => { let signerAddress: Address; let componentAddresses: Address[]; - let parameters: any; + let issuanceOrderParams: any; beforeEach(async () => { signerAddress = signerAccount; @@ -114,17 +114,17 @@ contract("CoreIssuanceOrder", (accounts) => { subjectCaller = takerAccount; subjectQuantityToIssue = ether(2); - parameters = await generateFillOrderParameters(setToken.address, signerAddress, componentAddresses[0]); + issuanceOrderParams = await generateFillOrderParameters(setToken.address, signerAddress, signerAddress, componentAddresses[0]); }); async function subject(): Promise { return core.fillOrder.sendTransactionAsync( - parameters.addresses, - parameters.values, + issuanceOrderParams.addresses, + issuanceOrderParams.values, subjectQuantityToIssue, - parameters.signature.v, - parameters.signature.r, - parameters.signature.s, + issuanceOrderParams.signature.v, + issuanceOrderParams.signature.r, + issuanceOrderParams.signature.s, { from: subjectCaller }, ); } @@ -159,7 +159,7 @@ contract("CoreIssuanceOrder", (accounts) => { }); describe("when the set was not created through core", async () => { beforeEach(async () => { - parameters = await generateFillOrderParameters(NULL_ADDRESS, signerAddress, componentAddresses[0]) + issuanceOrderParams = await generateFillOrderParameters(NULL_ADDRESS, signerAddress, signerAddress, componentAddresses[0]) }); it("should revert", async () => { @@ -177,7 +177,7 @@ contract("CoreIssuanceOrder", (accounts) => { }); describe("when the order has expired", async () => { beforeEach(async () => { - parameters = await generateFillOrderParameters(NULL_ADDRESS, signerAddress, componentAddresses[0], undefined, undefined, -1) + issuanceOrderParams = await generateFillOrderParameters(setToken.address, signerAddress, signerAddress, componentAddresses[0], undefined, undefined, -1) }); it("should revert", async () => { @@ -186,7 +186,7 @@ contract("CoreIssuanceOrder", (accounts) => { }); describe("when invalid Set Token quantity in Issuance Order", async () => { beforeEach(async () => { - parameters = await generateFillOrderParameters(NULL_ADDRESS, signerAddress, componentAddresses[0], ZERO) + issuanceOrderParams = await generateFillOrderParameters(setToken.address, signerAddress, signerAddress, componentAddresses[0], ZERO) }); it("should revert", async () => { @@ -195,7 +195,16 @@ contract("CoreIssuanceOrder", (accounts) => { }); describe("when invalid makerTokenAmount in Issuance Order", async () => { beforeEach(async () => { - parameters = await generateFillOrderParameters(NULL_ADDRESS, signerAddress, componentAddresses[0], undefined, ZERO) + issuanceOrderParams = await generateFillOrderParameters(setToken.address, signerAddress, signerAddress, componentAddresses[0], undefined, ZERO) + }); + + it("should revert", async () => { + await expectRevertError(subject()); + }); + }); + describe("when the message is not signed by the maker", async () => { + beforeEach(async () => { + issuanceOrderParams = await generateFillOrderParameters(setToken.address, signerAddress, makerAccount, componentAddresses[0]) }); it("should revert", async () => { diff --git a/test/core/lib/orderLibrary.spec.ts b/test/core/lib/orderLibrary.spec.ts index e4e558a70..158373c0c 100644 --- a/test/core/lib/orderLibrary.spec.ts +++ b/test/core/lib/orderLibrary.spec.ts @@ -59,7 +59,7 @@ contract("OrderLibrary", (accounts) => { let subjectMaker: Address; let signerAddress: Address; - let parameters: any; + let issuanceOrderParams: any; beforeEach(async () => { @@ -67,16 +67,16 @@ contract("OrderLibrary", (accounts) => { subjectMaker = signerAccount; signerAddress = signerAccount; - parameters = await generateFillOrderParameters(mockSetTokenAccount, signerAddress, mockTokenAccount); + issuanceOrderParams = await generateFillOrderParameters(mockSetTokenAccount, signerAddress, signerAddress, mockTokenAccount); }); async function subject(): Promise { return orderLib.testValidateSignature.callAsync( - parameters.orderHash, + issuanceOrderParams.orderHash, subjectMaker, - parameters.signature.v, - parameters.signature.r, - parameters.signature.s, + issuanceOrderParams.signature.v, + issuanceOrderParams.signature.r, + issuanceOrderParams.signature.s, { from: subjectCaller }, ); } @@ -101,18 +101,18 @@ contract("OrderLibrary", (accounts) => { describe("#generateOrderHash", async () => { let subjectCaller: Address; - let parameters: any; + let issuanceOrderParams: any; beforeEach(async () => { subjectCaller = takerAccount; - parameters = await generateFillOrderParameters(mockSetTokenAccount, makerAccount, mockTokenAccount); + issuanceOrderParams = await generateFillOrderParameters(mockSetTokenAccount, makerAccount, makerAccount, mockTokenAccount); }); async function subject(): Promise { return orderLib.testGenerateOrderHash.callAsync( - parameters.addresses, - parameters.values, + issuanceOrderParams.addresses, + issuanceOrderParams.values, { from: subjectCaller }, ); } @@ -120,7 +120,7 @@ contract("OrderLibrary", (accounts) => { it("should return true", async () => { const contractOrderHash = await subject(); - expect(contractOrderHash).to.equal(parameters.orderHash); + expect(contractOrderHash).to.equal(issuanceOrderParams.orderHash); }); }); }); \ No newline at end of file diff --git a/test/utils/orderWrapper.ts b/test/utils/orderWrapper.ts index b1b00a8fa..9277934d9 100644 --- a/test/utils/orderWrapper.ts +++ b/test/utils/orderWrapper.ts @@ -80,7 +80,7 @@ export function generateTimeStamp( min: number, ): BigNumber { const timeToExpiration = min * 60 * 1000; - const expiration = new BigNumber(Date.now() + timeToExpiration); + const expiration = new BigNumber(Math.floor((Date.now() + timeToExpiration)/1000)); return expiration; } @@ -120,6 +120,7 @@ export async function signMessage( export async function generateFillOrderParameters( setAddress: Address, signerAddress: Address, + makerAddress: Address, componentAddress: Address, quantity: BigNumber = ether(4), makerTokenAmount: BigNumber = ether(10), @@ -129,7 +130,7 @@ export async function generateFillOrderParameters( const order = { setAddress, quantity, - makerAddress: signerAddress, + makerAddress, makerToken: componentAddress, makerTokenAmount, expiration: generateTimeStamp(timeToExpiration), From 18c1ae04c547266fc159e4d53c2dfaf561ff5925 Mon Sep 17 00:00:00 2001 From: bweick Date: Mon, 2 Jul 2018 18:55:27 -0700 Subject: [PATCH 8/8] Removed .only on test. --- test/core/extensions/coreIssuanceOrder.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/extensions/coreIssuanceOrder.spec.ts b/test/core/extensions/coreIssuanceOrder.spec.ts index 7f26748f3..b44e7c120 100644 --- a/test/core/extensions/coreIssuanceOrder.spec.ts +++ b/test/core/extensions/coreIssuanceOrder.spec.ts @@ -82,7 +82,7 @@ contract("CoreIssuanceOrder", (accounts) => { await coreWrapper.setDefaultStateAndAuthorizationsAsync(core, vault, transferProxy, setTokenFactory); }); - describe.only("#fillOrder", async () => { + describe("#fillOrder", async () => { let subjectCaller: Address; let subjectQuantityToIssue: BigNumber;