Skip to content

Commit

Permalink
Handling the error when eth_estimateGas fails (#920)
Browse files Browse the repository at this point in the history 
When we did a transaction with binance smart chain and the dapp doesn't send a gas value, we were not being able to execute the eth_estimateGas, it was falling do a general RPC error.

Handle the error like the extension does it. Added a value to the Transaction object to be filled with an error if the eth_estimateGas fails.
  • Loading branch information
@tommasini
tommasini authored Oct 11, 2022
1 parent c7d670c commit 4fec100
Show file tree
Hide file tree
Showing 3 changed files with 190 additions and 22 deletions.
4 changes: 3 additions & 1 deletion src/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ export const ERC1155_TOKEN_RECEIVER_INTERFACE_ID = '0x4e2312e0';
// UNITS
export const GWEI = 'gwei';

// TRANSACTION CONTROLLER ERRORS
export const ESTIMATE_GAS_ERROR = 'eth_estimateGas rpc method error';

// ASSET TYPES
export const ASSET_TYPES = {
NATIVE: 'NATIVE',
Expand All @@ -40,7 +43,6 @@ export const TESTNET_TICKER_SYMBOLS = {
ROPSTEN: 'RopstenETH',
KOVAN: 'KovanETH',
};

// TYPED NetworkType TICKER SYMBOLS
export const TESTNET_NETWORK_TYPE_TO_TICKER_SYMBOL: {
[K in NetworkType]: string;
Expand Down
182 changes: 168 additions & 14 deletions src/transaction/TransactionController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
NetworkType,
NetworkState,
} from '../network/NetworkController';
import { ESTIMATE_GAS_ERROR } from '../constants';
import {
TransactionController,
TransactionStatus,
Expand All @@ -22,15 +23,22 @@ import {
const globalAny: any = global;

const mockFlags: { [key: string]: any } = {
estimateGas: null,
estimateGasError: null,
estimateGasValue: null,
getBlockByNumberValue: null,
};

jest.mock('eth-query', () =>
jest.fn().mockImplementation(() => {
return {
estimateGas: (_transaction: any, callback: any) => {
if (mockFlags.estimateGas) {
callback(new Error(mockFlags.estimateGas));
if (mockFlags.estimateGasError) {
callback(new Error(mockFlags.estimateGasError));
return;
}

if (mockFlags.estimateGasValue) {
callback(undefined, mockFlags.estimateGasValue);
return;
}
callback(undefined, '0x0');
Expand All @@ -43,6 +51,10 @@ jest.mock('eth-query', () =>
_fetchTxs: boolean,
callback: any,
) => {
if (mockFlags.getBlockByNumberValue) {
callback(undefined, { gasLimit: '0x12a05f200' });
return;
}
callback(undefined, { gasLimit: '0x0' });
},
getCode: (_to: any, callback: any) => {
Expand Down Expand Up @@ -354,6 +366,118 @@ describe('TransactionController', () => {
});
});

it('should on eth_estimateGas succeed when gasBn it is greater than maxGasBN', async () => {
const controller = new TransactionController({
getNetworkState: () => MOCK_NETWORK.state,
onNetworkStateChange: MOCK_NETWORK.subscribe,
getProvider: MOCK_NETWORK.getProvider,
});
mockFlags.estimateGasValue = '0x12a05f200';
const from = '0x4579d0ad79bfbdf4539a1ddf5f10b378d724a34c';
const result = await controller.estimateGas({ from, to: from });

expect(result.estimateGasError).toBeUndefined();
});

it('should on Mainnet eth_estimateGas succeed when gasBn it is higher than maxGasBN', async () => {
const controller = new TransactionController({
getNetworkState: () => MOCK_NETWORK.state,
onNetworkStateChange: MOCK_NETWORK.subscribe,
getProvider: MOCK_NETWORK.getProvider,
});
mockFlags.estimateGasValue = '0x12a05f200';
mockFlags.estimateGasError = ESTIMATE_GAS_ERROR;

const from = '0x4579d0ad79bfbdf4539a1ddf5f10b378d724a34c';
const result = await controller.estimateGas({ from, to: from });

expect(result.estimateGasError).toBeUndefined();
});

it('should on Custom Network eth_estimateGas succeed when gasBN is equal to maxGasBN', async () => {
const controller = new TransactionController({
getNetworkState: () => MOCK_CUSTOM_NETWORK.state,
onNetworkStateChange: MOCK_CUSTOM_NETWORK.subscribe,
getProvider: MOCK_CUSTOM_NETWORK.getProvider,
});
const from = '0x4579d0ad79bfbdf4539a1ddf5f10b378d724a34c';
const result = await controller.estimateGas({ from, to: from });
expect(result.estimateGasError).toBeUndefined();
});

it('should on Custom Network eth_estimateGas fail when gasBN is equal to maxGasBN', async () => {
const controller = new TransactionController({
getNetworkState: () => MOCK_CUSTOM_NETWORK.state,
onNetworkStateChange: MOCK_CUSTOM_NETWORK.subscribe,
getProvider: MOCK_CUSTOM_NETWORK.getProvider,
});
mockFlags.estimateGasError = ESTIMATE_GAS_ERROR;
const from = '0x4579d0ad79bfbdf4539a1ddf5f10b378d724a34c';
const result = await controller.estimateGas({ from, to: from });
expect(result.estimateGasError).toBe(ESTIMATE_GAS_ERROR);
});

it('should on Custom Network eth_estimateGas succeed when gasBN is less than maxGasBN', async () => {
const controller = new TransactionController({
getNetworkState: () => MOCK_CUSTOM_NETWORK.state,
onNetworkStateChange: MOCK_CUSTOM_NETWORK.subscribe,
getProvider: MOCK_CUSTOM_NETWORK.getProvider,
});

mockFlags.getBlockByNumberValue = '0x12a05f200';

const from = '0x4579d0ad79bfbdf4539a1ddf5f10b378d724a34c';
const result = await controller.estimateGas({ from, to: from });
expect(result.estimateGasError).toBeUndefined();
});

it('should on Custom Network eth_estimateGas fail when gasBN is less than maxGasBN', async () => {
const controller = new TransactionController({
getNetworkState: () => MOCK_CUSTOM_NETWORK.state,
onNetworkStateChange: MOCK_CUSTOM_NETWORK.subscribe,
getProvider: MOCK_CUSTOM_NETWORK.getProvider,
});

mockFlags.getBlockByNumberValue = '0x12a05f200';

mockFlags.estimateGasError = ESTIMATE_GAS_ERROR;
const from = '0x4579d0ad79bfbdf4539a1ddf5f10b378d724a34c';
const result = await controller.estimateGas({ from, to: from });

expect(result.estimateGasError).toBe(ESTIMATE_GAS_ERROR);
});

it('should eth_estimateGas succeed when gasBN is less than maxGasBN and paddedGasBN is less than MaxGasBN', async () => {
const controller = new TransactionController({
getNetworkState: () => MOCK_NETWORK.state,
onNetworkStateChange: MOCK_NETWORK.subscribe,
getProvider: MOCK_NETWORK.getProvider,
});

mockFlags.getBlockByNumberValue = '0x12a05f200';

const from = '0x4579d0ad79bfbdf4539a1ddf5f10b378d724a34c';
const result = await controller.estimateGas({ from, to: from });

expect(result.estimateGasError).toBeUndefined();
});

it('should eth_estimateGas fail when gasBN is less than maxGasBN and paddedGasBN is less than MaxGasBN', async () => {
const controller = new TransactionController({
getNetworkState: () => MOCK_NETWORK.state,
onNetworkStateChange: MOCK_NETWORK.subscribe,
getProvider: MOCK_NETWORK.getProvider,
});

mockFlags.getBlockByNumberValue = '0x12a05f200';
mockFlags.estimateGasError = ESTIMATE_GAS_ERROR;

const from = '0x4579d0ad79bfbdf4539a1ddf5f10b378d724a34c';
const result = await controller.estimateGas({ from, to: from });

expect(result.estimateGasError).toBe(ESTIMATE_GAS_ERROR);
});

it('should throw when adding invalid transaction', async () => {
const controller = new TransactionController({
getNetworkState: () => MOCK_NETWORK.state,
Expand Down Expand Up @@ -590,20 +714,50 @@ describe('TransactionController', () => {
await expect(result).rejects.toThrow('foo');
});

it('should fail transaction if gas calculation fails', async () => {
it('should have gasEstimatedError variable on transaction object if gas calculation fails', async () => {
const controller = new TransactionController({
getNetworkState: () => MOCK_NETWORK.state,
onNetworkStateChange: MOCK_NETWORK.subscribe,
getProvider: MOCK_NETWORK.getProvider,
getNetworkState: () => MOCK_MAINNET_NETWORK.state,
onNetworkStateChange: MOCK_MAINNET_NETWORK.subscribe,
getProvider: MOCK_MAINNET_NETWORK.getProvider,
});
mockFlags.estimateGasError = ESTIMATE_GAS_ERROR;
const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d';
mockFlags.estimateGas = 'Uh oh';
await expect(
controller.addTransaction({
from,
to: from,
}),
).rejects.toThrow('Uh oh');
await controller.addTransaction({
from,
to: from,
});

controller.hub.once(
`${controller.state.transactions[0].id}:finished`,
() => {
const { transaction, status } = controller.state.transactions[0];
expect(transaction.estimateGasError).toBe(ESTIMATE_GAS_ERROR);
expect(status).toBe(TransactionStatus.submitted);
},
);
});

it('should have gasEstimatedError variable on transaction object on custom network if gas calculation fails', async () => {
const controller = new TransactionController({
getNetworkState: () => MOCK_CUSTOM_NETWORK.state,
onNetworkStateChange: MOCK_CUSTOM_NETWORK.subscribe,
getProvider: MOCK_CUSTOM_NETWORK.getProvider,
});
mockFlags.estimateGasError = ESTIMATE_GAS_ERROR;
const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d';
await controller.addTransaction({
from,
to: from,
});

controller.hub.once(
`${controller.state.transactions[0].id}:finished`,
() => {
const { transaction, status } = controller.state.transactions[0];
expect(transaction.estimateGasError).toBe(ESTIMATE_GAS_ERROR);
expect(status).toBe(TransactionStatus.submitted);
},
);
});

it('should fail if no sign method defined', async () => {
Expand Down
26 changes: 19 additions & 7 deletions src/transaction/TransactionController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
validateGasValues,
validateMinimumIncrease,
} from '../util';
import { MAINNET, RPC } from '../constants';
import { ESTIMATE_GAS_ERROR, MAINNET, RPC } from '../constants';

const HARDFORK = 'london';

Expand Down Expand Up @@ -80,6 +80,7 @@ export interface Transaction {
maxFeePerGas?: string;
maxPriorityFeePerGas?: string;
estimatedBaseFee?: string;
estimateGasError?: string;
}

export interface GasPriceValue {
Expand Down Expand Up @@ -517,8 +518,9 @@ export class TransactionController extends BaseController<
};

try {
const { gas } = await this.estimateGas(transaction);
const { gas, estimateGasError } = await this.estimateGas(transaction);
transaction.gas = gas;
transaction.estimateGasError = estimateGasError;
} catch (error: any) {
this.failTransaction(transactionMeta, error);
return Promise.reject(error);
Expand Down Expand Up @@ -988,23 +990,33 @@ export class TransactionController extends BaseController<
typeof value === 'undefined' ? '0x0' : /* istanbul ignore next */ value;
const gasLimitBN = hexToBN(gasLimit);
estimatedTransaction.gas = BNToHex(fractionBN(gasLimitBN, 19, 20));
const gasHex = await query(this.ethQuery, 'estimateGas', [
estimatedTransaction,
]);

let gasHex;
let estimateGasError;
try {
gasHex = await query(this.ethQuery, 'estimateGas', [
estimatedTransaction,
]);
} catch (error) {
estimateGasError = ESTIMATE_GAS_ERROR;
}
// 4. Pad estimated gas without exceeding the most recent block gasLimit. If the network is a
// a custom network then return the eth_estimateGas value.
const gasBN = hexToBN(gasHex);
const maxGasBN = gasLimitBN.muln(0.9);
const paddedGasBN = gasBN.muln(1.5);
/* istanbul ignore next */
if (gasBN.gt(maxGasBN) || isCustomNetwork) {
return { gas: addHexPrefix(gasHex), gasPrice };
return { gas: addHexPrefix(gasHex), gasPrice, estimateGasError };
}

/* istanbul ignore next */
if (paddedGasBN.lt(maxGasBN)) {
return { gas: addHexPrefix(BNToHex(paddedGasBN)), gasPrice };
return {
gas: addHexPrefix(BNToHex(paddedGasBN)),
gasPrice,
estimateGasError,
};
}
return { gas: addHexPrefix(BNToHex(maxGasBN)), gasPrice };
}
Expand Down

0 comments on commit 4fec100

Please sign in to comment.