feat: use security api for simulations - #205
Conversation
|
The logic implemented by this PR is perfect and follows the standard set by Solana which is cool to have more code similarities. I have found some improvements though that should happen at the tests level which we are trying to enforce everywhere. Dropping them in the appropriate places |
| let mockTransactionScanService: jest.Mocked<TransactionScanService>; | ||
| let cronHandler: CronHandler; | ||
|
|
||
| beforeEach(() => { |
There was a problem hiding this comment.
Quoting Michele here, following our Contributor Guidelines for new tests (we can change the existing ones as we go) I would love it if we could remove the beforeEach and "instead, we should prefer setup functions that wrap the test logic and provide the necessary context for the test, e.g.: https://github.com/MetaMask/core/blob/78fe98b1e74e2d1aea047727ec789121f123eb50/packages/profile-metrics-controller/src/ProfileMetricsController.test.ts#L719-L719"
More can be read about it here. I have some open PRs that follow this approach that you can pass to your favorite LLM to adapt the changes for you:
| getInterfaceContext: jest.fn().mockResolvedValue(interfaceContext), | ||
| scheduleBackgroundEvent: jest.fn().mockResolvedValue(undefined), | ||
| getPreferences: jest.fn().mockResolvedValue({}), | ||
| } as unknown as jest.Mocked<SnapClient>; |
There was a problem hiding this comment.
Can we remove these unknown's? Even if it involves more boilerplate code
| const mockState = { | ||
| setKey: jest.fn().mockResolvedValue(undefined), | ||
| getKey: jest.fn().mockResolvedValue(mapInterfaceNameToId), | ||
| } as unknown as jest.Mocked<{ getKey: jest.Mock; setKey: jest.Mock }>; |
There was a problem hiding this comment.
Can we remove these unknown's? Even if it involves more boilerplate code
| const mockTransactionScanService = { | ||
| scanTransaction: jest.fn().mockResolvedValue(scanResult), | ||
| getSecurityAlertDescription: jest.fn().mockReturnValue('description'), | ||
| } as unknown as jest.Mocked<TransactionScanService>; |
There was a problem hiding this comment.
Can we remove these unknown's? Even if it involves more boilerplate code
| error: jest.fn(), | ||
| log: jest.fn(), | ||
| debug: jest.fn(), | ||
| } as any, |
There was a problem hiding this comment.
Can we remove this any?
| } as any, | ||
| accountsService: {} as AccountsService, | ||
| snapClient: mockSnapClient, | ||
| state: mockState as any, |
2 participants
Explanation
Add security scanning (Blockaid) for simple token sends, matching the existing pattern used for swaps/dapp transactions and the Solana Snap.
TransactionAlert(whenuseSecurityAlertsis enabled) andEstimatedChangesfrom the scan response (whensimulateOnChainActionsis enabled), replacing the previous static estimated changesRefreshConfirmationSendbackground jobSecurityScanCompletedandSecurityAlertDetectedanalytics eventsReferences
Fixes NEB-324
Test
You send. -0.000001 TRX
You receive. +0.000001 WTRX
Checklist