address comments

This commit is contained in:
Michael Zhu 2019-11-22 16:05:59 -08:00
parent e332b7535c
commit 1ca085ec4a
13 changed files with 99 additions and 15 deletions

View File

@ -95,7 +95,7 @@ blockchainTests.resets('Coordinator integration tests', env => {
}); });
after(async () => { after(async () => {
Actor.count = 0; Actor.reset();
}); });
function expectedFillEvent(order: SignedOrder): ExchangeFillEventArgs { function expectedFillEvent(order: SignedOrder): ExchangeFillEventArgs {

View File

@ -146,7 +146,7 @@ blockchainTests.resets('matchOrders integration tests', env => {
}); });
after(async () => { after(async () => {
Actor.count = 0; Actor.reset();
}); });
describe('batchMatchOrders and batchMatchOrdersWithMaximalFill rich errors', async () => { describe('batchMatchOrders and batchMatchOrdersWithMaximalFill rich errors', async () => {

View File

@ -124,7 +124,7 @@ blockchainTests.resets('Exchange wrappers', env => {
}); });
after(async () => { after(async () => {
Actor.count = 0; Actor.reset();
}); });
interface SignedOrderWithValidity { interface SignedOrderWithValidity {

View File

@ -109,7 +109,7 @@ blockchainTests.resets('fillOrder integration tests', env => {
}); });
after(async () => { after(async () => {
Actor.count = 0; Actor.reset();
}); });
function verifyFillEvents(order: SignedOrder, receipt: TransactionReceiptWithDecodedLogs): void { function verifyFillEvents(order: SignedOrder, receipt: TransactionReceiptWithDecodedLogs): void {

View File

@ -163,7 +163,7 @@ blockchainTests.resets('matchOrdersWithMaximalFill integration tests', env => {
}); });
after(async () => { after(async () => {
Actor.count = 0; Actor.reset();
}); });
describe('matchOrdersWithMaximalFill', () => { describe('matchOrdersWithMaximalFill', () => {

View File

@ -163,7 +163,7 @@ blockchainTests.resets('matchOrders integration tests', env => {
}); });
after(async () => { after(async () => {
Actor.count = 0; Actor.reset();
}); });
describe('matchOrders', () => { describe('matchOrders', () => {

View File

@ -1,7 +1,15 @@
// tslint:disable: max-file-line-count // tslint:disable: max-file-line-count
import { IAssetDataContract } from '@0x/contracts-asset-proxy'; import { IAssetDataContract } from '@0x/contracts-asset-proxy';
import { exchangeDataEncoder } from '@0x/contracts-exchange'; import { exchangeDataEncoder, ExchangeRevertErrors } from '@0x/contracts-exchange';
import { blockchainTests, constants, describe, ExchangeFunctionName } from '@0x/contracts-test-utils'; import {
blockchainTests,
constants,
describe,
ExchangeFunctionName,
expect,
orderHashUtils,
transactionHashUtils,
} from '@0x/contracts-test-utils';
import { SignedOrder, SignedZeroExTransaction } from '@0x/types'; import { SignedOrder, SignedZeroExTransaction } from '@0x/types';
import { BigNumber } from '@0x/utils'; import { BigNumber } from '@0x/utils';
@ -24,6 +32,7 @@ blockchainTests.resets('Transaction <> protocol fee integration tests', env => {
let alice: Taker; let alice: Taker;
let bob: Taker; let bob: Taker;
let charlie: Taker; let charlie: Taker;
let wethless: Taker; // Used to test revert scenarios
let order: SignedOrder; // All orders will have the same fields, modulo salt and expiration time let order: SignedOrder; // All orders will have the same fields, modulo salt and expiration time
let transactionA: SignedZeroExTransaction; // fillOrder transaction signed by Alice let transactionA: SignedZeroExTransaction; // fillOrder transaction signed by Alice
@ -42,6 +51,7 @@ blockchainTests.resets('Transaction <> protocol fee integration tests', env => {
alice = new Taker({ name: 'Alice', deployment }); alice = new Taker({ name: 'Alice', deployment });
bob = new Taker({ name: 'Bob', deployment }); bob = new Taker({ name: 'Bob', deployment });
charlie = new Taker({ name: 'Charlie', deployment }); charlie = new Taker({ name: 'Charlie', deployment });
wethless = new Taker({ name: 'wethless', deployment });
feeRecipient = new FeeRecipient({ feeRecipient = new FeeRecipient({
name: 'Fee recipient', name: 'Fee recipient',
deployment, deployment,
@ -63,6 +73,13 @@ blockchainTests.resets('Transaction <> protocol fee integration tests', env => {
await taker.configureERC20TokenAsync(takerFeeToken); await taker.configureERC20TokenAsync(takerFeeToken);
await taker.configureERC20TokenAsync(deployment.tokens.weth, deployment.staking.stakingProxy.address); await taker.configureERC20TokenAsync(deployment.tokens.weth, deployment.staking.stakingProxy.address);
} }
await wethless.configureERC20TokenAsync(takerToken);
await wethless.configureERC20TokenAsync(takerFeeToken);
await wethless.configureERC20TokenAsync(
deployment.tokens.weth,
deployment.staking.stakingProxy.address,
constants.ZERO_AMOUNT, // wethless taker has approved the proxy, but has no weth
);
await maker.configureERC20TokenAsync(makerToken); await maker.configureERC20TokenAsync(makerToken);
await maker.configureERC20TokenAsync(makerFeeToken); await maker.configureERC20TokenAsync(makerFeeToken);
@ -90,11 +107,28 @@ blockchainTests.resets('Transaction <> protocol fee integration tests', env => {
}); });
after(async () => { after(async () => {
Actor.count = 0; Actor.reset();
}); });
const REFUND_AMOUNT = new BigNumber(1); const REFUND_AMOUNT = new BigNumber(1);
function protocolFeeError(
failedOrder: SignedOrder,
failedTransaction: SignedZeroExTransaction,
): ExchangeRevertErrors.TransactionExecutionError {
const nestedError = new ExchangeRevertErrors.PayProtocolFeeError(
orderHashUtils.getOrderHashHex(failedOrder),
DeploymentManager.protocolFee,
maker.address,
wethless.address,
'0x',
).encode();
return new ExchangeRevertErrors.TransactionExecutionError(
transactionHashUtils.getTransactionHashHex(failedTransaction),
nestedError,
);
}
describe('executeTransaction', () => { describe('executeTransaction', () => {
const ETH_FEE_WITH_REFUND = DeploymentManager.protocolFee.plus(REFUND_AMOUNT); const ETH_FEE_WITH_REFUND = DeploymentManager.protocolFee.plus(REFUND_AMOUNT);
@ -133,6 +167,14 @@ blockchainTests.resets('Transaction <> protocol fee integration tests', env => {
.awaitTransactionSuccessAsync({ from: alice.address, value: REFUND_AMOUNT }); .awaitTransactionSuccessAsync({ from: alice.address, value: REFUND_AMOUNT });
expectedBalances.simulateFills([order], bob.address, txReceipt, deployment, REFUND_AMOUNT); expectedBalances.simulateFills([order], bob.address, txReceipt, deployment, REFUND_AMOUNT);
}); });
it('Alice executeTransaction => wETH-less taker fillOrder; reverts because protocol fee cannot be paid', async () => {
const data = exchangeDataEncoder.encodeOrdersToExchangeData(ExchangeFunctionName.FillOrder, [order]);
const transaction = await wethless.signTransactionAsync({ data });
const tx = deployment.exchange
.executeTransaction(transaction, transaction.signature)
.awaitTransactionSuccessAsync({ from: alice.address, value: REFUND_AMOUNT });
return expect(tx).to.revertWith(protocolFeeError(order, transaction));
});
it('Alice executeTransaction => Alice batchFillOrders; mixed protocol fees', async () => { it('Alice executeTransaction => Alice batchFillOrders; mixed protocol fees', async () => {
const orders = [order, await maker.signOrderAsync()]; const orders = [order, await maker.signOrderAsync()];
const data = exchangeDataEncoder.encodeOrdersToExchangeData( const data = exchangeDataEncoder.encodeOrdersToExchangeData(
@ -199,6 +241,22 @@ blockchainTests.resets('Transaction <> protocol fee integration tests', env => {
.awaitTransactionSuccessAsync({ from: alice.address, value: REFUND_AMOUNT }); .awaitTransactionSuccessAsync({ from: alice.address, value: REFUND_AMOUNT });
expectedBalances.simulateFills([order], bob.address, txReceipt, deployment, REFUND_AMOUNT); expectedBalances.simulateFills([order], bob.address, txReceipt, deployment, REFUND_AMOUNT);
}); });
it('Alice executeTransaction => Alice executeTransaction => wETH-less taker fillOrder; reverts because protocol fee cannot be paid', async () => {
const data = exchangeDataEncoder.encodeOrdersToExchangeData(ExchangeFunctionName.FillOrder, [order]);
const transaction = await wethless.signTransactionAsync({ data });
const recursiveData = deployment.exchange
.executeTransaction(transaction, transaction.signature)
.getABIEncodedTransactionData();
const recursiveTransaction = await alice.signTransactionAsync({ data: recursiveData });
const tx = deployment.exchange
.executeTransaction(recursiveTransaction, recursiveTransaction.signature)
.awaitTransactionSuccessAsync({ from: alice.address, value: REFUND_AMOUNT });
const expectedError = new ExchangeRevertErrors.TransactionExecutionError(
transactionHashUtils.getTransactionHashHex(recursiveTransaction),
protocolFeeError(order, transaction).encode(),
);
return expect(tx).to.revertWith(expectedError);
});
}); });
}); });
describe('batchExecuteTransactions', () => { describe('batchExecuteTransactions', () => {
@ -344,9 +402,19 @@ blockchainTests.resets('Transaction <> protocol fee integration tests', env => {
MIXED_FEES_WITH_REFUND, MIXED_FEES_WITH_REFUND,
); );
}); });
it('Alice batchExecuteTransactions => Alice fillOrder, Bob fillOrder, wETH-less taker fillOrder; reverts because protocol fee cannot be paid', async () => {
const data = exchangeDataEncoder.encodeOrdersToExchangeData(ExchangeFunctionName.FillOrder, [order]);
const failTransaction = await wethless.signTransactionAsync({ data });
const transactions = [transactionA, transactionB, failTransaction];
const signatures = transactions.map(transaction => transaction.signature);
const tx = deployment.exchange
.batchExecuteTransactions(transactions, signatures)
.awaitTransactionSuccessAsync({ from: alice.address, value: MIXED_FEES_WITH_REFUND });
expect(tx).to.revertWith(protocolFeeError(order, failTransaction));
});
}); });
describe('Nested', () => { describe('Nested', () => {
// First two orders' protocol fees paid in ETH by sender, the other three paid in WETH by their respective takers // First two orders' protocol fees paid in ETH by sender, the others paid in WETH by their respective takers
const MIXED_FEES_WITH_REFUND = DeploymentManager.protocolFee.times(2.5); const MIXED_FEES_WITH_REFUND = DeploymentManager.protocolFee.times(2.5);
// Alice batchExecuteTransactions => Alice fillOrder, Bob fillOrder, Charlie fillOrder // Alice batchExecuteTransactions => Alice fillOrder, Bob fillOrder, Charlie fillOrder
@ -373,6 +441,18 @@ blockchainTests.resets('Transaction <> protocol fee integration tests', env => {
nestedTransaction = await alice.signTransactionAsync({ data: recursiveData }); nestedTransaction = await alice.signTransactionAsync({ data: recursiveData });
}); });
it('Alice executeTransaction => nested batchExecuteTransactions; mixed protocol fees', async () => {
const txReceipt = await deployment.exchange
.executeTransaction(nestedTransaction, nestedTransaction.signature)
.awaitTransactionSuccessAsync({ from: alice.address, value: MIXED_FEES_WITH_REFUND });
expectedBalances.simulateFills(
[order, order, order],
[alice.address, bob.address, charlie.address],
txReceipt,
deployment,
MIXED_FEES_WITH_REFUND,
);
});
it('Alice batchExecuteTransactions => nested batchExecuteTransactions, Bob fillOrder, Charlie fillOrder; mixed protocol fees', async () => { it('Alice batchExecuteTransactions => nested batchExecuteTransactions, Bob fillOrder, Charlie fillOrder; mixed protocol fees', async () => {
const transactions = [nestedTransaction, transactionB2, transactionC2]; const transactions = [nestedTransaction, transactionB2, transactionC2];
const signatures = transactions.map(tx => tx.signature); const signatures = transactions.map(tx => tx.signature);

View File

@ -81,7 +81,7 @@ blockchainTests.resets('Transaction integration tests', env => {
}); });
after(async () => { after(async () => {
Actor.count = 0; Actor.reset();
}); });
function defaultFillEvent(order: SignedOrder): ExchangeFillEventArgs { function defaultFillEvent(order: SignedOrder): ExchangeFillEventArgs {

View File

@ -156,7 +156,7 @@ blockchainTests.resets('Forwarder <> ERC20Bridge integration tests', env => {
}); });
after(async () => { after(async () => {
Actor.count = 0; Actor.reset();
}); });
describe('marketSellOrdersWithEth', () => { describe('marketSellOrdersWithEth', () => {

View File

@ -106,7 +106,7 @@ blockchainTests('Forwarder integration tests', env => {
}); });
after(async () => { after(async () => {
Actor.count = 0; Actor.reset();
}); });
blockchainTests.resets('constructor', () => { blockchainTests.resets('constructor', () => {

View File

@ -31,6 +31,10 @@ export class Actor {
} = {}; } = {};
protected readonly _transactionFactory: TransactionFactory; protected readonly _transactionFactory: TransactionFactory;
public static reset(): void {
Actor.count = 0;
}
constructor(config: ActorConfig) { constructor(config: ActorConfig) {
Actor.count++; Actor.count++;

View File

@ -30,7 +30,7 @@ export class PoolManagementSimulation extends Simulation {
blockchainTests.skip('Pool management fuzz test', env => { blockchainTests.skip('Pool management fuzz test', env => {
after(async () => { after(async () => {
Actor.count = 0; Actor.reset();
}); });
it('fuzz', async () => { it('fuzz', async () => {

View File

@ -34,7 +34,7 @@ export class StakeManagementSimulation extends Simulation {
blockchainTests.skip('Stake management fuzz test', env => { blockchainTests.skip('Stake management fuzz test', env => {
after(async () => { after(async () => {
Actor.count = 0; Actor.reset();
}); });
it('fuzz', async () => { it('fuzz', async () => {