Skip to content

feat: Adding requestId field to dapp confirmations #7415

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.

Sign up for GitHub

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

Open
jpuri wants to merge 13 commits into
base: main
Choose a base branch
from
+41 −0
Open

feat: Adding requestId field to dapp confirmations #7415

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
2cfdb03
Adding requestId field to dapp confirmations
jpuri Dec 9, 2025
9abf1e0
Merge branch 'main' into request_id_support
jpuri Dec 9, 2025
665775d
update
jpuri Dec 9, 2025
71b0332
Merge branch 'request_id_support' of https://github.com/MetaMask/core…
jpuri Dec 9, 2025
c645831
update
jpuri Dec 9, 2025
d58f206
Merge branch 'main' into request_id_support
jpuri Dec 9, 2025
35a452d
Merge branch 'main' into request_id_support
jpuri Dec 9, 2025
c75349a
update
jpuri Dec 10, 2025
741816f
Merge branch 'request_id_support' of https://github.com/MetaMask/core…
jpuri Dec 10, 2025
ddd24c9
update
jpuri Dec 10, 2025
b7ed73e
Merge branch 'main' into request_id_support
jpuri Dec 10, 2025
a209dc8
update
jpuri Dec 10, 2025
e854e88
Merge branch 'request_id_support' of https://github.com/MetaMask/core…
jpuri Dec 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/eip-5792-middleware/src/hooks/processSendCalls.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ describe('EIP-5792', () => {
from: SEND_CALLS_MOCK.from,
networkClientId: NETWORK_CLIENT_ID_MOCK,
origin: ORIGIN_MOCK,
requestId: '1',
securityAlertId: expect.any(String),
transactions: [
{ params: SEND_CALLS_MOCK.calls[0] },
Expand Down Expand Up @@ -237,6 +238,7 @@ describe('EIP-5792', () => {
batchId: expect.any(String),
networkClientId: 'test-client',
origin: 'test.com',
requestId: '1',
securityAlertResponse: {
securityAlertId: expect.any(String),
},
Expand Down
12 changes: 12 additions & 0 deletions packages/eip-5792-middleware/src/hooks/processSendCalls.ts
View file
Open in desktop
Copy link
Member

@OGPoyraz OGPoyraz Dec 10, 2025

Choose a reason for hiding this comment

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

Do we want to note additions in changelog on this package and transaction-controller?

Copy link
Contributor Author

@jpuri jpuri Dec 10, 2025

Choose a reason for hiding this comment

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

Good point, let me do that.

Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ export async function processSendCalls(
const securityAlertId = uuid();
const validateSecurity = validateSecurityHook.bind(null, securityAlertId);

const requestId = String(req.id);
Copy link
Member

@OGPoyraz OGPoyraz Dec 10, 2025

Choose a reason for hiding this comment

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

What if we use JsonRpcId type from @metamask/utils in the TransactionController types? As v2 JSONRpcEngine referring the same types https://github.com/MetaMask/core/blob/main/packages/json-rpc-engine/src/v2/JsonRpcEngineV2.ts#L4

Then we wouldn't really need to cast here and just pass as is?

Copy link
Contributor Author

@jpuri jpuri Dec 10, 2025

Choose a reason for hiding this comment

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

String is I think more useful even later when we use it in the UI

Copy link
Member

@OGPoyraz OGPoyraz Dec 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry not sure I follow, what makes string to be useful and how's that related with UI?

It's a minor thing for sure, but am not completely convinced if casting here necessary. So I will defer this to @dan437

Copy link
Contributor

@dan437 dan437 Dec 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I would prefer to use the JsonRpcId type and not cast it into String for consistency reasons.

import {
  type JsonRpcId,
} from '@metamask/utils';

Copy link
Contributor Author

@jpuri jpuri Dec 12, 2025

Choose a reason for hiding this comment

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

We are using JSOP RPC request ID here but I do not want to tie completely to it, we may extend this field with some custom value and we are going to use it a lot in UI. string is more generic type, that is also reason it was casted for actionId.


let batchId: Hex;
if (Object.keys(transactions).length === 1) {
batchId = await processSingleTransaction({
Expand All @@ -110,6 +112,7 @@ export async function processSendCalls(
messenger,
networkClientId,
origin,
requestId,
securityAlertId,
sendCalls: params,
transactions,
Expand All @@ -126,6 +129,7 @@ export async function processSendCalls(
messenger,
networkClientId,
origin,
requestId,
sendCalls: params,
securityAlertId,
transactions,
Expand All @@ -147,6 +151,7 @@ export async function processSendCalls(
* @param params.messenger - Messenger instance for controller communication.
* @param params.networkClientId - The network client ID.
* @param params.origin - The origin of the request (optional).
* @param params.requestId - Unique requestId of the JSON-RPC request from DAPP.
* @param params.securityAlertId - The security alert ID for this transaction.
* @param params.sendCalls - The original sendCalls request.
* @param params.transactions - Array containing the single transaction.
Expand All @@ -161,6 +166,7 @@ async function processSingleTransaction({
messenger,
networkClientId,
origin,
requestId,
securityAlertId,
sendCalls,
transactions,
Expand All @@ -173,6 +179,7 @@ async function processSingleTransaction({
messenger: EIP5792Messenger;
networkClientId: string;
origin?: string;
requestId?: string;
securityAlertId: string;
sendCalls: SendCallsPayload;
transactions: { params: BatchTransactionParams }[];
Expand Down Expand Up @@ -209,6 +216,7 @@ async function processSingleTransaction({
const batchId = generateBatchId();

await addTransaction(txParams, {
requestId,
networkClientId,
origin,
securityAlertResponse: { securityAlertId } as SecurityAlertResponse,
Expand All @@ -229,6 +237,7 @@ async function processSingleTransaction({
* @param params.networkClientId - The network client ID.
* @param params.messenger - Messenger instance for controller communication.
* @param params.origin - The origin of the request (optional).
* @param params.requestId - Unique requestId of the JSON-RPC request from DAPP.
* @param params.sendCalls - The original sendCalls request.
* @param params.securityAlertId - The security alert ID for this batch.
* @param params.transactions - Array of transactions to process.
Expand All @@ -245,6 +254,7 @@ async function processMultipleTransaction({
networkClientId,
messenger,
origin,
requestId,
sendCalls,
securityAlertId,
transactions,
Expand All @@ -259,6 +269,7 @@ async function processMultipleTransaction({
messenger: EIP5792Messenger;
networkClientId: string;
origin?: string;
requestId?: string;
sendCalls: SendCallsPayload;
securityAlertId: string;
transactions: { params: BatchTransactionParams }[];
Expand Down Expand Up @@ -295,6 +306,7 @@ async function processMultipleTransaction({
from,
networkClientId,
origin,
requestId,
securityAlertId,
transactions,
validateSecurity,
Expand Down
1 change: 1 addition & 0 deletions packages/transaction-controller/src/TransactionController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1783,6 +1783,7 @@ describe('TransactionController', () => {
nestedTransactions: undefined,
networkClientId: NETWORK_CLIENT_ID_MOCK,
origin: undefined,
requestId: undefined,
securityAlertResponse: undefined,
selectedGasFeeToken: undefined,
sendFlowHistory: expect.any(Array),
Expand Down
2 changes: 2 additions & 0 deletions packages/transaction-controller/src/TransactionController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1269,6 +1269,7 @@ export class TransactionController extends BaseController<
networkClientId,
origin,
publishHook,
requestId,
requireApproval,
securityAlertResponse,
sendFlowHistory,
Expand Down Expand Up @@ -1366,6 +1367,7 @@ export class TransactionController extends BaseController<
nestedTransactions,
networkClientId,
origin,
requestId,
securityAlertResponse,
selectedGasFeeToken: gasFeeToken,
status: TransactionStatus.unapproved as const,
Expand Down
16 changes: 16 additions & 0 deletions packages/transaction-controller/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,11 @@ export type TransactionMeta = {
*/
replacedById?: string;

/**
* ID of JSON-RPC request from DAPP.
*/
requestId?: string;

/**
* IDs of any transactions that must be confirmed before this one is submitted.
* Unlike a transaction batch, these transactions can be on alternate chains.
Expand Down Expand Up @@ -592,6 +597,11 @@ export type TransactionBatchMeta = {
*/
origin?: string;

/**
* ID of the JSON-RPC request from DAPP.
*/
requestId?: string;

/** Current status of the transaction. */
status: TransactionStatus;

Expand Down Expand Up @@ -1754,6 +1764,9 @@ export type TransactionBatchRequest = {
/** Whether to overwrite existing EIP-7702 delegation with MetaMask contract. */
overwriteUpgrade?: boolean;

/** ID of the JSON-RPC request from DAPP. */
requestId?: string;

/** Whether an approval request should be created to require confirmation from the user. */
requireApproval?: boolean;

Expand Down Expand Up @@ -2107,6 +2120,9 @@ export type AddTransactionOptions = {
/** Custom logic to publish the transaction. */
publishHook?: PublishHook;

/** ID of JSON-RPC request from DAPP. */
requestId?: string;

/** Whether the transaction requires approval by the user, defaults to true unless explicitly disabled. */
requireApproval?: boolean | undefined;

Expand Down
5 changes: 5 additions & 0 deletions packages/transaction-controller/src/utils/batch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ type AddTransactionBatchRequest = {
) => Promise<Hex>;
publicKeyEIP7702?: Hex;
request: TransactionBatchRequest;
requestId?: string;
signTransaction: (
transactionMeta: TransactionMeta,
) => Promise<string | undefined>;
Expand Down Expand Up @@ -300,6 +301,7 @@ async function addTransactionBatchWith7702(
networkClientId,
origin,
overwriteUpgrade,
requestId,
requireApproval,
securityAlertId,
skipInitialGasEstimate,
Expand Down Expand Up @@ -430,6 +432,7 @@ async function addTransactionBatchWith7702(
nestedTransactions,
networkClientId,
origin,
requestId,
requireApproval,
securityAlertResponse,
skipInitialGasEstimate,
Expand Down Expand Up @@ -854,6 +857,7 @@ async function prepareApprovalData({
origin,
networkClientId,
transactions: nestedTransactions,
requestId,
} = userRequest;

const ethQuery = getEthQuery(networkClientId);
Expand Down Expand Up @@ -881,6 +885,7 @@ async function prepareApprovalData({
networkClientId,
origin,
transactions: nestedTransactions,
requestId,
});

const defaultGasFeeFlow = new DefaultGasFeeFlow();
Expand Down
Loading