Skip to content

Commit

Permalink
Normalize transaction data length (#3990)
Browse files Browse the repository at this point in the history
Normalize transaction data by padding to even length.
Export `normalizeTransactionParams` method.
  • Loading branch information
matthewwalsh0 authored Mar 1, 2024
1 parent 346331d commit c5dbb37
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 51 deletions.
8 changes: 4 additions & 4 deletions packages/transaction-controller/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ module.exports = merge(baseConfig, {
// An object that configures minimum threshold enforcement for coverage results
coverageThreshold: {
global: {
branches: 91.79,
functions: 98.56,
lines: 98.9,
statements: 98.91,
branches: 91.89,
functions: 98.86,
lines: 98.96,
statements: 98.97,
},
},

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5023,7 +5023,7 @@ describe('TransactionController', () => {
describe('updateEditableParams', () => {
const transactionId = '1';
const params = {
data: '0x0',
data: '0x12',
from: ACCOUNT_2_MOCK,
gas: '0x0',
gasPrice: '0x50fd51da',
Expand Down
11 changes: 7 additions & 4 deletions packages/transaction-controller/src/TransactionController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ import {
import { determineTransactionType } from './utils/transaction-type';
import {
getIncreasedPriceFromExisting,
normalizeTxParams,
normalizeTransactionParams,
isEIP1559Transaction,
isFeeMarketEIP1559Values,
isGasPriceValue,
Expand Down Expand Up @@ -699,7 +699,7 @@ export class TransactionController extends BaseControllerV1<
): Promise<Result> {
log('Adding transaction', txParams);

txParams = normalizeTxParams(txParams);
txParams = normalizeTransactionParams(txParams);
if (
networkClientId &&
!this.#multichainTrackingHelper.has(networkClientId)
Expand Down Expand Up @@ -1932,7 +1932,8 @@ export class TransactionController extends BaseControllerV1<
throw new Error('No sign method defined.');
}

const normalizedTransactionParams = normalizeTxParams(transactionParams);
const normalizedTransactionParams =
normalizeTransactionParams(transactionParams);
const type = isEIP1559Transaction(normalizedTransactionParams)
? TransactionEnvelopeType.feeMarket
: TransactionEnvelopeType.legacy;
Expand Down Expand Up @@ -3083,7 +3084,9 @@ export class TransactionController extends BaseControllerV1<
) {
const { transactions } = this.state;

transactionMeta.txParams = normalizeTxParams(transactionMeta.txParams);
transactionMeta.txParams = normalizeTransactionParams(
transactionMeta.txParams,
);

validateTxParams(transactionMeta.txParams);

Expand Down
5 changes: 4 additions & 1 deletion packages/transaction-controller/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
export * from './TransactionController';
export type { EtherscanTransactionMeta } from './utils/etherscan';
export { isEIP1559Transaction } from './utils/utils';
export {
isEIP1559Transaction,
normalizeTransactionParams,
} from './utils/utils';
export * from './types';
export { determineTransactionType } from './utils/transaction-type';
export { mergeGasFeeEstimates } from './utils/gas-flow';
109 changes: 70 additions & 39 deletions packages/transaction-controller/src/utils/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,30 @@ const GAS_PRICE = 'gasPrice';
const FAIL = 'lol';
const PASS = '0x1';

const TRANSACTION_PARAMS_MOCK: TransactionParams = {
data: 'data',
from: 'FROM',
gas: 'gas',
gasPrice: 'gasPrice',
nonce: 'nonce',
to: 'TO',
value: 'value',
maxFeePerGas: 'maxFeePerGas',
maxPriorityFeePerGas: 'maxPriorityFeePerGas',
estimatedBaseFee: 'estimatedBaseFee',
};

describe('utils', () => {
afterEach(() => {
jest.clearAllMocks();
});

describe('normalizeTxParams', () => {
const commonInput = {
data: 'data',
from: 'FROM',
gas: 'gas',
gasPrice: 'gasPrice',
nonce: 'nonce',
to: 'TO',
value: 'value',
maxFeePerGas: 'maxFeePerGas',
maxPriorityFeePerGas: 'maxPriorityFeePerGas',
estimatedBaseFee: 'estimatedBaseFee',
};
describe('normalizeTransactionParams', () => {
it('normalizes properties', () => {
const normalized = util.normalizeTransactionParams(
TRANSACTION_PARAMS_MOCK,
);

it('normalizeTransaction', () => {
const normalized = util.normalizeTxParams({
...commonInput,
});
expect(normalized).toStrictEqual({
data: '0xdata',
from: '0xfrom',
Expand All @@ -48,31 +49,35 @@ describe('utils', () => {
});
});

it('normalizeTransaction if type is zero', () => {
const normalized = util.normalizeTxParams({
...commonInput,
type: '0x0',
});
expect(normalized).toStrictEqual({
data: '0xdata',
from: '0xfrom',
gas: '0xgas',
gasPrice: '0xgasPrice',
nonce: '0xnonce',
to: '0xto',
value: '0xvalue',
maxFeePerGas: '0xmaxFeePerGas',
maxPriorityFeePerGas: '0xmaxPriorityFeePerGas',
estimatedBaseFee: '0xestimatedBaseFee',
type: '0x0',
});
it('retains legacy type if specified', () => {
expect(
util.normalizeTransactionParams({
...TRANSACTION_PARAMS_MOCK,
type: '0x0',
}),
).toStrictEqual(
expect.objectContaining({
type: '0x0',
}),
);
});

it('sets value if not specified', () => {
expect(util.normalizeTxParams({ from: '0xfrom' })).toStrictEqual({
from: '0xfrom',
value: '0x0',
});
expect(
util.normalizeTransactionParams({
...TRANSACTION_PARAMS_MOCK,
value: undefined,
}),
).toStrictEqual(expect.objectContaining({ value: '0x0' }));
});

it('ensures data is even length prefixed hex string', () => {
expect(
util.normalizeTransactionParams({
...TRANSACTION_PARAMS_MOCK,
data: '123',
}),
).toStrictEqual(expect.objectContaining({ data: '0x0123' }));
});
});

Expand Down Expand Up @@ -248,4 +253,30 @@ describe('utils', () => {
});
});
});

describe('padHexToEvenLength', () => {
it('returns same value if already even length and has prefix', () => {
expect(util.padHexToEvenLength('0x1234')).toBe('0x1234');
});

it('returns same value if already even length and no prefix', () => {
expect(util.padHexToEvenLength('1234')).toBe('1234');
});

it('returns padded value if not even length and has prefix', () => {
expect(util.padHexToEvenLength('0x123')).toBe('0x0123');
});

it('returns padded value if not even length and no prefix', () => {
expect(util.padHexToEvenLength('123')).toBe('0123');
});

it('returns same value if prefix only', () => {
expect(util.padHexToEvenLength('0x')).toBe('0x');
});

it('returns padded value if zero', () => {
expect(util.padHexToEvenLength('0x0')).toBe('0x00');
});
});
});
19 changes: 17 additions & 2 deletions packages/transaction-controller/src/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export const ESTIMATE_GAS_ERROR = 'eth_estimateGas rpc method error';
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const NORMALIZERS: { [param in keyof TransactionParams]: any } = {
data: (data: string) => add0x(data),
data: (data: string) => add0x(padHexToEvenLength(data)),
from: (from: string) => add0x(from).toLowerCase(),
gas: (gas: string) => add0x(gas),
gasLimit: (gas: string) => add0x(gas),
Expand All @@ -43,7 +43,7 @@ const NORMALIZERS: { [param in keyof TransactionParams]: any } = {
* @param txParams - The transaction params to normalize.
* @returns Normalized transaction params.
*/
export function normalizeTxParams(txParams: TransactionParams) {
export function normalizeTransactionParams(txParams: TransactionParams) {
const normalizedTxParams: TransactionParams = { from: '' };

for (const key of getKnownPropertyNames(NORMALIZERS)) {
Expand Down Expand Up @@ -191,3 +191,18 @@ export function normalizeGasFeeValues(
maxPriorityFeePerGas: normalize(gasFeeValues.maxPriorityFeePerGas),
};
}

/**
* Ensure a hex string is of even length by adding a leading 0 if necessary.
* Any existing `0x` prefix is preserved but is not added if missing.
*
* @param hex - The hex string to ensure is even.
* @returns The hex string with an even length.
*/
export function padHexToEvenLength(hex: string) {
const prefix = hex.toLowerCase().startsWith('0x') ? hex.slice(0, 2) : '';
const data = prefix ? hex.slice(2) : hex;
const evenData = data.length % 2 === 0 ? data : `0${data}`;

return prefix + evenData;
}

0 comments on commit c5dbb37

Please sign in to comment.