Skip to content

feat: add setting to dismiss prompt to enable smart contract #31609

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

Merged
merged 12 commits into from
Apr 9, 2025
+224 −7
Merged

feat: add setting to dismiss prompt to enable smart contract #31609

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
2bed795
Add setting to dismiss prompt to enable smart contract
jpuri Apr 4, 2025
75144bb
update
jpuri Apr 7, 2025
bb98225
Merge branch 'main' into advance_setting_smart_account
jpuri Apr 8, 2025
b44aef5
update
jpuri Apr 8, 2025
9d3afbf
update
jpuri Apr 8, 2025
d17b0d6
Merge branch 'advance_setting_smart_account' of github.com:MetaMask/m…
jpuri Apr 8, 2025
5f644e5
update
jpuri Apr 8, 2025
33a4883
Merge branch 'main' into advance_setting_smart_account
jpuri Apr 8, 2025
8d1bf81
merge
jpuri Apr 9, 2025
1786b55
update
jpuri Apr 9, 2025
c0e5a5f
Merge branch 'advance_setting_smart_account' of github.com:MetaMask/m…
jpuri Apr 9, 2025
79078c5
update
jpuri Apr 9, 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
39 changes: 39 additions & 0 deletions app/scripts/lib/transaction/eip5792.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ describe('EIP-5792', () => {
Parameters<typeof processSendCalls>[0]['validateSecurity']
>;

let getDismissSmartAccountSuggestionEnabledMock: jest.MockedFn<() => boolean>;

let messenger: EIP5792Messenger;

beforeEach(() => {
Expand All @@ -100,6 +102,7 @@ describe('EIP-5792', () => {
getNetworkClientByIdMock = jest.fn();
getDisabledAccountUpgradeChainsMock = jest.fn();
validateSecurityMock = jest.fn();
getDismissSmartAccountSuggestionEnabledMock = jest.fn();

messenger = new Messenger();

Expand Down Expand Up @@ -133,6 +136,8 @@ describe('EIP-5792', () => {
addTransactionBatch: addTransactionBatchMock,
getDisabledAccountUpgradeChains: getDisabledAccountUpgradeChainsMock,
validateSecurity: validateSecurityMock,
getDismissSmartAccountSuggestionEnabled:
getDismissSmartAccountSuggestionEnabledMock,
},
messenger,
SEND_CALLS_MOCK,
Expand All @@ -157,6 +162,8 @@ describe('EIP-5792', () => {
getDisabledAccountUpgradeChains:
getDisabledAccountUpgradeChainsMock,
validateSecurity: validateSecurityMock,
getDismissSmartAccountSuggestionEnabled:
getDismissSmartAccountSuggestionEnabledMock,
},
messenger,
SEND_CALLS_MOCK,
Expand All @@ -173,6 +180,8 @@ describe('EIP-5792', () => {
getDisabledAccountUpgradeChains:
getDisabledAccountUpgradeChainsMock,
validateSecurity: validateSecurityMock,
getDismissSmartAccountSuggestionEnabled:
getDismissSmartAccountSuggestionEnabledMock,
},
messenger,
{ ...SEND_CALLS_MOCK, version: '2.0' },
Expand All @@ -189,6 +198,8 @@ describe('EIP-5792', () => {
getDisabledAccountUpgradeChains:
getDisabledAccountUpgradeChainsMock,
validateSecurity: validateSecurityMock,
getDismissSmartAccountSuggestionEnabled:
getDismissSmartAccountSuggestionEnabledMock,
},
messenger,
{ ...SEND_CALLS_MOCK, chainId: CHAIN_ID_2_MOCK },
Expand All @@ -209,6 +220,8 @@ describe('EIP-5792', () => {
getDisabledAccountUpgradeChains:
getDisabledAccountUpgradeChainsMock,
validateSecurity: validateSecurityMock,
getDismissSmartAccountSuggestionEnabled:
getDismissSmartAccountSuggestionEnabledMock,
},
messenger,
SEND_CALLS_MOCK,
Expand All @@ -219,6 +232,28 @@ describe('EIP-5792', () => {
);
});

it('throws if user enabled preference to dismiss option to upgrade account', async () => {
getDismissSmartAccountSuggestionEnabledMock.mockReturnValue(true);

await expect(
processSendCalls(
{
addTransactionBatch: addTransactionBatchMock,
getDisabledAccountUpgradeChains:
getDisabledAccountUpgradeChainsMock,
validateSecurity: validateSecurityMock,
getDismissSmartAccountSuggestionEnabled:
getDismissSmartAccountSuggestionEnabledMock,
},
messenger,
SEND_CALLS_MOCK,
REQUEST_MOCK,
),
).rejects.toThrow(
'User does not want to update account to smart account.',
);
});

it('throws if top-level capability is required', async () => {
await expect(
processSendCalls(
Expand All @@ -227,6 +262,8 @@ describe('EIP-5792', () => {
getDisabledAccountUpgradeChains:
getDisabledAccountUpgradeChainsMock,
validateSecurity: validateSecurityMock,
getDismissSmartAccountSuggestionEnabled:
getDismissSmartAccountSuggestionEnabledMock,
},
messenger,
{
Expand All @@ -250,6 +287,8 @@ describe('EIP-5792', () => {
getDisabledAccountUpgradeChains:
getDisabledAccountUpgradeChainsMock,
validateSecurity: validateSecurityMock,
getDismissSmartAccountSuggestionEnabled:
getDismissSmartAccountSuggestionEnabledMock,
},
messenger,
{
Expand Down
23 changes: 21 additions & 2 deletions app/scripts/lib/transaction/eip5792.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export async function processSendCalls(
request: ValidateSecurityRequest,
chainId: Hex,
) => Promise<void>;
getDismissSmartAccountSuggestionEnabled: () => boolean;
},
messenger: EIP5792Messenger,
params: SendCalls,
Expand All @@ -46,6 +47,7 @@ export async function processSendCalls(
addTransactionBatch,
getDisabledAccountUpgradeChains,
validateSecurity: validateSecurityHook,
getDismissSmartAccountSuggestionEnabled,
} = hooks;

const { calls, from } = params;
Expand All @@ -58,8 +60,15 @@ export async function processSendCalls(
).configuration.chainId;

const disabledChains = getDisabledAccountUpgradeChains();

validateSendCalls(params, dappChainId, disabledChains);
const dismissSmartAccountSuggestionEnabled =
getDismissSmartAccountSuggestionEnabled();

validateSendCalls(
params,
dappChainId,
disabledChains,
dismissSmartAccountSuggestionEnabled,
);

const securityAlertId = generateSecurityAlertId();
const validateSecurity = validateSecurityHook.bind(null, securityAlertId);
Expand Down Expand Up @@ -127,11 +136,13 @@ function validateSendCalls(
sendCalls: SendCalls,
dappChainId: Hex,
disabledChains: Hex[],
dismissSmartAccountSuggestionEnabled: boolean,
) {
validateSendCallsVersion(sendCalls);
validateSendCallsChainId(sendCalls, dappChainId);
validateCapabilities(sendCalls);
validateUserDisabled(sendCalls, disabledChains, dappChainId);
validateUserPreference(dismissSmartAccountSuggestionEnabled);
}

function validateSendCallsVersion(sendCalls: SendCalls) {
Expand Down Expand Up @@ -199,6 +210,14 @@ function validateUserDisabled(
}
}

function validateUserPreference(dismissSmartAccountSuggestionEnabled: boolean) {
if (dismissSmartAccountSuggestionEnabled) {
throw rpcErrors.transactionRejected(
'User does not want to update account to smart account.',
Copy link
Member

Choose a reason for hiding this comment

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

Could we combine this with the above to use the same error, as they're ultimately both preferences to not upgrade?

Copy link
Contributor Author

@jpuri jpuri Apr 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Message for both will be bit different, as other will be address level now and this is more user level thing.

Copy link
Member

Choose a reason for hiding this comment

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

Even so, from the perspective of the dApp, it's simply not supported for that chain and account. Plus I think we want to stick with methodNotSupported for now since the user hasn't technically rejected the transaction, but the act of upgrading.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 I will implement code changes in following PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made these changes in PR while updating it to resolve conflicts

);
}
}

function getStatusCode(transactionMeta: TransactionMeta) {
const { hash, status } = transactionMeta;

Expand Down
3 changes: 3 additions & 0 deletions app/scripts/metamask-controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2008,6 +2008,9 @@ export default class MetamaskController extends EventEmitter {
updateSecurityAlertResponse:
this.updateSecurityAlertResponse.bind(this),
}),
getDismissSmartAccountSuggestionEnabled: () =>
this.preferencesController.state.preferences
.dismissSmartAccountSuggestionEnabled,
},
this.controllerMessenger,
),
Expand Down
8 changes: 0 additions & 8 deletions ui/pages/confirmations/components/confirm/footer/acknowledge/acknowledge.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { AuthorizationList } from '@metamask/transaction-controller';
import React from 'react';
import { act } from '@testing-library/react';
import * as ReduxFunctions from 'react-redux';

import configureStore from '../../../../../../store/store';
import { getMockConfirmStateForTransaction } from '../../../../../../../test/data/confirmations/helper';
import { genUnapprovedContractInteractionConfirmation } from '../../../../../../../test/data/confirmations/contract-interaction';
Expand Down Expand Up @@ -68,10 +66,4 @@ describe('Acknowledge', () => {
const { container } = render({});
expect(container).toBeEmptyDOMElement();
});

it('does not render if preference dismissSmartAccountSuggestionEnabled is enabled', () => {
jest.spyOn(ReduxFunctions, 'useSelector').mockReturnValue(true);
const { container } = render({});
expect(container).toBeEmptyDOMElement();
});
});
8 changes: 1 addition & 7 deletions ui/pages/confirmations/components/confirm/footer/acknowledge/acknowledge.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import React from 'react';
import { useSelector } from 'react-redux';

import { useIsUpgradeTransaction } from '../../info/hooks/useIsUpgradeTransaction';
import { getDismissSmartAccountSuggestionEnabled } from '../../../../../../selectors';
import { useI18nContext } from '../../../../../../hooks/useI18nContext';
import { Checkbox } from '../../../../../../components/component-library';
import { AlignItems } from '../../../../../../helpers/constants/design-system';
Expand All @@ -16,11 +13,8 @@ export function Acknowledge(props: AcknowledgeProps) {
const t = useI18nContext();
const isUpgradeTransaction = useIsUpgradeTransaction();
const { isAcknowledged, onAcknowledgeToggle } = props;
const dismissSmartAccountSuggestionEnabled = useSelector(
getDismissSmartAccountSuggestionEnabled,
);

if (!isUpgradeTransaction || dismissSmartAccountSuggestionEnabled) {
if (!isUpgradeTransaction) {
return null;
}

Expand Down
5 changes: 0 additions & 5 deletions ui/selectors/selectors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1998,11 +1998,6 @@ export function getShouldShowAggregatedBalancePopover(state) {
return shouldShowAggregatedBalancePopover;
}

export function getDismissSmartAccountSuggestionEnabled(state) {
const { dismissSmartAccountSuggestionEnabled } = getPreferences(state);
return dismissSmartAccountSuggestionEnabled;
}

export const getConnectedSnapsList = createDeepEqualSelector(
getSnapsList,
(snapsData) => {
Expand Down
2 changes: 1 addition & 1 deletion ui/store/actions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3263,7 +3263,7 @@ export function setShowExtensionInFullSizeView(value: boolean) {
}

export function setDismissSmartAccountSuggestionEnabled(value: boolean) {
return setPreference('setDismissSmartAccountSuggestionEnabled', value);
return setPreference('dismissSmartAccountSuggestionEnabled', value);
}

export function setTokenSortConfig(value: SortCriteria) {
Expand Down
Loading