@0x/contracts-exchange: Have TestValidatorWallet always accept WalletOrderValidator if makerAddress == this.

`@0x/contracts-exchange`: Update tests for repeatable signature validation.
This commit is contained in:
Lawrence Forman
2019-06-21 15:58:54 -04:00
committed by Amir Bandeali
parent 073930004d
commit dee5ff852d
4 changed files with 91 additions and 119 deletions

View File

@@ -33,7 +33,7 @@ contract TestValidatorWallet {
// Return false (default)
Reject,
// Return true
Allow,
Accept,
// Revert
Revert,
// Update state
@@ -85,7 +85,7 @@ contract TestValidatorWallet {
ValidatorAction action = _hashActions[hash];
if (action == ValidatorAction.Reject) {
isValid = false;
} else if (action == ValidatorAction.Allow) {
} else if (action == ValidatorAction.Accept) {
isValid = true;
} else if (action == ValidatorAction.Revert) {
revert(REVERT_REASON);
@@ -110,7 +110,7 @@ contract TestValidatorWallet {
ValidatorAction action = _hashActions[hash];
if (action == ValidatorAction.Reject) {
isValid = false;
} else if (action == ValidatorAction.Allow) {
} else if (action == ValidatorAction.Accept) {
isValid = true;
} else if (action == ValidatorAction.Revert) {
revert(REVERT_REASON);
@@ -121,7 +121,8 @@ contract TestValidatorWallet {
}
}
/// @dev Validates a hash with the `OrderValidator` signature type.
/// @dev Validates a hash with the `OrderValidator` and
/// `WalletOrderValidator` signature types.
/// @param order The order.
/// @param orderHash The order hash.
/// @param signature Proof of signing.
@@ -137,12 +138,16 @@ contract TestValidatorWallet {
ValidatorAction action = _hashActions[orderHash];
if (action == ValidatorAction.Reject) {
isValid = false;
} else if (action == ValidatorAction.Allow) {
} else if (action == ValidatorAction.Accept) {
isValid = true;
} else if (action == ValidatorAction.Revert) {
revert(REVERT_REASON);
} else if (action == ValidatorAction.ValidateSignature) {
isValid = _isSignedBy(orderHash, signature, order.makerAddress);
if (order.makerAddress == address(this)) {
isValid = true;
} else {
isValid = _isSignedBy(orderHash, signature, order.makerAddress);
}
} else { // if (action == ValidatorAction.UpdateState) {
_updateState();
}

View File

@@ -35,6 +35,7 @@ import { RevertReason, SignatureType, SignedOrder } from '@0x/types';
import { BigNumber, providerUtils, StringRevertError } from '@0x/utils';
import { Web3Wrapper } from '@0x/web3-wrapper';
import * as chai from 'chai';
import * as crypto from 'crypto';
import { LogWithDecodedArgs } from 'ethereum-types';
import ethUtil = require('ethereumjs-util');
import * as _ from 'lodash';
@@ -249,7 +250,7 @@ describe('Exchange core', () => {
};
describe('fillOrder reentrancy tests', () => reentrancyTest(exchangeConstants.FUNCTIONS_WITH_MUTEX));
describe('signatures', () => {
describe('repeatable signature types', () => {
beforeEach(async () => {
// Approve the ERC20 proxy with the test validator wallet.
await validatorWallet.approveERC20.awaitTransactionSuccessAsync(
@@ -262,105 +263,29 @@ describe('Exchange core', () => {
validatorWallet.address,
constants.INITIAL_ERC20_BALANCE,
);
// Approve the validator.
await exchange.setSignatureValidatorApproval.awaitTransactionSuccessAsync(
// Approve the order validator.
await exchange.setOrderValidatorApproval.awaitTransactionSuccessAsync(
validatorWallet.address,
true,
{ from: makerAddress },
);
});
it('should revert if signature is invalid', async () => {
signedOrder = await orderFactory.newSignedOrderAsync({
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
});
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
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;
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.BadSignature,
orderHashHex,
signedOrder.makerAddress,
invalidSigHex,
);
const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress);
return expect(tx).to.revertWith(expectedError);
});
it('should revert if validator tries to update state when SignatureType=Wallet', async () => {
signedOrder = await orderFactory.newSignedOrderAsync({
makerAddress: validatorWallet.address,
makerFee: constants.ZERO_AMOUNT,
takerFee: constants.ZERO_AMOUNT,
});
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
signedOrder.signature = `0x0${SignatureType.Wallet}`;
validatorWallet.setValidateAction.awaitTransactionSuccessAsync(
orderHashHex,
ValidatorWalletAction.UpdateState,
makerAddress,
);
const expectedError = new ExchangeRevertErrors.SignatureWalletError(
orderHashHex,
signedOrder.makerAddress,
signedOrder.signature,
constants.NULL_BYTES,
);
const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress);
return expect(tx).to.revertWith(expectedError);
});
it('should revert if validator tries to update state when SignatureType=Validator', async () => {
signedOrder.signature = `${validatorWallet.address}0${SignatureType.Validator}`;
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
validatorWallet.setValidateAction.awaitTransactionSuccessAsync(
orderHashHex,
ValidatorWalletAction.UpdateState,
makerAddress,
);
const expectedError = new ExchangeRevertErrors.SignatureValidatorError(
orderHashHex,
signedOrder.makerAddress,
signedOrder.signature,
constants.NULL_BYTES,
);
const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress);
return expect(tx).to.revertWith(expectedError);
});
it('should revert if validator is not approved when SignatureType=Validator', async () => {
signedOrder.signature = `${validatorWallet.address}0${SignatureType.Validator}`;
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
validatorWallet.setValidateAction.awaitTransactionSuccessAsync(
orderHashHex,
ValidatorWalletAction.Allow,
makerAddress,
);
// Disapprove the validator.
await exchange.setSignatureValidatorApproval.awaitTransactionSuccessAsync(
validatorWallet.address,
false,
{ from: makerAddress },
);
const expectedError = new ExchangeRevertErrors.SignatureValidatorNotApprovedError(
signedOrder.makerAddress,
signedOrder.signature,
);
const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress);
return expect(tx).to.revertWith(expectedError);
});
it('should revert if `OrderValidator` signature type rejects during a second fill', async () => {
signedOrder.signature = `${validatorWallet.address}0${SignatureType.OrderValidator}`;
const signature = Buffer.concat([
ethUtil.toBuffer(validatorWallet.address),
ethUtil.toBuffer([SignatureType.OrderValidator]),
]);
signedOrder.signature = ethUtil.bufferToHex(signature);
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
// Allow the signature check for the first fill.
await validatorWallet.setValidateAction.awaitTransactionSuccessAsync(
orderHashHex,
ValidatorWalletAction.Allow,
ValidatorWalletAction.Accept,
makerAddress,
);
const fillAmount = signedOrder.takerAssetAmount.div(10);
@@ -369,6 +294,7 @@ describe('Exchange core', () => {
takerAddress,
{ takerAssetFillAmount: fillAmount },
);
// Reject the signature check for the second fill.
await validatorWallet.setValidateAction.awaitTransactionSuccessAsync(
orderHashHex,
ValidatorWalletAction.Reject,
@@ -381,6 +307,46 @@ describe('Exchange core', () => {
);
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.BadSignature,
orderHashHex,
signedOrder.makerAddress,
signedOrder.signature,
);
return expect(tx).to.revertWith(expectedError);
});
it('should revert if `WalletOrderValidator` signature type rejects during a second fill', async () => {
const signature = Buffer.concat([
ethUtil.toBuffer([SignatureType.WalletOrderValidator]),
]);
signedOrder.makerAddress = validatorWallet.address;
signedOrder.signature = ethUtil.bufferToHex(signature);
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
// Allow the signature check for the first fill.
await validatorWallet.setValidateAction.awaitTransactionSuccessAsync(
orderHashHex,
ValidatorWalletAction.Accept,
makerAddress,
);
const fillAmount = signedOrder.takerAssetAmount.div(10);
await exchangeWrapper.fillOrderAsync(
signedOrder,
takerAddress,
{ takerAssetFillAmount: fillAmount },
);
// Reject the signature check for the second fill.
await validatorWallet.setValidateAction.awaitTransactionSuccessAsync(
orderHashHex,
ValidatorWalletAction.Reject,
makerAddress,
);
const tx = exchangeWrapper.fillOrderAsync(
signedOrder,
takerAddress,
{ takerAssetFillAmount: fillAmount },
);
const expectedError = new ExchangeRevertErrors.SignatureError(
ExchangeRevertErrors.SignatureErrorCode.BadSignature,
orderHashHex,
signedOrder.makerAddress,
signedOrder.signature,
);

View File

@@ -31,7 +31,7 @@ const expect = chai.expect;
const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
// tslint:disable:no-unnecessary-type-assertion
describe.only('MixinSignatureValidator', () => {
describe('MixinSignatureValidator', () => {
const SIGNATURE_LENGTH = 65;
let chainId: number;
let signedOrder: SignedOrder;
@@ -43,7 +43,7 @@ describe.only('MixinSignatureValidator', () => {
let signerAddress: string;
let signerPrivateKey: Buffer;
let notSignerAddress: string;
let notSignerPrivateKey: Buffer;
let signatureValidatorLogDecoder: LogDecoder;
before(async () => {
await blockchainLifecycle.startAsync();
@@ -100,8 +100,9 @@ describe.only('MixinSignatureValidator', () => {
chainId,
},
};
signatureValidatorLogDecoder = new LogDecoder(web3Wrapper, artifacts);
signerPrivateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddress)];
notSignerPrivateKey = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(notSignerAddress)];
orderFactory = new OrderFactory(signerPrivateKey, defaultOrderParams);
});
@@ -355,7 +356,7 @@ describe.only('MixinSignatureValidator', () => {
signedOrder,
signerAddress,
signatureHex,
ValidatorWalletAction.Allow,
ValidatorWalletAction.Accept,
);
expect(isValidSignature).to.be.true();
});
@@ -508,7 +509,7 @@ describe.only('MixinSignatureValidator', () => {
signedOrder,
signerAddress,
inappropriateSignatureHex,
ValidatorWalletAction.Allow,
ValidatorWalletAction.Accept,
);
return expect(tx).to.revertWith(expectedError);
});
@@ -678,20 +679,19 @@ describe.only('MixinSignatureValidator', () => {
signerAddress,
validatorWallet.address,
);
return expect(tx).to.rejectedWith(expectedError);
return expect(tx).to.revertWith(expectedError);
});
it('should return true when SignatureType=WalletOrderValidator and signature is valid', async () => {
signedOrder.makerAddress = validatorWallet.address;
const signature = Buffer.concat([
ethUtil.toBuffer(signedOrder.signature).slice(0, SIGNATURE_LENGTH),
ethUtil.toBuffer(validatorWallet.address),
ethUtil.toBuffer([SignatureType.OrderValidator]),
ethUtil.toBuffer([SignatureType.WalletOrderValidator]),
]);
const signatureHex = ethUtil.bufferToHex(signature);
// Validate signature
const isValidSignature = await validateCallAsync(
signedOrder,
signerAddress,
validatorWallet.address,
signatureHex,
ValidatorWalletAction.ValidateSignature,
);
@@ -699,16 +699,16 @@ describe.only('MixinSignatureValidator', () => {
});
it('should return false when SignatureType=WalletOrderValidator and signature is invalid', async () => {
signedOrder.makerAddress = notSignerAddress;
const signature = Buffer.concat([
crypto.randomBytes(SIGNATURE_LENGTH),
ethUtil.toBuffer(validatorWallet.address),
ethUtil.toBuffer([SignatureType.OrderValidator]),
ethUtil.toBuffer(signedOrder.signature).slice(0, SIGNATURE_LENGTH),
ethUtil.toBuffer([SignatureType.WalletOrderValidator]),
]);
const signatureHex = ethUtil.bufferToHex(signature);
// Validate signature
const isValidSignature = await validateCallAsync(
signedOrder,
signerAddress,
validatorWallet.address,
signatureHex,
ValidatorWalletAction.ValidateSignature,
);
@@ -716,11 +716,10 @@ describe.only('MixinSignatureValidator', () => {
});
it('should revert when validator attempts to update state and SignatureType=WalletOrderValidator', async () => {
signedOrder.makerAddress = validatorWallet.address;
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
const signature = Buffer.concat([
ethUtil.toBuffer(signedOrder.signature).slice(0, SIGNATURE_LENGTH),
ethUtil.toBuffer(validatorWallet.address),
ethUtil.toBuffer([SignatureType.OrderValidator]),
ethUtil.toBuffer([SignatureType.WalletOrderValidator]),
]);
const signatureHex = ethUtil.bufferToHex(signature);
const expectedError = new ExchangeRevertErrors.SignatureWalletOrderValidatorError(
@@ -739,12 +738,10 @@ describe.only('MixinSignatureValidator', () => {
});
it('should revert when validator reverts and SignatureType=WalletOrderValidator', async () => {
// Create EIP712 signature
signedOrder.makerAddress = validatorWallet.address;
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
const signature = Buffer.concat([
ethUtil.toBuffer(signedOrder.signature).slice(0, SIGNATURE_LENGTH),
ethUtil.toBuffer(validatorWallet.address),
ethUtil.toBuffer([SignatureType.OrderValidator]),
ethUtil.toBuffer([SignatureType.WalletOrderValidator]),
]);
const signatureHex = ethUtil.bufferToHex(signature);
const expectedError = new ExchangeRevertErrors.SignatureWalletOrderValidatorError(
@@ -776,7 +773,8 @@ describe.only('MixinSignatureValidator', () => {
},
);
expect(res.logs.length).to.equal(1);
const log = res.logs[0] as LogWithDecodedArgs<TestSignatureValidatorSignatureValidatorApprovalEventArgs>;
const log = signatureValidatorLogDecoder.decodeLogOrThrow(res.logs[0]) as
LogWithDecodedArgs<TestSignatureValidatorSignatureValidatorApprovalEventArgs>;
const logArgs = log.args;
expect(logArgs.signerAddress).to.equal(signerAddress);
expect(logArgs.validatorAddress).to.equal(validatorWallet.address);
@@ -792,7 +790,8 @@ describe.only('MixinSignatureValidator', () => {
},
);
expect(res.logs.length).to.equal(1);
const log = res.logs[0] as LogWithDecodedArgs<TestSignatureValidatorSignatureValidatorApprovalEventArgs>;
const log = signatureValidatorLogDecoder.decodeLogOrThrow(res.logs[0]) as
LogWithDecodedArgs<TestSignatureValidatorSignatureValidatorApprovalEventArgs>;
const logArgs = log.args;
expect(logArgs.signerAddress).to.equal(signerAddress);
expect(logArgs.validatorAddress).to.equal(validatorWallet.address);
@@ -808,7 +807,8 @@ describe.only('MixinSignatureValidator', () => {
},
);
expect(res.logs.length).to.equal(1);
const log = res.logs[0] as LogWithDecodedArgs<TestSignatureValidatorSignatureValidatorApprovalEventArgs>;
const log = signatureValidatorLogDecoder.decodeLogOrThrow(res.logs[0]) as
LogWithDecodedArgs<TestSignatureValidatorSignatureValidatorApprovalEventArgs>;
const logArgs = log.args;
expect(logArgs.signerAddress).to.equal(signerAddress);
expect(logArgs.validatorAddress).to.equal(validatorWallet.address);
@@ -824,7 +824,8 @@ describe.only('MixinSignatureValidator', () => {
},
);
expect(res.logs.length).to.equal(1);
const log = res.logs[0] as LogWithDecodedArgs<TestSignatureValidatorSignatureValidatorApprovalEventArgs>;
const log = signatureValidatorLogDecoder.decodeLogOrThrow(res.logs[0]) as
LogWithDecodedArgs<TestSignatureValidatorSignatureValidatorApprovalEventArgs>;
const logArgs = log.args;
expect(logArgs.signerAddress).to.equal(signerAddress);
expect(logArgs.validatorAddress).to.equal(validatorWallet.address);

View File

@@ -36,7 +36,7 @@ export const constants = {
export enum ValidatorWalletAction {
Reject = 0,
Allow = 1,
Accept = 1,
Revert = 2,
UpdateState = 3,
ValidateSignature = 4,