Skip to content

Commit

Permalink
Allow for speedUp and stop based on provided gasValues from consumer (#…
Browse files Browse the repository at this point in the history 
…535)

* Allow for speedUp and stop based on provided gasValues from consumer

* make this optional

* validate gas values are hex string

* use util

* rename

* update wording

* Update tests

* remove return true

* Use type predicate functions

* Move type predicate functions to util

* Update unit tests

* Add validation
ensure tx is increased by at least the minimum required amount

* Add validateMinimumIncrease method

* Address nit

* Add validation in stopTransaction

* fix spelling

* remove unnecessary optional chaining

* test
  • Loading branch information
@rickycodes @MajorLift
rickycodes authored and MajorLift committed Oct 11, 2023
1 parent b32fb8d commit 410570d
Show file tree
Hide file tree
Showing 3 changed files with 219 additions and 23 deletions.
121 changes: 99 additions & 22 deletions src/transaction/TransactionController.ts
View file
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);
}
const transactionMeta = this.state.transactions.find(
({ id }) => id === transactionID,
);
Expand All @@ -713,21 +732,48 @@ 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 gasPriceFromValues = isGasPriceValue(gasValues) && gasValues.gasPrice;

const newGasPrice =
(gasPriceFromValues &&
validateMinimumIncrease(gasPriceFromValues, 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 +790,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 +813,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 +834,49 @@ 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 gasPriceFromValues = isGasPriceValue(gasValues) && gasValues.gasPrice;

const newGasPrice =
(gasPriceFromValues &&
validateMinimumIncrease(gasPriceFromValues, 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 +890,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 +923,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
View file
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
View file
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

0 comments on commit 410570d

Please sign in to comment.