Skip to content

Commit edcf5ae

Browse files
committed
feat: transaction batch security validation (#5526)
Support triggering security validation in the client while processing transaction batches. Specifically: - Add `ValidateSecurityRequest` type. - Add optional `validateSecurity` callback to `TransactionBatchRequest` type. - Add optional `securityAlertId` to `AddTransactionBatchRequest` type. - Add optional `securityAlertId` to `SecurityAlertResponse` type. - Call `validateSecurity` callback after generating EIP-7702 transaction. - Include `delegationMock` if an EIP-7702 type 4 transaction. Relates to [ See `CHANGELOG.md`. - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate - [x] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
1 parent faad205 commit edcf5ae

File tree

4 files changed

+206
-4
lines changed

4 files changed

+206
-4
lines changed

packages/transaction-controller/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ export type {
6262
TransactionMeta,
6363
TransactionParams,
6464
TransactionReceipt,
65+
ValidateSecurityRequest,
6566
} from './types';
6667
export {
6768
GasFeeEstimateLevel,

packages/transaction-controller/src/types.ts

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1132,10 +1132,11 @@ export type TransactionError = {
11321132
* Type for security alert response from transaction validator.
11331133
*/
11341134
export type SecurityAlertResponse = {
1135-
reason: string;
11361135
features?: string[];
1137-
result_type: string;
11381136
providerRequestsCount?: Record<string, number>;
1137+
reason: string;
1138+
result_type: string;
1139+
securityAlertId?: string;
11391140
};
11401141

11411142
/** Alternate priority levels for which values are provided in gas fee estimates. */
@@ -1474,8 +1475,22 @@ export type TransactionBatchRequest = {
14741475
/** Whether an approval request should be created to require confirmation from the user. */
14751476
requireApproval?: boolean;
14761477

1478+
/** Security alert ID to persist on the transaction. */
1479+
securityAlertId?: string;
1480+
14771481
/** Transactions to be submitted as part of the batch. */
14781482
transactions: TransactionBatchSingleRequest[];
1483+
1484+
/**
1485+
* Callback to trigger security validation in the client.
1486+
*
1487+
* @param request - The JSON-RPC request to validate.
1488+
* @param chainId - The chain ID of the transaction batch.
1489+
*/
1490+
validateSecurity?: (
1491+
request: ValidateSecurityRequest,
1492+
chainId: Hex,
1493+
) => Promise<void>;
14791494
};
14801495

14811496
/**
@@ -1485,3 +1500,17 @@ export type TransactionBatchResult = {
14851500
/** ID of the batch to locate related transactions. */
14861501
batchId: Hex;
14871502
};
1503+
1504+
/**
1505+
* Request to validate security of a transaction in the client.
1506+
*/
1507+
export type ValidateSecurityRequest = {
1508+
/** JSON-RPC method to validate. */
1509+
method: string;
1510+
1511+
/** Parameters of the JSON-RPC method to validate. */
1512+
params: unknown[];
1513+
1514+
/** Optional EIP-7702 delegation to mock for the transaction sender. */
1515+
delegationMock?: Hex;
1516+
};

packages/transaction-controller/src/utils/batch.test.ts

Lines changed: 145 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,17 @@ const PUBLIC_KEY_MOCK = '0x112233';
4343
const BATCH_ID_CUSTOM_MOCK = '0x123456';
4444
const GET_ETH_QUERY_MOCK = jest.fn();
4545
const GET_INTERNAL_ACCOUNTS_MOCK = jest.fn().mockReturnValue([]);
46-
47-
const TRANSACTION_META_MOCK = {} as TransactionMeta;
46+
const SECURITY_ALERT_ID_MOCK = '123-456';
47+
48+
const TRANSACTION_META_MOCK = {
49+
id: BATCH_ID_CUSTOM_MOCK,
50+
txParams: {
51+
from: FROM_MOCK,
52+
to: TO_MOCK,
53+
data: DATA_MOCK,
54+
value: VALUE_MOCK,
55+
},
56+
} as TransactionMeta;
4857

4958
describe('Batch Utils', () => {
5059
const doesChainSupportEIP7702Mock = jest.mocked(doesChainSupportEIP7702);
@@ -84,6 +93,8 @@ describe('Batch Utils', () => {
8493
type: TransactionType.simpleSend,
8594
});
8695

96+
getChainIdMock.mockReturnValue(CHAIN_ID_MOCK);
97+
8798
request = {
8899
addTransaction: addTransactionMock,
89100
getChainId: getChainIdMock,
@@ -383,6 +394,138 @@ describe('Batch Utils', () => {
383394
'Validation Error',
384395
);
385396
});
397+
398+
it('adds security alert ID to transaction', async () => {
399+
doesChainSupportEIP7702Mock.mockReturnValueOnce(true);
400+
401+
isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({
402+
delegationAddress: undefined,
403+
isSupported: true,
404+
});
405+
406+
addTransactionMock.mockResolvedValueOnce({
407+
transactionMeta: TRANSACTION_META_MOCK,
408+
result: Promise.resolve(''),
409+
});
410+
411+
generateEIP7702BatchTransactionMock.mockReturnValueOnce({
412+
to: TO_MOCK,
413+
data: DATA_MOCK,
414+
value: VALUE_MOCK,
415+
});
416+
417+
request.request.securityAlertId = SECURITY_ALERT_ID_MOCK;
418+
419+
await addTransactionBatch(request);
420+
421+
expect(addTransactionMock).toHaveBeenCalledTimes(1);
422+
expect(addTransactionMock).toHaveBeenCalledWith(
423+
expect.any(Object),
424+
expect.objectContaining({
425+
securityAlertResponse: {
426+
securityAlertId: SECURITY_ALERT_ID_MOCK,
427+
},
428+
}),
429+
);
430+
});
431+
432+
describe('validates security', () => {
433+
it('using transaction params', async () => {
434+
doesChainSupportEIP7702Mock.mockReturnValueOnce(true);
435+
436+
isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({
437+
delegationAddress: undefined,
438+
isSupported: true,
439+
});
440+
441+
addTransactionMock.mockResolvedValueOnce({
442+
transactionMeta: TRANSACTION_META_MOCK,
443+
result: Promise.resolve(''),
444+
});
445+
446+
generateEIP7702BatchTransactionMock.mockReturnValueOnce({
447+
to: TO_MOCK,
448+
data: DATA_MOCK,
449+
value: VALUE_MOCK,
450+
});
451+
452+
const validateSecurityMock = jest.fn();
453+
validateSecurityMock.mockResolvedValueOnce({});
454+
455+
request.request.validateSecurity = validateSecurityMock;
456+
457+
await addTransactionBatch(request);
458+
459+
expect(validateSecurityMock).toHaveBeenCalledTimes(1);
460+
expect(validateSecurityMock).toHaveBeenCalledWith(
461+
{
462+
delegationMock: undefined,
463+
method: 'eth_sendTransaction',
464+
params: [
465+
{
466+
authorizationList: undefined,
467+
data: DATA_MOCK,
468+
from: FROM_MOCK,
469+
to: TO_MOCK,
470+
type: TransactionEnvelopeType.feeMarket,
471+
value: VALUE_MOCK,
472+
},
473+
],
474+
},
475+
CHAIN_ID_MOCK,
476+
);
477+
});
478+
479+
it('using delegation mock if not upgraded', async () => {
480+
doesChainSupportEIP7702Mock.mockReturnValueOnce(true);
481+
482+
isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({
483+
delegationAddress: undefined,
484+
isSupported: false,
485+
});
486+
487+
addTransactionMock.mockResolvedValueOnce({
488+
transactionMeta: TRANSACTION_META_MOCK,
489+
result: Promise.resolve(''),
490+
});
491+
492+
generateEIP7702BatchTransactionMock.mockReturnValueOnce({
493+
to: TO_MOCK,
494+
data: DATA_MOCK,
495+
value: VALUE_MOCK,
496+
});
497+
498+
getEIP7702UpgradeContractAddressMock.mockReturnValue(
499+
CONTRACT_ADDRESS_MOCK,
500+
);
501+
502+
const validateSecurityMock = jest.fn();
503+
validateSecurityMock.mockResolvedValueOnce({});
504+
505+
request.request.validateSecurity = validateSecurityMock;
506+
507+
await addTransactionBatch(request);
508+
509+
expect(validateSecurityMock).toHaveBeenCalledTimes(1);
510+
expect(validateSecurityMock).toHaveBeenCalledWith(
511+
{
512+
delegationMock: CONTRACT_ADDRESS_MOCK,
513+
method: 'eth_sendTransaction',
514+
params: [
515+
{
516+
authorizationList: undefined,
517+
data: DATA_MOCK,
518+
from: FROM_MOCK,
519+
to: TO_MOCK,
520+
type: TransactionEnvelopeType.feeMarket,
521+
value: VALUE_MOCK,
522+
},
523+
],
524+
},
525+
CHAIN_ID_MOCK,
526+
);
527+
});
528+
});
386529
});
387530

388531
describe('isAtomicBatchSupported', () => {

packages/transaction-controller/src/utils/batch.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ import {
2424
import { projectLogger } from '../logger';
2525
import type {
2626
NestedTransactionMetadata,
27+
SecurityAlertResponse,
2728
TransactionBatchSingleRequest,
29+
ValidateSecurityRequest,
2830
} from '../types';
2931
import {
3032
TransactionEnvelopeType,
@@ -83,7 +85,9 @@ export async function addTransactionBatch(
8385
from,
8486
networkClientId,
8587
requireApproval,
88+
securityAlertId,
8689
transactions,
90+
validateSecurity,
8791
} = userRequest;
8892

8993
log('Adding', userRequest);
@@ -144,15 +148,40 @@ export async function addTransactionBatch(
144148
txParams.authorizationList = [{ address: upgradeContractAddress }];
145149
}
146150

151+
if (validateSecurity) {
152+
const securityRequest: ValidateSecurityRequest = {
153+
method: 'eth_sendTransaction',
154+
params: [
155+
{
156+
...txParams,
157+
authorizationList: undefined,
158+
type: TransactionEnvelopeType.feeMarket,
159+
},
160+
],
161+
delegationMock: txParams.authorizationList?.[0]?.address,
162+
};
163+
164+
log('Security request', securityRequest);
165+
166+
validateSecurity(securityRequest, chainId).catch((error) => {
167+
log('Security validation failed', error);
168+
});
169+
}
170+
147171
log('Adding batch transaction', txParams, networkClientId);
148172

149173
const batchId = batchIdOverride ?? generateBatchId();
150174

175+
const securityAlertResponse = securityAlertId
176+
? ({ securityAlertId } as SecurityAlertResponse)
177+
: undefined;
178+
151179
const { result } = await addTransaction(txParams, {
152180
batchId,
153181
nestedTransactions,
154182
networkClientId,
155183
requireApproval,
184+
securityAlertResponse,
156185
type: TransactionType.batch,
157186
});
158187

0 commit comments

Comments
 (0)