Merge pull request #232 from JoinColony/fix/waitfortransactionreceipt

Add `waitForTransactionReceipt` abstraction to adapters

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## v.NEXT
 
+**Features**
+
+* `Adapter.getTransactionReceipt` now waits for an in-progress transaction to be mined before attempting to get the receipt (`@colony/colony-js-adapter-ethers`, `@colony/colony-js-client`)
+* `Adapter.getTransactionReceipt` and `Adapter.waitForTransaction` now accept a timeout argument (default: 5 minutes) (`@colony/colony-js-adapter-ethers`)
+
 **Bug fixes**
 
 * Partial empty hex strings (e.g. `0x0`) are now padded to full-length, which resolves an issue with `EthersAdapter` (`@colony/colony-js-contract-client`)

packages/colony-js-adapter-ethers/src/EthersAdapter.js

@@ -20,6 +20,8 @@ import ContractLoader from '@colony/colony-js-contract-loader';
 import type { ConstructorArgs } from './flowtypes';
 import EthersContract from './EthersContract';
 
+import { DEFAULT_TRANSACTION_WAIT_TIMEOUT } from './defaults';
+
 export default class EthersAdapter implements IAdapter {
  loader: ContractLoader;
  provider: IProvider;
@@ -54,11 +56,12 @@ export default class EthersAdapter implements IAdapter {
  this.wallet = wallet;
  }
  async getContract(query: Query): Promise<IContract> {
- const { address, abi } = await this.loader.load(query, {
- abi: true,
- address: true,
- bytecode: false,
- });
+ const { address, abi } =
+ (await this.loader.load(query, {
+ abi: true,
+ address: true,
+ bytecode: false,
+ })) || {};
 
  if (typeof address !== 'string')
  throw new Error('Unable to parse contract address');
@@ -105,11 +108,39 @@ export default class EthersAdapter implements IAdapter {
  });
  }
  }
- async getTransactionReceipt(transactionHash: string) {
- return this.provider.getTransactionReceipt(transactionHash);
+ async _getTransactionReceipt(transactionHash: string) {
+ const receipt = await this.provider.getTransactionReceipt(transactionHash);
+ if (receipt == null)
+ throw new Error(
+ `Transaction receipt not found (transaction: ${transactionHash})`,
+ );
+ return receipt;
  }
- async waitForTransaction(transactionHash: string) {
- return this.provider.waitForTransaction(transactionHash);
+ async waitForTransaction(
+ transactionHash: string,
+ timeoutMs: number = DEFAULT_TRANSACTION_WAIT_TIMEOUT,
+ ) {
+ return raceAgainstTimeout(
+ this.provider.waitForTransaction(transactionHash),
+ timeoutMs,
+ );
+ }
+ async getTransactionReceipt(transactionHash: string, timeoutMs?: number) {
+ let receipt;
+ try {
+ // Attempt to get the receipt immediately; the transaction may have
+ // already been mined, or we're running on TestRPC with no mining time.
+ receipt = await this._getTransactionReceipt(transactionHash);
+ } catch (error) {
+ // Ignore the error if the receipt wasn't found
+ if (!error.toString().includes('Transaction receipt not found'))
+ throw error;
+ }
+
+ // Wait until the transaction has been mined, then get the receipt.
+ await this.waitForTransaction(transactionHash, timeoutMs);
+ receipt = this._getTransactionReceipt(transactionHash);
+ return receipt;
  }
  /**
  * Sign a message hash (as binary) and return a split signature.

packages/colony-js-adapter-ethers/src/__tests__/EthersAdapter.test.js

@@ -28,8 +28,15 @@ jest.mock('ethers', () => {
  .fn()
  .mockReturnValue({ data: '0x123' });
  return {
+ utils: {
+ arrayify: jest.fn().mockReturnValue('bytes'),
+ splitSignature: jest.fn().mockReturnValue({ r: 'r', s: 's', v: 'v' }),
+ },
  Contract: MockContract,
  Interface: jest.fn(),
+ SigningKey: {
+ recover: jest.fn().mockReturnValue('recovered address'),
+ },
  };
 });
 
@@ -44,23 +51,22 @@ describe('EthersAdapter', () => {
 
  beforeEach(() => sandbox.clear());
 
+ jest.useFakeTimers();
+
  const mockLoader = {
- load: jest
- .fn()
- .mockReturnValue(Promise.resolve({ address, abi, bytecode })),
+ load: sandbox.fn().mockResolvedValue({ address, abi, bytecode }),
  };
  const mockProvider = {
- on: jest.fn(),
- resolveName: jest.fn(),
- getTransactionReceipt: jest.fn().mockImplementation(txHash => ({
- transactionHash: txHash,
- dummyReceipt: true,
- })),
+ on: sandbox.fn(),
+ resolveName: sandbox.fn(),
+ getTransactionReceipt: sandbox.fn(),
+ waitForTransaction: sandbox.fn(),
  };
 
  const mockWallet = {
- on: jest.fn(),
- resolveName: jest.fn(),
+ on: sandbox.fn(),
+ resolveName: sandbox.fn(),
+ signMessage: sandbox.fn(),
  };
 
  const adapter = new EthersAdapter({
@@ -74,17 +80,39 @@ describe('EthersAdapter', () => {
  });
 
  test('Adapter calls Contract with correct arguments', async () => {
- sandbox.spyOn(adapter.loader, 'load');
- const query = { name: 'myContractName' };
-
- const contract = await adapter.getContract(query);
-
- expect(adapter.loader.load).toHaveBeenCalledTimes(1);
- expect(adapter.loader.load).toHaveBeenCalledWith(query, {
+ const query = {
+ contractName: 'MyContract',
+ };
+ const defaults = {
  abi: true,
  address: true,
  bytecode: false,
+ };
+
+ // If the address is missing
+ adapter.loader.load.mockResolvedValueOnce({
+ address: null,
+ abi,
+ bytecode,
  });
+ try {
+ await adapter.getContract(query);
+ expect(false).toBe(true); // Should be unreachable
+ } catch (error) {
+ expect(error.toString()).toMatch('Unable to parse contract address');
+ expect(adapter.loader.load).toHaveBeenCalledWith(query, defaults);
+ }
+ adapter.loader.load.mockReset();
+
+ // If the address is found
+ adapter.loader.load.mockResolvedValue({
+ address,
+ abi,
+ bytecode,
+ });
+ const contract = await adapter.getContract(query);
+ expect(adapter.loader.load).toHaveBeenCalledTimes(1);
+ expect(adapter.loader.load).toHaveBeenCalledWith(query, defaults);
  expect(contract).toBeInstanceOf(EthersContract);
  expect(JSON.stringify(contract.provider)).toBe(
  JSON.stringify(adapter.provider),
@@ -217,16 +245,30 @@ describe('EthersAdapter', () => {
  expect(contract.removeListener).toHaveBeenCalledTimes(4);
  });
 
- test('getTransactionReceipt', async () => {
- const receipt = await adapter.getTransactionReceipt(transactionHash);
- expect(receipt).toMatchObject({ transactionHash, dummyReceipt: true });
- expect(mockProvider.getTransactionReceipt).toHaveBeenCalledTimes(1);
+ test('Getting a transaction receipt', async () => {
+ const receipt = { hash: transactionHash };
+
+ try {
+ mockProvider.getTransactionReceipt.mockResolvedValueOnce(null);
+ await adapter._getTransactionReceipt(transactionHash);
+ expect(false).toBe(true); // Should be unreachable
+ } catch (error) {
+ expect(error.toString()).toMatch('Transaction receipt not found');
+ expect(mockProvider.getTransactionReceipt).toHaveBeenCalledWith(
+ transactionHash,
+ );
+ mockProvider.getTransactionReceipt.mockReset();
+ }
+
+ mockProvider.getTransactionReceipt.mockResolvedValueOnce(receipt);
+ const receivedValue = await adapter._getTransactionReceipt(transactionHash);
+ expect(receivedValue).toEqual(receipt);
  expect(mockProvider.getTransactionReceipt).toHaveBeenCalledWith(
  transactionHash,
  );
  });
 
- test('getContractDeployTransaction', async () => {
+ test('Getting a contract deployment transaction', async () => {
  sandbox.spyOn(adapter.loader, 'load');
 
  const query = {
@@ -253,4 +295,64 @@ describe('EthersAdapter', () => {
  ...contractArgs,
  );
  });
+
+ test('Waiting for a transaction receipt', async () => {
+ const transaction = { hash: transactionHash };
+ const receipt = { hash: transactionHash };
+
+ // For this test, simulate that we couldn't get the receipt on the first
+ // try (i.e. it has not yet been mined).
+ mockProvider.getTransactionReceipt
+ .mockResolvedValueOnce(null)
+ .mockResolvedValueOnce(receipt);
+
+ mockProvider.waitForTransaction.mockResolvedValue(transaction);
+
+ const receivedValue = await adapter.getTransactionReceipt(transactionHash);
+
+ expect(mockProvider.getTransactionReceipt).toHaveBeenCalledTimes(2);
+ expect(mockProvider.getTransactionReceipt).toHaveBeenCalledWith(
+ transactionHash,
+ );
+ expect(mockProvider.waitForTransaction).toHaveBeenCalledWith(
+ transactionHash,
+ );
+ expect(receivedValue).toEqual(receipt);
+
+ // For this try, let's make getTransactionReceipt fail in a cool
+ // and interesting way
+ sandbox
+ .spyOn(adapter, 'getTransactionReceipt')
+ .mockImplementationOnce(async () => {
+ throw new Error('Kaboom!');
+ });
+ try {
+ await adapter.getTransactionReceipt(transactionHash);
+ expect(false).toBe(true); // Should be unreachable
+ } catch (error) {
+ expect(error.toString()).toMatch('Kaboom!');
+ }
+ sandbox.restore(adapter, 'getTransactionReceipt');
+ });
+
+ test('Signing a message', async () => {
+ const signature = 'signature';
+ const messageHash = '0x123';
+
+ adapter.wallet.signMessage.mockResolvedValue(signature);
+ const splitSignature = await adapter.signMessage(messageHash);
+
+ expect(splitSignature).toEqual({ sigR: 'r', sigS: 's', sigV: 'v' });
+ expect(ethers.utils.arrayify).toHaveBeenCalledWith(messageHash);
+ expect(ethers.utils.splitSignature).toHaveBeenCalledWith(signature);
+ expect(adapter.wallet.signMessage).toHaveBeenCalledWith('bytes');
+ });
+
+ test('Recovering an address from a digest/signature', () => {
+ const digest = [0, 1, 2, 3];
+ const signature = { sigR: 'r', sigS: 's', sigV: 28 };
+ const recoveredAddress = adapter.ecRecover(digest, signature);
+ expect(recoveredAddress).toBe('recovered address');
+ expect(ethers.SigningKey.recover).toHaveBeenCalledWith(digest, 'r', 's', 1);
+ });
 });

packages/colony-js-adapter-ethers/src/defaults.js

@@ -0,0 +1,4 @@
+/* @flow */
+
+// eslint-disable-next-line import/prefer-default-export
+export const DEFAULT_TRANSACTION_WAIT_TIMEOUT = 60 * 5 * 1000; // 5 mins

packages/colony-js-adapter/interface/Adapter.js

@@ -25,7 +25,13 @@ export interface Adapter {
  timeoutMs: number,
  transactionHash: string,
  }): Promise<any>;
- waitForTransaction(transactionHash: string): Promise<Transaction>;
- getTransactionReceipt(transactionHash: string): Promise<TransactionReceipt>;
+ waitForTransaction(
+ transactionHash: string,
+ timeoutMs?: number,
+ ): Promise<Transaction>;
+ getTransactionReceipt(
+ transactionHash: string,
+ timeoutMs?: number,
+ ): Promise<TransactionReceipt>;
  signMessage(messageHash: string): Promise<Signature>;
 }

packages/colony-js-client/src/ColonyNetworkClient/index.js

@@ -242,10 +242,6 @@ export default class ColonyNetworkClient extends ContractClient {
  [utf8ToHex(name), utf8ToHex(symbol), decimals],
  );
  const { hash } = await this.adapter.wallet.sendTransaction(transaction);
- const receipt = await this.adapter.getTransactionReceipt(hash);
- if (receipt != null) return receipt.contractAddress;
-
- await this.adapter.waitForTransaction(hash);
  const { contractAddress } = await this.adapter.getTransactionReceipt(hash);
  return contractAddress;
  }

packages/colony-js-contract-client/src/__tests__/ContractMethodSender.js

@@ -56,7 +56,6 @@ describe('ContractMethodSender', () => {
  };
  const adapter = {
  getTransactionReceipt: sandbox.fn(),
- waitForTransaction: sandbox.fn(),
  provider: {
  name: 'mainnet',
  },
@@ -160,7 +159,7 @@ describe('ContractMethodSender', () => {
  eventHandlers,
  });
  sandbox
- .spyOn(method, '_waitForTransactionReceipt')
+ .spyOn(method.client.adapter, 'getTransactionReceipt')
  .mockResolvedValue(receipt);
 
  const response = await method._sendWithWaitingForMining(
@@ -189,7 +188,7 @@ describe('ContractMethodSender', () => {
  timeoutMs: options.timeoutMs,
  transactionHash: transaction.hash,
  });
- expect(method._waitForTransactionReceipt).toHaveBeenCalledWith(
+ expect(method.client.adapter.getTransactionReceipt).toHaveBeenCalledWith(
  transaction.hash,
  );
  });
@@ -202,7 +201,7 @@ describe('ContractMethodSender', () => {
  eventHandlers,
  });
  sandbox
- .spyOn(method, '_waitForTransactionReceipt')
+ .spyOn(method.client.adapter, 'getTransactionReceipt')
  .mockImplementation(async () => receipt);
 
  const response = method._sendWithoutWaitingForMining(
@@ -226,38 +225,9 @@ describe('ContractMethodSender', () => {
  timeoutMs: options.timeoutMs,
  transactionHash: transaction.hash,
  });
- expect(method._waitForTransactionReceipt).toHaveBeenCalledWith(
- transaction.hash,
- );
- });
-
- test('Waiting for transaction receipt', async () => {
- const method = new ContractMethodSender({
- client,
- input,
- functionName,
- eventHandlers,
- });
- const hash = 'the tx hash';
-
- // For this test, simulate that we couldn't get the receipt on the first
- // try (i.e. it has not yet been mined).
- method.client.adapter.getTransactionReceipt
- .mockResolvedValueOnce(null)
- .mockResolvedValueOnce(receipt);
-
- method.client.adapter.waitForTransaction.mockResolvedValue(transaction);
-
- const txReceipt = await method._waitForTransactionReceipt(hash);
-
- expect(method.client.adapter.getTransactionReceipt).toHaveBeenCalledTimes(
- 2,
- );
  expect(method.client.adapter.getTransactionReceipt).toHaveBeenCalledWith(
- hash,
+ transaction.hash,
  );
- expect(method.client.adapter.waitForTransaction).toHaveBeenCalledWith(hash);
- expect(txReceipt).toEqual(receipt);
  });
 
  test('Default send options', () => {