Handle revert with reason when decoding call result

We use in-process ganache which throws on an RPC error. This means all of our tests get a nice revert error thrown when testing against ganache. This is not possible with RPC providers and a revert with reason result is returned. Our callAsync doesn't handle this and attempts to decode the revert with reason error log as a successful log, which results in an error while decoding.

This only works with our fork of ethers https://github.com/ethers-io/ethers.js/pull/188 and will need to be re-worked when updating to Ethers.js 4

packages/contract-wrappers/test/revert_validation_test.ts

@@ -0,0 +1,122 @@
+import { BlockchainLifecycle, devConstants, web3Factory } from '@0xproject/dev-utils';
+import { FillScenarios } from '@0xproject/fill-scenarios';
+import { runV2MigrationsAsync } from '@0xproject/migrations';
+import { assetDataUtils } from '@0xproject/order-utils';
+import { SignedOrder } from '@0xproject/types';
+import { BigNumber } from '@0xproject/utils';
+import { Web3Wrapper } from '@0xproject/web3-wrapper';
+import * as chai from 'chai';
+import 'mocha';
+
+import { ContractWrappers } from '../src';
+
+import { chaiSetup } from './utils/chai_setup';
+import { constants } from './utils/constants';
+import { tokenUtils } from './utils/token_utils';
+
+chaiSetup.configure();
+const expect = chai.expect;
+
+describe('Revert Validation ExchangeWrapper', () => {
+    let contractWrappers: ContractWrappers;
+    let userAddresses: string[];
+    let zrxTokenAddress: string;
+    let fillScenarios: FillScenarios;
+    let exchangeContractAddress: string;
+    let makerTokenAddress: string;
+    let takerTokenAddress: string;
+    let coinbase: string;
+    let makerAddress: string;
+    let anotherMakerAddress: string;
+    let takerAddress: string;
+    let makerAssetData: string;
+    let takerAssetData: string;
+    let feeRecipient: string;
+    let txHash: string;
+    let blockchainLifecycle: BlockchainLifecycle;
+    let web3Wrapper: Web3Wrapper;
+    const fillableAmount = new BigNumber(5);
+    const takerTokenFillAmount = new BigNumber(5);
+    let signedOrder: SignedOrder;
+    const config = {
+        networkId: constants.TESTRPC_NETWORK_ID,
+        blockPollingIntervalMs: 0,
+    };
+    before(async () => {
+        // vmErrorsOnRPCResponse is useful for quick feedback and testing during development
+        // but is not the default behaviour in production. Here we ensure our failure cases
+        // are handled in an environment which behaves similar to production
+        const provider = web3Factory.getRpcProvider({
+            shouldUseInProcessGanache: true,
+            shouldThrowErrorsOnGanacheRPCResponse: false,
+        });
+        web3Wrapper = new Web3Wrapper(provider);
+        blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
+        const txDefaults = {
+            gas: devConstants.GAS_LIMIT,
+            from: devConstants.TESTRPC_FIRST_ADDRESS,
+        };
+        const artifactsDir = `src/artifacts`;
+        // Re-deploy the artifacts in this provider, rather than in the default provider exposed in
+        // the beforeAll hook. This is due to the fact that the default provider enabled vmErrorsOnRPCResponse
+        // and we are explicity testing with vmErrorsOnRPCResponse disabled.
+        await runV2MigrationsAsync(provider, artifactsDir, txDefaults);
+        await blockchainLifecycle.startAsync();
+        contractWrappers = new ContractWrappers(provider, config);
+        exchangeContractAddress = contractWrappers.exchange.getContractAddress();
+        userAddresses = await web3Wrapper.getAvailableAddressesAsync();
+        zrxTokenAddress = tokenUtils.getProtocolTokenAddress();
+        fillScenarios = new FillScenarios(
+            provider,
+            userAddresses,
+            zrxTokenAddress,
+            exchangeContractAddress,
+            contractWrappers.erc20Proxy.getContractAddress(),
+            contractWrappers.erc721Proxy.getContractAddress(),
+        );
+        [coinbase, makerAddress, takerAddress, feeRecipient, anotherMakerAddress] = userAddresses;
+        [makerTokenAddress, takerTokenAddress] = tokenUtils.getDummyERC20TokenAddresses();
+        [makerAssetData, takerAssetData] = [
+            assetDataUtils.encodeERC20AssetData(makerTokenAddress),
+            assetDataUtils.encodeERC20AssetData(takerTokenAddress),
+        ];
+        signedOrder = await fillScenarios.createFillableSignedOrderAsync(
+            makerAssetData,
+            takerAssetData,
+            makerAddress,
+            takerAddress,
+            fillableAmount,
+        );
+    });
+    after(async () => {
+        await blockchainLifecycle.revertAsync();
+    });
+    beforeEach(async () => {
+        await blockchainLifecycle.startAsync();
+    });
+    afterEach(async () => {
+        await blockchainLifecycle.revertAsync();
+    });
+    describe('#fillOrderAsync', () => {
+        it('should throw the revert reason when shouldValidate is true and a fill would revert', async () => {
+            // Create a scenario where the fill will revert
+            const makerTokenBalance = await contractWrappers.erc20Token.getBalanceAsync(
+                makerTokenAddress,
+                makerAddress,
+            );
+            // Transfer all of the tokens from maker to create a failure scenario
+            txHash = await contractWrappers.erc20Token.transferAsync(
+                makerTokenAddress,
+                makerAddress,
+                takerAddress,
+                makerTokenBalance,
+            );
+            await web3Wrapper.awaitTransactionSuccessAsync(txHash, constants.AWAIT_TRANSACTION_MINED_MS);
+            expect(
+                contractWrappers.exchange.fillOrderAsync(signedOrder, takerTokenFillAmount, takerAddress, {
+                    shouldValidate: true,
+                }),
+            ).to.be.rejectedWith('TRANSFER_FAILED');
+        });
+    });
+});

packages/contract-wrappers/test/transaction_encoder_test.ts

@@ -1,6 +1,6 @@
 import { BlockchainLifecycle } from '@0xproject/dev-utils';
 import { FillScenarios } from '@0xproject/fill-scenarios';
-import { assetDataUtils, signatureUtils, generatePseudoRandomSalt, orderHashUtils } from '@0xproject/order-utils';
+import { assetDataUtils, generatePseudoRandomSalt, orderHashUtils, signatureUtils } from '@0xproject/order-utils';
 import { SignedOrder, SignerType } from '@0xproject/types';
 import { BigNumber } from '@0xproject/utils';
 import 'mocha';

packages/dev-utils/src/web3_factory.ts

@@ -14,6 +14,7 @@ import { env, EnvVars } from './env';
 export interface Web3Config {
     hasAddresses?: boolean; // default: true
     shouldUseInProcessGanache?: boolean; // default: false
+    shouldThrowErrorsOnGanacheRPCResponse?: boolean; // default: true
     rpcUrl?: string; // default: localhost:8545
     shouldUseFakeGasEstimate?: boolean; // default: true
 }
@@ -41,15 +42,19 @@ export const web3Factory = {
             if (!_.isUndefined(config.rpcUrl)) {
                 throw new Error('Cannot use both GanacheSubrovider and RPCSubprovider');
             }
+            const shouldThrowErrorsOnGanacheRPCResponse =
+                _.isUndefined(config.shouldThrowErrorsOnGanacheRPCResponse) ||
+                config.shouldThrowErrorsOnGanacheRPCResponse;
             provider.addProvider(
                 new GanacheSubprovider({
+                    vmErrorsOnRPCResponse: shouldThrowErrorsOnGanacheRPCResponse,
                     gasLimit: constants.GAS_LIMIT,
                     logger,
                     verbose: env.parseBoolean(EnvVars.VerboseGanache),
                     port: 8545,
                     network_id: 50,
                     mnemonic: 'concert load couple harbor equip island argue ramp clarify fence smart topic',
-                }),
+                } as any), // TODO remove any once types are merged in DefinitelyTyped
             );
         } else {
             provider.addProvider(new RPCSubprovider(config.rpcUrl || constants.RPC_URL));