From c5f8b9c2d2b1eed5789b40c4df07e98afe0431ab Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Thu, 23 Aug 2018 17:25:22 -0700 Subject: Add pragma experimental v0.5.0 to SignatureValidator and add tests --- .../contracts/test/exchange/signature_validator.ts | 53 +++++++++++++++++++++- packages/contracts/test/utils/artifacts.ts | 2 + 2 files changed, 54 insertions(+), 1 deletion(-) (limited to 'packages/contracts/test') diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index cd4f3d6f3..0e2967bc7 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -9,11 +9,12 @@ import { TestSignatureValidatorContract, TestSignatureValidatorSignatureValidatorApprovalEventArgs, } from '../../generated_contract_wrappers/test_signature_validator'; +import { TestStaticCallContract } from '../../generated_contract_wrappers/test_static_call'; import { ValidatorContract } from '../../generated_contract_wrappers/validator'; import { WalletContract } from '../../generated_contract_wrappers/wallet'; import { addressUtils } from '../utils/address_utils'; import { artifacts } from '../utils/artifacts'; -import { expectContractCallFailed } from '../utils/assertions'; +import { expectContractCallFailed, expectContractCallFailedWithoutReasonAsync } from '../utils/assertions'; import { chaiSetup } from '../utils/chai_setup'; import { constants } from '../utils/constants'; import { LogDecoder } from '../utils/log_decoder'; @@ -31,6 +32,8 @@ describe('MixinSignatureValidator', () => { let signatureValidator: TestSignatureValidatorContract; let testWallet: WalletContract; let testValidator: ValidatorContract; + let maliciousWallet: TestStaticCallContract; + let maliciousValidator: TestStaticCallContract; let signerAddress: string; let signerPrivateKey: Buffer; let notSignerAddress: string; @@ -65,6 +68,11 @@ describe('MixinSignatureValidator', () => { txDefaults, signerAddress, ); + maliciousWallet = maliciousValidator = await TestStaticCallContract.deployFrom0xArtifactAsync( + artifacts.TestStaticCall, + provider, + txDefaults, + ); signatureValidatorLogDecoder = new LogDecoder(web3Wrapper); await web3Wrapper.awaitTransactionSuccessAsync( await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync(testValidator.address, true, { @@ -72,6 +80,16 @@ describe('MixinSignatureValidator', () => { }), constants.AWAIT_TRANSACTION_MINED_MS, ); + await web3Wrapper.awaitTransactionSuccessAsync( + await signatureValidator.setSignatureValidatorApproval.sendTransactionAsync( + maliciousValidator.address, + true, + { + from: signerAddress, + }, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); const defaultOrderParams = { ...constants.STATIC_ORDER_PARAMS, @@ -334,6 +352,29 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.false(); }); + it('should not allow `isValidSignature` to update state when SignatureType=Wallet', 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 + await expectContractCallFailedWithoutReasonAsync( + signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + maliciousWallet.address, + signatureHex, + ), + ); + }); + 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}`); @@ -364,6 +405,16 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.false(); }); + it('should not allow `isValidSignature` to update state when SignatureType=Validator', async () => { + const validatorAddress = ethUtil.toBuffer(`${maliciousValidator.address}`); + const signatureType = ethUtil.toBuffer(`0x${SignatureType.Validator}`); + const signature = Buffer.concat([validatorAddress, signatureType]); + const signatureHex = ethUtil.bufferToHex(signature); + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + await expectContractCallFailedWithoutReasonAsync( + signatureValidator.publicIsValidSignature.callAsync(orderHashHex, signerAddress, signatureHex), + ); + }); 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( diff --git a/packages/contracts/test/utils/artifacts.ts b/packages/contracts/test/utils/artifacts.ts index e8a7585ac..1c922a3e4 100644 --- a/packages/contracts/test/utils/artifacts.ts +++ b/packages/contracts/test/utils/artifacts.ts @@ -23,6 +23,7 @@ import * as TestExchangeInternals from '../../artifacts/TestExchangeInternals.js import * as TestLibBytes from '../../artifacts/TestLibBytes.json'; import * as TestLibs from '../../artifacts/TestLibs.json'; import * as TestSignatureValidator from '../../artifacts/TestSignatureValidator.json'; +import * as TestStaticCall from '../../artifacts/TestStaticCall.json'; import * as TokenRegistry from '../../artifacts/TokenRegistry.json'; import * as Validator from '../../artifacts/Validator.json'; import * as Wallet from '../../artifacts/Wallet.json'; @@ -55,6 +56,7 @@ export const artifacts = { TestLibs: (TestLibs as any) as ContractArtifact, TestExchangeInternals: (TestExchangeInternals as any) as ContractArtifact, TestSignatureValidator: (TestSignatureValidator as any) as ContractArtifact, + TestStaticCall: (TestStaticCall as any) as ContractArtifact, Validator: (Validator as any) as ContractArtifact, Wallet: (Wallet as any) as ContractArtifact, TokenRegistry: (TokenRegistry as any) as ContractArtifact, -- cgit v1.2.3 From 0a1ae2c31139e446b2fb3b7f03caf371c6668ae2 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Fri, 24 Aug 2018 00:19:06 -0700 Subject: Remove pragma experimental v0.5.0 and use staticcall is assembly --- packages/contracts/test/exchange/core.ts | 59 +++++++++++++++++++++- .../contracts/test/exchange/signature_validator.ts | 22 ++++---- 2 files changed, 70 insertions(+), 11 deletions(-) (limited to 'packages/contracts/test') diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index bc2bad749..d3f9b95af 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -1,6 +1,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { assetDataUtils, orderHashUtils } from '@0xproject/order-utils'; -import { RevertReason, SignedOrder } from '@0xproject/types'; +import { RevertReason, SignatureType, SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; import * as chai from 'chai'; @@ -14,6 +14,7 @@ import { DummyNoReturnERC20TokenContract } from '../../generated_contract_wrappe import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy'; import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy'; import { ExchangeCancelEventArgs, ExchangeContract } from '../../generated_contract_wrappers/exchange'; +import { TestStaticCallContract } from '../../generated_contract_wrappers/test_static_call'; import { artifacts } from '../utils/artifacts'; import { expectTransactionFailedAsync } from '../utils/assertions'; import { getLatestBlockTimestampAsync, increaseTimeAndMineBlockAsync } from '../utils/block_timestamp'; @@ -44,6 +45,8 @@ describe('Exchange core', () => { let exchange: ExchangeContract; let erc20Proxy: ERC20ProxyContract; let erc721Proxy: ERC721ProxyContract; + let maliciousWallet: TestStaticCallContract; + let maliciousValidator: TestStaticCallContract; let signedOrder: SignedOrder; let erc20Balances: ERC20BalancesByOwner; @@ -109,6 +112,12 @@ describe('Exchange core', () => { constants.AWAIT_TRANSACTION_MINED_MS, ); + maliciousWallet = maliciousValidator = await TestStaticCallContract.deployFrom0xArtifactAsync( + artifacts.TestStaticCall, + provider, + txDefaults, + ); + defaultMakerAssetAddress = erc20TokenA.address; defaultTakerAssetAddress = erc20TokenB.address; @@ -161,6 +170,54 @@ describe('Exchange core', () => { RevertReason.OrderUnfillable, ); }); + + it('should revert if `isValidSignature` tries to update state when SignatureType=Wallet', async () => { + const maliciousMakerAddress = maliciousWallet.address; + await web3Wrapper.awaitTransactionSuccessAsync( + await erc20TokenA.setBalance.sendTransactionAsync( + maliciousMakerAddress, + constants.INITIAL_ERC20_BALANCE, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await maliciousWallet.approveERC20.sendTransactionAsync( + erc20TokenA.address, + erc20Proxy.address, + constants.INITIAL_ERC20_ALLOWANCE, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + signedOrder = await orderFactory.newSignedOrderAsync({ + makerAddress: maliciousMakerAddress, + makerFee: constants.ZERO_AMOUNT, + }); + signedOrder.signature = `0x0${SignatureType.Wallet}`; + await expectTransactionFailedAsync( + exchangeWrapper.fillOrderAsync(signedOrder, takerAddress), + RevertReason.InvalidOrderSignature, + ); + }); + + it('should revert if `isValidSignature` tries to update state when SignatureType=Validator', async () => { + const isApproved = true; + await web3Wrapper.awaitTransactionSuccessAsync( + await exchange.setSignatureValidatorApproval.sendTransactionAsync( + maliciousValidator.address, + isApproved, + { from: makerAddress }, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + signedOrder = await orderFactory.newSignedOrderAsync({ + makerAddress: maliciousValidator.address, + }); + signedOrder.signature = `${maliciousValidator.address}0${SignatureType.Validator}`; + await expectTransactionFailedAsync( + exchangeWrapper.fillOrderAsync(signedOrder, takerAddress), + RevertReason.InvalidOrderSignature, + ); + }); }); describe('Testing exchange of ERC20 tokens with no return values', () => { diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index 0e2967bc7..75656d992 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -352,7 +352,7 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.false(); }); - it('should not allow `isValidSignature` to update state when SignatureType=Wallet', async () => { + it('should return false when `isValidSignature` attempts to update state and not allow state to be updated when SignatureType=Wallet', async () => { // Create EIP712 signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); const orderHashBuffer = ethUtil.toBuffer(orderHashHex); @@ -366,13 +366,12 @@ describe('MixinSignatureValidator', () => { ]); const signatureHex = ethUtil.bufferToHex(signature); // Validate signature - await expectContractCallFailedWithoutReasonAsync( - signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - maliciousWallet.address, - signatureHex, - ), + const isValid = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + maliciousWallet.address, + signatureHex, ); + expect(isValid).to.be.equal(false); }); it('should return true when SignatureType=Validator, signature is valid and validator is approved', async () => { @@ -405,15 +404,18 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.false(); }); - it('should not allow `isValidSignature` to update state when SignatureType=Validator', async () => { + it('should return false when `isValidSignature` attempts to update state and not allow state to be updated when SignatureType=Validator', async () => { const validatorAddress = ethUtil.toBuffer(`${maliciousValidator.address}`); const signatureType = ethUtil.toBuffer(`0x${SignatureType.Validator}`); const signature = Buffer.concat([validatorAddress, signatureType]); const signatureHex = ethUtil.bufferToHex(signature); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - await expectContractCallFailedWithoutReasonAsync( - signatureValidator.publicIsValidSignature.callAsync(orderHashHex, signerAddress, signatureHex), + const isValid = await signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + maliciousValidator.address, + signatureHex, ); + expect(isValid).to.be.equal(false); }); it('should return false when SignatureType=Validator, signature is valid and validator is not approved', async () => { // Set approval of signature validator to false -- cgit v1.2.3 From 681ed822eca91085300e85e136ddcf4abd90495c Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Fri, 24 Aug 2018 09:24:33 -0700 Subject: Revert if undefined function called in AssetProxies --- packages/contracts/test/asset_proxy/proxies.ts | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) (limited to 'packages/contracts/test') diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts index 76a020222..6031e554d 100644 --- a/packages/contracts/test/asset_proxy/proxies.ts +++ b/packages/contracts/test/asset_proxy/proxies.ts @@ -12,7 +12,7 @@ import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_prox import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy'; import { IAssetProxyContract } from '../../generated_contract_wrappers/i_asset_proxy'; import { artifacts } from '../utils/artifacts'; -import { expectTransactionFailedAsync } from '../utils/assertions'; +import { expectTransactionFailedAsync, expectTransactionFailedWithoutReasonAsync } from '../utils/assertions'; import { chaiSetup } from '../utils/chai_setup'; import { constants } from '../utils/constants'; import { ERC20Wrapper } from '../utils/erc20_wrapper'; @@ -99,6 +99,17 @@ describe('Asset Transfer Proxies', () => { await blockchainLifecycle.revertAsync(); }); describe('Transfer Proxy - ERC20', () => { + it('should revert if undefined function is called', async () => { + const undefinedSelector = '0x01020304'; + await expectTransactionFailedWithoutReasonAsync( + web3Wrapper.sendTransactionAsync({ + from: owner, + to: erc20Proxy.address, + value: constants.ZERO_AMOUNT, + data: undefinedSelector, + }), + ); + }); describe('transferFrom', () => { it('should successfully transfer tokens', async () => { // Construct ERC20 asset data @@ -219,6 +230,17 @@ describe('Asset Transfer Proxies', () => { }); describe('Transfer Proxy - ERC721', () => { + it('should revert if undefined function is called', async () => { + const undefinedSelector = '0x01020304'; + await expectTransactionFailedWithoutReasonAsync( + web3Wrapper.sendTransactionAsync({ + from: owner, + to: erc721Proxy.address, + value: constants.ZERO_AMOUNT, + data: undefinedSelector, + }), + ); + }); describe('transferFrom', () => { it('should successfully transfer tokens', async () => { // Construct ERC721 asset data -- cgit v1.2.3 From 070eff6f3a2f3775ef72248c3f345c05cd833798 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Fri, 24 Aug 2018 09:27:29 -0700 Subject: Rename TestStaticCall => TestStaticCallReceiver --- packages/contracts/test/exchange/core.ts | 10 +++++----- packages/contracts/test/exchange/signature_validator.ts | 10 +++++----- packages/contracts/test/utils/artifacts.ts | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) (limited to 'packages/contracts/test') diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index d3f9b95af..fdfb40155 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -14,7 +14,7 @@ import { DummyNoReturnERC20TokenContract } from '../../generated_contract_wrappe import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy'; import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy'; import { ExchangeCancelEventArgs, ExchangeContract } from '../../generated_contract_wrappers/exchange'; -import { TestStaticCallContract } from '../../generated_contract_wrappers/test_static_call'; +import { TestStaticCallReceiverContract } from '../../generated_contract_wrappers/test_static_call_receiver'; import { artifacts } from '../utils/artifacts'; import { expectTransactionFailedAsync } from '../utils/assertions'; import { getLatestBlockTimestampAsync, increaseTimeAndMineBlockAsync } from '../utils/block_timestamp'; @@ -45,8 +45,8 @@ describe('Exchange core', () => { let exchange: ExchangeContract; let erc20Proxy: ERC20ProxyContract; let erc721Proxy: ERC721ProxyContract; - let maliciousWallet: TestStaticCallContract; - let maliciousValidator: TestStaticCallContract; + let maliciousWallet: TestStaticCallReceiverContract; + let maliciousValidator: TestStaticCallReceiverContract; let signedOrder: SignedOrder; let erc20Balances: ERC20BalancesByOwner; @@ -112,8 +112,8 @@ describe('Exchange core', () => { constants.AWAIT_TRANSACTION_MINED_MS, ); - maliciousWallet = maliciousValidator = await TestStaticCallContract.deployFrom0xArtifactAsync( - artifacts.TestStaticCall, + maliciousWallet = maliciousValidator = await TestStaticCallReceiverContract.deployFrom0xArtifactAsync( + artifacts.TestStaticCallReceiver, provider, txDefaults, ); diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index 75656d992..947ebffcc 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -9,7 +9,7 @@ import { TestSignatureValidatorContract, TestSignatureValidatorSignatureValidatorApprovalEventArgs, } from '../../generated_contract_wrappers/test_signature_validator'; -import { TestStaticCallContract } from '../../generated_contract_wrappers/test_static_call'; +import { TestStaticCallReceiverContract } from '../../generated_contract_wrappers/test_static_call_receiver'; import { ValidatorContract } from '../../generated_contract_wrappers/validator'; import { WalletContract } from '../../generated_contract_wrappers/wallet'; import { addressUtils } from '../utils/address_utils'; @@ -32,8 +32,8 @@ describe('MixinSignatureValidator', () => { let signatureValidator: TestSignatureValidatorContract; let testWallet: WalletContract; let testValidator: ValidatorContract; - let maliciousWallet: TestStaticCallContract; - let maliciousValidator: TestStaticCallContract; + let maliciousWallet: TestStaticCallReceiverContract; + let maliciousValidator: TestStaticCallReceiverContract; let signerAddress: string; let signerPrivateKey: Buffer; let notSignerAddress: string; @@ -68,8 +68,8 @@ describe('MixinSignatureValidator', () => { txDefaults, signerAddress, ); - maliciousWallet = maliciousValidator = await TestStaticCallContract.deployFrom0xArtifactAsync( - artifacts.TestStaticCall, + maliciousWallet = maliciousValidator = await TestStaticCallReceiverContract.deployFrom0xArtifactAsync( + artifacts.TestStaticCallReceiver, provider, txDefaults, ); diff --git a/packages/contracts/test/utils/artifacts.ts b/packages/contracts/test/utils/artifacts.ts index 1c922a3e4..2f6fcef71 100644 --- a/packages/contracts/test/utils/artifacts.ts +++ b/packages/contracts/test/utils/artifacts.ts @@ -23,7 +23,7 @@ import * as TestExchangeInternals from '../../artifacts/TestExchangeInternals.js import * as TestLibBytes from '../../artifacts/TestLibBytes.json'; import * as TestLibs from '../../artifacts/TestLibs.json'; import * as TestSignatureValidator from '../../artifacts/TestSignatureValidator.json'; -import * as TestStaticCall from '../../artifacts/TestStaticCall.json'; +import * as TestStaticCallReceiver from '../../artifacts/TestStaticCallReceiver.json'; import * as TokenRegistry from '../../artifacts/TokenRegistry.json'; import * as Validator from '../../artifacts/Validator.json'; import * as Wallet from '../../artifacts/Wallet.json'; @@ -56,7 +56,7 @@ export const artifacts = { TestLibs: (TestLibs as any) as ContractArtifact, TestExchangeInternals: (TestExchangeInternals as any) as ContractArtifact, TestSignatureValidator: (TestSignatureValidator as any) as ContractArtifact, - TestStaticCall: (TestStaticCall as any) as ContractArtifact, + TestStaticCallReceiver: (TestStaticCallReceiver as any) as ContractArtifact, Validator: (Validator as any) as ContractArtifact, Wallet: (Wallet as any) as ContractArtifact, TokenRegistry: (TokenRegistry as any) as ContractArtifact, -- cgit v1.2.3 From 6a9669a409a61c4645af43f39a4e4a0761354e32 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Fri, 24 Aug 2018 14:06:46 -0700 Subject: Rethrow Wallet and Validator errors --- packages/contracts/test/exchange/core.ts | 7 ++---- .../contracts/test/exchange/signature_validator.ts | 25 +++++++++++----------- 2 files changed, 14 insertions(+), 18 deletions(-) (limited to 'packages/contracts/test') diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index fdfb40155..f9d8b7783 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -195,7 +195,7 @@ describe('Exchange core', () => { signedOrder.signature = `0x0${SignatureType.Wallet}`; await expectTransactionFailedAsync( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress), - RevertReason.InvalidOrderSignature, + RevertReason.WalletError, ); }); @@ -209,13 +209,10 @@ describe('Exchange core', () => { ), constants.AWAIT_TRANSACTION_MINED_MS, ); - signedOrder = await orderFactory.newSignedOrderAsync({ - makerAddress: maliciousValidator.address, - }); signedOrder.signature = `${maliciousValidator.address}0${SignatureType.Validator}`; await expectTransactionFailedAsync( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress), - RevertReason.InvalidOrderSignature, + RevertReason.ValidatorError, ); }); }); diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index 947ebffcc..5b64d853c 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -352,7 +352,7 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.false(); }); - it('should return false when `isValidSignature` attempts to update state and not allow state to be updated when SignatureType=Wallet', async () => { + it('should revert when `isValidSignature` attempts to update state and SignatureType=Wallet', async () => { // Create EIP712 signature const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); const orderHashBuffer = ethUtil.toBuffer(orderHashHex); @@ -365,13 +365,14 @@ describe('MixinSignatureValidator', () => { ethUtil.toBuffer(`0x${SignatureType.Wallet}`), ]); const signatureHex = ethUtil.bufferToHex(signature); - // Validate signature - const isValid = await signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - maliciousWallet.address, - signatureHex, + await expectContractCallFailed( + signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + maliciousWallet.address, + signatureHex, + ), + RevertReason.WalletError, ); - expect(isValid).to.be.equal(false); }); it('should return true when SignatureType=Validator, signature is valid and validator is approved', async () => { @@ -404,18 +405,16 @@ describe('MixinSignatureValidator', () => { expect(isValidSignature).to.be.false(); }); - it('should return false when `isValidSignature` attempts to update state and not allow state to be updated when SignatureType=Validator', async () => { + it('should revert when `isValidSignature` attempts to update state and SignatureType=Validator', async () => { const validatorAddress = ethUtil.toBuffer(`${maliciousValidator.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 isValid = await signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - maliciousValidator.address, - signatureHex, + await expectContractCallFailed( + signatureValidator.publicIsValidSignature.callAsync(orderHashHex, signerAddress, signatureHex), + RevertReason.ValidatorError, ); - expect(isValid).to.be.equal(false); }); it('should return false when SignatureType=Validator, signature is valid and validator is not approved', async () => { // Set approval of signature validator to false -- cgit v1.2.3