Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for speedUp and stop based on provided gasValues from consumer #535

Merged
merged 23 commits into from
Aug 11, 2021
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 101 additions & 22 deletions src/transaction/TransactionController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ import {
query,
getIncreasedPriceFromExisting,
isEIP1559Transaction,
isGasPriceValue,
isFeeMarketEIP1559Values,
validateGasValues,
validateMinimumIncrease,
} from '../util';
import { MAINNET, RPC } from '../constants';

Expand Down Expand Up @@ -79,6 +83,15 @@ export interface Transaction {
estimatedBaseFee?: string;
}

export interface GasPriceValue {
gasPrice: string;
}

export interface FeeMarketEIP1559Values {
maxFeePerGas: string;
maxPriorityFeePerGas: string;
}

/**
* The status of the transaction. Each status represents the state of the transaction internally
* in the wallet. Some of these correspond with the state of the transaction on the network, but
Expand Down Expand Up @@ -701,7 +714,13 @@ export class TransactionController extends BaseController<
*
* @param transactionID - ID of the transaction to cancel
*/
async stopTransaction(transactionID: string) {
async stopTransaction(
transactionID: string,
gasValues?: GasPriceValue | FeeMarketEIP1559Values,
) {
if (gasValues) {
validateGasValues(gasValues);
Gudahtt marked this conversation as resolved.
Show resolved Hide resolved
}
const transactionMeta = this.state.transactions.find(
({ id }) => id === transactionID,
);
Expand All @@ -713,21 +732,49 @@ export class TransactionController extends BaseController<
throw new Error('No sign method defined.');
}

const gasPrice = getIncreasedPriceFromExisting(
// gasPrice (legacy non EIP1559)
const minGasPrice = getIncreasedPriceFromExisting(
transactionMeta.transaction.gasPrice,
CANCEL_RATE,
);

const gasPirceFromValues =
rickycodes marked this conversation as resolved.
Show resolved Hide resolved
isGasPriceValue(gasValues) && gasValues?.gasPrice;
rickycodes marked this conversation as resolved.
Show resolved Hide resolved

const newGasPrice =
(gasPirceFromValues &&
validateMinimumIncrease(gasPirceFromValues, minGasPrice)) ||
minGasPrice;

// maxFeePerGas (EIP1559)
const existingMaxFeePerGas = transactionMeta.transaction?.maxFeePerGas;
const minMaxFeePerGas = getIncreasedPriceFromExisting(
existingMaxFeePerGas,
CANCEL_RATE,
);
const maxFeePerGasValues =
isFeeMarketEIP1559Values(gasValues) && gasValues?.maxFeePerGas;
const newMaxFeePerGas =
(maxFeePerGasValues &&
validateMinimumIncrease(maxFeePerGasValues, minMaxFeePerGas)) ||
(existingMaxFeePerGas && minMaxFeePerGas);

// maxPriorityFeePerGas (EIP1559)
const existingMaxPriorityFeePerGas =
transactionMeta.transaction?.maxPriorityFeePerGas;

const newMaxFeePerGas =
existingMaxFeePerGas &&
getIncreasedPriceFromExisting(existingMaxFeePerGas, CANCEL_RATE);
const minMaxPriorityFeePerGas = getIncreasedPriceFromExisting(
existingMaxPriorityFeePerGas,
CANCEL_RATE,
);
const maxPriorityFeePerGasValues =
isFeeMarketEIP1559Values(gasValues) && gasValues?.maxPriorityFeePerGas;
const newMaxPriorityFeePerGas =
existingMaxPriorityFeePerGas &&
getIncreasedPriceFromExisting(existingMaxPriorityFeePerGas, CANCEL_RATE);
(maxPriorityFeePerGasValues &&
validateMinimumIncrease(
maxPriorityFeePerGasValues,
minMaxPriorityFeePerGas,
)) ||
(existingMaxPriorityFeePerGas && minMaxPriorityFeePerGas);

const txParams =
newMaxFeePerGas && newMaxPriorityFeePerGas
Expand All @@ -744,7 +791,7 @@ export class TransactionController extends BaseController<
: {
from: transactionMeta.transaction.from,
gasLimit: transactionMeta.transaction.gas,
gasPrice,
gasPrice: newGasPrice,
nonce: transactionMeta.transaction.nonce,
to: transactionMeta.transaction.from,
value: '0x0',
Expand All @@ -767,7 +814,13 @@ export class TransactionController extends BaseController<
*
* @param transactionID - ID of the transaction to speed up
*/
async speedUpTransaction(transactionID: string) {
async speedUpTransaction(
transactionID: string,
gasValues?: GasPriceValue | FeeMarketEIP1559Values,
) {
if (gasValues) {
validateGasValues(gasValues);
}
const transactionMeta = this.state.transactions.find(
({ id }) => id === transactionID,
);
Expand All @@ -782,24 +835,50 @@ export class TransactionController extends BaseController<
}

const { transactions } = this.state;
const gasPrice = getIncreasedPriceFromExisting(

// gasPrice (legacy non EIP1559)
const minGasPrice = getIncreasedPriceFromExisting(
transactionMeta.transaction.gasPrice,
SPEED_UP_RATE,
);

const gasPirceFromValues =
isGasPriceValue(gasValues) && gasValues?.gasPrice;

const newGasPrice =
(gasPirceFromValues &&
validateMinimumIncrease(gasPirceFromValues, minGasPrice)) ||
minGasPrice;

// maxFeePerGas (EIP1559)
const existingMaxFeePerGas = transactionMeta.transaction?.maxFeePerGas;
const minMaxFeePerGas = getIncreasedPriceFromExisting(
existingMaxFeePerGas,
SPEED_UP_RATE,
);
const maxFeePerGasValues =
isFeeMarketEIP1559Values(gasValues) && gasValues?.maxFeePerGas;
const newMaxFeePerGas =
(maxFeePerGasValues &&
validateMinimumIncrease(maxFeePerGasValues, minMaxFeePerGas)) ||
(existingMaxFeePerGas && minMaxFeePerGas);

// maxPriorityFeePerGas (EIP1559)
const existingMaxPriorityFeePerGas =
transactionMeta.transaction?.maxPriorityFeePerGas;

const newMaxFeePerGas =
existingMaxFeePerGas &&
getIncreasedPriceFromExisting(existingMaxFeePerGas, SPEED_UP_RATE);
const minMaxPriorityFeePerGas = getIncreasedPriceFromExisting(
existingMaxPriorityFeePerGas,
SPEED_UP_RATE,
);
const maxPriorityFeePerGasValues =
isFeeMarketEIP1559Values(gasValues) && gasValues?.maxPriorityFeePerGas;
const newMaxPriorityFeePerGas =
existingMaxPriorityFeePerGas &&
getIncreasedPriceFromExisting(
existingMaxPriorityFeePerGas,
SPEED_UP_RATE,
);
(maxPriorityFeePerGasValues &&
validateMinimumIncrease(
maxPriorityFeePerGasValues,
minMaxPriorityFeePerGas,
)) ||
(existingMaxPriorityFeePerGas && minMaxPriorityFeePerGas);

const txParams =
newMaxFeePerGas && newMaxPriorityFeePerGas
Expand All @@ -813,7 +892,7 @@ export class TransactionController extends BaseController<
: {
...transactionMeta.transaction,
gasLimit: transactionMeta.transaction.gas,
gasPrice,
gasPrice: newGasPrice,
};

const unsignedEthTx = this.prepareUnsignedEthTx(txParams);
Expand Down Expand Up @@ -846,7 +925,7 @@ export class TransactionController extends BaseController<
...baseTransactionMeta,
transaction: {
...transactionMeta.transaction,
gasPrice,
gasPrice: newGasPrice,
},
};
transactions.push(newTransactionMeta);
Expand Down
85 changes: 84 additions & 1 deletion src/util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,22 @@ import nock from 'nock';
import HttpProvider from 'ethjs-provider-http';
import EthQuery from 'eth-query';
import * as util from './util';
import { Transaction } from './transaction/TransactionController';
import {
Transaction,
GasPriceValue,
FeeMarketEIP1559Values,
} from './transaction/TransactionController';

const VALID = '4e1fF7229BDdAf0A73DF183a88d9c3a04cc975e0';
const SOME_API = 'https://someapi.com';
const SOME_FAILING_API = 'https://somefailingapi.com';

const MAX_FEE_PER_GAS = 'maxFeePerGas';
const MAX_PRIORITY_FEE_PER_GAS = 'maxPriorityFeePerGas';
const GAS_PRICE = 'gasPrice';
const FAIL = 'lol';
const PASS = '0x1';

const mockFlags: { [key: string]: any } = {
estimateGas: null,
gasPrice: null,
Expand Down Expand Up @@ -960,4 +970,77 @@ describe('util', () => {
expect(util.isEIP1559Transaction(tx)).toBe(false);
});
});

describe('validateGasValues', () => {
it('should throw when provided invalid gas values', () => {
const gasValues: GasPriceValue = {
[GAS_PRICE]: FAIL,
};
expect(() => util.validateGasValues(gasValues)).toThrow(TypeError);
expect(() => util.validateGasValues(gasValues)).toThrow(
`expected hex string for ${GAS_PRICE} but received: ${FAIL}`,
);
});
it('should throw when any provided gas values are invalid', () => {
const gasValues: FeeMarketEIP1559Values = {
[MAX_PRIORITY_FEE_PER_GAS]: PASS,
[MAX_FEE_PER_GAS]: FAIL,
};
expect(() => util.validateGasValues(gasValues)).toThrow(TypeError);
expect(() => util.validateGasValues(gasValues)).toThrow(
`expected hex string for ${MAX_FEE_PER_GAS} but received: ${FAIL}`,
);
});
it('should return true when provided valid gas values', () => {
const gasValues: FeeMarketEIP1559Values = {
[MAX_FEE_PER_GAS]: PASS,
[MAX_PRIORITY_FEE_PER_GAS]: PASS,
};
expect(() => util.validateGasValues(gasValues)).not.toThrow(TypeError);
});
});

describe('isFeeMarketEIP1559Values', () => {
it('should detect if isFeeMarketEIP1559Values', () => {
const gasValues = {
[MAX_PRIORITY_FEE_PER_GAS]: PASS,
[MAX_FEE_PER_GAS]: FAIL,
};
expect(util.isFeeMarketEIP1559Values(gasValues)).toBe(true);
expect(util.isGasPriceValue(gasValues)).toBe(false);
});
});

describe('isGasPriceValue', () => {
it('should detect if isGasPriceValue', () => {
const gasValues: GasPriceValue = {
[GAS_PRICE]: PASS,
};
expect(util.isGasPriceValue(gasValues)).toBe(true);
expect(util.isFeeMarketEIP1559Values(gasValues)).toBe(false);
});
});

describe('validateMinimumIncrease', () => {
it('should throw if increase does not meet minimum requirement', () => {
expect(() =>
util.validateMinimumIncrease('0x50fd51da', '0x5916a6d6'),
).toThrow(Error);
expect(() =>
util.validateMinimumIncrease('0x50fd51da', '0x5916a6d6'),
).toThrow(
'The proposed value: 1358778842 should meet or exceed the minimum value: 1494656726',
);
});
it('should not throw if increase meets minimum requirement', () => {
expect(() =>
util.validateMinimumIncrease('0x5916a6d6', '0x5916a6d6'),
).not.toThrow(Error);
});
it('should not throw if increase exceeds minimum requirement', () => {
expect(() =>
util.validateMinimumIncrease('0x7162a5ca', '0x5916a6d6'),
).not.toThrow(Error);
});
});
});
36 changes: 36 additions & 0 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { validate } from 'jsonschema';
import {
Transaction,
FetchAllOptions,
GasPriceValue,
FeeMarketEIP1559Values,
} from './transaction/TransactionController';
import { MessageParams } from './message-manager/MessageManager';
import { PersonalMessageParams } from './message-manager/PersonalMessageManager';
Expand Down Expand Up @@ -710,6 +712,40 @@ export const getIncreasedPriceFromExisting = (
return getIncreasedPriceHex(convertPriceToDecimal(value), rate);
};

export const validateGasValues = (
gasValues: GasPriceValue | FeeMarketEIP1559Values,
) => {
Object.keys(gasValues).forEach((key) => {
const value = (gasValues as any)[key];
if (typeof value !== 'string' || !isHexString(value)) {
throw new TypeError(
`expected hex string for ${key} but received: ${value}`,
);
}
});
};

export const isFeeMarketEIP1559Values = (
gasValues?: GasPriceValue | FeeMarketEIP1559Values,
): gasValues is FeeMarketEIP1559Values =>
(gasValues as FeeMarketEIP1559Values)?.maxFeePerGas !== undefined ||
(gasValues as FeeMarketEIP1559Values)?.maxPriorityFeePerGas !== undefined;

export const isGasPriceValue = (
gasValues?: GasPriceValue | FeeMarketEIP1559Values,
): gasValues is GasPriceValue =>
(gasValues as GasPriceValue)?.gasPrice !== undefined;

export function validateMinimumIncrease(proposed: string, min: string) {
const proposedDecimal = convertPriceToDecimal(proposed);
const minDecimal = convertPriceToDecimal(min);
if (proposedDecimal >= minDecimal) {
return proposed;
}
const errorMsg = `The proposed value: ${proposedDecimal} should meet or exceed the minimum value: ${minDecimal}`;
throw new Error(errorMsg);
}

export default {
BNToHex,
fractionBN,
Expand Down