From d6ba03916a7d4971ef721f5e09f96364f141c862 Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Wed, 26 Jun 2019 16:10:31 -0400 Subject: [PATCH] `@0x/contracts-exchange`: Update tests for new/consolidated signature types. `@0x/contracts-exchange`: Update `Whitelist` example for new signature types. --- contracts/exchange/CHANGELOG.json | 28 +- .../exchange/contracts/examples/Whitelist.sol | 22 +- .../contracts/test/TestValidatorWallet.sol | 4 +- contracts/exchange/test/core.ts | 27 +- .../exchange/test/signature_validator.ts | 893 +++++++++--------- contracts/exchange/test/transactions.ts | 29 +- 6 files changed, 494 insertions(+), 509 deletions(-) diff --git a/contracts/exchange/CHANGELOG.json b/contracts/exchange/CHANGELOG.json index 20ad46df27..3043249ae5 100644 --- a/contracts/exchange/CHANGELOG.json +++ b/contracts/exchange/CHANGELOG.json @@ -79,15 +79,23 @@ "pr": 1868 }, { - "note": "Always check `OrderValidator` and `WalletOrderValidator` signature types on every fill", + "note": "Add `EIP1271Wallet` signature type", "pr": 1885 }, { - "note": "Rename `WalletOrderValidator` to `OrderWallet` signature type", + "note": "Remove `WalletOrderValidator` and `OrderValidator` signature types", "pr": 1885 }, { - "note": "Rename `SignatureWalletOrderValidatorError` to `SignatureOrderWalletError`", + "note": "Make the regular `Validator` signature type have EIP1271 behavior", + "pr": 1885 + }, + { + "note": "Always check signature types that are validated via contract (not just on first fill).", + "pr": 1885 + }, + { + "note": "Remove unecessary rich revert error types.", "pr": 1885 }, { @@ -95,19 +103,7 @@ "pr": 1885 }, { - "note": "Add `EIP1271Wallet` and `EIP1271OrderWallet` to `SignatureType`", - "pr": 1885 - }, - { - "note": "Always check `OrderValidator`, `OrderWallet`, `EIP1271OrderWallet` signature types on every fill", - "pr": 1885 - }, - { - "note": "Add `validatorAddress` field to `SignatureValidatorError` and `SignatureOrderValidatorError` rich reverts", - "pr": 1885 - }, - { - "note": "Add separate `SignatureOrderValidatorNotApprovedError` for `OrderValidator` signatures", + "note": "Add `validatorAddress` field to `SignatureValidatorError` rich reverts", "pr": 1885 } ] diff --git a/contracts/exchange/contracts/examples/Whitelist.sol b/contracts/exchange/contracts/examples/Whitelist.sol index 4cdc389c95..af2bdae981 100644 --- a/contracts/exchange/contracts/examples/Whitelist.sol +++ b/contracts/exchange/contracts/examples/Whitelist.sol @@ -23,10 +23,12 @@ import "../src/interfaces/IExchange.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibZeroExTransaction.sol"; import "@0x/contracts-utils/contracts/src/Ownable.sol"; +import "@0x/contracts-utils/contracts/src/LibEIP1271.sol"; contract Whitelist is - Ownable + Ownable, + LibEIP1271 { // Mapping of address => whitelist status. mapping (address => bool) public isWhitelisted; @@ -59,24 +61,28 @@ contract Whitelist is isWhitelisted[target] = isApproved; } - /// @dev Verifies signer is same as signer of current Ethereum transaction. + /// @dev Verifies a ZeroExTransaction's 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 signerAddress Address that should have signed the given hash. + /// @param data The abi-encoded ZeroExTransaction. /// @param signature Proof of signing. - /// @return Validity of order signature. + /// @return magicValue `EIP1271_MAGIC_VALUE` if the signature is authorized. // solhint-disable no-unused-vars function isValidSignature( - bytes32 hash, - address signerAddress, + bytes calldata data, bytes calldata signature ) external view - returns (bool isValid) + returns (bytes4 magicValue) { + // Decode the ZeroExTransaction. + LibZeroExTransaction.ZeroExTransaction memory transaction = + abi.decode(data, (LibZeroExTransaction.ZeroExTransaction)); // solhint-disable-next-line avoid-tx-origin - return signerAddress == tx.origin; + if (transaction.signerAddress == tx.origin) { + magicValue = EIP1271_MAGIC_VALUE; + } } // solhint-enable no-unused-vars diff --git a/contracts/exchange/contracts/test/TestValidatorWallet.sol b/contracts/exchange/contracts/test/TestValidatorWallet.sol index ad830c0b14..7da9523ec7 100644 --- a/contracts/exchange/contracts/test/TestValidatorWallet.sol +++ b/contracts/exchange/contracts/test/TestValidatorWallet.sol @@ -196,10 +196,10 @@ contract TestValidatorWallet is view returns (bytes32 hash) { - // HACK(dorothy-zbornak): First we want the hash, which is the second + // First we want the hash, which is the second // encoded parameter. We will initially treat all fields as inline // `bytes32`s and ignore the first one to extract it. - (,hash) = abi.decode(data, (bytes32, bytes32)); + (, hash) = abi.decode(data, (bytes32, bytes32)); // Now we can figure out what the data type is from a previous call to // `prepare()`. DataType dataType = _hashDataTypes[hash]; diff --git a/contracts/exchange/test/core.ts b/contracts/exchange/test/core.ts index 4e0eb9c429..11ed9a76f8 100644 --- a/contracts/exchange/test/core.ts +++ b/contracts/exchange/test/core.ts @@ -22,6 +22,7 @@ import { constants, ERC20BalancesByOwner, getLatestBlockTimestampAsync, + hexConcat, increaseTimeAndMineBlockAsync, OrderFactory, OrderStatus, @@ -36,7 +37,6 @@ import { BigNumber, providerUtils, StringRevertError } from '@0x/utils'; import { Web3Wrapper } from '@0x/web3-wrapper'; import * as chai from 'chai'; import { LogWithDecodedArgs } from 'ethereum-types'; -import ethUtil = require('ethereumjs-util'); import * as _ from 'lodash'; import { Erc1155Wrapper } from '../../erc1155/lib/src'; @@ -267,9 +267,13 @@ describe('Exchange core', () => { constants.INITIAL_ERC20_BALANCE, ); // Approve the validator. - await exchange.setSignatureValidatorApproval.awaitTransactionSuccessAsync(validatorWallet.address, true, { - from: makerAddress, - }); + await exchange.setSignatureValidatorApproval.awaitTransactionSuccessAsync( + validatorWallet.address, + true, + { + from: makerAddress, + }, + ); signedOrder = await orderFactory.newSignedOrderAsync({ makerFee: constants.ZERO_AMOUNT, takerFee: constants.ZERO_AMOUNT, @@ -277,11 +281,8 @@ describe('Exchange core', () => { }); it('should revert if `Validator` signature type rejects during a second fill', async () => { - const signature = Buffer.concat([ - ethUtil.toBuffer(validatorWallet.address), - ethUtil.toBuffer([SignatureType.Validator]), - ]); - signedOrder.signature = ethUtil.bufferToHex(signature); + const signatureHex = hexConcat(validatorWallet.address, SignatureType.Validator); + signedOrder.signature = signatureHex; const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); // Allow the signature check for the first fill. await validatorWallet.prepare.awaitTransactionSuccessAsync( @@ -312,9 +313,9 @@ describe('Exchange core', () => { }); it('should revert if `Wallet` signature type rejects during a second fill', async () => { - const signature = Buffer.concat([ethUtil.toBuffer([SignatureType.Wallet])]); + const signatureHex = hexConcat(SignatureType.Wallet); signedOrder.makerAddress = validatorWallet.address; - signedOrder.signature = ethUtil.bufferToHex(signature); + signedOrder.signature = signatureHex; const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); // Allow the signature check for the first fill. await validatorWallet.prepare.awaitTransactionSuccessAsync( @@ -345,9 +346,9 @@ describe('Exchange core', () => { }); it('should revert if `EIP1271Wallet` signature type rejects during a second fill', async () => { - const signature = Buffer.concat([ethUtil.toBuffer([SignatureType.EIP1271Wallet])]); + const signatureHex = hexConcat(SignatureType.EIP1271Wallet); signedOrder.makerAddress = validatorWallet.address; - signedOrder.signature = ethUtil.bufferToHex(signature); + signedOrder.signature = signatureHex; const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); // Allow the signature check for the first fill. await validatorWallet.prepare.awaitTransactionSuccessAsync( diff --git a/contracts/exchange/test/signature_validator.ts b/contracts/exchange/test/signature_validator.ts index 6123ce871a..6bc75c06c0 100644 --- a/contracts/exchange/test/signature_validator.ts +++ b/contracts/exchange/test/signature_validator.ts @@ -2,15 +2,24 @@ import { addressUtils, chaiSetup, constants, + hexConcat, LogDecoder, OrderFactory, + orderUtils, provider, + TransactionFactory, txDefaults, web3Wrapper, } from '@0x/contracts-test-utils'; import { BlockchainLifecycle } from '@0x/dev-utils'; -import { assetDataUtils, ExchangeRevertErrors, orderHashUtils, signatureUtils } from '@0x/order-utils'; -import { SignatureType, SignedOrder } from '@0x/types'; +import { + assetDataUtils, + ExchangeRevertErrors, + orderHashUtils, + signatureUtils, + transactionHashUtils, +} from '@0x/order-utils'; +import { SignatureType, SignedOrder, SignedZeroExTransaction } from '@0x/types'; import { BigNumber, providerUtils, StringRevertError } from '@0x/utils'; import * as chai from 'chai'; import * as crypto from 'crypto'; @@ -28,21 +37,16 @@ import { ValidatorWalletAction, ValidatorWalletDataType } from './utils'; chaiSetup.configure(); const expect = chai.expect; - const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); // tslint:disable:no-unnecessary-type-assertion -describe.only('MixinSignatureValidator', () => { - const SIGNATURE_LENGTH = 65; +describe('MixinSignatureValidator', () => { let chainId: number; - let signedOrder: SignedOrder; - let orderFactory: OrderFactory; let signatureValidator: TestSignatureValidatorContract; let validatorWallet: TestValidatorWalletContract; let validatorWalletRevertReason: string; let signerAddress: string; let signerPrivateKey: Buffer; let notSignerAddress: string; - let signatureValidatorLogDecoder: LogDecoder; before(async () => { await blockchainLifecycle.startAsync(); @@ -53,8 +57,7 @@ describe.only('MixinSignatureValidator', () => { before(async () => { chainId = await providerUtils.getChainIdAsync(provider); const accounts = await web3Wrapper.getAvailableAddressesAsync(); - const makerAddress = accounts[0]; - signerAddress = makerAddress; + signerAddress = accounts[0]; notSignerAddress = accounts[1]; signatureValidator = await TestSignatureValidatorContract.deployFrom0xArtifactAsync( artifacts.TestSignatureValidator, @@ -81,25 +84,7 @@ describe.only('MixinSignatureValidator', () => { }), ); - const defaultOrderParams = { - ...constants.STATIC_ORDER_PARAMS, - makerAddress, - feeRecipientAddress: addressUtils.generatePseudoRandomAddress(), - makerAssetData: assetDataUtils.encodeERC20AssetData(addressUtils.generatePseudoRandomAddress()), - takerAssetData: assetDataUtils.encodeERC20AssetData(addressUtils.generatePseudoRandomAddress()), - makerFeeAssetData: assetDataUtils.encodeERC20AssetData(addressUtils.generatePseudoRandomAddress()), - takerFeeAssetData: assetDataUtils.encodeERC20AssetData(addressUtils.generatePseudoRandomAddress()), - makerFee: constants.ZERO_AMOUNT, - takerFee: constants.ZERO_AMOUNT, - domain: { - verifyingContractAddress: signatureValidator.address, - chainId, - }, - }; - - signatureValidatorLogDecoder = new LogDecoder(web3Wrapper, artifacts); - signerPrivateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddress)]; - orderFactory = new OrderFactory(signerPrivateKey, defaultOrderParams); + signerPrivateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(signerAddress)]; }); beforeEach(async () => { @@ -109,418 +94,252 @@ describe.only('MixinSignatureValidator', () => { await blockchainLifecycle.revertAsync(); }); - type ValidateCallAsync = ( - order: SignedOrder, + const SIGNATURE_LENGTH = 65; + const generateRandomBytes = (count: number): string => ethUtil.bufferToHex(crypto.randomBytes(count)); + const generateRandomSignature = (): string => generateRandomBytes(SIGNATURE_LENGTH); + const hashBytes = (bytesHex: string): string => ethUtil.bufferToHex(ethUtil.sha3(ethUtil.toBuffer(bytesHex))); + const signDataHex = (dataHex: string, privateKey: Buffer): string => { + const ecSignature = ethUtil.ecsign(ethUtil.toBuffer(dataHex), signerPrivateKey); + return hexConcat(ecSignature.v, ecSignature.r, ecSignature.s); + }; + + type ValidateHashSignatureAsync = ( + hashHex: string, signerAddress: string, signatureHex: string, validatorAction?: ValidatorWalletAction, + validatorExpectedSignatureHex?: string, ) => Promise; - const createHashSignatureTests = (validateCallAsync: ValidateCallAsync) => { + const createHashSignatureTests = ( + getCurrentHashHex: (signerAddress?: string) => string, + validateAsync: ValidateHashSignatureAsync, + ) => { it('should revert when signature is empty', async () => { + const hashHex = getCurrentHashHex(); const emptySignature = constants.NULL_BYTES; - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); const expectedError = new ExchangeRevertErrors.SignatureError( ExchangeRevertErrors.SignatureErrorCode.InvalidLength, - orderHashHex, - signedOrder.makerAddress, + hashHex, + signerAddress, emptySignature, ); - const tx = validateCallAsync(signedOrder, signedOrder.makerAddress, emptySignature); + const tx = validateAsync(hashHex, signerAddress, emptySignature); return expect(tx).to.revertWith(expectedError); }); it('should revert when signature type is unsupported', async () => { - const unsupportedSignatureType = SignatureType.NSignatureTypes; - const unsupportedSignatureHex = `0x${Buffer.from([unsupportedSignatureType]).toString('hex')}`; - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const hashHex = getCurrentHashHex(); + const signatureHex = hexConcat(SignatureType.NSignatureTypes); const expectedError = new ExchangeRevertErrors.SignatureError( ExchangeRevertErrors.SignatureErrorCode.Unsupported, - orderHashHex, - signedOrder.makerAddress, - unsupportedSignatureHex, + hashHex, + signerAddress, + signatureHex, ); - const tx = validateCallAsync(signedOrder, signedOrder.makerAddress, unsupportedSignatureHex); + const tx = validateAsync(hashHex, signerAddress, signatureHex); return expect(tx).to.revertWith(expectedError); }); it('should revert when SignatureType=Illegal', async () => { - const illegalSignatureHex = `0x${Buffer.from([SignatureType.Illegal]).toString('hex')}`; - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const hashHex = getCurrentHashHex(); + const signatureHex = hexConcat(SignatureType.Illegal); const expectedError = new ExchangeRevertErrors.SignatureError( ExchangeRevertErrors.SignatureErrorCode.Illegal, - orderHashHex, - signedOrder.makerAddress, - illegalSignatureHex, + hashHex, + signerAddress, + signatureHex, ); - const tx = validateCallAsync(signedOrder, signedOrder.makerAddress, illegalSignatureHex); + const tx = validateAsync(hashHex, signerAddress, signatureHex); return expect(tx).to.revertWith(expectedError); }); it('should return false when SignatureType=Invalid and signature has a length of zero', async () => { - const signatureHex = `0x${Buffer.from([SignatureType.Invalid]).toString('hex')}`; - const isValidSignature = await validateCallAsync(signedOrder, signedOrder.makerAddress, signatureHex); + const hashHex = getCurrentHashHex(); + const signatureHex = hexConcat(SignatureType.Invalid); + const isValidSignature = await validateAsync(hashHex, signerAddress, 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); + const hashHex = getCurrentHashHex(); + const signatureHex = hexConcat('0xdeadbeef', SignatureType.Invalid); const expectedError = new ExchangeRevertErrors.SignatureError( ExchangeRevertErrors.SignatureErrorCode.InvalidLength, - orderHashHex, - signedOrder.makerAddress, + hashHex, + signerAddress, signatureHex, ); - const tx = validateCallAsync(signedOrder, signedOrder.makerAddress, signatureHex); + const tx = validateAsync(hashHex, signerAddress, signatureHex); return expect(tx).to.revertWith(expectedError); }); it('should return true when SignatureType=EIP712 and signature is valid', async () => { - // Validate signature - const isValidSignature = await validateCallAsync(signedOrder, signerAddress, signedOrder.signature); + const hashHex = getCurrentHashHex(); + const signatureHex = hexConcat(signDataHex(hashHex, signerPrivateKey), SignatureType.EIP712); + const isValidSignature = await validateAsync(hashHex, signerAddress, signatureHex); expect(isValidSignature).to.be.true(); }); it('should return false when SignatureType=EIP712 and signature is invalid', async () => { - const signature = Buffer.concat([ - crypto.randomBytes(SIGNATURE_LENGTH), - ethUtil.toBuffer([SignatureType.EIP712]), - ]); - const signatureHex = ethUtil.bufferToHex(signature); - // Validate signature. - // This will fail because the signature is random, not signed by `signerAddress`. - const isValidSignature = await validateCallAsync(signedOrder, notSignerAddress, signatureHex); + const hashHex = getCurrentHashHex(); + const signatureHex = hexConcat(generateRandomSignature(), SignatureType.EIP712); + const isValidSignature = await validateAsync(hashHex, signerAddress, 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 = signatureUtils.addSignedMessagePrefix(orderHashHex); - 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([SignatureType.EthSign]), - ]); - const signatureHex = ethUtil.bufferToHex(signature); - // Validate signature - const isValidSignature = await validateCallAsync(signedOrder, signerAddress, signatureHex); + const hashHex = getCurrentHashHex(); + const orderHashWithEthSignPrefixHex = signatureUtils.addSignedMessagePrefix(hashHex); + const signatureHex = hexConcat( + signDataHex(orderHashWithEthSignPrefixHex, signerPrivateKey), + SignatureType.EthSign, + ); + const isValidSignature = await validateAsync(hashHex, signerAddress, signatureHex); expect(isValidSignature).to.be.true(); }); it('should return false when SignatureType=EthSign and signature is invalid', async () => { + const hashHex = getCurrentHashHex(); // Create EthSign signature - const signature = Buffer.concat([ - crypto.randomBytes(SIGNATURE_LENGTH), - ethUtil.toBuffer([SignatureType.EthSign]), - ]); - const signatureHex = ethUtil.bufferToHex(signature); - // Validate signature. - // This will fail because the signature is random, not signed by `signerAddress`. - const isValidSignature = await validateCallAsync(signedOrder, signerAddress, signatureHex); + const signatureHex = hexConcat(generateRandomSignature(), SignatureType.EthSign); + const isValidSignature = await validateAsync(hashHex, signerAddress, signatureHex); expect(isValidSignature).to.be.false(); }); it('should return true when SignatureType=Wallet and signature is valid', async () => { - signedOrder.makerAddress = validatorWallet.address; - const signature = Buffer.concat([ethUtil.toBuffer([SignatureType.Wallet])]); - const signatureHex = ethUtil.bufferToHex(signature); - // Validate signature - const isValidSignature = await validateCallAsync( - signedOrder, + const hashHex = getCurrentHashHex(validatorWallet.address); + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureDataHex = generateRandomSignature(); + const signatureHex = hexConcat(signatureDataHex, SignatureType.Wallet); + const isValidSignature = await validateAsync( + hashHex, validatorWallet.address, signatureHex, ValidatorWalletAction.MatchSignatureHash, + signatureDataHex, ); expect(isValidSignature).to.be.true(); }); it('should return false when SignatureType=Wallet and signature is invalid', async () => { - signedOrder.makerAddress = validatorWallet.address; - const signature = Buffer.concat([ethUtil.toBuffer([SignatureType.Wallet])]); - const signatureHex = ethUtil.bufferToHex(signature); + const hashHex = getCurrentHashHex(validatorWallet.address); + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureDataHex = generateRandomSignature(); + const notSignatureDataHex = generateRandomSignature(); + const signatureHex = hexConcat(notSignatureDataHex, SignatureType.Wallet); // Validate signature - const isValidSignature = await validateCallAsync( - signedOrder, - notSignerAddress, + const isValidSignature = await validateAsync( + hashHex, + validatorWallet.address, signatureHex, ValidatorWalletAction.MatchSignatureHash, + signatureDataHex, ); expect(isValidSignature).to.be.false(); }); it('should revert when validator attempts to update state and SignatureType=Wallet', async () => { - signedOrder.makerAddress = validatorWallet.address; - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const signature = Buffer.concat([ethUtil.toBuffer([SignatureType.Wallet])]); - const signatureHex = ethUtil.bufferToHex(signature); + const hashHex = getCurrentHashHex(validatorWallet.address); + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureHex = hexConcat(generateRandomSignature(), SignatureType.Wallet); const expectedError = new ExchangeRevertErrors.SignatureWalletError( - orderHashHex, + hashHex, validatorWallet.address, signatureHex, constants.NULL_BYTES, ); - const tx = validateCallAsync( - signedOrder, - validatorWallet.address, - signatureHex, - ValidatorWalletAction.UpdateState, - ); + const tx = validateAsync(hashHex, validatorWallet.address, signatureHex, ValidatorWalletAction.UpdateState); return expect(tx).to.revertWith(expectedError); }); it('should revert when validator reverts and SignatureType=Wallet', async () => { - signedOrder.makerAddress = validatorWallet.address; - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const signature = Buffer.concat([ethUtil.toBuffer([SignatureType.Wallet])]); - const signatureHex = ethUtil.bufferToHex(signature); + const hashHex = getCurrentHashHex(validatorWallet.address); + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureHex = hexConcat(generateRandomSignature(), SignatureType.Wallet); const expectedError = new ExchangeRevertErrors.SignatureWalletError( - orderHashHex, + hashHex, validatorWallet.address, signatureHex, new StringRevertError(validatorWalletRevertReason).encode(), ); - const tx = validateCallAsync( - signedOrder, - validatorWallet.address, - signatureHex, - ValidatorWalletAction.Revert, - ); - return expect(tx).to.revertWith(expectedError); - }); - - it('should return true when SignatureType=Validator, signature is valid and validator is approved', async () => { - const signature = Buffer.concat([ - ethUtil.toBuffer(signedOrder.signature).slice(0, SIGNATURE_LENGTH), - ethUtil.toBuffer(validatorWallet.address), - ethUtil.toBuffer([SignatureType.Validator]), - ]); - const signatureHex = ethUtil.bufferToHex(signature); - const isValidSignature = await validateCallAsync( - signedOrder, - signerAddress, - signatureHex, - ValidatorWalletAction.MatchSignatureHash, - ); - expect(isValidSignature).to.be.true(); - }); - - it('should return false when SignatureType=Validator, signature is invalid and validator is approved', async () => { - const signature = Buffer.concat([ - crypto.randomBytes(SIGNATURE_LENGTH), - ethUtil.toBuffer(validatorWallet.address), - ethUtil.toBuffer([SignatureType.Validator]), - ]); - const isValidSignature = await validateCallAsync( - signedOrder, - signerAddress, - ethUtil.bufferToHex(signature), - ValidatorWalletAction.MatchSignatureHash, - ); - expect(isValidSignature).to.be.false(); - }); - - it('should revert when validator attempts to update state and SignatureType=Validator', async () => { - const signature = Buffer.concat([ - ethUtil.toBuffer(signedOrder.signature).slice(0, SIGNATURE_LENGTH), - ethUtil.toBuffer(validatorWallet.address), - ethUtil.toBuffer([SignatureType.Validator]), - ]); - const signatureHex = ethUtil.bufferToHex(signature); - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const expectedError = new ExchangeRevertErrors.SignatureValidatorError( - orderHashHex, - signerAddress, - validatorWallet.address, - signatureHex, - constants.NULL_BYTES, - ); - const tx = validateCallAsync(signedOrder, signerAddress, signatureHex, ValidatorWalletAction.UpdateState); - return expect(tx).to.revertWith(expectedError); - }); - - it('should revert when validator reverts and SignatureType=Validator', async () => { - const signature = Buffer.concat([ - ethUtil.toBuffer(signedOrder.signature).slice(0, SIGNATURE_LENGTH), - ethUtil.toBuffer(validatorWallet.address), - ethUtil.toBuffer([SignatureType.Validator]), - ]); - const signatureHex = ethUtil.bufferToHex(signature); - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const expectedError = new ExchangeRevertErrors.SignatureValidatorError( - orderHashHex, - signerAddress, - validatorWallet.address, - signatureHex, - new StringRevertError(validatorWalletRevertReason).encode(), - ); - const tx = validateCallAsync(signedOrder, signerAddress, signatureHex, ValidatorWalletAction.Revert); - return expect(tx).to.revertWith(expectedError); - }); - - it('should revert when SignatureType=Validator, signature is valid and validator is not approved', async () => { - // Set approval of signature validator to false - await signatureValidator.setSignatureValidatorApproval.awaitTransactionSuccessAsync( - validatorWallet.address, - false, - { from: signerAddress }, - ); - const signature = Buffer.concat([ - ethUtil.toBuffer(signedOrder.signature).slice(0, SIGNATURE_LENGTH), - ethUtil.toBuffer(validatorWallet.address), - ethUtil.toBuffer([SignatureType.Validator]), - ]); - // Validate signature - const signatureHex = ethUtil.bufferToHex(signature); - const tx = validateCallAsync( - signedOrder, - signerAddress, - signatureHex, - ValidatorWalletAction.MatchSignatureHash, - ); - const expectedError = new ExchangeRevertErrors.SignatureValidatorNotApprovedError( - signerAddress, - validatorWallet.address, - ); - return expect(tx).to.revertWith(expectedError); - }); - - it('should return true when SignatureType=EIP1271Wallet and signature is valid', async () => { - signedOrder.makerAddress = validatorWallet.address; - const signature = Buffer.concat([ethUtil.toBuffer([SignatureType.EIP1271Wallet])]); - const signatureHex = ethUtil.bufferToHex(signature); - // Validate signature - const isValidSignature = await validateCallAsync( - signedOrder, - validatorWallet.address, - signatureHex, - ValidatorWalletAction.MatchSignatureHash, - ); - expect(isValidSignature).to.be.true(); - }); - - it('should return false when SignatureType=EIP1271Wallet and signature is invalid', async () => { - signedOrder.makerAddress = validatorWallet.address; - const signature = Buffer.concat([ethUtil.toBuffer([SignatureType.EIP1271Wallet])]); - const signatureHex = ethUtil.bufferToHex(signature); - // Validate signature - const isValidSignature = await validateCallAsync( - signedOrder, - validatorWallet.address, - signatureHex, - ValidatorWalletAction.Reject, - ); - expect(isValidSignature).to.be.false(); - }); - - it('should revert when validator attempts to update state and SignatureType=EIP1271Wallet', async () => { - signedOrder.makerAddress = validatorWallet.address; - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const signature = Buffer.concat([ethUtil.toBuffer([SignatureType.EIP1271Wallet])]); - const signatureHex = ethUtil.bufferToHex(signature); - const expectedError = new ExchangeRevertErrors.SignatureWalletError( - orderHashHex, - validatorWallet.address, - signatureHex, - constants.NULL_BYTES, - ); - const tx = validateCallAsync( - signedOrder, - validatorWallet.address, - signatureHex, - ValidatorWalletAction.UpdateState, - ); - return expect(tx).to.revertWith(expectedError); - }); - - it('should revert when validator reverts and SignatureType=EIP1271Wallet', async () => { - signedOrder.makerAddress = validatorWallet.address; - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const signature = Buffer.concat([ethUtil.toBuffer([SignatureType.EIP1271Wallet])]); - const signatureHex = ethUtil.bufferToHex(signature); - const expectedError = new ExchangeRevertErrors.SignatureWalletError( - orderHashHex, - validatorWallet.address, - signatureHex, - new StringRevertError(validatorWalletRevertReason).encode(), - ); - const tx = validateCallAsync( - signedOrder, - validatorWallet.address, - signatureHex, - ValidatorWalletAction.Revert, - ); + const tx = validateAsync(hashHex, validatorWallet.address, signatureHex, ValidatorWalletAction.Revert); return expect(tx).to.revertWith(expectedError); }); it('should return true when SignatureType=Presigned and signer has presigned hash', async () => { - // Presign hash - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - await signatureValidator.preSign.awaitTransactionSuccessAsync(orderHashHex, { - from: signedOrder.makerAddress, - }); + const hashHex = getCurrentHashHex(); + // Presign the hash + await signatureValidator.preSign.awaitTransactionSuccessAsync(hashHex, { from: signerAddress }); // Validate presigned signature - const signature = ethUtil.toBuffer(`0x${SignatureType.PreSigned}`); - const signatureHex = ethUtil.bufferToHex(signature); - const isValidSignature = await validateCallAsync(signedOrder, signedOrder.makerAddress, signatureHex); + const signatureHex = hexConcat(SignatureType.PreSigned); + const isValidSignature = await validateAsync(hashHex, signerAddress, signatureHex); expect(isValidSignature).to.be.true(); }); 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 isValidSignature = await validateCallAsync(signedOrder, signedOrder.makerAddress, signatureHex); + const hashHex = getCurrentHashHex(); + const signatureHex = hexConcat(SignatureType.PreSigned); + const isValidSignature = await validateAsync(hashHex, signerAddress, signatureHex); expect(isValidSignature).to.be.false(); }); }; describe('isValidHashSignature', () => { - const validateCallAsync = async ( - order: SignedOrder, - expectedSignatureHex: string, - signatureHex: string, - validatorAction?: ValidatorWalletAction, - ) => { - const orderHashHex = orderHashUtils.getOrderHashHex(order); - const signatureHashHex = ethUtil.bufferToHex(ethUtil.sha3(expectedSignatureHex)); - if (validatorAction !== undefined) { - await validatorWallet.prepare.awaitTransactionSuccessAsync( - orderHashHex, - ValidatorWalletDataType.None, - validatorAction, - signatureHashHex, - ); - } - return signatureValidator.isValidHashSignature.callAsync(orderHashHex, order.makerAddress, signatureHex); - }; + let hashHex: string; beforeEach(async () => { - signedOrder = await orderFactory.newSignedOrderAsync(); + hashHex = orderUtils.generatePseudoRandomOrderHash(); }); + const validateAsync = async ( + _hashHex: string, + _signerAddress: string, + signatureHex: string, + validatorAction?: ValidatorWalletAction, + validatorExpectedSignatureHex?: string, + ) => { + const expectedSignatureHashHex = + validatorExpectedSignatureHex === undefined + ? constants.NULL_BYTES + : hashBytes(validatorExpectedSignatureHex); + if (validatorAction !== undefined) { + await validatorWallet.prepare.awaitTransactionSuccessAsync( + _hashHex, + ValidatorWalletDataType.None, + validatorAction, + expectedSignatureHashHex, + ); + } + return signatureValidator.isValidHashSignature.callAsync(_hashHex, _signerAddress, signatureHex); + }; + it('should revert when SignatureType=Validator', async () => { - const inappropriateSignatureHex = `0x${Buffer.from([SignatureType.Validator]).toString('hex')}`; - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const signatureHex = hexConcat(SignatureType.Validator); const expectedError = new ExchangeRevertErrors.SignatureError( ExchangeRevertErrors.SignatureErrorCode.InappropriateSignatureType, - orderHashHex, - signedOrder.makerAddress, - inappropriateSignatureHex, - ); - const tx = validateCallAsync( - signedOrder, + hashHex, signerAddress, - inappropriateSignatureHex, - ValidatorWalletAction.Accept, + signatureHex, ); + const tx = validateAsync(hashHex, signerAddress, signatureHex); + return expect(tx).to.revertWith(expectedError); + }); + + it('should revert when SignatureType=EIP1271Wallet', async () => { + const signatureHex = hexConcat(SignatureType.EIP1271Wallet); + const expectedError = new ExchangeRevertErrors.SignatureError( + ExchangeRevertErrors.SignatureErrorCode.InappropriateSignatureType, + hashHex, + signerAddress, + signatureHex, + ); + const tx = validateAsync(hashHex, signerAddress, signatureHex); return expect(tx).to.revertWith(expectedError); }); @@ -560,72 +379,93 @@ describe.only('MixinSignatureValidator', () => { expect(isValidSignature).to.be.true(); }); - createHashSignatureTests(validateCallAsync); + createHashSignatureTests((_signerAddress?: string) => hashHex, validateAsync); }); describe('isValidOrderSignature', () => { - const validateCallAsync = async ( - order: SignedOrder, - expectedSignatureHex: string, - signatureHex: string, - validatorAction?: ValidatorWalletAction, - ) => { - const orderHashHex = orderHashUtils.getOrderHashHex(order); - const signatureHashHex = ethUtil.bufferToHex(ethUtil.sha3(expectedSignatureHex)); - if (validatorAction !== undefined) { - await validatorWallet.prepare.awaitTransactionSuccessAsync( - orderHashHex, - ValidatorWalletDataType.Order, - validatorAction, - signatureHashHex, - ); - } - return signatureValidator.isValidOrderSignature.callAsync(order, order.makerAddress, signatureHex); - }; + let orderFactory: OrderFactory; + let signedOrder: SignedOrder; + + before(async () => { + const makerAddress = signerAddress; + const defaultOrderParams = { + ...constants.STATIC_ORDER_PARAMS, + makerAddress, + feeRecipientAddress: addressUtils.generatePseudoRandomAddress(), + makerAssetData: assetDataUtils.encodeERC20AssetData(addressUtils.generatePseudoRandomAddress()), + takerAssetData: assetDataUtils.encodeERC20AssetData(addressUtils.generatePseudoRandomAddress()), + makerFeeAssetData: assetDataUtils.encodeERC20AssetData(addressUtils.generatePseudoRandomAddress()), + takerFeeAssetData: assetDataUtils.encodeERC20AssetData(addressUtils.generatePseudoRandomAddress()), + makerFee: constants.ZERO_AMOUNT, + takerFee: constants.ZERO_AMOUNT, + domain: { + verifyingContractAddress: signatureValidator.address, + chainId, + }, + }; + orderFactory = new OrderFactory(signerPrivateKey, defaultOrderParams); + }); beforeEach(async () => { signedOrder = await orderFactory.newSignedOrderAsync(); }); + const validateAsync = async ( + order: SignedOrder, + signatureHex: string, + validatorAction?: ValidatorWalletAction, + validatorExpectedSignatureHex?: string, + ) => { + const orderHashHex = orderHashUtils.getOrderHashHex(order); + const expectedSignatureHashHex = + validatorExpectedSignatureHex === undefined + ? constants.NULL_BYTES + : hashBytes(validatorExpectedSignatureHex); + if (validatorAction !== undefined) { + await validatorWallet.prepare.awaitTransactionSuccessAsync( + orderHashHex, + ValidatorWalletDataType.Order, + validatorAction, + expectedSignatureHashHex, + ); + } + return signatureValidator.isValidOrderSignature.callAsync(order, order.makerAddress, signatureHex); + }; + it('should return true when SignatureType=Validator, signature is valid and validator is approved', async () => { - const signature = Buffer.concat([ - ethUtil.toBuffer(signedOrder.signature).slice(0, SIGNATURE_LENGTH), - ethUtil.toBuffer(validatorWallet.address), - ethUtil.toBuffer([SignatureType.Validator]), - ]); - const signatureHex = ethUtil.bufferToHex(signature); - const isValidSignature = await validateCallAsync( + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureDataHex = generateRandomSignature(); + const signatureHex = hexConcat(signatureDataHex, validatorWallet.address, SignatureType.Validator); + const isValidSignature = await validateAsync( signedOrder, - signerAddress, signatureHex, ValidatorWalletAction.MatchSignatureHash, + signatureDataHex, ); expect(isValidSignature).to.be.true(); }); it('should return false when SignatureType=Validator, signature is invalid and validator is approved', async () => { - const signature = Buffer.concat([ - crypto.randomBytes(SIGNATURE_LENGTH), - ethUtil.toBuffer(validatorWallet.address), - ethUtil.toBuffer([SignatureType.Validator]), - ]); - const signatureHex = ethUtil.bufferToHex(signature); - const isValidSignature = await validateCallAsync( + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureDataHex = generateRandomSignature(); + const notSignatureDataHex = generateRandomSignature(); + const signatureHex = hexConcat(notSignatureDataHex, validatorWallet.address, SignatureType.Validator); + const isValidSignature = await validateAsync( signedOrder, - signerAddress, signatureHex, ValidatorWalletAction.MatchSignatureHash, + signatureDataHex, ); expect(isValidSignature).to.be.false(); }); - it('should revert when `isValidOrderSignature` attempts to update state and SignatureType=Validator', async () => { - const signature = Buffer.concat([ - ethUtil.toBuffer(signedOrder.signature).slice(0, SIGNATURE_LENGTH), - ethUtil.toBuffer(validatorWallet.address), - ethUtil.toBuffer([SignatureType.Validator]), - ]); - const signatureHex = ethUtil.bufferToHex(signature); + it('should revert when validator attempts to update state and SignatureType=Validator', async () => { + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureDataHex = generateRandomSignature(); + const signatureHex = hexConcat(signatureDataHex, validatorWallet.address, SignatureType.Validator); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); const expectedError = new ExchangeRevertErrors.SignatureValidatorError( orderHashHex, @@ -634,26 +474,24 @@ describe.only('MixinSignatureValidator', () => { signatureHex, constants.NULL_BYTES, ); - const tx = validateCallAsync(signedOrder, signerAddress, signatureHex, ValidatorWalletAction.UpdateState); + const tx = validateAsync(signedOrder, signatureHex, ValidatorWalletAction.UpdateState); return expect(tx).to.revertWith(expectedError); }); - it('should revert when `isValidOrderSignature` reverts and SignatureType=Validator', async () => { - const signature = Buffer.concat([ - ethUtil.toBuffer(signedOrder.signature).slice(0, SIGNATURE_LENGTH), - ethUtil.toBuffer(validatorWallet.address), - ethUtil.toBuffer([SignatureType.Validator]), - ]); - const signatureHex = ethUtil.bufferToHex(signature); + it('should revert when validator reverts and SignatureType=Validator', async () => { + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureDataHex = generateRandomSignature(); + const signatureHex = hexConcat(signatureDataHex, validatorWallet.address, SignatureType.Validator); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); const expectedError = new ExchangeRevertErrors.SignatureValidatorError( orderHashHex, - signerAddress, + signedOrder.makerAddress, validatorWallet.address, signatureHex, new StringRevertError(validatorWalletRevertReason).encode(), ); - const tx = validateCallAsync(signedOrder, signerAddress, signatureHex, ValidatorWalletAction.Revert); + const tx = validateAsync(signedOrder, signatureHex, ValidatorWalletAction.Revert); return expect(tx).to.revertWith(expectedError); }); @@ -662,167 +500,316 @@ describe.only('MixinSignatureValidator', () => { await signatureValidator.setSignatureValidatorApproval.awaitTransactionSuccessAsync( validatorWallet.address, false, - { from: signerAddress }, - ); - const signature = Buffer.concat([ - ethUtil.toBuffer(signedOrder.signature).slice(0, SIGNATURE_LENGTH), - ethUtil.toBuffer(validatorWallet.address), - ethUtil.toBuffer([SignatureType.Validator]), - ]); - const signatureHex = ethUtil.bufferToHex(signature); - const tx = validateCallAsync( - signedOrder, - signerAddress, - signatureHex, - ValidatorWalletAction.MatchSignatureHash, + { from: signedOrder.makerAddress }, ); + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureDataHex = generateRandomSignature(); + const signatureHex = hexConcat(signatureDataHex, validatorWallet.address, SignatureType.Validator); const expectedError = new ExchangeRevertErrors.SignatureValidatorNotApprovedError( - signerAddress, + signedOrder.makerAddress, validatorWallet.address, ); + const tx = validateAsync(signedOrder, signatureHex, ValidatorWalletAction.Revert); return expect(tx).to.revertWith(expectedError); }); it('should return true when SignatureType=EIP1271Wallet and signature is valid', async () => { signedOrder.makerAddress = validatorWallet.address; - const signature = Buffer.concat([ethUtil.toBuffer([SignatureType.EIP1271Wallet])]); - const signatureHex = ethUtil.bufferToHex(signature); + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureDataHex = generateRandomSignature(); + const signatureHex = hexConcat(signatureDataHex, SignatureType.EIP1271Wallet); // Validate signature - const isValidSignature = await validateCallAsync( + const isValidSignature = await validateAsync( signedOrder, - validatorWallet.address, signatureHex, ValidatorWalletAction.MatchSignatureHash, + signatureDataHex, ); expect(isValidSignature).to.be.true(); }); it('should return false when SignatureType=EIP1271Wallet and signature is invalid', async () => { signedOrder.makerAddress = validatorWallet.address; - const signature = Buffer.concat([ethUtil.toBuffer([SignatureType.EIP1271Wallet])]); - const signatureHex = ethUtil.bufferToHex(signature); + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureDataHex = generateRandomSignature(); + const notSignatureDataHex = generateRandomSignature(); + const signatureHex = hexConcat(notSignatureDataHex, SignatureType.EIP1271Wallet); // Validate signature - const isValidSignature = await validateCallAsync( + const isValidSignature = await validateAsync( signedOrder, - validatorWallet.address, signatureHex, - ValidatorWalletAction.Reject, + ValidatorWalletAction.MatchSignatureHash, + signatureDataHex, ); expect(isValidSignature).to.be.false(); }); it('should revert when validator attempts to update state and SignatureType=EIP1271Wallet', async () => { signedOrder.makerAddress = validatorWallet.address; + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureHex = hexConcat(generateRandomSignature(), SignatureType.EIP1271Wallet); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const signature = Buffer.concat([ethUtil.toBuffer([SignatureType.EIP1271Wallet])]); - const signatureHex = ethUtil.bufferToHex(signature); const expectedError = new ExchangeRevertErrors.SignatureWalletError( orderHashHex, validatorWallet.address, signatureHex, constants.NULL_BYTES, ); - const tx = validateCallAsync( - signedOrder, - validatorWallet.address, - signatureHex, - ValidatorWalletAction.UpdateState, - ); + const tx = validateAsync(signedOrder, signatureHex, ValidatorWalletAction.UpdateState); return expect(tx).to.revertWith(expectedError); }); it('should revert when validator reverts and SignatureType=EIP1271Wallet', async () => { signedOrder.makerAddress = validatorWallet.address; + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureHex = hexConcat(generateRandomSignature(), SignatureType.EIP1271Wallet); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const signature = Buffer.concat([ethUtil.toBuffer([SignatureType.EIP1271Wallet])]); - const signatureHex = ethUtil.bufferToHex(signature); const expectedError = new ExchangeRevertErrors.SignatureWalletError( orderHashHex, validatorWallet.address, signatureHex, new StringRevertError(validatorWalletRevertReason).encode(), ); - const tx = validateCallAsync( - signedOrder, + const tx = validateAsync(signedOrder, signatureHex, ValidatorWalletAction.Revert); + return expect(tx).to.revertWith(expectedError); + }); + + // Run hash-only signature type tests as well. + const validateOrderHashAsync = async ( + hashHex: string, + _signerAddress: string, + signatureHex: string, + validatorAction?: ValidatorWalletAction, + validatorExpectedSignatureHex?: string, + ): Promise => { + signedOrder.makerAddress = _signerAddress; + return validateAsync(signedOrder, signatureHex, validatorAction, validatorExpectedSignatureHex); + }; + createHashSignatureTests((_signerAddress?: string) => { + signedOrder.makerAddress = _signerAddress === undefined ? signerAddress : _signerAddress; + return orderHashUtils.getOrderHashHex(signedOrder); + }, validateOrderHashAsync); + }); + + describe('isValidTransactionSignature', () => { + let transactionFactory: TransactionFactory; + let signedTransaction: SignedZeroExTransaction; + const TRANSACTION_DATA_LENGTH = 100; + + before(async () => { + transactionFactory = new TransactionFactory(signerPrivateKey, signatureValidator.address, chainId); + }); + + beforeEach(async () => { + // We don't actually do anything with the transaction so we can just + // fill it with random data. + signedTransaction = await transactionFactory.newSignedTransactionAsync({ + data: generateRandomBytes(TRANSACTION_DATA_LENGTH), + }); + }); + + const validateAsync = async ( + transaction: SignedZeroExTransaction, + signatureHex: string, + validatorAction?: ValidatorWalletAction, + validatorExpectedSignatureHex?: string, + ) => { + const transactionHashHex = transactionHashUtils.getTransactionHashHex(transaction); + const expectedSignatureHashHex = + validatorExpectedSignatureHex === undefined + ? constants.NULL_BYTES + : hashBytes(validatorExpectedSignatureHex); + if (validatorAction !== undefined) { + await validatorWallet.prepare.awaitTransactionSuccessAsync( + transactionHashHex, + ValidatorWalletDataType.ZeroExTransaction, + validatorAction, + expectedSignatureHashHex, + ); + } + return signatureValidator.isValidTransactionSignature.callAsync( + transaction, + transaction.signerAddress, + signatureHex, + ); + }; + + it('should return true when SignatureType=Validator, signature is valid and validator is approved', async () => { + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureDataHex = generateRandomSignature(); + const signatureHex = hexConcat(signatureDataHex, validatorWallet.address, SignatureType.Validator); + const isValidSignature = await validateAsync( + signedTransaction, + signatureHex, + ValidatorWalletAction.MatchSignatureHash, + signatureDataHex, + ); + expect(isValidSignature).to.be.true(); + }); + + it('should return false when SignatureType=Validator, signature is invalid and validator is approved', async () => { + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureDataHex = generateRandomSignature(); + const notSignatureDataHex = generateRandomSignature(); + const signatureHex = hexConcat(notSignatureDataHex, validatorWallet.address, SignatureType.Validator); + const isValidSignature = await validateAsync( + signedTransaction, + signatureHex, + ValidatorWalletAction.MatchSignatureHash, + signatureDataHex, + ); + expect(isValidSignature).to.be.false(); + }); + + it('should revert when validator attempts to update state and SignatureType=Validator', async () => { + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureDataHex = generateRandomSignature(); + const signatureHex = hexConcat(signatureDataHex, validatorWallet.address, SignatureType.Validator); + const transactionHashHex = transactionHashUtils.getTransactionHashHex(signedTransaction); + const expectedError = new ExchangeRevertErrors.SignatureValidatorError( + transactionHashHex, + signedTransaction.signerAddress, validatorWallet.address, signatureHex, - ValidatorWalletAction.Revert, + constants.NULL_BYTES, ); + const tx = validateAsync(signedTransaction, signatureHex, ValidatorWalletAction.UpdateState); + return expect(tx).to.revertWith(expectedError); + }); + + it('should revert when validator reverts and SignatureType=Validator', async () => { + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureDataHex = generateRandomSignature(); + const signatureHex = hexConcat(signatureDataHex, validatorWallet.address, SignatureType.Validator); + const transactionHashHex = transactionHashUtils.getTransactionHashHex(signedTransaction); + const expectedError = new ExchangeRevertErrors.SignatureValidatorError( + transactionHashHex, + signedTransaction.signerAddress, + validatorWallet.address, + signatureHex, + new StringRevertError(validatorWalletRevertReason).encode(), + ); + const tx = validateAsync(signedTransaction, signatureHex, ValidatorWalletAction.Revert); + return expect(tx).to.revertWith(expectedError); + }); + + it('should throw when SignatureType=Validator, signature is valid and validator is not approved', async () => { + // Set approval of signature validator to false + await signatureValidator.setSignatureValidatorApproval.awaitTransactionSuccessAsync( + validatorWallet.address, + false, + { from: signedTransaction.signerAddress }, + ); + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureDataHex = generateRandomSignature(); + const signatureHex = hexConcat(signatureDataHex, validatorWallet.address, SignatureType.Validator); + const expectedError = new ExchangeRevertErrors.SignatureValidatorNotApprovedError( + signedTransaction.signerAddress, + validatorWallet.address, + ); + const tx = validateAsync(signedTransaction, signatureHex, ValidatorWalletAction.Revert); return expect(tx).to.revertWith(expectedError); }); it('should return true when SignatureType=EIP1271Wallet and signature is valid', async () => { - signedOrder.makerAddress = validatorWallet.address; - const signature = Buffer.concat([ethUtil.toBuffer([SignatureType.EIP1271Wallet])]); - const signatureHex = ethUtil.bufferToHex(signature); + signedTransaction.signerAddress = validatorWallet.address; + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureDataHex = generateRandomSignature(); + const signatureHex = hexConcat(signatureDataHex, SignatureType.EIP1271Wallet); // Validate signature - const isValidSignature = await validateCallAsync( - signedOrder, - validatorWallet.address, + const isValidSignature = await validateAsync( + signedTransaction, signatureHex, ValidatorWalletAction.MatchSignatureHash, + signatureDataHex, ); expect(isValidSignature).to.be.true(); }); it('should return false when SignatureType=EIP1271Wallet and signature is invalid', async () => { - signedOrder.makerAddress = validatorWallet.address; - const signature = Buffer.concat([ethUtil.toBuffer([SignatureType.EIP1271Wallet])]); - const signatureHex = ethUtil.bufferToHex(signature); + signedTransaction.signerAddress = validatorWallet.address; + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureDataHex = generateRandomSignature(); + const notSignatureDataHex = generateRandomSignature(); + const signatureHex = hexConcat(notSignatureDataHex, SignatureType.EIP1271Wallet); // Validate signature - const isValidSignature = await validateCallAsync( - signedOrder, - validatorWallet.address, + const isValidSignature = await validateAsync( + signedTransaction, signatureHex, - ValidatorWalletAction.Reject, + ValidatorWalletAction.MatchSignatureHash, + signatureDataHex, ); expect(isValidSignature).to.be.false(); }); it('should revert when validator attempts to update state and SignatureType=EIP1271Wallet', async () => { - signedOrder.makerAddress = validatorWallet.address; - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const signature = Buffer.concat([ethUtil.toBuffer([SignatureType.EIP1271Wallet])]); - const signatureHex = ethUtil.bufferToHex(signature); + signedTransaction.signerAddress = validatorWallet.address; + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureHex = hexConcat(generateRandomSignature(), SignatureType.EIP1271Wallet); + const transactionHashHex = transactionHashUtils.getTransactionHashHex(signedTransaction); const expectedError = new ExchangeRevertErrors.SignatureWalletError( - orderHashHex, + transactionHashHex, validatorWallet.address, signatureHex, constants.NULL_BYTES, ); - const tx = validateCallAsync( - signedOrder, - validatorWallet.address, - signatureHex, - ValidatorWalletAction.UpdateState, - ); + const tx = validateAsync(signedTransaction, signatureHex, ValidatorWalletAction.UpdateState); return expect(tx).to.revertWith(expectedError); }); it('should revert when validator reverts and SignatureType=EIP1271Wallet', async () => { - signedOrder.makerAddress = validatorWallet.address; - const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - const signature = Buffer.concat([ethUtil.toBuffer([SignatureType.EIP1271Wallet])]); - const signatureHex = ethUtil.bufferToHex(signature); + signedTransaction.signerAddress = validatorWallet.address; + // Doesn't have to contain a real signature since our wallet contract + // just does a hash comparison. + const signatureHex = hexConcat(generateRandomSignature(), SignatureType.EIP1271Wallet); + const transactionHashHex = transactionHashUtils.getTransactionHashHex(signedTransaction); const expectedError = new ExchangeRevertErrors.SignatureWalletError( - orderHashHex, + transactionHashHex, validatorWallet.address, signatureHex, new StringRevertError(validatorWalletRevertReason).encode(), ); - const tx = validateCallAsync( - signedOrder, - validatorWallet.address, - signatureHex, - ValidatorWalletAction.Revert, - ); + const tx = validateAsync(signedTransaction, signatureHex, ValidatorWalletAction.Revert); return expect(tx).to.revertWith(expectedError); }); - createHashSignatureTests(validateCallAsync); + // Run hash-only signature type tests as well. + const validateOrderHashAsync = async ( + hashHex: string, + _signerAddress: string, + signatureHex: string, + validatorAction?: ValidatorWalletAction, + validatorExpectedSignatureHex?: string, + ): Promise => { + signedTransaction.signerAddress = _signerAddress; + return validateAsync(signedTransaction, signatureHex, validatorAction, validatorExpectedSignatureHex); + }; + createHashSignatureTests((_signerAddress?: string) => { + signedTransaction.signerAddress = _signerAddress === undefined ? signerAddress : _signerAddress; + return transactionHashUtils.getTransactionHashHex(signedTransaction); + }, validateOrderHashAsync); }); describe('setSignatureValidatorApproval', () => { + let signatureValidatorLogDecoder: LogDecoder; + + before(async () => { + signatureValidatorLogDecoder = new LogDecoder(web3Wrapper, artifacts); + }); + it('should emit a SignatureValidatorApprovalSet with correct args when a validator is approved', async () => { const approval = true; const res = await signatureValidator.setSignatureValidatorApproval.awaitTransactionSuccessAsync( diff --git a/contracts/exchange/test/transactions.ts b/contracts/exchange/test/transactions.ts index cb574ead92..d9073f71e8 100644 --- a/contracts/exchange/test/transactions.ts +++ b/contracts/exchange/test/transactions.ts @@ -1101,13 +1101,10 @@ describe('Exchange transactions', () => { exchangeInstance.address, ); const isApproved = true; - await web3Wrapper.awaitTransactionSuccessAsync( - await exchangeInstance.setSignatureValidatorApproval.sendTransactionAsync( - whitelistContract.address, - isApproved, - { from: takerAddress }, - ), - constants.AWAIT_TRANSACTION_MINED_MS, + await exchangeInstance.setSignatureValidatorApproval.awaitTransactionSuccessAsync( + whitelistContract.address, + isApproved, + { from: takerAddress }, ); }); @@ -1161,18 +1158,16 @@ describe('Exchange transactions', () => { it('should fill the order if maker and taker have been whitelisted', async () => { const isApproved = true; - await web3Wrapper.awaitTransactionSuccessAsync( - await whitelistContract.updateWhitelistStatus.sendTransactionAsync(makerAddress, isApproved, { - from: owner, - }), - constants.AWAIT_TRANSACTION_MINED_MS, + await whitelistContract.updateWhitelistStatus.awaitTransactionSuccessAsync( + makerAddress, + isApproved, + { from: owner }, ); - await web3Wrapper.awaitTransactionSuccessAsync( - await whitelistContract.updateWhitelistStatus.sendTransactionAsync(takerAddress, isApproved, { - from: owner, - }), - constants.AWAIT_TRANSACTION_MINED_MS, + await whitelistContract.updateWhitelistStatus.awaitTransactionSuccessAsync( + takerAddress, + isApproved, + { from: owner }, ); const signedOrder = await orderFactory.newSignedOrderAsync({