diff --git a/contracts/exchange/src/artifacts.ts b/contracts/exchange/src/artifacts.ts index a5551815fd..72d4f1c186 100644 --- a/contracts/exchange/src/artifacts.ts +++ b/contracts/exchange/src/artifacts.ts @@ -14,11 +14,11 @@ import * as IExchange from '../generated-artifacts/IExchange.json'; import * as IExchangeCore from '../generated-artifacts/IExchangeCore.json'; import * as IMatchOrders from '../generated-artifacts/IMatchOrders.json'; import * as ISignatureValidator from '../generated-artifacts/ISignatureValidator.json'; +import * as IsolatedExchange from '../generated-artifacts/IsolatedExchange.json'; import * as ITransactions from '../generated-artifacts/ITransactions.json'; import * as ITransferSimulator from '../generated-artifacts/ITransferSimulator.json'; import * as IWallet from '../generated-artifacts/IWallet.json'; import * as IWrapperFunctions from '../generated-artifacts/IWrapperFunctions.json'; -import * as IsolatedExchange from '../generated-artifacts/IsolatedExchange.json'; import * as LibExchangeRichErrorDecoder from '../generated-artifacts/LibExchangeRichErrorDecoder.json'; import * as MixinAssetProxyDispatcher from '../generated-artifacts/MixinAssetProxyDispatcher.json'; import * as MixinExchangeCore from '../generated-artifacts/MixinExchangeCore.json'; diff --git a/contracts/exchange/test/transactions.ts b/contracts/exchange/test/transactions.ts index f5a0e8dd1f..140e2868de 100644 --- a/contracts/exchange/test/transactions.ts +++ b/contracts/exchange/test/transactions.ts @@ -43,7 +43,7 @@ import { const artifacts = { ...erc20Artifacts, ...localArtifacts }; // tslint:disable:no-unnecessary-type-assertion -blockchainTests.resets('Exchange transactions', env => { +blockchainTests.resets.only('Exchange transactions', env => { let chainId: number; let senderAddress: string; let owner: string; @@ -349,9 +349,9 @@ blockchainTests.resets('Exchange transactions', env => { const recursiveTransactionHashHex = transactionHashUtils.getTransactionHashHex( recursiveTransaction, ); - const noReentrancyError = new ExchangeRevertErrors.TransactionError( - ExchangeRevertErrors.TransactionErrorCode.NoReentrancy, + const noReentrancyError = new ExchangeRevertErrors.TransactionInvalidContextError( transactionHashHex, + transaction.signerAddress, ).encode(); const expectedError = new ExchangeRevertErrors.TransactionExecutionError( recursiveTransactionHashHex, diff --git a/contracts/exchange/test/transactions_unit_tests.ts b/contracts/exchange/test/transactions_unit_tests.ts index 6e198c52b6..fab0ee0851 100644 --- a/contracts/exchange/test/transactions_unit_tests.ts +++ b/contracts/exchange/test/transactions_unit_tests.ts @@ -357,28 +357,6 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef }); return expect(tx).to.revertWith(expectedError); }); - // FIXME - This should be unskipped when the contracts have been updated to fix this problem. - it.skip('should revert if reentrancy occurs in the middle of an executeTransaction call and msg.sender == signer for both calls', async () => { - const validSignature = randomSignature(); - const transaction1 = await generateZeroExTransactionAsync({ signerAddress: accounts[0] }); - const transactionHash1 = transactionHashUtils.getTransactionHashHex(transaction1); - const transaction2 = await generateZeroExTransactionAsync({ - signerAddress: accounts[0], - callData: getExecuteTransactionCallData(transaction1, validSignature), - returnData: '0xdeadbeef', - }); - const transactionHash2 = transactionHashUtils.getTransactionHashHex(transaction2); - const abiEncoder = AbiEncoder.createMethod('TransactionError', ['uint8', 'bytes32']); - const errorData = abiEncoder.encode([ - ExchangeRevertErrors.TransactionErrorCode.NoReentrancy, - transactionHash1, - ]); - const expectedError = new ExchangeRevertErrors.TransactionExecutionError(transactionHash2, errorData); - const tx = transactionsContract.executeTransaction.sendTransactionAsync(transaction2, validSignature, { - from: accounts[0], - }); - 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 () => { const validSignature = randomSignature(); const transaction1 = await generateZeroExTransactionAsync({ signerAddress: accounts[0] }); @@ -389,11 +367,10 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef returnData: '0xdeadbeef', }); const transactionHash2 = transactionHashUtils.getTransactionHashHex(transaction2); - const abiEncoder = AbiEncoder.createMethod('TransactionError', ['uint8', 'bytes32']); - const errorData = abiEncoder.encode([ - ExchangeRevertErrors.TransactionErrorCode.NoReentrancy, + const errorData = new ExchangeRevertErrors.TransactionInvalidContextError( transactionHash1, - ]); + accounts[0], + ).encode(); const expectedError = new ExchangeRevertErrors.TransactionExecutionError(transactionHash2, errorData); const tx = transactionsContract.executeTransaction.sendTransactionAsync(transaction2, validSignature, { from: accounts[1], // Different then the signing addresses @@ -410,33 +387,10 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef returnData: '0xdeadbeef', }); const transactionHash2 = transactionHashUtils.getTransactionHashHex(transaction2); - const abiEncoder = AbiEncoder.createMethod('TransactionError', ['uint8', 'bytes32']); - const errorData = abiEncoder.encode([ - ExchangeRevertErrors.TransactionErrorCode.NoReentrancy, + const errorData = new ExchangeRevertErrors.TransactionInvalidContextError( transactionHash1, - ]); - const expectedError = new ExchangeRevertErrors.TransactionExecutionError(transactionHash2, errorData); - const tx = transactionsContract.executeTransaction.sendTransactionAsync(transaction2, validSignature, { - from: accounts[1], // Different then the signing addresses - }); - return expect(tx).to.revertWith(expectedError); - }); - // FIXME - This should be unskipped when the contracts have been updated to fix this problem. - it.skip('should revert if reentrancy occurs in the middle of an executeTransaction call and msg.sender == signer and then msg.sender != sender', async () => { - const validSignature = randomSignature(); - const transaction1 = await generateZeroExTransactionAsync({ signerAddress: accounts[0] }); - const transactionHash1 = transactionHashUtils.getTransactionHashHex(transaction1); - const transaction2 = await generateZeroExTransactionAsync({ - signerAddress: accounts[0], - callData: getExecuteTransactionCallData(transaction1, validSignature), - returnData: '0xdeadbeef', - }); - const transactionHash2 = transactionHashUtils.getTransactionHashHex(transaction2); - const abiEncoder = AbiEncoder.createMethod('TransactionError', ['uint8', 'bytes32']); - const errorData = abiEncoder.encode([ - ExchangeRevertErrors.TransactionErrorCode.NoReentrancy, - transactionHash1, - ]); + accounts[0], + ).encode(); const expectedError = new ExchangeRevertErrors.TransactionExecutionError(transactionHash2, errorData); const tx = transactionsContract.executeTransaction.sendTransactionAsync(transaction2, validSignature, { from: accounts[1], // Different then the signing addresses @@ -598,10 +552,7 @@ blockchainTests.resets('Transaction Unit Tests', ({ provider, web3Wrapper, txDef await transactionsContract.setCurrentContextAddress.awaitTransactionSuccessAsync(accounts[0]); const transaction = await generateZeroExTransactionAsync(); const transactionHash = transactionHashUtils.getTransactionHashHex(transaction); - const expectedError = new ExchangeRevertErrors.TransactionError( - ExchangeRevertErrors.TransactionErrorCode.NoReentrancy, - transactionHash, - ); + const expectedError = new ExchangeRevertErrors.TransactionInvalidContextError(transactionHash, accounts[0]); expect( transactionsContract.assertExecutableTransaction.callAsync(transaction, randomSignature()), ).to.revertWith(expectedError);