Skip to content

Commit

Permalink
Transaction input and data refactors (#6294)
Browse files Browse the repository at this point in the history
* Remove extra formatting check for input and data for transactionSchema

* Update transaciton builder to only use input or data provided by user, don't autofill unless both or nullish

* Add check for transaction.data for getEthereumjsTxDataFromTransaction

* Update PopulatedUnsignedBaseTransaction to have optional input and data properties

* Update sign_transaction test

* Update CHANGELOGs

* Add fillInputAndData option to formatTransaction. Make options.transactionSchema optional with transactoinInfoSchema the default value

* Update tests

* Update CHANGELOGs

* Convert formattedTransaction. input and data to hex string before comparing equality for TransactionDataAndInputError check

* Update test fixtures

* Add options.fillInputAndData to decodeSignedTransaction

* Update sign_transaction fixture data
  • Loading branch information
spacesailor24 committed Aug 3, 2023
1 parent 0e5c53c commit 51d087e
Show file tree
Hide file tree
Showing 15 changed files with 129 additions and 51 deletions.
20 changes: 16 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1815,6 +1815,10 @@ If there are any bugs, improvements, optimizations or any new feature proposal f

- Fixed: "'disconnect' in Eip1193 provider must emit ProviderRpcError #6003".(#6230)

#### web3-eth

- Missing `blockHeaderSchema` properties causing some properties to not appear in response of `newHeads` subscription (#6243)

### Changed

#### web3-core
Expand All @@ -1824,18 +1828,26 @@ If there are any bugs, improvements, optimizations or any new feature proposal f
- A new optional protected method `formatSubscriptionResult` could be used to customize data formatting instead of re-implementing `_processSubscriptionResult`. (#6262)
- No more needed to pass `CommonSubscriptionEvents & ` for the first generic parameter of `Web3Subscription` when inheriting from it. (#6262)

#### web3-eth

- `input` and `data` are no longer auto populated for transaction objects if they are not present. Instead, whichever property is provided by the user is formatted and sent to the RPC provider. Transaction objects returned from RPC responses are still formatted to contain both `input` and `data` properties (#6294)

#### web3-types

- `input` and `data` are now optional properties on `PopulatedUnsignedBaseTransaction` (previously `input` was a required property, and `data` was not available) (#6294)

#### web3-validator

- Replace `is-my-json-valid` with `zod` dependency. Related code was changed (#6264)
- Types `ValidationError` and `JsonSchema` were changed (#6264)

### Removed

#### web3-validator

- Rpc method `getPastLogs` accept blockHash as a parameter https://ethereum.org/en/developers/docs/apis/json-rpc/#eth_getlogs (#6181)

#### web3-eth

- Missing `blockHeaderSchema` properties causing some properties to not appear in response of `newHeads` subscription (#6243)
- Type `RawValidationError` was removed (#6264)

#### web3-validator

- Type `RawValidationError` was removed (#6264)
11 changes: 10 additions & 1 deletion packages/web3-eth/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,17 @@ Documentation:
### Changed

- `MissingGasError` error message changed for clarity (#6215)
-
-

### Added

- A `rpc_method_wrapper` (`signTypedData`) for the rpc calls `eth_signTypedData` and `eth_signTypedData_v4` (#6286)
- A `signTypedData` method to the `Web3Eth` class (#6286)

### Fixed

- Missing `blockHeaderSchema` properties causing some properties to not appear in response of `newHeads` subscription (#6243)

### Changed

- `input` and `data` are no longer auto populated for transaction objects if they are not present. Instead, whichever property is provided by the user is formatted and sent to the RPC provider. Transaction objects returned from RPC responses are still formatted to contain both `input` and `data` properties (#6294)
17 changes: 11 additions & 6 deletions packages/web3-eth/src/rpc_method_wrappers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ import {
feeHistorySchema,
logSchema,
transactionReceiptSchema,
transactionInfoSchema,
accessListResultSchema,
SignatureObjectSchema,
} from './schemas.js';
Expand Down Expand Up @@ -375,7 +374,7 @@ export async function getTransaction<ReturnFormat extends DataFormat>(

return isNullish(response)
? response
: formatTransaction(response, returnFormat, { transactionSchema: transactionInfoSchema });
: formatTransaction(response, returnFormat, { fillInputAndData: true });
}

/**
Expand All @@ -389,7 +388,9 @@ export async function getPendingTransactions<ReturnFormat extends DataFormat>(
const response = await ethRpcMethods.getPendingTransactions(web3Context.requestManager);

return response.map(transaction =>
formatTransaction(transaction as unknown as Transaction, returnFormat),
formatTransaction(transaction as unknown as Transaction, returnFormat, {
fillInputAndData: true,
}),
);
}

Expand Down Expand Up @@ -426,7 +427,7 @@ export async function getTransactionFromBlock<ReturnFormat extends DataFormat>(

return isNullish(response)
? response
: formatTransaction(response, returnFormat, { transactionSchema: transactionInfoSchema });
: formatTransaction(response, returnFormat, { fillInputAndData: true });
}

/**
Expand Down Expand Up @@ -917,14 +918,18 @@ export async function signTransaction<ReturnFormat extends DataFormat>(
// Some clients only return the encoded signed transaction (e.g. Ganache)
// while clients such as Geth return the desired SignedTransactionInfoAPI object
return isString(response as HexStringBytes)
? decodeSignedTransaction(response as HexStringBytes, returnFormat)
? decodeSignedTransaction(response as HexStringBytes, returnFormat, {
fillInputAndData: true,
})
: {
raw: format(
{ format: 'bytes' },
(response as SignedTransactionInfoAPI).raw,
returnFormat,
),
tx: formatTransaction((response as SignedTransactionInfoAPI).tx, returnFormat),
tx: formatTransaction((response as SignedTransactionInfoAPI).tx, returnFormat, {
fillInputAndData: true,
}),
};
}

Expand Down
2 changes: 2 additions & 0 deletions packages/web3-eth/src/utils/decode_signed_transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { formatTransaction } from './format_transaction.js';
export function decodeSignedTransaction<ReturnFormat extends DataFormat>(
encodedSignedTransaction: HexStringBytes,
returnFormat: ReturnFormat,
options: { fillInputAndData?: boolean } = { fillInputAndData: false },
): SignedTransactionInfoAPI {
return {
raw: format({ format: 'bytes' }, encodedSignedTransaction, returnFormat),
Expand All @@ -47,6 +48,7 @@ export function decodeSignedTransaction<ReturnFormat extends DataFormat>(
type: detectRawTransactionType(hexToBytes(encodedSignedTransaction)),
} as TransactionSignedAPI,
returnFormat,
{ fillInputAndData: options.fillInputAndData },
),
};
}
48 changes: 31 additions & 17 deletions packages/web3-eth/src/utils/format_transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,23 @@ along with web3.js. If not, see <http://www.gnu.org/licenses/>.

import { Transaction, DataFormat, DEFAULT_RETURN_FORMAT, FormatType } from 'web3-types';
import { isNullish, ValidationSchemaInput } from 'web3-validator';
import { mergeDeep, format, bytesToHex, toHex } from 'web3-utils';
import { TransactionDataAndInputError } from 'web3-errors';
import { mergeDeep, bytesToHex, format } from 'web3-utils';
import { transactionSchema } from '../schemas.js';

import { transactionInfoSchema, transactionSchema } from '../schemas.js';

export function formatTransaction<
ReturnFormat extends DataFormat = typeof DEFAULT_RETURN_FORMAT,
TransactionType extends Transaction = Transaction,
>(
transaction: TransactionType,
returnFormat: ReturnFormat = DEFAULT_RETURN_FORMAT as ReturnFormat,
options: { transactionSchema: ValidationSchemaInput | typeof transactionSchema } = {
transactionSchema,
options: {
transactionSchema?: ValidationSchemaInput | typeof transactionSchema;
fillInputAndData?: boolean;
} = {
transactionSchema: transactionInfoSchema,
fillInputAndData: false,
},
): FormatType<TransactionType, ReturnFormat> {
let formattedTransaction = mergeDeep({}, transaction as Record<string, unknown>) as Transaction;
Expand All @@ -38,21 +43,30 @@ export function formatTransaction<
formattedTransaction.common.customChain = { ...transaction.common.customChain };
}

formattedTransaction = format(options.transactionSchema, formattedTransaction, returnFormat);
formattedTransaction = format(
options.transactionSchema ?? transactionInfoSchema,
formattedTransaction,
returnFormat,
);

if (!isNullish(formattedTransaction.data)) {
if (
!isNullish(formattedTransaction.input) &&
formattedTransaction.data !== formattedTransaction.input
)
throw new TransactionDataAndInputError({
data: bytesToHex(formattedTransaction.data),
input: bytesToHex(formattedTransaction.input),
});
if (
!isNullish(formattedTransaction.data) &&
!isNullish(formattedTransaction.input) &&
// Converting toHex is accounting for data and input being Uint8Arrays
// since comparing Uint8Array is not as straightforward as comparing strings
toHex(formattedTransaction.data) !== toHex(formattedTransaction.input)
)
throw new TransactionDataAndInputError({
data: bytesToHex(formattedTransaction.data),
input: bytesToHex(formattedTransaction.input),
});

formattedTransaction.input = formattedTransaction.data;
} else if (!isNullish(formattedTransaction.input)) {
formattedTransaction.data = formattedTransaction.input;
if (options.fillInputAndData) {
if (!isNullish(formattedTransaction.data)) {
formattedTransaction.input = formattedTransaction.data;
} else if (!isNullish(formattedTransaction.input)) {
formattedTransaction.data = formattedTransaction.input;
}
}

if (!isNullish(formattedTransaction.gasLimit)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const getEthereumjsTxDataFromTransaction = (
gasLimit: transaction.gasLimit ?? transaction.gas,
to: transaction.to,
value: transaction.value,
data: transaction.input,
data: transaction.data ?? transaction.input,
type: transaction.type,
chainId: transaction.chainId,
accessList: (
Expand Down
5 changes: 0 additions & 5 deletions packages/web3-eth/src/utils/transaction_builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,16 +171,11 @@ export async function defaultTransactionBuilder<ReturnType = Transaction>(option

if (!populatedTransaction.data.startsWith('0x'))
populatedTransaction.data = `0x${populatedTransaction.data}`;

populatedTransaction.input = populatedTransaction.data;
} else if (!isNullish(populatedTransaction.input)) {
if (!populatedTransaction.input.startsWith('0x'))
populatedTransaction.input = `0x${populatedTransaction.input}`;

populatedTransaction.data = populatedTransaction.input;
} else {
populatedTransaction.input = '0x';
populatedTransaction.data = '0x';
}

if (isNullish(populatedTransaction.common)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('Web3Eth.signTransaction', () => {
gasPrice: '0x3b9aca01',
};
const response = await web3Eth.signTransaction(transaction);
expect(response).toMatchObject({
const expectedResponse: { tx: Transaction } = {
tx: {
type: BigInt(0),
nonce: BigInt(nonce),
Expand All @@ -56,8 +56,12 @@ describe('Web3Eth.signTransaction', () => {
value: BigInt(1),
to: transaction.to,
input: '0x',
data: '0x',
},
});
};

expect(response).toMatchObject(expectedResponse);

// Pulling out of toMatchObject to be compatiable with Cypress
expect(response.raw).toMatch(/0[xX][0-9a-fA-F]+/);
expect(typeof (response.tx as TransactionLegacySignedAPI).v).toBe('bigint');
Expand All @@ -77,16 +81,19 @@ describe('Web3Eth.signTransaction', () => {
gasPrice: '0x3b9aca01',
};
const response = await web3Eth.signTransaction(transaction);
// eslint-disable-next-line jest/no-standalone-expect
expect(response).toMatchObject({
const expectedResponse: { tx: Transaction } = {
tx: {
type: BigInt(0),
nonce: BigInt(nonce),
gasPrice: BigInt(1000000001),
gas: BigInt(475320),
input: greeterContractDeploymentData,
data: greeterContractDeploymentData,
},
});
};

// eslint-disable-next-line jest/no-standalone-expect
expect(response).toMatchObject(expectedResponse);
// Pulling out of toMatchObject to be compatiable with Cypress
expect(response.raw).toMatch(/0[xX][0-9a-fA-F]+/);
expect(typeof (response.tx as TransactionLegacySignedAPI).v).toBe('bigint');
Expand Down
24 changes: 19 additions & 5 deletions packages/web3-eth/test/unit/default_transaction_builder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,8 @@ describe('defaultTransactionBuilder', () => {
});

describe('should populate input/data', () => {
it('should populate with 0x', async () => {
it('should populate input with 0x', async () => {
const input = { ...transaction };
delete input.input;
delete input.maxPriorityFeePerGas;
delete input.maxFeePerGas;
delete input.data;
Expand All @@ -259,22 +258,37 @@ describe('defaultTransactionBuilder', () => {
fillGasPrice: true,
});
expect(result.input).toBe('0x');
expect(result.data).toBe('0x');
expect(result.data).toBeUndefined();
});

it('should prefix with 0x', async () => {
it('should prefix input with 0x', async () => {
const input = { ...transaction };
input.input = '123';
delete input.maxPriorityFeePerGas;
delete input.maxFeePerGas;
input.data = '123';
delete input.data;

const result = await defaultTransactionBuilder({
transaction: input,
web3Context,
fillGasPrice: true,
});
expect(result.input).toBe('0x123');
expect(result.data).toBeUndefined();
});

it('should prefix data with 0x', async () => {
const input = { ...transaction };
delete input.maxPriorityFeePerGas;
delete input.maxFeePerGas;
input.data = '123';

const result = await defaultTransactionBuilder({
transaction: input,
web3Context,
fillGasPrice: true,
});
expect(result.input).toBeUndefined();
expect(result.data).toBe('0x123');
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const transaction: Transaction = {
maxFeePerGas: '0x1229298c00',
maxPriorityFeePerGas: '0x49504f80',
data: '0x',
input: '0x',
nonce: '0x4',
chain: 'mainnet',
hardfork: 'berlin',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export const mockRpcResponse: TransactionInfo = {
gasPrice: '0xa83613262',
hash: '0x5f67b495f9c53b942cb1bfacaf175ad887372d7227454a971f15f5e6a7639ad1',
input: '0x38ed17390000000000000000000000000000000000000000000000147ebc6d689cc81c8c0000000000000000000000000000000000000000000000005b7471df733ea75c00000000000000000000000000000000000000000000000000000000000000a0000000000000000000000000cfb162c6de7ee2b49048b270cb5e297da5b6e6c30000000000000000000000000000000000000000000000000000000061134c8f0000000000000000000000000000000000000000000000000000000000000003000000000000000000000000d084b83c305dafd76ae3e1b4e1f1fe2ecccb3988000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2000000000000000000000000d2877702675e6ceb975b4a1dff9fb7baf4c91ea9',
data: '0x38ed17390000000000000000000000000000000000000000000000147ebc6d689cc81c8c0000000000000000000000000000000000000000000000005b7471df733ea75c00000000000000000000000000000000000000000000000000000000000000a0000000000000000000000000cfb162c6de7ee2b49048b270cb5e297da5b6e6c30000000000000000000000000000000000000000000000000000000061134c8f0000000000000000000000000000000000000000000000000000000000000003000000000000000000000000d084b83c305dafd76ae3e1b4e1f1fe2ecccb3988000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2000000000000000000000000d2877702675e6ceb975b4a1dff9fb7baf4c91ea9',
maxFeePerGas: '0xf2cec3661',
maxPriorityFeePerGas: '0xb2d05e00',
nonce: '0xb8',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export const mockRpcResponse: Transaction = {
maxFeePerGas: '0x1229298c00',
maxPriorityFeePerGas: '0x49504f80',
data: '0x',
input: '0x',
nonce: '0x4',
chain: 'mainnet',
hardfork: 'berlin',
Expand Down
Loading

0 comments on commit 51d087e

Please sign in to comment.