From d46e3f677805c8e3e4b4873fb9b0cec2988a4d63 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 19 Jun 2018 17:20:53 -0700 Subject: Twenty new tests for MixinSignatureValidator. Fixed handling of unsupported types. Fixed trezor prefix. --- packages/contracts/compiler.json | 2 + packages/contracts/package.json | 2 +- .../protocol/Exchange/MixinSignatureValidator.sol | 11 +- .../Exchange/mixins/MSignatureValidator.sol | 19 +- .../current/test/TestValidator/TestValidator.sol | 52 +++ .../current/test/TestWallet/TestWallet.sol | 65 ++++ packages/contracts/src/utils/artifacts.ts | 4 + packages/contracts/src/utils/types.ts | 1 + .../contracts/test/exchange/signature_validator.ts | 401 ++++++++++++++++++++- 9 files changed, 530 insertions(+), 27 deletions(-) create mode 100644 packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol create mode 100644 packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol (limited to 'packages/contracts') diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json index a5cfa8761..fa7ede817 100644 --- a/packages/contracts/compiler.json +++ b/packages/contracts/compiler.json @@ -36,6 +36,8 @@ "TestLibMem", "TestLibs", "TestSignatureValidator", + "TestValidator", + "TestWallet", "TokenRegistry", "Whitelist", "WETH9", diff --git a/packages/contracts/package.json b/packages/contracts/package.json index cf3f6f01d..9e7dad674 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -34,7 +34,7 @@ }, "config": { "abis": - "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|ExchangeWrapper|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetDataDecoders|TestAssetProxyDispatcher|TestLibBytes|TestLibMem|TestLibs|TestSignatureValidator|TokenRegistry|Whitelist|WETH9|ZRXToken).json" + "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|ExchangeWrapper|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetDataDecoders|TestAssetProxyDispatcher|TestLibBytes|TestLibMem|TestLibs|TestSignatureValidator|TestValidator|TestWallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json" }, "repository": { "type": "git", diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol index 8ad15aaff..4a2beff57 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -33,7 +33,7 @@ contract MixinSignatureValidator is { // Personal message headers string constant ETH_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n32"; - string constant TREZOR_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n\x41"; + string constant TREZOR_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n\x20"; // Mapping of hash => signer => signed mapping (bytes32 => mapping (address => bool)) public preSigned; @@ -92,8 +92,15 @@ contract MixinSignatureValidator is LENGTH_GREATER_THAN_0_REQUIRED ); + // Ensure signature is supported + uint8 signatureTypeRaw = uint8(popLastByte(signature)); + require( + signatureTypeRaw < uint8(SignatureType.NSignatureTypes), + SIGNATURE_UNSUPPORTED + ); + // Pop last byte off of signature byte array. - SignatureType signatureType = SignatureType(uint8(popLastByte(signature))); + SignatureType signatureType = SignatureType(signatureTypeRaw); // Variables are not scoped in Solidity. uint8 v; diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol index 5e286e43a..0cab95193 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol @@ -25,14 +25,15 @@ contract MSignatureValidator is { // Allowed signature types. enum SignatureType { - Illegal, // 0x00, default value - Invalid, // 0x01 - EIP712, // 0x02 - EthSign, // 0x03 - Caller, // 0x04 - Wallet, // 0x05 - Validator, // 0x06 - PreSigned, // 0x07 - Trezor // 0x08 + Illegal, // 0x00, default value + Invalid, // 0x01 + EIP712, // 0x02 + EthSign, // 0x03 + Caller, // 0x04 + Wallet, // 0x05 + Validator, // 0x06 + PreSigned, // 0x07 + Trezor, // 0x08 + NSignatureTypes // 0x09, always leave at end. } } diff --git a/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol b/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol new file mode 100644 index 000000000..ba661f89d --- /dev/null +++ b/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol @@ -0,0 +1,52 @@ +/* + + 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; + +import "../../protocol/Exchange/interfaces/IValidator.sol"; + +contract TestValidator is + IValidator +{ + + // The single valid signer for this wallet. + address validSigner; + + /// @dev constructs a new `TestValidator` with a single valid signer. + /// @param _validSigner The sole signer for this wallet. + constructor (address _validSigner) public { + validSigner = _validSigner; + } + + /// @dev Verifies that a signature is valid. + /// @param hash Message hash that is signed. + /// @param signer Address that should have signed the given hash. + /// @param signature Proof of signing. + /// @return Validity of order signature. + function isValidSignature( + bytes32 hash, + address signer, + bytes signature + ) + external + view + returns (bool isValid) + { + return (signer == validSigner); + } +} diff --git a/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol b/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol new file mode 100644 index 000000000..5baba1d98 --- /dev/null +++ b/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol @@ -0,0 +1,65 @@ +/* + + 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; + +import "../../protocol/Exchange/interfaces/IWallet.sol"; +import "../../utils/LibBytes/LibBytes.sol"; + +contract TestWallet is + IWallet, + LibBytes +{ + + string constant LENGTH_65_REQUIRED = "LENGTH_65_REQUIRED"; + + // The owner of this wallet. + address walletOwner; + + /// @dev constructs a new `TestWallet` with a single owner. + /// @param _walletOwner The owner of this wallet. + constructor (address _walletOwner) public { + walletOwner = _walletOwner; + } + + /// @dev Validates an EIP712 signature. + /// The signer must match the signer of this wallet. + /// @param hash Message hash that is signed. + /// @param eip721Signature Proof of signing. + /// @return Validity of order signature. + function isValidSignature( + bytes32 hash, + bytes eip721Signature + ) + external + view + returns (bool isValid) + { + require( + eip721Signature.length == 65, + LENGTH_65_REQUIRED + ); + + uint8 v = uint8(eip721Signature[0]); + bytes32 r = readBytes32(eip721Signature, 1); + bytes32 s = readBytes32(eip721Signature, 33); + address recoveredAddress = ecrecover(hash, v, r, s); + isValid = walletOwner == recoveredAddress; + return isValid; + } +} diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index 7ba467708..4375d87c6 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -17,6 +17,8 @@ import * as TestLibBytes from '../artifacts/TestLibBytes.json'; import * as TestLibMem from '../artifacts/TestLibMem.json'; import * as TestLibs from '../artifacts/TestLibs.json'; import * as TestSignatureValidator from '../artifacts/TestSignatureValidator.json'; +import * as TestValidator from '../artifacts/TestValidator.json'; +import * as TestWallet from '../artifacts/TestWallet.json'; import * as TokenRegistry from '../artifacts/TokenRegistry.json'; import * as EtherToken from '../artifacts/WETH9.json'; import * as Whitelist from '../artifacts/Whitelist.json'; @@ -41,6 +43,8 @@ export const artifacts = { TestLibMem: (TestLibMem as any) as ContractArtifact, TestLibs: (TestLibs as any) as ContractArtifact, TestSignatureValidator: (TestSignatureValidator as any) as ContractArtifact, + TestValidator: (TestValidator as any) as ContractArtifact, + TestWallet: (TestWallet as any) as ContractArtifact, TokenRegistry: (TokenRegistry as any) as ContractArtifact, Whitelist: (Whitelist as any) as ContractArtifact, ZRX: (ZRX as any) as ContractArtifact, diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index bb8c12088..a6d0c2a88 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -100,6 +100,7 @@ export enum ContractName { DummyERC721Receiver = 'DummyERC721Receiver', DummyERC721Token = 'DummyERC721Token', TestLibBytes = 'TestLibBytes', + TestWallet = 'TestWallet', Authorizable = 'Authorizable', Whitelist = 'Whitelist', } diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index c39fd6ee4..8f796361f 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -1,12 +1,16 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { assetProxyUtils, orderHashUtils } from '@0xproject/order-utils'; -import { SignedOrder } from '@0xproject/types'; +import { addSignedMessagePrefix, assetProxyUtils, MessagePrefixType, orderHashUtils } from '@0xproject/order-utils'; +import { SignatureType, SignedOrder } from '@0xproject/types'; import * as chai from 'chai'; import ethUtil = require('ethereumjs-util'); +import * as _ from 'lodash'; import { TestSignatureValidatorContract } from '../../src/generated_contract_wrappers/test_signature_validator'; +import { TestValidatorContract } from '../../src/generated_contract_wrappers/test_validator'; +import { TestWalletContract } from '../../src/generated_contract_wrappers/test_wallet'; import { addressUtils } from '../../src/utils/address_utils'; import { artifacts } from '../../src/utils/artifacts'; +import { expectRevertOrAlwaysFailingTransactionAsync } from '../../src/utils/assertions'; import { chaiSetup } from '../../src/utils/chai_setup'; import { constants } from '../../src/utils/constants'; import { OrderFactory } from '../../src/utils/order_factory'; @@ -21,6 +25,12 @@ describe('MixinSignatureValidator', () => { let signedOrder: SignedOrder; let orderFactory: OrderFactory; let signatureValidator: TestSignatureValidatorContract; + let testWallet: TestWalletContract; + let testValidator: TestValidatorContract; + let signerAddress: string; + let signerPrivateKey: Buffer; + let notSignerAddress: string; + let notSignerPrivateKey: Buffer; before(async () => { await blockchainLifecycle.startAsync(); @@ -31,11 +41,31 @@ describe('MixinSignatureValidator', () => { before(async () => { const accounts = await web3Wrapper.getAvailableAddressesAsync(); const makerAddress = accounts[0]; + signerAddress = makerAddress; + notSignerAddress = accounts[1]; signatureValidator = await TestSignatureValidatorContract.deployFrom0xArtifactAsync( artifacts.TestSignatureValidator, provider, txDefaults, ); + testWallet = await TestWalletContract.deployFrom0xArtifactAsync( + artifacts.TestWallet, + provider, + txDefaults, + signerAddress, + ); + testValidator = await TestValidatorContract.deployFrom0xArtifactAsync( + artifacts.TestValidator, + provider, + txDefaults, + signerAddress, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync(testValidator.address, true, { + from: signerAddress, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); const defaultOrderParams = { ...constants.STATIC_ORDER_PARAMS, @@ -45,8 +75,9 @@ describe('MixinSignatureValidator', () => { makerAssetData: assetProxyUtils.encodeERC20AssetData(addressUtils.generatePseudoRandomAddress()), takerAssetData: assetProxyUtils.encodeERC20AssetData(addressUtils.generatePseudoRandomAddress()), }; - const privateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddress)]; - orderFactory = new OrderFactory(privateKey, defaultOrderParams); + signerPrivateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddress)]; + notSignerPrivateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(notSignerAddress)]; + orderFactory = new OrderFactory(signerPrivateKey, defaultOrderParams); }); beforeEach(async () => { @@ -61,29 +92,369 @@ describe('MixinSignatureValidator', () => { signedOrder = orderFactory.newSignedOrder(); }); - it('should return true with a valid signature', async () => { + it('should revert with an empty signature', async () => { + const emptySignature = '0x'; + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + return expectRevertOrAlwaysFailingTransactionAsync( + signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signedOrder.makerAddress, + emptySignature, + ), + ); + }); + + it('should revert with an unsupported signature type', async () => { + const unsupportedSignatureType = SignatureType.NSignatureTypes; + const unsupportedSignatureHex = `0x${unsupportedSignatureType}`; + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + return expectRevertOrAlwaysFailingTransactionAsync( + signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signedOrder.makerAddress, + unsupportedSignatureHex, + ), + ); + }); + + it('should revert when SignatureType=Illegal', async () => { + const unsupportedSignatureHex = `0x${SignatureType.Illegal}`; + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + return expectRevertOrAlwaysFailingTransactionAsync( + signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signedOrder.makerAddress, + unsupportedSignatureHex, + ), + ); + }); + + it('should return false when SignatureType=Invalid and signature has a length of zero', async () => { + const signatureHex = `0x${SignatureType.Invalid}`; + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signedOrder.makerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.false(); + }); + + it('should revert when SignatureType=Invalid and signature length is non-zero', async () => { + const fillerData = ethUtil.toBuffer('0xdeadbeef'); + const signatureType = ethUtil.toBuffer(`0x${SignatureType.Invalid}`); + const signatureBuffer = Buffer.concat([fillerData, signatureType]); + const signatureHex = ethUtil.bufferToHex(signatureBuffer); + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + return expectRevertOrAlwaysFailingTransactionAsync( + signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signedOrder.makerAddress, + signatureHex, + ), + ); + }); + + it('should return true when SignatureType=EIP712 and signature is valid', async () => { + // Create EIP712 signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const orderHashBuffer = ethUtil.toBuffer(orderHashHex); + const ecSignature = ethUtil.ecsign(orderHashBuffer, signerPrivateKey); + // Create 0x signature from EIP712 signature + const signature = Buffer.concat([ + ethUtil.toBuffer(ecSignature.v), + ecSignature.r, + ecSignature.s, + ethUtil.toBuffer(`0x${SignatureType.EIP712}`), + ]); + const signatureHex = ethUtil.bufferToHex(signature); + // Validate signature + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.true(); + }); + + it('should return true when SignatureType=EIP712 and signature is valid', async () => { + // Create EIP712 signature + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const orderHashBuffer = ethUtil.toBuffer(orderHashHex); + const ecSignature = ethUtil.ecsign(orderHashBuffer, signerPrivateKey); + // Create 0x signature from EIP712 signature + const signature = Buffer.concat([ + ethUtil.toBuffer(ecSignature.v), + ecSignature.r, + ecSignature.s, + ethUtil.toBuffer(`0x${SignatureType.EIP712}`), + ]); + const signatureHex = ethUtil.bufferToHex(signature); + // Validate signature + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + notSignerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.false(); + }); + + it('should return true when SignatureType=EthSign and signature is valid', async () => { + // Create EthSign signature + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.EthSign); + const orderHashWithEthSignPrefixBuffer = ethUtil.toBuffer(orderHashWithEthSignPrefixHex); + const ecSignature = ethUtil.ecsign(orderHashWithEthSignPrefixBuffer, signerPrivateKey); + // Create 0x signature from EthSign signature + const signature = Buffer.concat([ + ethUtil.toBuffer(ecSignature.v), + ecSignature.r, + ecSignature.s, + ethUtil.toBuffer(`0x${SignatureType.EthSign}`), + ]); + const signatureHex = ethUtil.bufferToHex(signature); + // Validate signature + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.true(); + }); + + it('should return true when SignatureType=EthSign and signature is invalid', async () => { + // Create EthSign signature + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.EthSign); + const orderHashWithEthSignPrefixBuffer = ethUtil.toBuffer(orderHashWithEthSignPrefixHex); + const ecSignature = ethUtil.ecsign(orderHashWithEthSignPrefixBuffer, signerPrivateKey); + // Create 0x signature from EthSign signature + const signature = Buffer.concat([ + ethUtil.toBuffer(ecSignature.v), + ecSignature.r, + ecSignature.s, + ethUtil.toBuffer(`0x${SignatureType.EthSign}`), + ]); + const signatureHex = ethUtil.bufferToHex(signature); + // Validate signature. + // This will fail because `signerAddress` signed the message, but we're passing in `notSignerAddress` + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + notSignerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.false(); + }); + + it('should return true when SignatureType=Caller and signer is caller', async () => { + const signature = ethUtil.toBuffer(`0x${SignatureType.Caller}`); + const signatureHex = ethUtil.bufferToHex(signature); + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signerAddress, + signatureHex, + { from: signerAddress }, + ); + expect(isValidSignature).to.be.true(); + }); + + it('should return false when SignatureType=Caller and signer is not caller', async () => { + const signature = ethUtil.toBuffer(`0x${SignatureType.Caller}`); + const signatureHex = ethUtil.bufferToHex(signature); + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signerAddress, + signatureHex, + { from: notSignerAddress }, + ); + expect(isValidSignature).to.be.false(); + }); + + it('should return true when SignatureType=Wallet and signature is valid', async () => { + // Create EIP712 signature + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const orderHashBuffer = ethUtil.toBuffer(orderHashHex); + const ecSignature = ethUtil.ecsign(orderHashBuffer, signerPrivateKey); + // Create 0x signature from EIP712 signature + const signature = Buffer.concat([ + ethUtil.toBuffer(ecSignature.v), + ecSignature.r, + ecSignature.s, + ethUtil.toBuffer(`0x${SignatureType.Wallet}`), + ]); + const signatureHex = ethUtil.bufferToHex(signature); + // Validate signature + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + testWallet.address, + signatureHex, + ); + expect(isValidSignature).to.be.true(); + }); + + it('should return false when SignatureType=Wallet and signature is invalid', async () => { + // Create EIP712 signature using a private key that does not belong to the wallet owner. + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const orderHashBuffer = ethUtil.toBuffer(orderHashHex); + const notWalletOwnerPrivateKey = notSignerPrivateKey; + const ecSignature = ethUtil.ecsign(orderHashBuffer, notWalletOwnerPrivateKey); + // Create 0x signature from EIP712 signature + const signature = Buffer.concat([ + ethUtil.toBuffer(ecSignature.v), + ecSignature.r, + ecSignature.s, + ethUtil.toBuffer(`0x${SignatureType.Wallet}`), + ]); + const signatureHex = ethUtil.bufferToHex(signature); + // Validate signature + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + testWallet.address, + signatureHex, + ); + expect(isValidSignature).to.be.false(); + }); + + it('should return true when SignatureType=Validator, signature is valid and validator is approved', async () => { + const validatorAddress = ethUtil.toBuffer(`${testValidator.address}`); + const signatureType = ethUtil.toBuffer(`0x${SignatureType.Validator}`); + const signature = Buffer.concat([validatorAddress, signatureType]); + const signatureHex = ethUtil.bufferToHex(signature); + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.true(); + }); + + it('should return false when SignatureType=Validator, signature is invalid and validator is approved', async () => { + const validatorAddress = ethUtil.toBuffer(`${testValidator.address}`); + const signatureType = ethUtil.toBuffer(`0x${SignatureType.Validator}`); + const signature = Buffer.concat([validatorAddress, signatureType]); + const signatureHex = ethUtil.bufferToHex(signature); + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + notSignerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.false(); + }); + + it('should return false when SignatureType=Validator, signature is valid and validator is not approved', async () => { + // Set approval of signature validator to false + await web3Wrapper.awaitTransactionSuccessAsync( + await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync( + testValidator.address, + false, + { from: signerAddress }, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Validate signature + const validatorAddress = ethUtil.toBuffer(`${testValidator.address}`); + const signatureType = ethUtil.toBuffer(`0x${SignatureType.Validator}`); + const signature = Buffer.concat([validatorAddress, signatureType]); + const signatureHex = ethUtil.bufferToHex(signature); + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.false(); + // Set approval of signature validator back to true + await web3Wrapper.awaitTransactionSuccessAsync( + await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync( + testValidator.address, + true, + { from: signerAddress }, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + }); + + it('should return true when SignatureType=Trezor and signature is valid', async () => { + // Create Trezor signature + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.Trezor); + const orderHashWithTrezorPrefixBuffer = ethUtil.toBuffer(orderHashWithTrezorPrefixHex); + const ecSignature = ethUtil.ecsign(orderHashWithTrezorPrefixBuffer, signerPrivateKey); + // Create 0x signature from Trezor signature + const signature = Buffer.concat([ + ethUtil.toBuffer(ecSignature.v), + ecSignature.r, + ecSignature.s, + ethUtil.toBuffer(`0x${SignatureType.Trezor}`), + ]); + const signatureHex = ethUtil.bufferToHex(signature); + // Validate signature + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.true(); + }); + + it('should return true when SignatureType=Trezor and signature is invalid', async () => { + // Create Trezor signature + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.Trezor); + const orderHashWithTrezorPrefixBuffer = ethUtil.toBuffer(orderHashWithTrezorPrefixHex); + const ecSignature = ethUtil.ecsign(orderHashWithTrezorPrefixBuffer, signerPrivateKey); + // Create 0x signature from Trezor signature + const signature = Buffer.concat([ + ethUtil.toBuffer(ecSignature.v), + ecSignature.r, + ecSignature.s, + ethUtil.toBuffer(`0x${SignatureType.Trezor}`), + ]); + const signatureHex = ethUtil.bufferToHex(signature); + // Validate signature. + // This will fail because `signerAddress` signed the message, but we're passing in `notSignerAddress` + const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + notSignerAddress, + signatureHex, + ); + expect(isValidSignature).to.be.false(); + }); + + it('should return true when SignatureType=Presigned and signer has presigned hash', async () => { + // Presign hash + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + await web3Wrapper.awaitTransactionSuccessAsync( + await signatureValidator.preSign.sendTransactionAsync( + orderHashHex, + signedOrder.makerAddress, + signedOrder.signature, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Validate presigned signature + const signature = ethUtil.toBuffer(`0x${SignatureType.PreSigned}`); + const signatureHex = ethUtil.bufferToHex(signature); const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( orderHashHex, signedOrder.makerAddress, - signedOrder.signature, + signatureHex, ); expect(isValidSignature).to.be.true(); }); - it('should return false with an invalid signature', async () => { - const v = ethUtil.toBuffer(signedOrder.signature.slice(0, 4)); - const invalidR = ethUtil.sha3('invalidR'); - const invalidS = ethUtil.sha3('invalidS'); - const signatureType = ethUtil.toBuffer(`0x${signedOrder.signature.slice(-2)}`); - const invalidSigBuff = Buffer.concat([v, invalidR, invalidS, signatureType]); - const invalidSigHex = `0x${invalidSigBuff.toString('hex')}`; - signedOrder.signature = invalidSigHex; + it('should return false when SignatureType=Presigned has not presigned hash', async () => { + const signature = ethUtil.toBuffer(`0x${SignatureType.PreSigned}`); + const signatureHex = ethUtil.bufferToHex(signature); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( orderHashHex, signedOrder.makerAddress, - signedOrder.signature, + signatureHex, ); expect(isValidSignature).to.be.false(); }); -- cgit v1.2.3 From 2c7358d64ffb0e9fcbc462bbe3cd89edd67cec81 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 19 Jun 2018 17:27:09 -0700 Subject: Minor style tweaks --- .../current/protocol/Exchange/mixins/MSignatureValidator.sol | 2 +- .../contracts/current/test/TestValidator/TestValidator.sol | 2 +- .../src/contracts/current/test/TestWallet/TestWallet.sol | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol index 0cab95193..9c6fbe22b 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol @@ -34,6 +34,6 @@ contract MSignatureValidator is Validator, // 0x06 PreSigned, // 0x07 Trezor, // 0x08 - NSignatureTypes // 0x09, always leave at end. + NSignatureTypes // 0x09, number of signature types. Always leave at end. } } diff --git a/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol b/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol index ba661f89d..c0daf8f38 100644 --- a/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol +++ b/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol @@ -33,7 +33,7 @@ contract TestValidator is validSigner = _validSigner; } - /// @dev Verifies that a signature is valid. + /// @dev Verifies that a signature is valid. /// @param hash Message hash that is signed. /// @param signer Address that should have signed the given hash. /// @param signature Proof of signing. diff --git a/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol b/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol index 5baba1d98..51556971d 100644 --- a/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol +++ b/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol @@ -40,24 +40,24 @@ contract TestWallet is /// @dev Validates an EIP712 signature. /// The signer must match the signer of this wallet. /// @param hash Message hash that is signed. - /// @param eip721Signature Proof of signing. + /// @param eip712Signature Proof of signing. /// @return Validity of order signature. function isValidSignature( bytes32 hash, - bytes eip721Signature + bytes eip712Signature ) external view returns (bool isValid) { require( - eip721Signature.length == 65, + eip712Signature.length == 65, LENGTH_65_REQUIRED ); - uint8 v = uint8(eip721Signature[0]); - bytes32 r = readBytes32(eip721Signature, 1); - bytes32 s = readBytes32(eip721Signature, 33); + uint8 v = uint8(eip712Signature[0]); + bytes32 r = readBytes32(eip712Signature, 1); + bytes32 s = readBytes32(eip712Signature, 33); address recoveredAddress = ecrecover(hash, v, r, s); isValid = walletOwner == recoveredAddress; return isValid; -- cgit v1.2.3 From 8d003dbc30735d72b860e739fd7dc47fad557df9 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 19 Jun 2018 18:26:42 -0700 Subject: Fixed two mislabelled revert reasons + Signature Validator revert tests working on Geth --- .../current/protocol/Exchange/libs/LibExchangeErrors.sol | 4 ++-- packages/contracts/src/utils/constants.ts | 5 +++++ packages/contracts/test/exchange/signature_validator.ts | 16 ++++++++++------ 3 files changed, 17 insertions(+), 8 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol index adf27bec3..aab428e74 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol @@ -56,6 +56,6 @@ contract LibExchangeErrors { /// Length validation errors /// string constant LENGTH_GREATER_THAN_0_REQUIRED = "LENGTH_GREATER_THAN_0_REQUIRED"; // Byte array must have a length greater than 0. - string constant LENGTH_0_REQUIRED = "LENGTH_1_REQUIRED"; // Byte array must have a length of 1. - string constant LENGTH_65_REQUIRED = "LENGTH_66_REQUIRED"; // Byte array must have a length of 66. + string constant LENGTH_0_REQUIRED = "LENGTH_0_REQUIRED"; // Byte array must have a length of 0. + string constant LENGTH_65_REQUIRED = "LENGTH_65_REQUIRED"; // Byte array must have a length of 65. } diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts index ec3c8fd36..f21b8c7a0 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -27,6 +27,11 @@ export const constants = { LIB_BYTES_GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED', ERC20_INSUFFICIENT_BALANCE: 'Insufficient balance to complete transfer.', ERC20_INSUFFICIENT_ALLOWANCE: 'Insufficient allowance to complete transfer.', + EXCHANGE_LENGTH_GREATER_THAN_0_REQUIRED: 'LENGTH_GREATER_THAN_0_REQUIRED', + EXCHANGE_SIGNATURE_UNSUPPORTED: 'SIGNATURE_UNSUPPORTED', + EXCHANGE_SIGNATURE_ILLEGAL: 'SIGNATURE_ILLEGAL', + EXCHANGE_LENGTH_0_REQUIRED: 'LENGTH_0_REQUIRED', + EXCHANGE_LENGTH_65_REQUIRED: 'LENGTH_65_REQUIRED', TESTRPC_NETWORK_ID: 50, // Note(albrow): In practice V8 and most other engines limit the minimum // interval for setInterval to 10ms. We still set it to 0 here in order to diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index 8f796361f..ca6897d7f 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -10,7 +10,7 @@ import { TestValidatorContract } from '../../src/generated_contract_wrappers/tes import { TestWalletContract } from '../../src/generated_contract_wrappers/test_wallet'; import { addressUtils } from '../../src/utils/address_utils'; import { artifacts } from '../../src/utils/artifacts'; -import { expectRevertOrAlwaysFailingTransactionAsync } from '../../src/utils/assertions'; +import { expectRevertOrOtherErrorAsync, expectRevertOrAlwaysFailingTransactionAsync } from '../../src/utils/assertions'; import { chaiSetup } from '../../src/utils/chai_setup'; import { constants } from '../../src/utils/constants'; import { OrderFactory } from '../../src/utils/order_factory'; @@ -21,7 +21,7 @@ const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); -describe('MixinSignatureValidator', () => { +describe.only('MixinSignatureValidator', () => { let signedOrder: SignedOrder; let orderFactory: OrderFactory; let signatureValidator: TestSignatureValidatorContract; @@ -95,12 +95,13 @@ describe('MixinSignatureValidator', () => { it('should revert with an empty signature', async () => { const emptySignature = '0x'; const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertOrOtherErrorAsync( signatureValidator.publicIsValidSignature.callAsync( orderHashHex, signedOrder.makerAddress, emptySignature, ), + constants.EXCHANGE_LENGTH_GREATER_THAN_0_REQUIRED, ); }); @@ -108,24 +109,26 @@ describe('MixinSignatureValidator', () => { const unsupportedSignatureType = SignatureType.NSignatureTypes; const unsupportedSignatureHex = `0x${unsupportedSignatureType}`; const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertOrOtherErrorAsync( signatureValidator.publicIsValidSignature.callAsync( orderHashHex, signedOrder.makerAddress, unsupportedSignatureHex, ), + constants.EXCHANGE_SIGNATURE_UNSUPPORTED, ); }); it('should revert when SignatureType=Illegal', async () => { const unsupportedSignatureHex = `0x${SignatureType.Illegal}`; const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertOrOtherErrorAsync( signatureValidator.publicIsValidSignature.callAsync( orderHashHex, signedOrder.makerAddress, unsupportedSignatureHex, ), + constants.EXCHANGE_SIGNATURE_ILLEGAL, ); }); @@ -146,12 +149,13 @@ describe('MixinSignatureValidator', () => { const signatureBuffer = Buffer.concat([fillerData, signatureType]); const signatureHex = ethUtil.bufferToHex(signatureBuffer); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertOrOtherErrorAsync( signatureValidator.publicIsValidSignature.callAsync( orderHashHex, signedOrder.makerAddress, signatureHex, ), + constants.EXCHANGE_LENGTH_0_REQUIRED, ); }); -- cgit v1.2.3 From d0df25d9e2ced5593a0718ee174eeb833f6599d3 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 19 Jun 2018 18:30:49 -0700 Subject: Remove .only --- packages/contracts/test/exchange/signature_validator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index ca6897d7f..5937dee12 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -21,7 +21,7 @@ const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); -describe.only('MixinSignatureValidator', () => { +describe('MixinSignatureValidator', () => { let signedOrder: SignedOrder; let orderFactory: OrderFactory; let signatureValidator: TestSignatureValidatorContract; -- cgit v1.2.3 From 4a136cafda2621b2d539b0d7b406db878586f2ff Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 19 Jun 2018 18:45:57 -0700 Subject: Minor improvements to MixinSignatureValidator tests --- .../contracts/test/exchange/signature_validator.ts | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index 5937dee12..483099c10 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -181,7 +181,7 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.true(); }); - it('should return true when SignatureType=EIP712 and signature is valid', async () => { + it('should return false when SignatureType=EIP712 and signature is invalid', async () => { // Create EIP712 signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); const orderHashBuffer = ethUtil.toBuffer(orderHashHex); @@ -194,7 +194,8 @@ describe('MixinSignatureValidator', () => { ethUtil.toBuffer(`0x${SignatureType.EIP712}`), ]); const signatureHex = ethUtil.bufferToHex(signature); - // Validate signature + // Validate signature. + // This will fail because `signerAddress` signed the message, but we're passing in `notSignerAddress` const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( orderHashHex, notSignerAddress, @@ -226,7 +227,7 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.true(); }); - it('should return true when SignatureType=EthSign and signature is invalid', async () => { + it('should return false when SignatureType=EthSign and signature is invalid', async () => { // Create EthSign signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); const orderHashWithEthSignPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.EthSign); @@ -341,6 +342,8 @@ describe('MixinSignatureValidator', () => { const signature = Buffer.concat([validatorAddress, signatureType]); const signatureHex = ethUtil.bufferToHex(signature); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + // This will return false because we signed the message with `signerAddress`, but + // are validating against `notSignerAddress` const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( orderHashHex, notSignerAddress, @@ -371,15 +374,6 @@ describe('MixinSignatureValidator', () => { signatureHex, ); expect(isValidSignature).to.be.false(); - // Set approval of signature validator back to true - await web3Wrapper.awaitTransactionSuccessAsync( - await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync( - testValidator.address, - true, - { from: signerAddress }, - ), - constants.AWAIT_TRANSACTION_MINED_MS, - ); }); it('should return true when SignatureType=Trezor and signature is valid', async () => { @@ -405,7 +399,7 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.true(); }); - it('should return true when SignatureType=Trezor and signature is invalid', async () => { + it('should return false when SignatureType=Trezor and signature is invalid', async () => { // Create Trezor signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); const orderHashWithTrezorPrefixHex = addSignedMessagePrefix(orderHashHex, MessagePrefixType.Trezor); @@ -451,7 +445,7 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.true(); }); - it('should return false when SignatureType=Presigned has not presigned hash', async () => { + it('should return false when SignatureType=Presigned and signer has not presigned hash', async () => { const signature = ethUtil.toBuffer(`0x${SignatureType.PreSigned}`); const signatureHex = ethUtil.bufferToHex(signature); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); -- cgit v1.2.3 From 322151b0d5d811bf1925f8479de3151316ce44dc Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 19 Jun 2018 18:51:02 -0700 Subject: Changed wording of two tests #nit --- packages/contracts/test/exchange/signature_validator.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index 483099c10..0d7722697 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -92,7 +92,7 @@ describe('MixinSignatureValidator', () => { signedOrder = orderFactory.newSignedOrder(); }); - it('should revert with an empty signature', async () => { + it('should revert when signature is empty', async () => { const emptySignature = '0x'; const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); return expectRevertOrOtherErrorAsync( @@ -105,7 +105,7 @@ describe('MixinSignatureValidator', () => { ); }); - it('should revert with an unsupported signature type', async () => { + it('should revert when signature type is unsupported', async () => { const unsupportedSignatureType = SignatureType.NSignatureTypes; const unsupportedSignatureHex = `0x${unsupportedSignatureType}`; const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); -- cgit v1.2.3 From 7814a391d8698f0b865d5f2675b9cc068eeab986 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 19 Jun 2018 18:55:19 -0700 Subject: Few more minor #nit wording changes --- .../src/contracts/current/test/TestValidator/TestValidator.sol | 6 +++--- .../contracts/src/contracts/current/test/TestWallet/TestWallet.sol | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol b/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol index c0daf8f38..7eba61073 100644 --- a/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol +++ b/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol @@ -28,16 +28,16 @@ contract TestValidator is address validSigner; /// @dev constructs a new `TestValidator` with a single valid signer. - /// @param _validSigner The sole signer for this wallet. + /// @param _validSigner The sole, valid signer. constructor (address _validSigner) public { validSigner = _validSigner; } - /// @dev Verifies that a signature is valid. + /// @dev Verifies that a signature is valid. `signer` must match `validSigner`. /// @param hash Message hash that is signed. /// @param signer Address that should have signed the given hash. /// @param signature Proof of signing. - /// @return Validity of order signature. + /// @return Validity of signature. function isValidSignature( bytes32 hash, address signer, diff --git a/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol b/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol index 51556971d..e7ddf1d9f 100644 --- a/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol +++ b/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol @@ -38,10 +38,10 @@ contract TestWallet is } /// @dev Validates an EIP712 signature. - /// The signer must match the signer of this wallet. + /// The signer must match the owner of this wallet. /// @param hash Message hash that is signed. /// @param eip712Signature Proof of signing. - /// @return Validity of order signature. + /// @return Validity of signature. function isValidSignature( bytes32 hash, bytes eip712Signature -- cgit v1.2.3 From 12e16d532bd1844b561a5c1d32fc1c9b1fae2efc Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Wed, 20 Jun 2018 13:50:11 -0700 Subject: Renamed constants in test wallet/validator --- .../contracts/current/test/TestValidator/TestValidator.sol | 12 ++++++------ .../src/contracts/current/test/TestWallet/TestWallet.sol | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol b/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol index 7eba61073..13953b482 100644 --- a/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol +++ b/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol @@ -25,15 +25,15 @@ contract TestValidator is { // The single valid signer for this wallet. - address validSigner; + address VALID_SIGNER; /// @dev constructs a new `TestValidator` with a single valid signer. - /// @param _validSigner The sole, valid signer. - constructor (address _validSigner) public { - validSigner = _validSigner; + /// @param validSigner The sole, valid signer. + constructor (address validSigner) public { + VALID_SIGNER = validSigner; } - /// @dev Verifies that a signature is valid. `signer` must match `validSigner`. + /// @dev Verifies that a signature is valid. `signer` must match `VALID_SIGNER`. /// @param hash Message hash that is signed. /// @param signer Address that should have signed the given hash. /// @param signature Proof of signing. @@ -47,6 +47,6 @@ contract TestValidator is view returns (bool isValid) { - return (signer == validSigner); + return (signer == VALID_SIGNER); } } diff --git a/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol b/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol index e7ddf1d9f..aca74b409 100644 --- a/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol +++ b/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol @@ -29,12 +29,12 @@ contract TestWallet is string constant LENGTH_65_REQUIRED = "LENGTH_65_REQUIRED"; // The owner of this wallet. - address walletOwner; + address WALLET_OWNER; /// @dev constructs a new `TestWallet` with a single owner. - /// @param _walletOwner The owner of this wallet. - constructor (address _walletOwner) public { - walletOwner = _walletOwner; + /// @param walletOwner The owner of this wallet. + constructor (address walletOwner) public { + WALLET_OWNER = walletOwner; } /// @dev Validates an EIP712 signature. @@ -59,7 +59,7 @@ contract TestWallet is bytes32 r = readBytes32(eip712Signature, 1); bytes32 s = readBytes32(eip712Signature, 33); address recoveredAddress = ecrecover(hash, v, r, s); - isValid = walletOwner == recoveredAddress; + isValid = WALLET_OWNER == recoveredAddress; return isValid; } } -- cgit v1.2.3 From 491a322ceb6b31d66598ef90a3ce74bf0fbb0b96 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Wed, 20 Jun 2018 13:51:39 -0700 Subject: Linter --- packages/contracts/test/exchange/signature_validator.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index 0d7722697..2a94ab25a 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -3,14 +3,13 @@ import { addSignedMessagePrefix, assetProxyUtils, MessagePrefixType, orderHashUt import { SignatureType, SignedOrder } from '@0xproject/types'; import * as chai from 'chai'; import ethUtil = require('ethereumjs-util'); -import * as _ from 'lodash'; import { TestSignatureValidatorContract } from '../../src/generated_contract_wrappers/test_signature_validator'; import { TestValidatorContract } from '../../src/generated_contract_wrappers/test_validator'; import { TestWalletContract } from '../../src/generated_contract_wrappers/test_wallet'; import { addressUtils } from '../../src/utils/address_utils'; import { artifacts } from '../../src/utils/artifacts'; -import { expectRevertOrOtherErrorAsync, expectRevertOrAlwaysFailingTransactionAsync } from '../../src/utils/assertions'; +import { expectRevertOrOtherErrorAsync } from '../../src/utils/assertions'; import { chaiSetup } from '../../src/utils/chai_setup'; import { constants } from '../../src/utils/constants'; import { OrderFactory } from '../../src/utils/order_factory'; -- cgit v1.2.3 From 8ee6e26608a832e48f235fbcb948721afcee648f Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 12 Jun 2018 13:33:06 -0700 Subject: Removed MixinSettlement. Moved `settleOrder` into `MixinExchangeCore` and `settleMatchedOrders` into `MixinMatchOrders` --- .../current/protocol/Exchange/Exchange.sol | 5 +- .../protocol/Exchange/MixinExchangeCore.sol | 52 +++++- .../current/protocol/Exchange/MixinMatchOrders.sol | 93 ++++++++++- .../current/protocol/Exchange/MixinSettlement.sol | 182 --------------------- .../protocol/Exchange/libs/LibConstants.sol | 37 +++++ .../protocol/Exchange/mixins/MExchangeCore.sol | 12 ++ .../protocol/Exchange/mixins/MMatchOrders.sol | 13 ++ .../protocol/Exchange/mixins/MSettlement.sol | 49 ------ 8 files changed, 205 insertions(+), 238 deletions(-) delete mode 100644 packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol create mode 100644 packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol delete mode 100644 packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol (limited to 'packages/contracts') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol b/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol index 51f99bafa..d36e9633e 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol @@ -19,9 +19,9 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; +import "./libs/LibConstants.sol"; import "./MixinExchangeCore.sol"; import "./MixinSignatureValidator.sol"; -import "./MixinSettlement.sol"; import "./MixinWrapperFunctions.sol"; import "./MixinAssetProxyDispatcher.sol"; import "./MixinTransactions.sol"; @@ -30,7 +30,6 @@ import "./MixinMatchOrders.sol"; contract Exchange is MixinExchangeCore, MixinMatchOrders, - MixinSettlement, MixinSignatureValidator, MixinTransactions, MixinAssetProxyDispatcher, @@ -42,9 +41,9 @@ contract Exchange is // Mixins are instantiated in the order they are inherited constructor (bytes memory _zrxAssetData) public + LibConstants(_zrxAssetData) // @TODO: Remove when we deploy. MixinExchangeCore() MixinMatchOrders() - MixinSettlement(_zrxAssetData) MixinSignatureValidator() MixinTransactions() MixinAssetProxyDispatcher() diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index c406354a7..2e6c86d48 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -19,22 +19,26 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; +import "./libs/LibConstants.sol"; +import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibFillResults.sol"; import "./libs/LibOrder.sol"; import "./libs/LibMath.sol"; import "./libs/LibExchangeErrors.sol"; import "./mixins/MExchangeCore.sol"; -import "./mixins/MSettlement.sol"; import "./mixins/MSignatureValidator.sol"; import "./mixins/MTransactions.sol"; +import "./mixins/MAssetProxyDispatcher.sol"; contract MixinExchangeCore is + LibConstants, + LibBytes, LibMath, LibOrder, LibFillResults, LibExchangeErrors, + MAssetProxyDispatcher, MExchangeCore, - MSettlement, MSignatureValidator, MTransactions { @@ -389,4 +393,48 @@ contract MixinExchangeCore is return fillResults; } + + /// @dev Settles an order by transferring assets between counterparties. + /// @param order Order struct containing order specifications. + /// @param takerAddress Address selling takerAsset and buying makerAsset. + /// @param fillResults Amounts to be filled and fees paid by maker and taker. + function settleOrder( + LibOrder.Order memory order, + address takerAddress, + LibFillResults.FillResults memory fillResults + ) + internal + { + uint8 makerAssetProxyId = uint8(popLastByte(order.makerAssetData)); + uint8 takerAssetProxyId = uint8(popLastByte(order.takerAssetData)); + bytes memory zrxAssetData = ZRX_ASSET_DATA; + dispatchTransferFrom( + order.makerAssetData, + makerAssetProxyId, + order.makerAddress, + takerAddress, + fillResults.makerAssetFilledAmount + ); + dispatchTransferFrom( + order.takerAssetData, + takerAssetProxyId, + takerAddress, + order.makerAddress, + fillResults.takerAssetFilledAmount + ); + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + order.makerAddress, + order.feeRecipientAddress, + fillResults.makerFeePaid + ); + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + takerAddress, + order.feeRecipientAddress, + fillResults.takerFeePaid + ); + } } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol index 517b743fe..945153731 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol @@ -14,21 +14,25 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; +import "./libs/LibConstants.sol"; +import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibMath.sol"; import "./libs/LibOrder.sol"; import "./libs/LibFillResults.sol"; import "./libs/LibExchangeErrors.sol"; import "./mixins/MExchangeCore.sol"; import "./mixins/MMatchOrders.sol"; -import "./mixins/MSettlement.sol"; import "./mixins/MTransactions.sol"; +import "./mixins/MAssetProxyDispatcher.sol"; contract MixinMatchOrders is + LibConstants, + LibBytes, LibMath, LibExchangeErrors, + MAssetProxyDispatcher, MExchangeCore, MMatchOrders, - MSettlement, MTransactions { @@ -224,4 +228,89 @@ contract MixinMatchOrders is // Return fill results return matchedFillResults; } + + /// @dev Settles matched order by transferring appropriate funds between order makers, taker, and fee recipient. + /// @param leftOrder First matched order. + /// @param rightOrder Second matched order. + /// @param takerAddress Address that matched the orders. The taker receives the spread between orders as profit. + /// @param matchedFillResults Struct holding amounts to transfer between makers, taker, and fee recipients. + function settleMatchedOrders( + LibOrder.Order memory leftOrder, + LibOrder.Order memory rightOrder, + address takerAddress, + LibFillResults.MatchedFillResults memory matchedFillResults + ) + internal + { + uint8 leftMakerAssetProxyId = uint8(popLastByte(leftOrder.makerAssetData)); + uint8 rightMakerAssetProxyId = uint8(popLastByte(rightOrder.makerAssetData)); + bytes memory zrxAssetData = ZRX_ASSET_DATA; + // Order makers and taker + dispatchTransferFrom( + leftOrder.makerAssetData, + leftMakerAssetProxyId, + leftOrder.makerAddress, + rightOrder.makerAddress, + matchedFillResults.right.takerAssetFilledAmount + ); + dispatchTransferFrom( + rightOrder.makerAssetData, + rightMakerAssetProxyId, + rightOrder.makerAddress, + leftOrder.makerAddress, + matchedFillResults.left.takerAssetFilledAmount + ); + dispatchTransferFrom( + leftOrder.makerAssetData, + leftMakerAssetProxyId, + leftOrder.makerAddress, + takerAddress, + matchedFillResults.leftMakerAssetSpreadAmount + ); + + // Maker fees + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + leftOrder.makerAddress, + leftOrder.feeRecipientAddress, + matchedFillResults.left.makerFeePaid + ); + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + rightOrder.makerAddress, + rightOrder.feeRecipientAddress, + matchedFillResults.right.makerFeePaid + ); + + // Taker fees + if (leftOrder.feeRecipientAddress == rightOrder.feeRecipientAddress) { + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + takerAddress, + leftOrder.feeRecipientAddress, + safeAdd( + matchedFillResults.left.takerFeePaid, + matchedFillResults.right.takerFeePaid + ) + ); + } else { + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + takerAddress, + leftOrder.feeRecipientAddress, + matchedFillResults.left.takerFeePaid + ); + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + takerAddress, + rightOrder.feeRecipientAddress, + matchedFillResults.right.takerFeePaid + ); + } + } } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol deleted file mode 100644 index 29a9c87bd..000000000 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol +++ /dev/null @@ -1,182 +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; -pragma experimental ABIEncoderV2; - -import "../../utils/LibBytes/LibBytes.sol"; -import "./libs/LibMath.sol"; -import "./libs/LibFillResults.sol"; -import "./libs/LibOrder.sol"; -import "./libs/LibExchangeErrors.sol"; -import "./mixins/MMatchOrders.sol"; -import "./mixins/MSettlement.sol"; -import "./mixins/MAssetProxyDispatcher.sol"; - -contract MixinSettlement is - LibBytes, - LibMath, - LibExchangeErrors, - MMatchOrders, - MSettlement, - MAssetProxyDispatcher -{ - // ZRX address encoded as a byte array. - // This will be constant throughout the life of the Exchange contract, - // since ZRX will always be transferred via the ERC20 AssetProxy. - bytes internal ZRX_ASSET_DATA; - uint8 constant ZRX_PROXY_ID = 1; - - /// TODO: _zrxAssetData should be a constant in production. - /// @dev Constructor sets the metadata that will be used for paying ZRX fees. - /// @param _zrxAssetData Byte array containing ERC20 proxy id concatenated with address of ZRX. - constructor (bytes memory _zrxAssetData) - public - { - ZRX_ASSET_DATA = _zrxAssetData; - } - - /// @dev Settles an order by transferring assets between counterparties. - /// @param order Order struct containing order specifications. - /// @param takerAddress Address selling takerAsset and buying makerAsset. - /// @param fillResults Amounts to be filled and fees paid by maker and taker. - function settleOrder( - LibOrder.Order memory order, - address takerAddress, - LibFillResults.FillResults memory fillResults - ) - internal - { - uint8 makerAssetProxyId = uint8(popLastByte(order.makerAssetData)); - uint8 takerAssetProxyId = uint8(popLastByte(order.takerAssetData)); - bytes memory zrxAssetData = ZRX_ASSET_DATA; - dispatchTransferFrom( - order.makerAssetData, - makerAssetProxyId, - order.makerAddress, - takerAddress, - fillResults.makerAssetFilledAmount - ); - dispatchTransferFrom( - order.takerAssetData, - takerAssetProxyId, - takerAddress, - order.makerAddress, - fillResults.takerAssetFilledAmount - ); - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - order.makerAddress, - order.feeRecipientAddress, - fillResults.makerFeePaid - ); - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - takerAddress, - order.feeRecipientAddress, - fillResults.takerFeePaid - ); - } - - /// @dev Settles matched order by transferring appropriate funds between order makers, taker, and fee recipient. - /// @param leftOrder First matched order. - /// @param rightOrder Second matched order. - /// @param takerAddress Address that matched the orders. The taker receives the spread between orders as profit. - /// @param matchedFillResults Struct holding amounts to transfer between makers, taker, and fee recipients. - function settleMatchedOrders( - LibOrder.Order memory leftOrder, - LibOrder.Order memory rightOrder, - address takerAddress, - LibFillResults.MatchedFillResults memory matchedFillResults - ) - internal - { - uint8 leftMakerAssetProxyId = uint8(popLastByte(leftOrder.makerAssetData)); - uint8 rightMakerAssetProxyId = uint8(popLastByte(rightOrder.makerAssetData)); - bytes memory zrxAssetData = ZRX_ASSET_DATA; - // Order makers and taker - dispatchTransferFrom( - leftOrder.makerAssetData, - leftMakerAssetProxyId, - leftOrder.makerAddress, - rightOrder.makerAddress, - matchedFillResults.right.takerAssetFilledAmount - ); - dispatchTransferFrom( - rightOrder.makerAssetData, - rightMakerAssetProxyId, - rightOrder.makerAddress, - leftOrder.makerAddress, - matchedFillResults.left.takerAssetFilledAmount - ); - dispatchTransferFrom( - leftOrder.makerAssetData, - leftMakerAssetProxyId, - leftOrder.makerAddress, - takerAddress, - matchedFillResults.leftMakerAssetSpreadAmount - ); - - // Maker fees - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - leftOrder.makerAddress, - leftOrder.feeRecipientAddress, - matchedFillResults.left.makerFeePaid - ); - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - rightOrder.makerAddress, - rightOrder.feeRecipientAddress, - matchedFillResults.right.makerFeePaid - ); - - // Taker fees - if (leftOrder.feeRecipientAddress == rightOrder.feeRecipientAddress) { - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - takerAddress, - leftOrder.feeRecipientAddress, - safeAdd( - matchedFillResults.left.takerFeePaid, - matchedFillResults.right.takerFeePaid - ) - ); - } else { - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - takerAddress, - leftOrder.feeRecipientAddress, - matchedFillResults.left.takerFeePaid - ); - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - takerAddress, - rightOrder.feeRecipientAddress, - matchedFillResults.right.takerFeePaid - ); - } - } -} diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol new file mode 100644 index 000000000..4a9452448 --- /dev/null +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol @@ -0,0 +1,37 @@ +/* + + 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; + +contract LibConstants { + + // Asset data for ZRX token. Used for fee transfers. + // @TODO: Hardcode constant when we deploy. Currently + // not constant to make testing easier. + bytes public ZRX_ASSET_DATA; + + // Proxy Id for ZRX token. + uint8 constant ZRX_PROXY_ID = 1; + + // @TODO: Remove when we deploy. + constructor (bytes memory zrxAssetData) + public + { + ZRX_ASSET_DATA = zrxAssetData; + } +} diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol index fb345294c..c752d928c 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol @@ -121,4 +121,16 @@ contract MExchangeCore is internal pure returns (LibFillResults.FillResults memory fillResults); + + /// @dev Settles an order by transferring assets between counterparties. + /// @param order Order struct containing order specifications. + /// @param takerAddress Address selling takerAsset and buying makerAsset. + /// @param fillResults Amounts to be filled and fees paid by maker and taker. + function settleOrder( + LibOrder.Order memory order, + address takerAddress, + LibFillResults.FillResults memory fillResults + ) + internal; + } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol index e52963a26..6b4cff972 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol @@ -54,4 +54,17 @@ contract MMatchOrders is internal pure returns (LibFillResults.MatchedFillResults memory matchedFillResults); + + /// @dev Settles matched order by transferring appropriate funds between order makers, taker, and fee recipient. + /// @param leftOrder First matched order. + /// @param rightOrder Second matched order. + /// @param takerAddress Address that matched the orders. The taker receives the spread between orders as profit. + /// @param matchedFillResults Struct holding amounts to transfer between makers, taker, and fee recipients. + function settleMatchedOrders( + LibOrder.Order memory leftOrder, + LibOrder.Order memory rightOrder, + address takerAddress, + LibFillResults.MatchedFillResults memory matchedFillResults + ) + internal; } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol deleted file mode 100644 index 2c403162f..000000000 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol +++ /dev/null @@ -1,49 +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; - -import "../libs/LibOrder.sol"; -import "../libs/LibFillResults.sol"; - -contract MSettlement { - - /// @dev Settles an order by transferring assets between counterparties. - /// @param order Order struct containing order specifications. - /// @param takerAddress Address selling takerAsset and buying makerAsset. - /// @param fillResults Amounts to be filled and fees paid by maker and taker. - function settleOrder( - LibOrder.Order memory order, - address takerAddress, - LibFillResults.FillResults memory fillResults - ) - internal; - - /// @dev Settles matched order by transferring appropriate funds between order makers, taker, and fee recipient. - /// @param leftOrder First matched order. - /// @param rightOrder Second matched order. - /// @param takerAddress Address that matched the orders. The taker receives the spread between orders as profit. - /// @param matchedFillResults Struct holding amounts to transfer between makers, taker, and fee recipients. - function settleMatchedOrders( - LibOrder.Order memory leftOrder, - LibOrder.Order memory rightOrder, - address takerAddress, - LibFillResults.MatchedFillResults memory matchedFillResults - ) - internal; -} -- cgit v1.2.3 From 6d5b16725d364a632f6838129086e915fcc0b338 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Wed, 20 Jun 2018 17:27:45 -0700 Subject: Set settleOrder and settleMatchedOrders to private --- .../current/protocol/Exchange/MixinExchangeCore.sol | 2 +- .../contracts/current/protocol/Exchange/MixinMatchOrders.sol | 2 +- .../current/protocol/Exchange/mixins/MExchangeCore.sol | 11 ----------- .../current/protocol/Exchange/mixins/MMatchOrders.sol | 12 ------------ 4 files changed, 2 insertions(+), 25 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index 2e6c86d48..8539342b7 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -403,7 +403,7 @@ contract MixinExchangeCore is address takerAddress, LibFillResults.FillResults memory fillResults ) - internal + private { uint8 makerAssetProxyId = uint8(popLastByte(order.makerAssetData)); uint8 takerAssetProxyId = uint8(popLastByte(order.takerAssetData)); diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol index 945153731..82325a29f 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol @@ -240,7 +240,7 @@ contract MixinMatchOrders is address takerAddress, LibFillResults.MatchedFillResults memory matchedFillResults ) - internal + private { uint8 leftMakerAssetProxyId = uint8(popLastByte(leftOrder.makerAssetData)); uint8 rightMakerAssetProxyId = uint8(popLastByte(rightOrder.makerAssetData)); diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol index c752d928c..1737f5baf 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol @@ -122,15 +122,4 @@ contract MExchangeCore is pure returns (LibFillResults.FillResults memory fillResults); - /// @dev Settles an order by transferring assets between counterparties. - /// @param order Order struct containing order specifications. - /// @param takerAddress Address selling takerAsset and buying makerAsset. - /// @param fillResults Amounts to be filled and fees paid by maker and taker. - function settleOrder( - LibOrder.Order memory order, - address takerAddress, - LibFillResults.FillResults memory fillResults - ) - internal; - } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol index 6b4cff972..abe7c3596 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol @@ -55,16 +55,4 @@ contract MMatchOrders is pure returns (LibFillResults.MatchedFillResults memory matchedFillResults); - /// @dev Settles matched order by transferring appropriate funds between order makers, taker, and fee recipient. - /// @param leftOrder First matched order. - /// @param rightOrder Second matched order. - /// @param takerAddress Address that matched the orders. The taker receives the spread between orders as profit. - /// @param matchedFillResults Struct holding amounts to transfer between makers, taker, and fee recipients. - function settleMatchedOrders( - LibOrder.Order memory leftOrder, - LibOrder.Order memory rightOrder, - address takerAddress, - LibFillResults.MatchedFillResults memory matchedFillResults - ) - internal; } -- cgit v1.2.3 From a3ba7683f4734d82cf8e2508cc00709a775bbc58 Mon Sep 17 00:00:00 2001 From: Austin Roberts Date: Thu, 21 Jun 2018 14:15:20 -0500 Subject: Remove unreachable PreSigned check This code was unreachable, as it had the exact same condition as line 206. --- .../contracts/current/protocol/Exchange/MixinSignatureValidator.sol | 4 ---- 1 file changed, 4 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol index 4a2beff57..5897e6416 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -232,10 +232,6 @@ contract MixinSignatureValidator is isValid = signer == recovered; return isValid; - // Signer signed hash previously using the preSign function - } else if (signatureType == SignatureType.PreSigned) { - isValid = preSigned[hash][signer]; - return isValid; } // Anything else is illegal (We do not return false because -- cgit v1.2.3 From 6a073d5f866665c43ab57d7246c7ea693264ed88 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Wed, 20 Jun 2018 14:29:30 -0700 Subject: Add senderAddress to Fill and Cancel logs, add comments to events and types --- .../protocol/Exchange/MixinExchangeCore.sol | 4 ++- .../protocol/Exchange/libs/LibFillResults.sol | 14 ++++---- .../current/protocol/Exchange/libs/LibOrder.sol | 33 +++++++++---------- .../protocol/Exchange/mixins/MExchangeCore.sol | 38 ++++++++++++---------- packages/contracts/test/exchange/core.ts | 2 ++ 5 files changed, 47 insertions(+), 44 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index 8539342b7..901e71936 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -225,8 +225,9 @@ contract MixinExchangeCore is // Log order emit Fill( order.makerAddress, - takerAddress, order.feeRecipientAddress, + takerAddress, + msg.sender, fillResults.makerAssetFilledAmount, fillResults.takerAssetFilledAmount, fillResults.makerFeePaid, @@ -255,6 +256,7 @@ contract MixinExchangeCore is emit Cancel( order.makerAddress, order.feeRecipientAddress, + msg.sender, orderHash, order.makerAssetData, order.takerAssetData diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibFillResults.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibFillResults.sol index b7550d6d2..63f1b8c87 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibFillResults.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibFillResults.sol @@ -25,16 +25,16 @@ contract LibFillResults is { struct FillResults { - uint256 makerAssetFilledAmount; - uint256 takerAssetFilledAmount; - uint256 makerFeePaid; - uint256 takerFeePaid; + uint256 makerAssetFilledAmount; // Total amount of makerAsset(s) filled. + uint256 takerAssetFilledAmount; // Total amount of takerAsset(s) filled. + uint256 makerFeePaid; // Total amount of ZRX paid by maker(s) to feeRecipient(s). + uint256 takerFeePaid; // Total amount of ZRX paid by taker to feeRecipients(s). } struct MatchedFillResults { - FillResults left; - FillResults right; - uint256 leftMakerAssetSpreadAmount; + FillResults left; // Amounts filled and fees paid of left order. + FillResults right; // Amounts filled and fees paid of right order. + uint256 leftMakerAssetSpreadAmount; // Spread between price of left and right order, denominated in the left order's makerAsset, paid to taker. } /// @dev Adds properties of both FillResults instances. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol index bfc7aaae0..f3f1e9277 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol @@ -55,27 +55,24 @@ contract LibOrder is } struct Order { - address makerAddress; - address takerAddress; - address feeRecipientAddress; - address senderAddress; - uint256 makerAssetAmount; - uint256 takerAssetAmount; - uint256 makerFee; - uint256 takerFee; - uint256 expirationTimeSeconds; - uint256 salt; - bytes makerAssetData; - bytes takerAssetData; + address makerAddress; // Address that created the order. + address takerAddress; // Address that is allowed to fill the order. If set to 0, any address is allowed to fill the order. + address feeRecipientAddress; // Address that will recieve fees when order is filled. + address senderAddress; // Address that is allowed to call Exchange contract methods that affect this order. If set to 0, any address is allowed to call these methods. + uint256 makerAssetAmount; // Amount of makerAsset being offered by maker. Must be greater than 0. + uint256 takerAssetAmount; // Amount of takerAsset being bid on by maker. Must be greater than 0. + uint256 makerFee; // Amount of ZRX paid to feeRecipient by maker when order is filled. If set to 0, no transfer of ZRX from maker to feeRecipient will be attempted. + uint256 takerFee; // Amount of ZRX paid to feeRecipient by taker when order is filled. If set to 0, no transfer of ZRX from taker to feeRecipient will be attempted. + uint256 expirationTimeSeconds; // Timestamp in seconds at which order expires. + uint256 salt; // Arbitrary number to facilitate uniqueness of the order's hash. + bytes makerAssetData; // Encoded data that can be decoded by a specified proxy contract when transferring makerAsset. The last byte references the id of this proxy. + bytes takerAssetData; // Encoded data that can be decoded by a specified proxy contract when transferring takerAsset. The last byte references the id of this proxy. } struct OrderInfo { - // See LibStatus for a complete description of order statuses - uint8 orderStatus; - // Keccak-256 EIP712 hash of the order - bytes32 orderHash; - // Amount of order that has been filled - uint256 orderTakerAssetFilledAmount; + uint8 orderStatus; // Status that describes order's validity and fillability. + bytes32 orderHash; // EIP712 hash of the order (see LibOrder.getOrderHash). + uint256 orderTakerAssetFilledAmount; // Amount of order that has already been filled. } /// @dev Calculates Keccak-256 hash of the order. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol index 1737f5baf..b0dbceff6 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol @@ -28,32 +28,34 @@ contract MExchangeCore is { // Fill event is emitted whenever an order is filled. event Fill( - address indexed makerAddress, - address takerAddress, - address indexed feeRecipientAddress, - uint256 makerAssetFilledAmount, - uint256 takerAssetFilledAmount, - uint256 makerFeePaid, - uint256 takerFeePaid, - bytes32 indexed orderHash, - bytes makerAssetData, - bytes takerAssetData + address indexed makerAddress, // Address that created the order. + address indexed feeRecipientAddress, // Address that received fees. + address takerAddress, // Address that filled the order. + address senderAddress, // Address that called the Exchange contract (msg.sender). + uint256 makerAssetFilledAmount, // Amount of makerAsset sold by maker and bought by taker. + uint256 takerAssetFilledAmount, // Amount of takerAsset sold by taker and bought by maker. + uint256 makerFeePaid, // Amount of ZRX paid to feeRecipient by maker. + uint256 takerFeePaid, // Amount of ZRX paid to feeRecipient by taker. + bytes32 indexed orderHash, // EIP712 hash of order (see LibOrder.getOrderHash) + bytes makerAssetData, // Encoded data specific to makerAsset. + bytes takerAssetData // Encoded data specific to takerAsset. ); // Cancel event is emitted whenever an individual order is cancelled. event Cancel( - address indexed makerAddress, - address indexed feeRecipientAddress, - bytes32 indexed orderHash, - bytes makerAssetData, - bytes takerAssetData + address indexed makerAddress, // Address that created the order. + address indexed feeRecipientAddress, // Address that would have recieved fees if order was filled. + address senderAddress, // Address that called the Exchange contract (msg.sender). + bytes32 indexed orderHash, // EIP712 hash of order (see LibOrder.getOrderHash) + bytes makerAssetData, // Encoded data specific to makerAsset. + bytes takerAssetData // Encoded data specific to takerAsset. ); // CancelUpTo event is emitted whenever `cancelOrdersUpTo` is executed succesfully. event CancelUpTo( - address indexed makerAddress, - address indexed senderAddress, - uint256 orderEpoch + address indexed makerAddress, // Orders cancelled must have been created by this address. + address indexed senderAddress, // Orders cancelled must have a `senderAddress` equal to this address. + uint256 orderEpoch // Orders specified makerAddress and senderAddress with a salt <= this value are considered cancelled. ); /// @dev Updates state with results of a fill order. diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index ea37a1e99..89a1c6492 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -399,6 +399,7 @@ describe('Exchange core', () => { expect(signedOrder.makerAddress).to.be.equal(logArgs.makerAddress); expect(takerAddress).to.be.equal(logArgs.takerAddress); + expect(takerAddress).to.be.equal(logArgs.senderAddress); expect(signedOrder.feeRecipientAddress).to.be.equal(logArgs.feeRecipientAddress); expect(signedOrder.makerAssetData).to.be.equal(logArgs.makerAssetData); expect(signedOrder.takerAssetData).to.be.equal(logArgs.takerAssetData); @@ -577,6 +578,7 @@ describe('Exchange core', () => { const logArgs = log.args; expect(signedOrder.makerAddress).to.be.equal(logArgs.makerAddress); + expect(signedOrder.makerAddress).to.be.equal(logArgs.senderAddress); expect(signedOrder.feeRecipientAddress).to.be.equal(logArgs.feeRecipientAddress); expect(signedOrder.makerAssetData).to.be.equal(logArgs.makerAssetData); expect(signedOrder.takerAssetData).to.be.equal(logArgs.takerAssetData); -- cgit v1.2.3 From b333ed91de8d4f08cd614a52e2b0d602995c2b27 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Thu, 21 Jun 2018 16:08:45 -0700 Subject: Add event to setSignatureValidatorApproval, rename signer => signerAddress accross all contracts --- .../protocol/Exchange/MixinExchangeCore.sol | 6 ++- .../protocol/Exchange/MixinSignatureValidator.sol | 50 ++++++++++++--------- .../protocol/Exchange/MixinTransactions.sol | 28 +++++++----- .../Exchange/interfaces/ISignatureValidator.sol | 12 ++--- .../protocol/Exchange/interfaces/ITransactions.sol | 4 +- .../protocol/Exchange/interfaces/IValidator.sol | 4 +- .../Exchange/mixins/MAssetProxyDispatcher.sol | 6 +-- .../protocol/Exchange/mixins/MExchangeCore.sol | 4 +- .../Exchange/mixins/MSignatureValidator.sol | 26 ++++++----- .../current/test/TestValidator/TestValidator.sol | 6 +-- .../contracts/current/test/Whitelist/Whitelist.sol | 6 +-- packages/contracts/src/utils/exchange_wrapper.ts | 2 +- .../contracts/src/utils/transaction_factory.ts | 6 +-- packages/contracts/src/utils/types.ts | 2 +- .../contracts/test/exchange/signature_validator.ts | 52 +++++++++++++++++++++- 15 files changed, 144 insertions(+), 70 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index 901e71936..c227d210f 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -312,7 +312,11 @@ contract MixinExchangeCore is // Validate Maker signature (check only if first time seen) if (orderInfo.orderTakerAssetFilledAmount == 0) { require( - isValidSignature(orderInfo.orderHash, order.makerAddress, signature), + isValidSignature( + orderInfo.orderHash, + order.makerAddress, + signature + ), INVALID_ORDER_SIGNATURE ); } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol index 5897e6416..881d6e7b3 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -43,43 +43,52 @@ contract MixinSignatureValidator is /// @dev Approves a hash on-chain using any valid signature type. /// After presigning a hash, the preSign signature type will become valid for that hash and signer. - /// @param signer Address that should have signed the given hash. + /// @param signerAddress Address that should have signed the given hash. /// @param signature Proof that the hash has been signed by signer. function preSign( bytes32 hash, - address signer, + address signerAddress, bytes signature ) external { require( - isValidSignature(hash, signer, signature), + isValidSignature( + hash, + signerAddress, + signature + ), INVALID_SIGNATURE ); - preSigned[hash][signer] = true; + preSigned[hash][signerAddress] = true; } /// @dev Approves/unnapproves a Validator contract to verify signatures on signer's behalf. - /// @param validator Address of Validator contract. + /// @param validatorAddress Address of Validator contract. /// @param approval Approval or disapproval of Validator contract. function setSignatureValidatorApproval( - address validator, + address validatorAddress, bool approval ) external { - address signer = getCurrentContextAddress(); - allowedValidators[signer][validator] = approval; + address signerAddress = getCurrentContextAddress(); + allowedValidators[signerAddress][validatorAddress] = approval; + emit SignatureValidatorApproval( + signerAddress, + validatorAddress, + approval + ); } /// @dev Verifies that a hash has been signed by the given signer. /// @param hash Any 32 byte hash. - /// @param signer Address that should have signed the given hash. + /// @param signerAddress Address that should have signed the given hash. /// @param signature Proof that the hash has been signed by signer. /// @return True if the address recovered from the provided signature matches the input signer address. function isValidSignature( bytes32 hash, - address signer, + address signerAddress, bytes memory signature ) public @@ -138,7 +147,7 @@ contract MixinSignatureValidator is r = readBytes32(signature, 1); s = readBytes32(signature, 33); recovered = ecrecover(hash, v, r, s); - isValid = signer == recovered; + isValid = signerAddress == recovered; return isValid; // Signed using web3.eth_sign @@ -156,7 +165,7 @@ contract MixinSignatureValidator is r, s ); - isValid = signer == recovered; + isValid = signerAddress == recovered; return isValid; // Implicitly signed by caller. @@ -172,13 +181,13 @@ contract MixinSignatureValidator is signature.length == 0, LENGTH_0_REQUIRED ); - isValid = signer == msg.sender; + isValid = signerAddress == msg.sender; return isValid; // Signature verified by wallet contract. // If used with an order, the maker of the order is the wallet contract. } else if (signatureType == SignatureType.Wallet) { - isValid = IWallet(signer).isValidSignature(hash, signature); + isValid = IWallet(signerAddress).isValidSignature(hash, signature); return isValid; // Signature verified by validator contract. @@ -190,21 +199,21 @@ contract MixinSignatureValidator is // | 0x14 + x | 1 | Signature type is always "\x06" | } else if (signatureType == SignatureType.Validator) { // Pop last 20 bytes off of signature byte array. - address validator = popLast20Bytes(signature); + address validatorAddress = popLast20Bytes(signature); // Ensure signer has approved validator. - if (!allowedValidators[signer][validator]) { + if (!allowedValidators[signerAddress][validatorAddress]) { return false; } - isValid = IValidator(validator).isValidSignature( + isValid = IValidator(validatorAddress).isValidSignature( hash, - signer, + signerAddress, signature ); return isValid; // Signer signed hash previously using the preSign function. } else if (signatureType == SignatureType.PreSigned) { - isValid = preSigned[hash][signer]; + isValid = preSigned[hash][signerAddress]; return isValid; // Signature from Trezor hardware wallet. @@ -229,9 +238,8 @@ contract MixinSignatureValidator is r, s ); - isValid = signer == recovered; + isValid = signerAddress == recovered; return isValid; - } // Anything else is illegal (We do not return false because diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol index f1332363c..d3d22d48f 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol @@ -41,17 +41,17 @@ contract MixinTransactions is bytes32 constant EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH = keccak256(abi.encodePacked( "ZeroExTransaction(", "uint256 salt,", - "address signer,", + "address signerAddress,", "bytes data", ")" )); /// @dev Calculates EIP712 hash of the Transaction. /// @param salt Arbitrary number to ensure uniqueness of transaction hash. - /// @param signer Address of transaction signer. + /// @param signerAddress Address of transaction signer. /// @param data AbiV2 encoded calldata. /// @return EIP712 hash of the Transaction. - function hashZeroExTransaction(uint256 salt, address signer, bytes data) + function hashZeroExTransaction(uint256 salt, address signerAddress, bytes data) internal pure returns (bytes32) @@ -59,19 +59,19 @@ contract MixinTransactions is return keccak256(abi.encode( EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH, salt, - signer, + signerAddress, keccak256(data) )); } /// @dev Executes an exchange method call in the context of signer. /// @param salt Arbitrary number to ensure uniqueness of transaction hash. - /// @param signer Address of transaction signer. + /// @param signerAddress Address of transaction signer. /// @param data AbiV2 encoded calldata. /// @param signature Proof of signer transaction by signer. function executeTransaction( uint256 salt, - address signer, + address signerAddress, bytes data, bytes signature ) @@ -83,7 +83,11 @@ contract MixinTransactions is REENTRANCY_ILLEGAL ); - bytes32 transactionHash = hashEIP712Message(hashZeroExTransaction(salt, signer, data)); + bytes32 transactionHash = hashEIP712Message(hashZeroExTransaction( + salt, + signerAddress, + data + )); // Validate transaction has not been executed require( @@ -92,15 +96,19 @@ contract MixinTransactions is ); // Transaction always valid if signer is sender of transaction - if (signer != msg.sender) { + if (signerAddress != msg.sender) { // Validate signature require( - isValidSignature(transactionHash, signer, signature), + isValidSignature( + transactionHash, + signerAddress, + signature + ), INVALID_TX_SIGNATURE ); // Set the current transaction signer - currentContextAddress = signer; + currentContextAddress = signerAddress; } // Execute transaction diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol index 02aa9776e..511463309 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol @@ -22,32 +22,32 @@ contract ISignatureValidator { /// @dev Approves a hash on-chain using any valid signature type. /// After presigning a hash, the preSign signature type will become valid for that hash and signer. - /// @param signer Address that should have signed the given hash. + /// @param signerAddress Address that should have signed the given hash. /// @param signature Proof that the hash has been signed by signer. function preSign( bytes32 hash, - address signer, + address signerAddress, bytes signature ) external; /// @dev Approves/unnapproves a Validator contract to verify signatures on signer's behalf. - /// @param validator Address of Validator contract. + /// @param validatorAddress Address of Validator contract. /// @param approval Approval or disapproval of Validator contract. function setSignatureValidatorApproval( - address validator, + address validatorAddress, bool approval ) external; /// @dev Verifies that a signature is valid. /// @param hash Message hash that is signed. - /// @param signer Address of signer. + /// @param signerAddress Address of signer. /// @param signature Proof of signing. /// @return Validity of order signature. function isValidSignature( bytes32 hash, - address signer, + address signerAddress, bytes memory signature ) public diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol index 2f9a5bc7c..a7cab8f55 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol @@ -21,12 +21,12 @@ contract ITransactions { /// @dev Executes an exchange method call in the context of signer. /// @param salt Arbitrary number to ensure uniqueness of transaction hash. - /// @param signer Address of transaction signer. + /// @param signerAddress Address of transaction signer. /// @param data AbiV2 encoded calldata. /// @param signature Proof of signer transaction by signer. function executeTransaction( uint256 salt, - address signer, + address signerAddress, bytes data, bytes signature ) diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol index 3e5ccc190..0b1796a66 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol @@ -22,12 +22,12 @@ contract IValidator { /// @dev Verifies that a signature is valid. /// @param hash Message hash that is signed. - /// @param signer Address that should have signed the given hash. + /// @param signerAddress Address that should have signed the given hash. /// @param signature Proof of signing. /// @return Validity of order signature. function isValidSignature( bytes32 hash, - address signer, + address signerAddress, bytes signature ) external diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol index d16a830f4..788f42c60 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol @@ -27,9 +27,9 @@ contract MAssetProxyDispatcher is // Logs registration of new asset proxy event AssetProxySet( - uint8 id, - address newAssetProxy, - address oldAssetProxy + uint8 id, // Id of new registered AssetProxy. + address newAssetProxy, // Address of new registered AssetProxy. + address oldAssetProxy // Address of AssetProxy that was overwritten at given id (or null address). ); /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol index b0dbceff6..6e406e1c4 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol @@ -36,7 +36,7 @@ contract MExchangeCore is uint256 takerAssetFilledAmount, // Amount of takerAsset sold by taker and bought by maker. uint256 makerFeePaid, // Amount of ZRX paid to feeRecipient by maker. uint256 takerFeePaid, // Amount of ZRX paid to feeRecipient by taker. - bytes32 indexed orderHash, // EIP712 hash of order (see LibOrder.getOrderHash) + bytes32 indexed orderHash, // EIP712 hash of order (see LibOrder.getOrderHash). bytes makerAssetData, // Encoded data specific to makerAsset. bytes takerAssetData // Encoded data specific to takerAsset. ); @@ -46,7 +46,7 @@ contract MExchangeCore is address indexed makerAddress, // Address that created the order. address indexed feeRecipientAddress, // Address that would have recieved fees if order was filled. address senderAddress, // Address that called the Exchange contract (msg.sender). - bytes32 indexed orderHash, // EIP712 hash of order (see LibOrder.getOrderHash) + bytes32 indexed orderHash, // EIP712 hash of order (see LibOrder.getOrderHash). bytes makerAssetData, // Encoded data specific to makerAsset. bytes takerAssetData // Encoded data specific to takerAsset. ); diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol index 9c6fbe22b..6cc1d7a10 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol @@ -23,17 +23,23 @@ import "../interfaces/ISignatureValidator.sol"; contract MSignatureValidator is ISignatureValidator { + event SignatureValidatorApproval( + address indexed signerAddress, // Address that approves or disapproves a contract to verify signatures. + address indexed validatorAddress, // Address of signature validator contract. + bool approved // Approval or disapproval of validator contract. + ); + // Allowed signature types. enum SignatureType { - Illegal, // 0x00, default value - Invalid, // 0x01 - EIP712, // 0x02 - EthSign, // 0x03 - Caller, // 0x04 - Wallet, // 0x05 - Validator, // 0x06 - PreSigned, // 0x07 - Trezor, // 0x08 - NSignatureTypes // 0x09, number of signature types. Always leave at end. + Illegal, // 0x00, default value + Invalid, // 0x01 + EIP712, // 0x02 + EthSign, // 0x03 + Caller, // 0x04 + Wallet, // 0x05 + Validator, // 0x06 + PreSigned, // 0x07 + Trezor, // 0x08 + NSignatureTypes // 0x09, number of signature types. Always leave at end. } } diff --git a/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol b/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol index 13953b482..f9271bf7a 100644 --- a/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol +++ b/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol @@ -35,18 +35,18 @@ contract TestValidator is /// @dev Verifies that a signature is valid. `signer` must match `VALID_SIGNER`. /// @param hash Message hash that is signed. - /// @param signer Address that should have signed the given hash. + /// @param signerAddress Address that should have signed the given hash. /// @param signature Proof of signing. /// @return Validity of signature. function isValidSignature( bytes32 hash, - address signer, + address signerAddress, bytes signature ) external view returns (bool isValid) { - return (signer == VALID_SIGNER); + return (signerAddress == VALID_SIGNER); } } diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol index 460c9ea42..d35815474 100644 --- a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol +++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol @@ -116,18 +116,18 @@ contract Whitelist is /// @dev Verifies signer is same as signer of current Ethereum transaction. /// NOTE: This function can currently be used to validate signatures coming from outside of this contract. /// Extra safety checks can be added for a production contract. - /// @param signer Address that should have signed the given hash. + /// @param signerAddress Address that should have signed the given hash. /// @param signature Proof of signing. /// @return Validity of order signature. function isValidSignature( bytes32 hash, - address signer, + address signerAddress, bytes signature ) external view returns (bool isValid) { - return signer == tx.origin; + return signerAddress == tx.origin; } } diff --git a/packages/contracts/src/utils/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts index 4cc8f0b89..06eb70644 100644 --- a/packages/contracts/src/utils/exchange_wrapper.ts +++ b/packages/contracts/src/utils/exchange_wrapper.ts @@ -215,7 +215,7 @@ export class ExchangeWrapper { ): Promise { const txHash = await this._exchange.executeTransaction.sendTransactionAsync( signedTx.salt, - signedTx.signer, + signedTx.signerAddress, signedTx.data, signedTx.signature, { from }, diff --git a/packages/contracts/src/utils/transaction_factory.ts b/packages/contracts/src/utils/transaction_factory.ts index 19ef4e1bf..348c0715d 100644 --- a/packages/contracts/src/utils/transaction_factory.ts +++ b/packages/contracts/src/utils/transaction_factory.ts @@ -9,7 +9,7 @@ const EIP712_ZEROEX_TRANSACTION_SCHEMA: EIP712Schema = { name: 'ZeroExTransaction', parameters: [ { name: 'salt', type: EIP712Types.Uint256 }, - { name: 'signer', type: EIP712Types.Address }, + { name: 'signerAddress', type: EIP712Types.Address }, { name: 'data', type: EIP712Types.Bytes }, ], }; @@ -25,10 +25,10 @@ export class TransactionFactory { } public newSignedTransaction(data: string, signatureType: SignatureType = SignatureType.EthSign): SignedTransaction { const salt = generatePseudoRandomSalt(); - const signer = `0x${this._signerBuff.toString('hex')}`; + const signerAddress = `0x${this._signerBuff.toString('hex')}`; const executeTransactionData = { salt, - signer, + signerAddress, data, }; const executeTransactionHashBuff = EIP712Utils.structHash( diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index a6d0c2a88..7a1f92afd 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -108,7 +108,7 @@ export enum ContractName { export interface SignedTransaction { exchangeAddress: string; salt: BigNumber; - signer: string; + signerAddress: string; data: string; signature: string; } diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index 2a94ab25a..8e221e3f1 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -2,9 +2,13 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { addSignedMessagePrefix, assetProxyUtils, MessagePrefixType, orderHashUtils } from '@0xproject/order-utils'; import { SignatureType, SignedOrder } from '@0xproject/types'; import * as chai from 'chai'; +import { LogWithDecodedArgs } from 'ethereum-types'; import ethUtil = require('ethereumjs-util'); -import { TestSignatureValidatorContract } from '../../src/generated_contract_wrappers/test_signature_validator'; +import { + SignatureValidatorApprovalContractEventArgs, + TestSignatureValidatorContract, +} from '../../src/generated_contract_wrappers/test_signature_validator'; import { TestValidatorContract } from '../../src/generated_contract_wrappers/test_validator'; import { TestWalletContract } from '../../src/generated_contract_wrappers/test_wallet'; import { addressUtils } from '../../src/utils/address_utils'; @@ -12,6 +16,7 @@ import { artifacts } from '../../src/utils/artifacts'; import { expectRevertOrOtherErrorAsync } from '../../src/utils/assertions'; import { chaiSetup } from '../../src/utils/chai_setup'; import { constants } from '../../src/utils/constants'; +import { LogDecoder } from '../../src/utils/log_decoder'; import { OrderFactory } from '../../src/utils/order_factory'; import { provider, txDefaults, web3Wrapper } from '../../src/utils/web3_wrapper'; @@ -19,7 +24,7 @@ chaiSetup.configure(); const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); - +// tslint:disable:no-unnecessary-type-assertion describe('MixinSignatureValidator', () => { let signedOrder: SignedOrder; let orderFactory: OrderFactory; @@ -30,6 +35,7 @@ describe('MixinSignatureValidator', () => { let signerPrivateKey: Buffer; let notSignerAddress: string; let notSignerPrivateKey: Buffer; + let signatureValidatorLogDecoder: LogDecoder; before(async () => { await blockchainLifecycle.startAsync(); @@ -59,6 +65,7 @@ describe('MixinSignatureValidator', () => { txDefaults, signerAddress, ); + signatureValidatorLogDecoder = new LogDecoder(web3Wrapper, signatureValidator.address); await web3Wrapper.awaitTransactionSuccessAsync( await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync(testValidator.address, true, { from: signerAddress, @@ -456,4 +463,45 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.false(); }); }); + + describe('setSignatureValidatorApproval', () => { + it('should emit a SignatureValidatorApprovalSet with correct args when a validator is approved', async () => { + const approval = true; + const res = await signatureValidatorLogDecoder.getTxWithDecodedLogsAsync( + await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync( + testValidator.address, + approval, + { + from: signerAddress, + }, + ), + ); + expect(res.logs.length).to.equal(1); + const log = res.logs[0] as LogWithDecodedArgs; + const logArgs = log.args; + expect(logArgs.signerAddress).to.equal(signerAddress); + expect(logArgs.validatorAddress).to.equal(testValidator.address); + expect(logArgs.approved).to.equal(approval); + }); + it('should emit a SignatureValidatorApprovalSet with correct args when a validator is disapproved', async () => { + const approval = false; + const res = await signatureValidatorLogDecoder.getTxWithDecodedLogsAsync( + await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync( + testValidator.address, + approval, + { + from: signerAddress, + }, + ), + ); + expect(res.logs.length).to.equal(1); + const log = res.logs[0] as LogWithDecodedArgs; + const logArgs = log.args; + expect(logArgs.signerAddress).to.equal(signerAddress); + expect(logArgs.validatorAddress).to.equal(testValidator.address); + expect(logArgs.approved).to.equal(approval); + }); + }); }); +// tslint:disable:max-file-line-count +// tslint:enable:no-unnecessary-type-assertion -- cgit v1.2.3 From c131d8269997bf5342107110e2abb1da1c3c28e6 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 21 Jun 2018 17:58:39 -0700 Subject: Updated compiler runs to be 1,000,000 --- packages/contracts/compiler.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/contracts') diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json index fa7ede817..38580f4dc 100644 --- a/packages/contracts/compiler.json +++ b/packages/contracts/compiler.json @@ -4,7 +4,7 @@ "compilerSettings": { "optimizer": { "enabled": true, - "runs": 200 + "runs": 1000000 }, "outputSelection": { "*": { -- cgit v1.2.3 From 4012e31115dd0571d742b42a48094f41fd53a460 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Thu, 21 Jun 2018 19:17:33 -0700 Subject: Use make-promises-safe as a preloader instead of manually importing --- packages/contracts/package.json | 2 +- packages/contracts/test/asset_proxy/authorizable.ts | 1 - packages/contracts/test/asset_proxy_owner.ts | 1 - packages/contracts/test/ether_token.ts | 1 - packages/contracts/test/exchange/core.ts | 1 - packages/contracts/test/exchange/wrapper.ts | 1 - packages/contracts/test/multi_sig_with_time_lock.ts | 1 - packages/contracts/test/token_registry.ts | 1 - packages/contracts/test/tutorials/arbitrage.ts | 1 - packages/contracts/test/unlimited_allowance_token.ts | 1 - packages/contracts/test/zrx_token.ts | 1 - 11 files changed, 1 insertion(+), 11 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/package.json b/packages/contracts/package.json index 9e7dad674..3bb66067d 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -20,7 +20,7 @@ "test:coverage": "SOLIDITY_COVERAGE=true run-s build run_mocha coverage:report:text coverage:report:lcov", "test:profiler": "SOLIDITY_PROFILER=true run-s build run_mocha profiler:report:html", "test:trace": "SOLIDITY_REVERT_TRACE=true run-s build run_mocha", - "run_mocha": "mocha --require source-map-support/register 'lib/test/**/*.js' --timeout 100000 --bail --exit", + "run_mocha": "mocha --require source-map-support/register --require make-promises-safe 'lib/test/**/*.js' --timeout 100000 --bail --exit", "compile": "sol-compiler", "clean": "shx rm -rf lib src/generated_contract_wrappers", "generate_contract_wrappers": diff --git a/packages/contracts/test/asset_proxy/authorizable.ts b/packages/contracts/test/asset_proxy/authorizable.ts index 347d060d6..ed582b63e 100644 --- a/packages/contracts/test/asset_proxy/authorizable.ts +++ b/packages/contracts/test/asset_proxy/authorizable.ts @@ -1,6 +1,5 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import * as chai from 'chai'; -import 'make-promises-safe'; import { MixinAuthorizableContract } from '../../src/generated_contract_wrappers/mixin_authorizable'; import { artifacts } from '../../src/utils/artifacts'; diff --git a/packages/contracts/test/asset_proxy_owner.ts b/packages/contracts/test/asset_proxy_owner.ts index cbe429b12..aa4999e95 100644 --- a/packages/contracts/test/asset_proxy_owner.ts +++ b/packages/contracts/test/asset_proxy_owner.ts @@ -2,7 +2,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; import { LogWithDecodedArgs } from 'ethereum-types'; -import 'make-promises-safe'; import { AssetProxyOwnerContract, diff --git a/packages/contracts/test/ether_token.ts b/packages/contracts/test/ether_token.ts index e2fc69aee..01093d309 100644 --- a/packages/contracts/test/ether_token.ts +++ b/packages/contracts/test/ether_token.ts @@ -2,7 +2,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { BigNumber } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; import * as chai from 'chai'; -import 'make-promises-safe'; import { WETH9Contract } from '../src/generated_contract_wrappers/weth9'; import { artifacts } from '../src/utils/artifacts'; diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index 89a1c6492..ff652d3aa 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -6,7 +6,6 @@ import { Web3Wrapper } from '@0xproject/web3-wrapper'; import * as chai from 'chai'; import { LogWithDecodedArgs } from 'ethereum-types'; import ethUtil = require('ethereumjs-util'); -import 'make-promises-safe'; import { DummyERC20TokenContract } from '../../src/generated_contract_wrappers/dummy_e_r_c20_token'; import { DummyERC721TokenContract } from '../../src/generated_contract_wrappers/dummy_e_r_c721_token'; diff --git a/packages/contracts/test/exchange/wrapper.ts b/packages/contracts/test/exchange/wrapper.ts index abba1ac4f..703f644b8 100644 --- a/packages/contracts/test/exchange/wrapper.ts +++ b/packages/contracts/test/exchange/wrapper.ts @@ -5,7 +5,6 @@ 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 { DummyERC20TokenContract } from '../../src/generated_contract_wrappers/dummy_e_r_c20_token'; import { DummyERC721TokenContract } from '../../src/generated_contract_wrappers/dummy_e_r_c721_token'; diff --git a/packages/contracts/test/multi_sig_with_time_lock.ts b/packages/contracts/test/multi_sig_with_time_lock.ts index f630743e1..aa82b9edf 100644 --- a/packages/contracts/test/multi_sig_with_time_lock.ts +++ b/packages/contracts/test/multi_sig_with_time_lock.ts @@ -2,7 +2,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; import { LogWithDecodedArgs } from 'ethereum-types'; -import 'make-promises-safe'; import { MultiSigWalletWithTimeLockContract, diff --git a/packages/contracts/test/token_registry.ts b/packages/contracts/test/token_registry.ts index f368fd73d..095cecfce 100644 --- a/packages/contracts/test/token_registry.ts +++ b/packages/contracts/test/token_registry.ts @@ -3,7 +3,6 @@ import { BigNumber, NULL_BYTES } from '@0xproject/utils'; import * as chai from 'chai'; import ethUtil = require('ethereumjs-util'); import * as _ from 'lodash'; -import 'make-promises-safe'; import { TokenRegistryContract } from '../src/generated_contract_wrappers/token_registry'; import { artifacts } from '../src/utils/artifacts'; diff --git a/packages/contracts/test/tutorials/arbitrage.ts b/packages/contracts/test/tutorials/arbitrage.ts index 32fcedb43..6851483cc 100644 --- a/packages/contracts/test/tutorials/arbitrage.ts +++ b/packages/contracts/test/tutorials/arbitrage.ts @@ -4,7 +4,6 @@ // import { BigNumber } from '@0xproject/utils'; // import { Web3Wrapper } from '@0xproject/web3-wrapper'; // import * as chai from 'chai'; -// import 'make-promises-safe'; // import ethUtil = require('ethereumjs-util'); // import * as Web3 from 'web3'; diff --git a/packages/contracts/test/unlimited_allowance_token.ts b/packages/contracts/test/unlimited_allowance_token.ts index 0c3f5094b..7132c57bf 100644 --- a/packages/contracts/test/unlimited_allowance_token.ts +++ b/packages/contracts/test/unlimited_allowance_token.ts @@ -1,7 +1,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; -import 'make-promises-safe'; import { DummyERC20TokenContract } from '../src/generated_contract_wrappers/dummy_e_r_c20_token'; import { artifacts } from '../src/utils/artifacts'; diff --git a/packages/contracts/test/zrx_token.ts b/packages/contracts/test/zrx_token.ts index 0629c7a88..01ae57d4a 100644 --- a/packages/contracts/test/zrx_token.ts +++ b/packages/contracts/test/zrx_token.ts @@ -2,7 +2,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { BigNumber } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; import * as chai from 'chai'; -import 'make-promises-safe'; import { ZRXTokenContract } from '../src/generated_contract_wrappers/zrx_token'; import { artifacts } from '../src/utils/artifacts'; -- cgit v1.2.3