Skip to content

Commit

Permalink
Rename transactionHash property to hash in the persisted transaction …
Browse files Browse the repository at this point in the history
…metadata (#1620)

## Explanation
Rename the `transactionHash` property to `hash` in the persisted
transaction metadata as part of the unification initiative between the
core and extension.

The following task will take place to handle the migration and adoption
of the patch/release.

## References

Fixes MetaMask/MetaMask-planning#1077

## Changelog

### `@metamask/transaction-controller`

- **BREAKING**: rename the `transactionHash` property to `hash` in the
persisted `TransactionMeta`.
  • Loading branch information
vinistevam authored Sep 12, 2023
1 parent 18987b8 commit ae43985
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ const ETHERSCAN_TOKEN_TRANSACTION_RESPONSE_EMPTY_MOCK: EtherscanTransactionRespo
const EXPECTED_NORMALISED_TRANSACTION_BASE = {
blockNumber: ETHERSCAN_TRANSACTION_SUCCESS_MOCK.blockNumber,
chainId: undefined,
hash: ETHERSCAN_TRANSACTION_SUCCESS_MOCK.hash,
id: ID_MOCK,
networkID: undefined,
status: TransactionStatus.confirmed,
Expand All @@ -102,7 +103,6 @@ const EXPECTED_NORMALISED_TRANSACTION_BASE = {
to: ETHERSCAN_TRANSACTION_SUCCESS_MOCK.to,
value: '0xb1a2bc2ec50000',
},
transactionHash: ETHERSCAN_TRANSACTION_SUCCESS_MOCK.hash,
verifiedOnBlockchain: false,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ export class EtherscanRemoteTransactionSource
return {
blockNumber: txMeta.blockNumber,
chainId: currentChainId,
hash: txMeta.hash,
id: random({ msecs: time }),
networkID: currentNetworkId,
status: TransactionStatus.confirmed,
Expand All @@ -154,7 +155,6 @@ export class EtherscanRemoteTransactionSource
to: txMeta.to,
value: BNToHex(new BN(txMeta.value)),
},
transactionHash: txMeta.hash,
verifiedOnBlockchain: false,
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,18 @@ const CONTROLLER_ARGS_MOCK = {
const TRANSACTION_MOCK: TransactionMeta = {
blockNumber: '123',
chainId: '0x1',
hash: '0x1',
status: TransactionStatus.submitted,
time: 0,
transaction: { to: '0x1', gasUsed: '0x1' },
transactionHash: '0x1',
} as unknown as TransactionMeta;

const TRANSACTION_MOCK_2: TransactionMeta = {
blockNumber: '234',
hash: '0x2',
chainId: '0x1',
time: 1,
transaction: { to: '0x1' },
transactionHash: '0x2',
} as unknown as TransactionMeta;

const createRemoteTransactionSourceMock = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,7 @@ export class IncomingTransactionHelper {
localTxs: TransactionMeta[],
): TransactionMeta[] {
return remoteTxs.filter(
(tx) =>
!localTxs.some(
({ transactionHash }) => transactionHash === tx.transactionHash,
),
(tx) => !localTxs.some(({ hash }) => hash === tx.hash),
);
}

Expand All @@ -199,7 +196,7 @@ export class IncomingTransactionHelper {
return remoteTxs.filter((remoteTx) =>
localTxs.some(
(localTx) =>
remoteTx.transactionHash === localTx.transactionHash &&
remoteTx.hash === localTx.hash &&
this.#isTransactionOutdated(remoteTx, localTx),
),
);
Expand Down
48 changes: 22 additions & 26 deletions packages/transaction-controller/src/TransactionController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,10 @@ jest.mock('@metamask/eth-query', () =>
},
getTransactionByHash: (_hash: string, callback: any) => {
const txs: any = [
{ transactionHash: '1337', blockNumber: '0x1' },
{ transactionHash: '1338', blockNumber: null },
{ blockNumber: '0x1', hash: '1337' },
{ blockNumber: null, hash: '1338' },
];
const tx: any = txs.find(
(element: any) => element.transactionHash === _hash,
);
const tx: any = txs.find((element: any) => element.hash === _hash);
callback(undefined, tx);
},
getTransactionCount: (_from: any, _to: any, callback: any) => {
Expand All @@ -101,30 +99,28 @@ jest.mock('@metamask/eth-query', () =>
getTransactionReceipt: (_hash: any, callback: any) => {
const txs: any = [
{
transactionHash: '1337',
blockHash: '1337',
gasUsed: '0x5208',
hash: '1337',
status: '0x1',
transactionIndex: 1337,
blockHash: '1337',
},
{
transactionHash: '1111',
gasUsed: '0x1108',
hash: '1111',
status: '0x0',
transactionIndex: 1111,
},
];
const tx: any = txs.find(
(element: any) => element.transactionHash === _hash,
);
const tx: any = txs.find((element: any) => element.hash === _hash);
callback(undefined, tx);
},
getBlockByHash: (_blockHash: any, callback: any) => {
const blocks: any = [
{
baseFeePerGas: '0x14',
hash: '1337',
number: '0x1',
baseFeePerGas: '0x14',
timestamp: '628dc0c8',
},
{ hash: '1338', number: '0x2' },
Expand Down Expand Up @@ -399,21 +395,21 @@ const NONCE_MOCK = 12;
const ACTION_ID_MOCK = '123456';

const TRANSACTION_META_MOCK = {
hash: '0x1',
status: TransactionStatus.confirmed,
time: 123456789,
transaction: {
from: ACCOUNT_MOCK,
},
transactionHash: '0x1',
time: 123456789,
} as TransactionMeta;

const TRANSACTION_META_2_MOCK = {
hash: '0x2',
status: TransactionStatus.confirmed,
time: 987654321,
transaction: {
from: '0x3',
},
transactionHash: '0x2',
time: 987654321,
} as TransactionMeta;

describe('TransactionController', () => {
Expand Down Expand Up @@ -1383,10 +1379,10 @@ describe('TransactionController', () => {

controller.state.transactions.push({
from: MOCK_PREFERENCES.state.selectedAddress,
hash: '1337',
id: 'foo',
networkID: '5',
status: TransactionStatus.submitted,
transactionHash: '1337',
} as any);

controller.wipeTransactions();
Expand Down Expand Up @@ -1462,12 +1458,12 @@ describe('TransactionController', () => {
const controller = newController();

controller.state.transactions.push({
chainId: toHex(5),
from: MOCK_PREFERENCES.state.selectedAddress,
hash: '1337',
id: 'foo',
networkID: '5',
chainId: toHex(5),
status: TransactionStatus.submitted,
transactionHash: '1337',
} as any);

controller.state.transactions.push({} as any);
Expand All @@ -1489,10 +1485,10 @@ describe('TransactionController', () => {

controller.state.transactions.push({
from: MOCK_PREFERENCES.state.selectedAddress,
hash: '1337',
id: 'foo',
networkID: '5',
status: TransactionStatus.submitted,
transactionHash: '1337',
} as any);

controller.state.transactions.push({} as any);
Expand All @@ -1515,7 +1511,7 @@ describe('TransactionController', () => {
id: 'foo',
networkID: '5',
status: TransactionStatus.submitted,
transactionHash: '1338',
hash: '1338',
} as any);

await controller.queryTransactionStatuses();
Expand All @@ -1528,16 +1524,16 @@ describe('TransactionController', () => {
const controller = newController();

controller.state.transactions.push({
chainId: toHex(5),
from: MOCK_PREFERENCES.state.selectedAddress,
hash: '1337',
id: 'foo',
networkID: '5',
chainId: toHex(5),
status: TransactionStatus.confirmed,
transactionHash: '1337',
verifiedOnBlockchain: false,
transaction: {
gasUsed: undefined,
},
verifiedOnBlockchain: false,
} as any);

await controller.queryTransactionStatuses();
Expand Down Expand Up @@ -1785,17 +1781,17 @@ describe('TransactionController', () => {
it('creates approvals for all unapproved transaction', async () => {
const transaction = {
from: ACCOUNT_MOCK,
hash: '1337',
id: 'mocked',
networkID: '5',
status: TransactionStatus.unapproved,
transactionHash: '1337',
};
const controller = newController();
controller.state.transactions.push(transaction as any);
controller.state.transactions.push({
...transaction,
id: 'mocked1',
transactionHash: '1338',
hash: '1338',
} as any);

controller.initApprovals();
Expand Down
25 changes: 10 additions & 15 deletions packages/transaction-controller/src/TransactionController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -709,15 +709,13 @@ export class TransactionController extends BaseController<
transactionMeta.transaction.from,
);
const rawTx = bufferToHex(signedTx.serialize());
const transactionHash = await query(this.ethQuery, 'sendRawTransaction', [
rawTx,
]);
const hash = await query(this.ethQuery, 'sendRawTransaction', [rawTx]);
const baseTransactionMeta = {
...transactionMeta,
estimatedBaseFee,
id: random(),
time: Date.now(),
transactionHash,
hash,
actionId,
originalGasEstimate: transactionMeta.transaction.gas,
};
Expand Down Expand Up @@ -1031,7 +1029,7 @@ export class TransactionController extends BaseController<

case TransactionStatus.submitted:
resultCallbacks?.success();
return finalMeta.transactionHash as string;
return finalMeta.hash as string;

default:
const internalError = ethErrors.rpc.internal(
Expand Down Expand Up @@ -1122,10 +1120,8 @@ export class TransactionController extends BaseController<

transactionMeta.rawTx = rawTx;
this.updateTransaction(transactionMeta);
const transactionHash = await query(this.ethQuery, 'sendRawTransaction', [
rawTx,
]);
transactionMeta.transactionHash = transactionHash;
const hash = await query(this.ethQuery, 'sendRawTransaction', [rawTx]);
transactionMeta.hash = hash;
transactionMeta.status = TransactionStatus.submitted;
transactionMeta.submittedTime = new Date().getTime();
this.updateTransaction(transactionMeta);
Expand Down Expand Up @@ -1249,11 +1245,11 @@ export class TransactionController extends BaseController<
private async blockchainTransactionStateReconciler(
meta: TransactionMeta,
): Promise<[TransactionMeta, boolean]> {
const { status, transactionHash } = meta;
const { status, hash } = meta;
switch (status) {
case TransactionStatus.confirmed:
const txReceipt = await query(this.ethQuery, 'getTransactionReceipt', [
transactionHash,
hash,
]);

if (!txReceipt) {
Expand Down Expand Up @@ -1283,12 +1279,12 @@ export class TransactionController extends BaseController<
return [meta, true];
case TransactionStatus.submitted:
const txObj = await query(this.ethQuery, 'getTransactionByHash', [
transactionHash,
hash,
]);

if (!txObj) {
const receiptShowsFailedStatus =
await this.checkTxReceiptStatusIsFailed(transactionHash);
await this.checkTxReceiptStatusIsFailed(hash);

// Case the txObj is evaluated as false, a second check will
// determine if the tx failed or it is pending or confirmed
Expand Down Expand Up @@ -1445,8 +1441,7 @@ export class TransactionController extends BaseController<
...added,
...currentTransactions.map((originalTransaction) => {
const updatedTransaction = updated.find(
({ transactionHash }) =>
transactionHash === originalTransaction.transactionHash,
({ hash }) => hash === originalTransaction.hash,
);

return updatedTransaction ?? originalTransaction;
Expand Down
5 changes: 0 additions & 5 deletions packages/transaction-controller/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,6 @@ type TransactionMetaBase = {
*/
transaction: Transaction;

/**
* Hash of a successful transaction.
*/
transactionHash?: string;

/**
* Additional transfer information.
*/
Expand Down

0 comments on commit ae43985

Please sign in to comment.