Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

Commit

Permalink
Address remaining PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@abandeali1
abandeali1 committed Aug 23, 2019
1 parent 830d6f7 commit 106b171
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 28 deletions.
2 changes: 1 addition & 1 deletion contracts/exchange-forwarder/test/forwarder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { artifacts, ForwarderContract, ForwarderTestFactory, ForwarderWrapper }

const DECIMALS_DEFAULT = 18;

blockchainTests.only(ContractName.Forwarder, env => {
blockchainTests(ContractName.Forwarder, env => {
let chainId: number;
let makerAddress: string;
let owner: string;
Expand Down
84 changes: 57 additions & 27 deletions contracts/exchange/test/transactions_unit_tests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef
);

// Call the `batchExecuteTransactions()` function and ensure that it reverts with the expected revert error.
const tx = transactionsContract.batchExecuteTransactions.sendTransactionAsync(
const tx = transactionsContract.batchExecuteTransactions.awaitTransactionSuccessAsync(
[transaction],
[randomSignature()],
);
Expand All @@ -132,7 +132,7 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef
);

// Call the `batchExecuteTransactions()` function and ensure that it reverts with the expected revert error.
const tx = transactionsContract.batchExecuteTransactions.sendTransactionAsync(
const tx = transactionsContract.batchExecuteTransactions.awaitTransactionSuccessAsync(
[transaction1, transaction2],
[randomSignature(), randomSignature()],
);
Expand All @@ -155,7 +155,7 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef
);

// Call the `batchExecuteTransactions()` function and ensure that it reverts with the expected revert error.
const tx = transactionsContract.batchExecuteTransactions.sendTransactionAsync(
const tx = transactionsContract.batchExecuteTransactions.awaitTransactionSuccessAsync(
[transaction1, transaction2],
[randomSignature(), randomSignature()],
);
Expand All @@ -176,7 +176,7 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef
ExchangeRevertErrors.TransactionErrorCode.AlreadyExecuted,
transactionHash2,
);
const tx = transactionsContract.batchExecuteTransactions.sendTransactionAsync(
const tx = transactionsContract.batchExecuteTransactions.awaitTransactionSuccessAsync(
[transaction1, transaction2],
[randomSignature(), randomSignature()],
{
Expand Down Expand Up @@ -293,7 +293,7 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef
outerExecuteTransactionHash,
innerExpectedError.encode(),
);
const tx = transactionsContract.batchExecuteTransactions.sendTransactionAsync(
const tx = transactionsContract.batchExecuteTransactions.awaitTransactionSuccessAsync(
[outerExecuteTransaction],
[randomSignature()],
{ from: accounts[2] },
Expand Down Expand Up @@ -344,7 +344,10 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef
ExchangeRevertErrors.TransactionErrorCode.Expired,
transactionHash,
);
const tx = transactionsContract.executeTransaction.sendTransactionAsync(transaction, randomSignature());
const tx = transactionsContract.executeTransaction.awaitTransactionSuccessAsync(
transaction,
randomSignature(),
);
return expect(tx).to.revertWith(expectedError);
});
it('should revert if the transaction is submitted with a gasPrice that does not equal the required gasPrice', async () => {
Expand All @@ -356,9 +359,13 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef
actualGasPrice,
transaction.gasPrice,
);
const tx = transactionsContract.executeTransaction.sendTransactionAsync(transaction, randomSignature(), {
gasPrice: actualGasPrice,
});
const tx = transactionsContract.executeTransaction.awaitTransactionSuccessAsync(
transaction,
randomSignature(),
{
gasPrice: actualGasPrice,
},
);
return expect(tx).to.revertWith(expectedError);
});
it('should revert if reentrancy occurs in the middle of an executeTransaction call and msg.sender != signer for both calls', async () => {
Expand All @@ -376,9 +383,13 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef
accounts[0],
).encode();
const expectedError = new ExchangeRevertErrors.TransactionExecutionError(outerTransactionHash, errorData);
const tx = transactionsContract.executeTransaction.sendTransactionAsync(outerTransaction, validSignature, {
from: accounts[1], // Different then the signing addresses
});
const tx = transactionsContract.executeTransaction.awaitTransactionSuccessAsync(
outerTransaction,
validSignature,
{
from: accounts[1], // Different then the signing addresses
},
);
return expect(tx).to.revertWith(expectedError);
});
it('should revert if reentrancy occurs in the middle of an executeTransaction call and msg.sender != signer and then msg.sender == signer', async () => {
Expand All @@ -396,9 +407,13 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef
accounts[0],
).encode();
const expectedError = new ExchangeRevertErrors.TransactionExecutionError(outerTransactionHash, errorData);
const tx = transactionsContract.executeTransaction.sendTransactionAsync(outerTransaction, validSignature, {
from: accounts[1], // Different then the signing addresses
});
const tx = transactionsContract.executeTransaction.awaitTransactionSuccessAsync(
outerTransaction,
validSignature,
{
from: accounts[1], // Different then the signing addresses
},
);
return expect(tx).to.revertWith(expectedError);
});
it('should allow reentrancy in the middle of an executeTransaction call if msg.sender == signer for both calls', async () => {
Expand All @@ -410,7 +425,7 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef
returnData: DEADBEEF_RETURN_DATA,
});
return expect(
transactionsContract.executeTransaction.sendTransactionAsync(outerTransaction, validSignature, {
transactionsContract.executeTransaction.awaitTransactionSuccessAsync(outerTransaction, validSignature, {
from: accounts[0],
}),
).to.be.fulfilled('');
Expand All @@ -424,7 +439,7 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef
returnData: DEADBEEF_RETURN_DATA,
});
return expect(
transactionsContract.executeTransaction.sendTransactionAsync(outerTransaction, validSignature, {
transactionsContract.executeTransaction.awaitTransactionSuccessAsync(outerTransaction, validSignature, {
from: accounts[0],
}),
).to.be.fulfilled('');
Expand All @@ -442,7 +457,10 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef
ExchangeRevertErrors.TransactionErrorCode.AlreadyExecuted,
transactionHash,
);
const tx = transactionsContract.executeTransaction.sendTransactionAsync(transaction, validSignature);
const tx = transactionsContract.executeTransaction.awaitTransactionSuccessAsync(
transaction,
validSignature,
);
return expect(tx).to.revertWith(expectedError);
});
it('should revert if the signer != msg.sender and the signature is not valid', async () => {
Expand All @@ -453,9 +471,13 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef
accounts[1],
INVALID_SIGNATURE,
);
const tx = transactionsContract.executeTransaction.sendTransactionAsync(transaction, INVALID_SIGNATURE, {
from: accounts[0],
});
const tx = transactionsContract.executeTransaction.awaitTransactionSuccessAsync(
transaction,
INVALID_SIGNATURE,
{
from: accounts[0],
},
);
return expect(tx).to.revertWith(expectedError);
});
it('should revert if the signer == msg.sender but the delegatecall fails', async () => {
Expand All @@ -470,9 +492,13 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef
transactionHash,
executableError.encode(),
);
const tx = transactionsContract.executeTransaction.sendTransactionAsync(transaction, randomSignature(), {
from: accounts[1],
});
const tx = transactionsContract.executeTransaction.awaitTransactionSuccessAsync(
transaction,
randomSignature(),
{
from: accounts[1],
},
);
return expect(tx).to.revertWith(expectedError);
});
it('should revert if the signer != msg.sender and the signature is valid but the delegatecall fails', async () => {
Expand All @@ -488,9 +514,13 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef
transactionHash,
executableError.encode(),
);
const tx = transactionsContract.executeTransaction.sendTransactionAsync(transaction, validSignature, {
from: accounts[0],
});
const tx = transactionsContract.executeTransaction.awaitTransactionSuccessAsync(
transaction,
validSignature,
{
from: accounts[0],
},
);
return expect(tx).to.revertWith(expectedError);
});
it('should succeed with the correct return hash and event emitted when msg.sender != signer', async () => {
Expand Down

0 comments on commit 106b171

Please sign in to comment.