Skip to content

fix: cp-12.17.3 Remove upgrade rejection persistence #32697

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 13 commits into from
May 13, 2025
+113 −323
Merged

fix: cp-12.17.3 Remove upgrade rejection persistence #32697

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
c902774
Remove upgrade rejection persistence
jpuri May 8, 2025
2f57318
update
jpuri May 8, 2025
f5843e4
update
jpuri May 8, 2025
cad9473
update
jpuri May 8, 2025
d664488
update
jpuri May 9, 2025
2d3821f
update
jpuri May 9, 2025
0458ff4
update
jpuri May 9, 2025
b1a3647
update
jpuri May 9, 2025
5cd15af
update
jpuri May 9, 2025
9538c5b
update
jpuri May 13, 2025
b2fff02
merge
jpuri May 13, 2025
da803bb
update
jpuri May 13, 2025
69837ca
update
jpuri May 13, 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
69 changes: 0 additions & 69 deletions app/scripts/controllers/preferences-controller.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
*/
import { Messenger } from '@metamask/base-controller';
import { AccountsController } from '@metamask/accounts-controller';
import { Hex } from '@metamask/utils';
import { KeyringControllerStateChangeEvent } from '@metamask/keyring-controller';
import type { MultichainNetworkControllerNetworkDidChangeEvent } from '@metamask/multichain-network-controller';
import { SnapControllerStateChangeEvent } from '@metamask/snaps-controllers';
Expand Down Expand Up @@ -841,74 +840,6 @@ describe('preferences controller', () => {
});
});

describe('getDisabledUpgradeAccountsByChain', () => {
it('returns empty object if disabledAccountUpgradeChainsAddresses is empty', () => {
const { controller } = setupController({});
expect(controller.getDisabledUpgradeAccountsByChain()).toStrictEqual({});
});

it('returns disabledAccountUpgrades state', () => {
const mockStateObject = {
[CHAIN_IDS.MAINNET]: ['0x0'] as Hex[],
[CHAIN_IDS.GOERLI]: ['0x1'] as Hex[],
};
const { controller } = setupController({
state: {
disabledUpgradeAccountsByChain: mockStateObject,
},
});

expect(controller.getDisabledUpgradeAccountsByChain()).toStrictEqual(
mockStateObject,
);
});
});

describe('disableAccountUpgrade', () => {
it('adds chain ID, address to disabledAccountUpgrades if empty', () => {
const { controller } = setupController({});

controller.disableAccountUpgrade(CHAIN_IDS.GOERLI, '0x0');

expect(controller.state.disabledUpgradeAccountsByChain).toStrictEqual({
[CHAIN_IDS.GOERLI]: ['0x0'],
});
});

it('adds chain ID, address to disabledAccountUpgrades if not empty', () => {
const { controller } = setupController({
state: {
disabledUpgradeAccountsByChain: {
[CHAIN_IDS.MAINNET]: ['0x0'],
},
},
});

controller.disableAccountUpgrade(CHAIN_IDS.GOERLI, '0x1');

expect(controller.state.disabledUpgradeAccountsByChain).toStrictEqual({
[CHAIN_IDS.MAINNET]: ['0x0'],
[CHAIN_IDS.GOERLI]: ['0x1'],
});
});

it('does not add chain ID to disabledAccountUpgrades if duplicate', () => {
const { controller } = setupController({
state: {
disabledUpgradeAccountsByChain: {
[CHAIN_IDS.MAINNET]: ['0x0'],
},
},
});

controller.disableAccountUpgrade(CHAIN_IDS.MAINNET, '0x0');

expect(controller.state.disabledUpgradeAccountsByChain).toStrictEqual({
[CHAIN_IDS.MAINNET]: ['0x0'],
});
});
});

describe('manageInstitutionalWallets', () => {
it('defaults manageInstitutionalWallets to false', () => {
const { controller } = setupController({});
Expand Down
25 changes: 1 addition & 24 deletions app/scripts/controllers/preferences-controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
AccountsControllerSetSelectedAccountAction,
AccountsControllerState,
} from '@metamask/accounts-controller';
import { Hex, Json } from '@metamask/utils';
import { Json } from '@metamask/utils';
import {
BaseController,
ControllerGetStateAction,
Expand Down Expand Up @@ -150,7 +150,6 @@ export type PreferencesControllerState = Omit<
useExternalServices: boolean;
textDirection?: string;
manageInstitutionalWallets: boolean;
disabledUpgradeAccountsByChain?: Record<Hex, Hex[]>;
};

/**
Expand Down Expand Up @@ -256,7 +255,6 @@ export const getDefaultPreferencesControllerState =
[ETHERSCAN_SUPPORTED_CHAIN_IDS.GNOSIS]: true,
},
manageInstitutionalWallets: false,
disabledUpgradeAccountsByChain: {},
});

/**
Expand Down Expand Up @@ -428,7 +426,6 @@ const controllerMetadata = {
isMultiAccountBalancesEnabled: { persist: true, anonymous: true },
showIncomingTransactions: { persist: true, anonymous: true },
manageInstitutionalWallets: { persist: true, anonymous: false },
disabledUpgradeAccountsByChain: { persist: true, anonymous: false },
};

export class PreferencesController extends BaseController<
Expand Down Expand Up @@ -964,26 +961,6 @@ export class PreferencesController extends BaseController<
});
}

getDisabledUpgradeAccountsByChain(): Record<Hex, Hex[]> {
return this.state.disabledUpgradeAccountsByChain ?? {};
}

disableAccountUpgrade(chainId: Hex, address: Hex): void {
this.update((state) => {
const { disabledUpgradeAccountsByChain = {} } = state;
const addressLowerCase = address.toLowerCase() as Hex;

if (
!disabledUpgradeAccountsByChain[chainId]?.includes(addressLowerCase)
) {
if (!disabledUpgradeAccountsByChain[chainId]) {
disabledUpgradeAccountsByChain[chainId] = [];
}
disabledUpgradeAccountsByChain[chainId].push(addressLowerCase);
}
});
}

///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
setSnapsAddSnapAccountModalDismissed(value: boolean): void {
this.update((state) => {
Expand Down
95 changes: 2 additions & 93 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 @@ -14,7 +14,7 @@ import {
SendCalls,
SendCallsParams,
} from '@metamask/eth-json-rpc-middleware';
import { Hex, JsonRpcRequest } from '@metamask/utils';
import { JsonRpcRequest } from '@metamask/utils';
import { Messenger } from '@metamask/base-controller';
import { AccountsControllerGetSelectedAccountAction } from '@metamask/accounts-controller';
import { InternalAccount } from '@metamask/keyring-internal-api';
Expand Down Expand Up @@ -93,10 +93,6 @@ describe('EIP-5792', () => {
TransactionControllerGetStateAction['handler']
> = jest.fn();

const getDisabledUpgradeAccountsByChainMock: jest.MockedFn<
() => Record<Hex, Hex[]>
> = jest.fn();

const isAtomicBatchSupportedMock: jest.MockedFn<
TransactionController['isAtomicBatchSupported']
> = jest.fn();
Expand All @@ -113,7 +109,6 @@ describe('EIP-5792', () => {

const sendCallsHooks = {
addTransactionBatch: addTransactionBatchMock,
getDisabledUpgradeAccountsByChain: getDisabledUpgradeAccountsByChainMock,
getDismissSmartAccountSuggestionEnabled:
getDismissSmartAccountSuggestionEnabledMock,
isAtomicBatchSupported: isAtomicBatchSupportedMock,
Expand Down Expand Up @@ -150,7 +145,6 @@ describe('EIP-5792', () => {
batchId: BATCH_ID_MOCK,
});

getDisabledUpgradeAccountsByChainMock.mockReturnValue({});
getDismissSmartAccountSuggestionEnabledMock.mockReturnValue(false);

isAtomicBatchSupportedMock.mockResolvedValue([
Expand Down Expand Up @@ -236,46 +230,6 @@ describe('EIP-5792', () => {
);
});

it('throws if disabled preference for chain and account', async () => {
getDisabledUpgradeAccountsByChainMock.mockReturnValue({
[CHAIN_ID_MOCK]: [FROM_MOCK],
});

await expect(
processSendCalls(
sendCallsHooks,
messenger,
SEND_CALLS_MOCK,
REQUEST_MOCK,
),
).rejects.toThrow(
`EIP-7702 upgrade rejected for this chain and account - Chain ID: ${CHAIN_ID_MOCK}, Account: ${SEND_CALLS_MOCK.from}`,
);
});

it('does not throw if disabled preference for chain and account if already upgraded', async () => {
getDisabledUpgradeAccountsByChainMock.mockReturnValue({
[CHAIN_ID_MOCK]: [FROM_MOCK],
});

isAtomicBatchSupportedMock.mockResolvedValueOnce([
{
chainId: CHAIN_ID_MOCK,
delegationAddress: DELEGATION_ADDRESS_MOCK,
isSupported: true,
},
]);

expect(
await processSendCalls(
sendCallsHooks,
messenger,
SEND_CALLS_MOCK,
REQUEST_MOCK,
),
).toBeDefined();
});

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

Expand All @@ -286,9 +240,7 @@ describe('EIP-5792', () => {
SEND_CALLS_MOCK,
REQUEST_MOCK,
),
).rejects.toThrow(
`EIP-7702 upgrade rejected for this chain and account - Chain ID: ${CHAIN_ID_MOCK}, Account: ${SEND_CALLS_MOCK.from}`,
);
).rejects.toThrow('EIP-7702 upgrade disabled by the user');
});

it('does not throw if user enabled preference to dismiss option to upgrade account if already upgraded', async () => {
Expand Down Expand Up @@ -530,8 +482,6 @@ describe('EIP-5792', () => {

const capabilities = await getCapabilities(
{
getDisabledUpgradeAccountsByChain:
getDisabledUpgradeAccountsByChainMock,
getDismissSmartAccountSuggestionEnabled:
getDismissSmartAccountSuggestionEnabledMock,
isAtomicBatchSupported: isAtomicBatchSupportedMock,
Expand Down Expand Up @@ -561,8 +511,6 @@ describe('EIP-5792', () => {

const capabilities = await getCapabilities(
{
getDisabledUpgradeAccountsByChain:
getDisabledUpgradeAccountsByChainMock,
getDismissSmartAccountSuggestionEnabled:
getDismissSmartAccountSuggestionEnabledMock,
isAtomicBatchSupported: isAtomicBatchSupportedMock,
Expand All @@ -585,8 +533,6 @@ describe('EIP-5792', () => {

const capabilities = await getCapabilities(
{
getDisabledUpgradeAccountsByChain:
getDisabledUpgradeAccountsByChainMock,
getDismissSmartAccountSuggestionEnabled:
getDismissSmartAccountSuggestionEnabledMock,
isAtomicBatchSupported: isAtomicBatchSupportedMock,
Expand All @@ -612,37 +558,6 @@ describe('EIP-5792', () => {

const capabilities = await getCapabilities(
{
getDisabledUpgradeAccountsByChain:
getDisabledUpgradeAccountsByChainMock,
getDismissSmartAccountSuggestionEnabled:
getDismissSmartAccountSuggestionEnabledMock,
isAtomicBatchSupported: isAtomicBatchSupportedMock,
},
FROM_MOCK,
[CHAIN_ID_MOCK],
);

expect(capabilities).toStrictEqual({});
});

it('does not include atomic capability if upgrade disabled for chain and account', async () => {
isAtomicBatchSupportedMock.mockResolvedValueOnce([
{
chainId: CHAIN_ID_MOCK,
delegationAddress: undefined,
isSupported: false,
upgradeContractAddress: DELEGATION_ADDRESS_MOCK,
},
]);

getDisabledUpgradeAccountsByChainMock.mockReturnValue({
[CHAIN_ID_MOCK]: [FROM_MOCK],
});

const capabilities = await getCapabilities(
{
getDisabledUpgradeAccountsByChain:
getDisabledUpgradeAccountsByChainMock,
getDismissSmartAccountSuggestionEnabled:
getDismissSmartAccountSuggestionEnabledMock,
isAtomicBatchSupported: isAtomicBatchSupportedMock,
Expand All @@ -664,14 +579,8 @@ describe('EIP-5792', () => {
},
]);

getDisabledUpgradeAccountsByChainMock.mockReturnValue({
[CHAIN_ID_MOCK]: [FROM_MOCK],
});

const capabilities = await getCapabilities(
{
getDisabledUpgradeAccountsByChain:
getDisabledUpgradeAccountsByChainMock,
getDismissSmartAccountSuggestionEnabled:
getDismissSmartAccountSuggestionEnabledMock,
isAtomicBatchSupported: isAtomicBatchSupportedMock,
Expand Down
Loading
Loading