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

Add verifiedOnBlockchain property to transaction meta #20890

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ const EXPECTED_NORMALISED_TRANSACTION_BASE = {
value: '0xb1a2bc2ec50000',
},
type: TransactionType.incoming,
verifiedOnBlockchain: false,
};

const EXPECTED_NORMALISED_TRANSACTION_SUCCESS = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ export class EtherscanRemoteTransactionSource
value: BNToHex(new BN(txMeta.value)),
},
type: TransactionType.incoming,
verifiedOnBlockchain: false,
} as TransactionMeta;
}
}
2 changes: 2 additions & 0 deletions app/scripts/controllers/transactions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -963,6 +963,8 @@ export default class TransactionController extends EventEmitter {
if (blockTimestamp) {
txMeta.blockTimestamp = blockTimestamp;
}
// transaction verified on the blockchain
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, but is this comment adding much additional detail?

txMeta.verifiedOnBlockchain = true;

this.txStateManager.setTxStatusConfirmed(txId);
this._markNonceDuplicatesDropped(txId);
Expand Down
5 changes: 5 additions & 0 deletions app/scripts/controllers/transactions/pending-tx-tracker.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,11 @@ export default class PendingTransactionTracker extends EventEmitter {
return;
}

// Skip processing transactions verified on the blockchain
matthewwalsh0 marked this conversation as resolved.
Show resolved Hide resolved
if (txMeta.verifiedOnBlockchain) {
matthewwalsh0 marked this conversation as resolved.
Show resolved Hide resolved
return;
}

// extra check in case there was an uncaught error during the
// signature and submission process

Expand Down
1 change: 1 addition & 0 deletions app/scripts/controllers/transactions/tx-state-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ export default class TransactionStateManager extends EventEmitter {
loadingDefaults: true,
dappSuggestedGasFees,
sendFlowHistory: [],
verifiedOnBlockchain: false,
...opts,
};
}
Expand Down
117 changes: 117 additions & 0 deletions app/scripts/migrations/098.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
import { migrate, version } from './098';

const oldVersion = 97;
describe('migration #98', () => {
it('updates the version metadata', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {},
};

const newStorage = await migrate(oldStorage);

expect(newStorage.meta).toStrictEqual({ version });
});

it('handles missing TransactionController', async () => {
const oldState = {
OtherController: {},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: oldState,
});

expect(transformedState.data).toEqual(oldState);
});

it('handles empty transactions', async () => {
const oldState = {
TransactionController: {
transactions: {},
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: oldState,
});

expect(transformedState.data).toEqual(oldState);
});

it('handles missing state', async () => {
const transformedState = await migrate({
meta: { version: oldVersion },
data: {},
});

expect(transformedState.data).toEqual({});
});

it('adds verifiedOnBlockchain in transaction based on the presence of txReceipt', async () => {
const oldState = {
TransactionController: {
transactions: {
tx1: {
to: '0x9ef57335bc7d5b6cbc06dca6064a604b75e09ace',
txReceipt: {
blockHash:
'0xafa4e1fd95e429d9c6e6c7c1d282b2bd0bbeb50d0a68743e9392b9c95a06e2eb',
},
otherProp: 'value',
},
tx2: {
to: '0x9ef57335bc7d5b6cbc06dca6064a604b75e09ace',
otherProp: 'value',
},
tx3: {
to: '0x9ef57335bc7d5b6cbc06dca6064a604b75e09ace',
txReceipt: {
blockHash:
'0xafa4e1fd95e429d9c6e6c7c1d282b2bd0bbeb50d0a68743e9392b9c95a06e2eb',
},
otherProp: 'value',
},
},
},
};
const oldStorage = {
meta: { version: oldVersion },
data: oldState,
};

const newStorage = await migrate(oldStorage);

expect(newStorage.data).toEqual({
TransactionController: {
transactions: {
tx1: {
to: '0x9ef57335bc7d5b6cbc06dca6064a604b75e09ace',
txReceipt: {
blockHash:
'0xafa4e1fd95e429d9c6e6c7c1d282b2bd0bbeb50d0a68743e9392b9c95a06e2eb',
},
otherProp: 'value',
verifiedOnBlockchain: true,
},
tx2: {
to: '0x9ef57335bc7d5b6cbc06dca6064a604b75e09ace',
otherProp: 'value',
verifiedOnBlockchain: false,
},
tx3: {
to: '0x9ef57335bc7d5b6cbc06dca6064a604b75e09ace',
txReceipt: {
blockHash:
'0xafa4e1fd95e429d9c6e6c7c1d282b2bd0bbeb50d0a68743e9392b9c95a06e2eb',
},
otherProp: 'value',
verifiedOnBlockchain: true,
},
},
},
});
});
});
52 changes: 52 additions & 0 deletions app/scripts/migrations/098.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { cloneDeep, isEmpty } from 'lodash';

type VersionedData = {
meta: { version: number };
data: Record<string, unknown>;
};

export const version = 98; // Increment the version number

/**
* Add `verifiedOnBlockchain` property to transactions based on the presence of `txReceipt`
*
* @param originalVersionedData - Versioned MetaMask extension state, exactly what we persist to dist.
* @param originalVersionedData.meta - State metadata.
* @param originalVersionedData.meta.version - The current state version.
* @param originalVersionedData.data - The persisted MetaMask state, keyed by controller.
* @returns Updated versioned MetaMask extension state.
*/
export async function migrate(
originalVersionedData: VersionedData,
): Promise<VersionedData> {
const versionedData = cloneDeep(originalVersionedData);
versionedData.meta.version = version;
transformState(versionedData.data);
return versionedData;
}

function transformState(state: Record<string, any>) {
const transactionControllerState = state?.TransactionController || {};
const transactions = transactionControllerState?.transactions || {};

if (isEmpty(transactions)) {
return;
}

const newTxs = Object.keys(transactions).reduce((txs, txId) => {
const transaction = transactions[txId];

// Add the `verifiedOnBlockchain` property based on the presence of `txReceipt`
transaction.verifiedOnBlockchain = Boolean(transaction.txReceipt);

return {
...txs,
[txId]: transaction,
};
}, {});

state.TransactionController = {
...transactionControllerState,
transactions: newTxs,
};
}
2 changes: 2 additions & 0 deletions app/scripts/migrations/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ import * as m094 from './094';
import * as m095 from './095';
import * as m096 from './096';
import * as m097 from './097';
import * as m098 from './098';

const migrations = [
m002,
Expand Down Expand Up @@ -205,5 +206,6 @@ const migrations = [
m095,
m096,
m097,
m098,
];
export default migrations;
4 changes: 4 additions & 0 deletions shared/constants/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,10 @@ export interface TransactionMeta {
submittedTime?: number;
/** The error encountered during the transaction */
txErr?: TxError;
/**
* Whether the transaction is verified on the blockchain.
*/
verifiedOnBlockchain?: boolean;
}

/**
Expand Down