From fc3058c1e2fdf9a11eedd3d4c775d54fbf61b6c9 Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Mon, 5 Feb 2018 14:27:58 -0800 Subject: Remove re-fetch of transaction count on error --- packages/subproviders/CHANGELOG.md | 2 +- packages/subproviders/src/subproviders/ledger.ts | 2 + .../subproviders/src/subproviders/nonce_tracker.ts | 56 ++++++++++------------ .../subproviders/src/subproviders/redundant_rpc.ts | 1 + packages/subproviders/src/types.ts | 13 +++++ 5 files changed, 41 insertions(+), 33 deletions(-) diff --git a/packages/subproviders/CHANGELOG.md b/packages/subproviders/CHANGELOG.md index 512e8ce8b..8adaa1c08 100644 --- a/packages/subproviders/CHANGELOG.md +++ b/packages/subproviders/CHANGELOG.md @@ -2,7 +2,7 @@ ## v0.4.1 - _Febuary 2, 2018_ - * Added NonceTrackerSubprovider + * Added NonceTrackerSubprovider (#355) ## v0.4.0 - _January 28, 2018_ diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts index 7267a793e..5966a88bb 100644 --- a/packages/subproviders/src/subproviders/ledger.ts +++ b/packages/subproviders/src/subproviders/ledger.ts @@ -60,6 +60,8 @@ export class LedgerSubprovider extends Subprovider { public setPathIndex(pathIndex: number) { this._derivationPathIndex = pathIndex; } + // Required to implement this public interface which doesn't conform to our linting rule. + // tslint:disable-next-line:async-suffix public async handleRequest( payload: Web3.JSONRPCRequestPayload, next: () => void, diff --git a/packages/subproviders/src/subproviders/nonce_tracker.ts b/packages/subproviders/src/subproviders/nonce_tracker.ts index a1e499629..2f94ea581 100644 --- a/packages/subproviders/src/subproviders/nonce_tracker.ts +++ b/packages/subproviders/src/subproviders/nonce_tracker.ts @@ -4,42 +4,49 @@ import EthereumTx = require('ethereumjs-tx'); import ethUtil = require('ethereumjs-util'); import providerEngineUtils = require('web3-provider-engine/util/rpc-cache-utils'); -import { JSONRPCPayload } from '../types'; +import { + BlockParamLiteral, + ErrorCallback, + JSONRPCPayload, + NonceSubproviderErrors, + OptionalNextCallback, +} from '../types'; import { Subprovider } from './subprovider'; const NONCE_TOO_LOW_ERROR_MESSAGE = 'Transaction nonce is too low'; - -export type OptionalNextCallback = (callback?: (err: Error | null, result: any, cb: any) => void) => void; -export type ErrorCallback = (err: Error | null, data?: any) => void; - export class NonceTrackerSubprovider extends Subprovider { private _nonceCache: { [address: string]: string } = {}; private static _reconstructTransaction(payload: JSONRPCPayload): EthereumTx { const raw = payload.params[0]; if (_.isUndefined(raw)) { - throw new Error('Invalid transaction: empty parameters'); + throw new Error(NonceSubproviderErrors.EmptyParametersFound); } const rawData = ethUtil.toBuffer(raw); - return new EthereumTx(rawData); + const transaction = new EthereumTx(rawData); + return transaction; } private static _determineAddress(payload: JSONRPCPayload): string { + let address: string; switch (payload.method) { case 'eth_getTransactionCount': - return payload.params[0].toLowerCase(); + address = payload.params[0].toLowerCase(); + return address; case 'eth_sendRawTransaction': const transaction = NonceTrackerSubprovider._reconstructTransaction(payload); - return `0x${transaction.getSenderAddress().toString('hex')}`.toLowerCase(); + address = `0x${transaction.getSenderAddress().toString('hex')}`.toLowerCase(); + return address; default: - throw new Error(`Invalid Method: ${payload.method}`); + throw new Error(NonceSubproviderErrors.CannotDetermineAddressFromPayload); } } + // Required to implement this public interface which doesn't conform to our linting rule. // tslint:disable-next-line:async-suffix public async handleRequest(payload: JSONRPCPayload, next: OptionalNextCallback, end: ErrorCallback): Promise { switch (payload.method) { case 'eth_getTransactionCount': - const blockTag = providerEngineUtils.blockTagForPayload(payload); - if (!_.isNull(blockTag) && blockTag === 'pending') { + const requestDefaultBlock = providerEngineUtils.blockTagForPayload(payload); + if (requestDefaultBlock === BlockParamLiteral.Pending) { const address = NonceTrackerSubprovider._determineAddress(payload); const cachedResult = this._nonceCache[address]; if (!_.isUndefined(cachedResult)) { @@ -56,11 +63,11 @@ export class NonceTrackerSubprovider extends Subprovider { return next(); } case 'eth_sendRawTransaction': - return next(async (sendTransactionError: Error | null, txResult: any, cb: any) => { + return next((sendTransactionError: Error | null, txResult: any, cb: any) => { if (_.isNull(sendTransactionError)) { this._handleSuccessfulTransaction(payload); } else { - await this._handleSendTransactionErrorAsync(payload, sendTransactionError); + this._handleSendTransactionError(payload, sendTransactionError); } cb(); }); @@ -81,25 +88,10 @@ export class NonceTrackerSubprovider extends Subprovider { nextHexNonce = `0x${nextHexNonce}`; this._nonceCache[address] = nextHexNonce; } - private async _handleSendTransactionErrorAsync(payload: JSONRPCPayload, err: Error): Promise { + private _handleSendTransactionError(payload: JSONRPCPayload, err: Error): void { const address = NonceTrackerSubprovider._determineAddress(payload); - if (this._nonceCache[address]) { - if (_.includes(err.message, NONCE_TOO_LOW_ERROR_MESSAGE)) { - await this._handleNonceTooLowErrorAsync(address); - } - } - } - private async _handleNonceTooLowErrorAsync(address: string): Promise { - const oldNonceInt = ethUtil.bufferToInt(new Buffer(this._nonceCache[address], 'hex')); - delete this._nonceCache[address]; - const nonceResult = await this.emitPayloadAsync({ - method: 'eth_getTransactionCount', - params: [address, 'pending'], - }); - const nonce = nonceResult.result; - const latestNonceInt = ethUtil.bufferToInt(new Buffer(nonce, 'hex')); - if (latestNonceInt > oldNonceInt) { - this._nonceCache[address] = nonce; + if (this._nonceCache[address] && _.includes(err.message, NONCE_TOO_LOW_ERROR_MESSAGE)) { + delete this._nonceCache[address]; } } } diff --git a/packages/subproviders/src/subproviders/redundant_rpc.ts b/packages/subproviders/src/subproviders/redundant_rpc.ts index a3cb463a8..5a94f93d7 100644 --- a/packages/subproviders/src/subproviders/redundant_rpc.ts +++ b/packages/subproviders/src/subproviders/redundant_rpc.ts @@ -35,6 +35,7 @@ export class RedundantRPCSubprovider extends Subprovider { }); }); } + // Required to implement this public interface which doesn't conform to our linting rule. // tslint:disable-next-line:async-suffix public async handleRequest( payload: JSONRPCPayload, diff --git a/packages/subproviders/src/types.ts b/packages/subproviders/src/types.ts index 3db8be943..86b118767 100644 --- a/packages/subproviders/src/types.ts +++ b/packages/subproviders/src/types.ts @@ -112,3 +112,16 @@ export enum LedgerSubproviderErrors { SenderInvalidOrNotSupplied = 'SENDER_INVALID_OR_NOT_SUPPLIED', MultipleOpenConnectionsDisallowed = 'MULTIPLE_OPEN_CONNECTIONS_DISALLOWED', } + +export enum NonceSubproviderErrors { + EmptyParametersFound = 'EMPTY_PARAMETERS_FOUND', + CannotDetermineAddressFromPayload = 'CANNOT_DETERMINE_ADDRESS_FROM_PAYLOAD', +} + +// Re-defined BlockParamLiteral here, rather than import it from 0x.js. +export enum BlockParamLiteral { + Pending = 'pending', +} + +export type OptionalNextCallback = (callback?: (err: Error | null, result: any, cb: any) => void) => void; +export type ErrorCallback = (err: Error | null, data?: any) => void; -- cgit v1.2.3