Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
]
},
"resolutions": {
"@metamask/snaps-sdk": "9.0.0"
"@metamask/snaps-sdk": "9.3.0"
},
"devDependencies": {
"@commitlint/cli": "^17.7.1",
Expand Down
2 changes: 1 addition & 1 deletion packages/snap/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
"@metamask/keyring-snap-sdk": "^4.0.0",
"@metamask/snaps-cli": "^8.1.1",
"@metamask/snaps-jest": "9.3.0",
"@metamask/snaps-sdk": "^9.2.0",
"@metamask/snaps-sdk": "^9.3.0",
"@metamask/superstruct": "^3.1.0",
"@metamask/utils": "11.4.0",
"@noble/ed25519": "2.1.0",
Expand Down
21 changes: 3 additions & 18 deletions packages/snap/snap.manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"url": "https://github.com/MetaMask/snap-solana-wallet.git"
},
"source": {
"shasum": "W86v0OUr3ogUxyukFcAgbTjaqxtra+nN67DvfgLuLJ8=",
"shasum": "8ufRG4iBVewqxExSv+Ri41hNIHkWbdWamb0dy9J08Cw=",
"location": {
"npm": {
"filePath": "dist/bundle.js",
Expand Down Expand Up @@ -38,22 +38,7 @@
"endowment:lifecycle-hooks": {},
"endowment:network-access": {},
"endowment:cronjob": {
"jobs": [
{
"duration": "PT30S",
"request": {
"method": "refreshSend",
"params": {}
}
},
{
"duration": "PT20S",
"request": {
"method": "refreshConfirmationEstimation",
"params": {}
}
}
]
"jobs": []
},
"endowment:protocol": {
"scopes": {
Expand Down Expand Up @@ -90,6 +75,6 @@
"snap_dialog": {},
"snap_getPreferences": {}
},
"platformVersion": "9.0.0",
"platformVersion": "9.3.0",
"manifestVersion": "0.1"
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,8 @@ export enum ScheduleBackgroundEventMethod {
OnTransactionRejected = 'onTransactionRejected',
/** Use it to schedule a background event to asynchronously fetch the transactions of an account */
OnSyncAccountTransactions = 'onSyncAccountTransactions',
/** Use it to schedule a background event to refresh the send form */
RefreshSend = 'refreshSend',
/** Use it to schedule a background event to refresh the confirmation estimation */
RefreshConfirmationEstimation = 'refreshConfirmationEstimation',
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { onSyncAccountTransactions } from './onSyncAccountTransactions';
import { onTransactionAdded } from './onTransactionAdded';
import { onTransactionApproved } from './onTransactionApproved';
import { onTransactionRejected } from './onTransactionRejected';
import { refreshConfirmationEstimation } from './refreshConfirmationEstimation';
import { refreshSend } from './refreshSend';
import { ScheduleBackgroundEventMethod } from './ScheduleBackgroundEventMethod';

export const handlers: Record<ScheduleBackgroundEventMethod, OnCronjobHandler> =
Expand All @@ -15,4 +17,7 @@ export const handlers: Record<ScheduleBackgroundEventMethod, OnCronjobHandler> =
onTransactionRejected,
[ScheduleBackgroundEventMethod.OnSyncAccountTransactions]:
onSyncAccountTransactions,
[ScheduleBackgroundEventMethod.RefreshSend]: refreshSend,
[ScheduleBackgroundEventMethod.RefreshConfirmationEstimation]:
refreshConfirmationEstimation,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
import type { OnCronjobHandler } from '@metamask/snaps-sdk';

import { ConfirmTransactionRequest } from '../../../../features/confirmation/views/ConfirmTransactionRequest/ConfirmTransactionRequest';
import type { ConfirmTransactionRequestContext } from '../../../../features/confirmation/views/ConfirmTransactionRequest/types';
import { state, transactionScanService } from '../../../../snapContext';
import type { UnencryptedStateValue } from '../../../services/state/State';
import {
CONFIRM_SIGN_AND_SEND_TRANSACTION_INTERFACE_NAME,
getInterfaceContextOrThrow,
updateInterface,
} from '../../../utils/interface';
import baseLogger, { createPrefixedLogger } from '../../../utils/logger';

export const refreshConfirmationEstimation: OnCronjobHandler = async () => {
const logger = createPrefixedLogger(
baseLogger,
'[refreshConfirmationEstimation]',
);

logger.info(`Background event triggered`);

const mapInterfaceNameToId =
(await state.getKey<UnencryptedStateValue['mapInterfaceNameToId']>(
'mapInterfaceNameToId',
)) ?? {};

const confirmationInterfaceId =
mapInterfaceNameToId[CONFIRM_SIGN_AND_SEND_TRANSACTION_INTERFACE_NAME];

// Don't do anything if the confirmation interface is not open
if (!confirmationInterfaceId) {
logger.info(`No interface context found`);
return;
}

// Schedule the next run
await snap.request({
method: 'snap_scheduleBackgroundEvent',
params: {
duration: 'PT20S',
request: { method: 'refreshConfirmationEstimation' },
},
});
Comment on lines +37 to +43

Choose a reason for hiding this comment

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

Q: This will run every 20 seconds or it will run just once? In the case of being periodic will it get cancelled when the confirmation interfaced gets closed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This only schedules the next run.
Incidentally, if the confirmation interface gets closed, it runs, but doesn't schedule an other run.

It's basically a loop system, where we enter the loop when the confirmation interface renders, and exit the loop when the interface get closed.


// Update the interface context with the new rates.
try {
// Get the current context
const interfaceContext =
await getInterfaceContextOrThrow<ConfirmTransactionRequestContext>(
confirmationInterfaceId,
);

if (
!interfaceContext.account?.address ||
!interfaceContext.transaction ||
!interfaceContext.scope ||
!interfaceContext.method
) {
logger.info(`Context is missing required fields`);
return;
}

// Skip transaction simulation if the preference is disabled
if (!interfaceContext.preferences?.simulateOnChainActions) {
logger.info(`Transaction simulation is disabled in preferences`);
return;
}

const fetchingConfirmationContext = {
...interfaceContext,
scanFetchStatus: 'fetching',
} as ConfirmTransactionRequestContext;

await updateInterface(
confirmationInterfaceId,
<ConfirmTransactionRequest context={fetchingConfirmationContext} />,
fetchingConfirmationContext,
);

const [scan, updatedInterfaceContextFinal] = await Promise.all([
transactionScanService.scanTransaction({
method: interfaceContext.method,
accountAddress: interfaceContext.account.address,
transaction: interfaceContext.transaction,
scope: interfaceContext.scope,
origin: interfaceContext.origin,
account: interfaceContext.account,
}),
getInterfaceContextOrThrow<ConfirmTransactionRequestContext>(
confirmationInterfaceId,
),
]);

// Update the current context with the new rates
const updatedInterfaceContext = {
...updatedInterfaceContextFinal,
scanFetchStatus: 'fetched' as const,
scan,
};

logger.info(`New scan fetched`);

await updateInterface(
confirmationInterfaceId,
<ConfirmTransactionRequest context={updatedInterfaceContext} />,
updatedInterfaceContext,
);

logger.info(`Background event suceeded`);
} catch (error) {
const fetchedInterfaceContext =
await getInterfaceContextOrThrow<ConfirmTransactionRequestContext>(
confirmationInterfaceId,
);

const fetchingConfirmationContext = {
...fetchedInterfaceContext,
scanFetchStatus: 'fetched',
} as ConfirmTransactionRequestContext;

await updateInterface(
confirmationInterfaceId,
<ConfirmTransactionRequest context={fetchingConfirmationContext} />,
fetchingConfirmationContext,
);

logger.info(
{ error },
`Could not update the interface. But rolled back status to fetched.`,
);
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import type { OnCronjobHandler } from '@metamask/snaps-sdk';

import { DEFAULT_SEND_CONTEXT } from '../../../../features/send/render';
import { Send } from '../../../../features/send/Send';
import type { SendContext } from '../../../../features/send/types';
import { assetsService, priceApiClient, state } from '../../../../snapContext';
import type { UnencryptedStateValue } from '../../../services/state/State';
import {
getInterfaceContextOrThrow,
getPreferences,
SEND_FORM_INTERFACE_NAME,
updateInterface,
} from '../../../utils/interface';
import baseLogger, { createPrefixedLogger } from '../../../utils/logger';

export const refreshSend: OnCronjobHandler = async () => {
const logger = createPrefixedLogger(baseLogger, '[refreshSend]');

logger.info(`Background event triggered`);

const [assets, mapInterfaceNameToId, preferences] = await Promise.all([
assetsService.getAll(),
state.getKey<UnencryptedStateValue['mapInterfaceNameToId']>(
'mapInterfaceNameToId',
),
getPreferences().catch(() => DEFAULT_SEND_CONTEXT.preferences),
]);

const assetTypes = assets.flatMap((asset) => asset.assetType);

const sendFormInterfaceId = mapInterfaceNameToId?.[SEND_FORM_INTERFACE_NAME];

// Don't do anything if the send form interface is not open
if (!sendFormInterfaceId) {
logger.info(`No send form interface found`);
return;
}

// Schedule the next run
await snap.request({
method: 'snap_scheduleBackgroundEvent',
params: { duration: 'PT30S', request: { method: 'refreshSend' } },
});

// First, fetch the token prices
const tokenPrices = await priceApiClient.getMultipleSpotPrices(
assetTypes,
preferences.currency,
);

// Save them in the state
await state.setKey('tokenPrices', tokenPrices);
Copy link

Choose a reason for hiding this comment

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

Bug: Token Refresh Errors and Unnecessary Scheduling

The refreshSend background event has two issues:

  1. Missing Error Handling: It lacks error handling for token price fetching and state updates. Failures in these operations cause the event to crash silently, preventing token price updates.
  2. Unnecessary Scheduling: It unconditionally schedules its next run before checking if the send form interface is open. This results in unnecessary background events being scheduled even when the interface is closed, contrary to the intended behavior.
Fix in Cursor Fix in Web


// Get the current context
const interfaceContext =
await getInterfaceContextOrThrow<SendContext>(sendFormInterfaceId);

// We only want to refresh the token prices when the user is in the transaction confirmation stage
if (interfaceContext.stage !== 'transaction-confirmation') {
logger.info(`❌ Not in transaction confirmation stage`);
return;
}

if (!interfaceContext.assets) {
logger.info(`❌ No assets found`);
return;
}

// Update the current context with the new rates
const updatedInterfaceContext = {
...interfaceContext,
tokenPrices: {
...interfaceContext.tokenPrices,
...tokenPrices,
},
};

await updateInterface(
sendFormInterfaceId,
<Send context={updatedInterfaceContext} />,
updatedInterfaceContext,
);

logger.info(`✅ Background event suceeded`);
};
Copy link

Choose a reason for hiding this comment

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

Bug

The refreshSend background event has two main issues related to its execution flow and error handling:

  1. Missing Error Handling: Critical operations, such as fetching token prices, saving them to state, and updating the interface, lack try-catch blocks. This differs from the previous cronjob version, which handled these failures gracefully.
  2. Incorrect Scheduling Order: The next background event is scheduled early in the handler, before checks for the send interface being open and before the token price fetching/state saving operations.

As a result:

  • If the send interface is closed, the event still schedules its next run before exiting, leading to an infinite loop of unnecessary background events.
  • If token price fetching or state saving fails, the current event throws an unhandled error, but the next event has already been scheduled, resulting in repeated, failed background event executions every 30 seconds, wasting resources.
Fix in Cursor Fix in Web

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Errors are already handled globally at the upper level, in src/index.ts. The try/catch was redundant.
  2. Wrong, we do schedule the next run after we've checked that the send form is open, which is the relevant condition that make sense to be checked.

Original file line number Diff line number Diff line change
@@ -1,4 +1 @@
export enum CronjobMethod {
RefreshSend = 'refreshSend',
RefreshConfirmationEstimation = 'refreshConfirmationEstimation',
}
export enum CronjobMethod {}
9 changes: 2 additions & 7 deletions packages/snap/src/core/handlers/onCronjob/cronjobs/index.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
import { type OnCronjobHandler } from '@metamask/snaps-sdk';

import { CronjobMethod } from './CronjobMethod';
import { refreshConfirmationEstimation } from './refreshConfirmationEstimation';
import { refreshSend } from './refreshSend';
import type { CronjobMethod } from './CronjobMethod';

export const handlers: Record<CronjobMethod, OnCronjobHandler> = {
[CronjobMethod.RefreshSend]: refreshSend,
[CronjobMethod.RefreshConfirmationEstimation]: refreshConfirmationEstimation,
};
export const handlers: Record<CronjobMethod, OnCronjobHandler> = {};
Loading
Loading