From 17c51b899043909a49960f4af3d60eed2cfc6064 Mon Sep 17 00:00:00 2001 From: sednaoui Date: Fri, 29 May 2026 08:16:41 +0200 Subject: [PATCH 1/8] Add Safe 4337 module migration helpers and fallback-handler reader Migrating a deployed Safe between EntryPoint versions means swapping its 4337 module and fallback handler. The only built-in helper covered v0.6 -> v0.7; there was no reusable path to v0.9 and no way to read a Safe's current fallback handler. - SafeAccount.createModuleMigrationMetaTransactions(node, oldModule, newModule, overrides): generic [disableOld, enableNew, setFallbackHandler] builder. Both Safe4337Module and Safe4337MultiChainSignatureModule are stateless, so no storage clearing is needed. - SafeAccountV0_3_0.createMigrateToSafeMultiChainSigAccountV1MetaTransactions: convenience wrapper for the v0.7 -> v0.9 path. - SafeAccountV0_2_0.createMigrateToSafeAccountV0_3_0MetaTransactions now delegates to the generic helper (removes duplicated body). - SafeAccount.getFallbackHandler(node): reads the fallback handler (the 4337 module) so callers can confirm which EntryPoint version an account is on. - JsonRpcNode.getStorageAt(address, slot, blockTag): eth_getStorageAt. - Export SAFE_FALLBACK_HANDLER_STORAGE_SLOT constant. --- src/abstractionkit.ts | 1 + src/account/Safe/SafeAccount.ts | 77 +++++++++++++++++++++++++++ src/account/Safe/SafeAccountV0_2_0.ts | 27 +--------- src/account/Safe/SafeAccountV0_3_0.ts | 43 +++++++++++++++ src/constants.ts | 8 +++ src/transport/JsonRpcNode.ts | 26 +++++++++ 6 files changed, 157 insertions(+), 25 deletions(-) diff --git a/src/abstractionkit.ts b/src/abstractionkit.ts index 3d58aba4..b84c363d 100644 --- a/src/abstractionkit.ts +++ b/src/abstractionkit.ts @@ -79,6 +79,7 @@ export { ENTRYPOINT_V7, ENTRYPOINT_V8, ENTRYPOINT_V9, + SAFE_FALLBACK_HANDLER_STORAGE_SLOT, ZeroAddress, } from "./constants"; export { AbstractionKitError } from "./errors"; diff --git a/src/account/Safe/SafeAccount.ts b/src/account/Safe/SafeAccount.ts index 5193d2bb..60acca93 100644 --- a/src/account/Safe/SafeAccount.ts +++ b/src/account/Safe/SafeAccount.ts @@ -22,6 +22,7 @@ import { ENTRYPOINT_V6, ENTRYPOINT_V7, ENTRYPOINT_V9, + SAFE_FALLBACK_HANDLER_STORAGE_SLOT, Safe_L2_V1_4_1, ZeroAddress, } from "../../constants"; @@ -2739,6 +2740,25 @@ export class SafeAccount extends SmartAccount { return decodedCalldata[0]; } + /** + * read the Safe's current fallback handler address from storage. + * For Safe ERC-4337 accounts the fallback handler is the 4337 module, so this + * is the canonical way to confirm which module/EntryPoint version an account + * is on (e.g. after a module migration). + * @param nodeRpcUrl - The JSON-RPC API url for the target chain + * @returns a promise of the fallback handler address (checksummed) + */ + public async getFallbackHandler( + nodeRpcUrl: string | Transport | JsonRpcNode, + ): Promise { + const word = await JsonRpcNode.from(nodeRpcUrl).getStorageAt( + this.accountAddress, + SAFE_FALLBACK_HANDLER_STORAGE_SLOT, + "latest", + ); + return getAddress("0x" + word.slice(-40)); + } + /** * create a list of dummy signer/signature pairs for gas estimation based on the expected signers. * @param expectedSigners - signers whose signatures will be produced at sign time @@ -3010,6 +3030,63 @@ export class SafeAccount extends SmartAccount { }; } + /** + * create the MetaTransactions that migrate a DEPLOYED Safe from one ERC-4337 + * module (and EntryPoint) to another. For Safe 4337 accounts the module is + * both the enabled module and the fallback handler, so a migration is exactly: + * 1. disableModule(oldModule) + * 2. enableModule(newModule) + * 3. setFallbackHandler(newModule) + * + * @note Both the v0.6/v0.7 `Safe4337Module` and the v0.9 + * `Safe4337MultiChainSignatureModule` are stateless (no per-account storage), + * so there is NO storage to clear when swapping modules — these three + * transactions are the whole migration. The batch is validated and executed + * by the OLD module on the OLD EntryPoint; disabling that module mid-batch is + * safe because validation has already completed. + * + * @param nodeRpcUrl - The JSON-RPC API url for the target chain (used to find + * the previous module in the linked list when not provided) + * @param oldModuleAddress - the currently-enabled 4337 module to disable + * @param newModuleAddress - the 4337 module to enable and set as fallback handler + * @param overrides - overrides for finding the previous module + * @returns a promise of [disableOld, enableNew, setFallbackHandler] MetaTransactions + */ + public async createModuleMigrationMetaTransactions( + nodeRpcUrl: string | Transport | JsonRpcNode, + oldModuleAddress: string, + newModuleAddress: string, + overrides: { + prevModuleAddress?: string; + modulesStart?: string; + modulesPageSize?: bigint; + } = {}, + ): Promise { + const disableOldModule = await this.createDisableModuleMetaTransaction( + nodeRpcUrl, + oldModuleAddress, + this.accountAddress, + overrides, + ); + + const enableNewModule = SafeAccount.createEnableModuleMetaTransaction( + newModuleAddress, + this.accountAddress, + ); + + const setFallbackHandler: MetaTransaction = { + to: this.accountAddress, + value: 0n, + data: createCallData( + "0xf08a0323", //setFallbackHandler(address) + ["address"], + [newModuleAddress], + ), + }; + + return [disableOldModule, enableNewModule, setFallbackHandler]; + } + /** * Simulate the encoded calldata for this account on Tenderly and optionally return a share link. * When `isInit` isn't provided, nonce is fetched via `nodeRpcUrl` to decide whether to include diff --git a/src/account/Safe/SafeAccountV0_2_0.ts b/src/account/Safe/SafeAccountV0_2_0.ts index 8749fa39..1e7705f4 100644 --- a/src/account/Safe/SafeAccountV0_2_0.ts +++ b/src/account/Safe/SafeAccountV0_2_0.ts @@ -2,7 +2,6 @@ import type {Bundler} from "src/Bundler"; import {ENTRYPOINT_V6} from "src/constants"; import type {SignContext, Signer as AkSigner} from "src/signer/types"; import type {JsonRpcNode, Transport} from "src/transport"; -import {createCallData} from "src/utils"; import type {MetaTransaction, OnChainIdentifierParamsType, StateOverrideSet, UserOperationV6,} from "../../types"; import {SafeAccount} from "./SafeAccount"; import {SafeAccountV0_3_0} from "./SafeAccountV0_3_0"; @@ -380,38 +379,16 @@ export class SafeAccountV0_2_0 extends SafeAccount { const moduleV07Address = overrides.safeV07ModuleAddress ?? SafeAccountV0_3_0.DEFAULT_SAFE_4337_MODULE_ADDRESS; - const disableModuleMetaTransaction = await this.createDisableModuleMetaTransaction( + return this.createModuleMigrationMetaTransactions( nodeRpcUrl, moduleV06Address, - this.accountAddress, + moduleV07Address, { prevModuleAddress: overrides.safeV06ModuleAddress, modulesPageSize: overrides.pageSize, modulesStart: overrides.modulesStart, }, ); - - const enableModuleMetaTransaction = SafeAccount.createEnableModuleMetaTransaction( - moduleV07Address, - this.accountAddress, - ); - - const setFallbackHandlerCallData = createCallData( - "0xf08a0323", //setFallbackHandler(address) - ["address"], - [moduleV07Address], - ); - const setFallbackHandlerMetaTransaction: MetaTransaction = { - to: this.accountAddress, - value: 0n, - data: setFallbackHandlerCallData, - }; - - return [ - disableModuleMetaTransaction, - enableModuleMetaTransaction, - setFallbackHandlerMetaTransaction, - ]; } /** diff --git a/src/account/Safe/SafeAccountV0_3_0.ts b/src/account/Safe/SafeAccountV0_3_0.ts index 44d77f8e..5bd763a2 100644 --- a/src/account/Safe/SafeAccountV0_3_0.ts +++ b/src/account/Safe/SafeAccountV0_3_0.ts @@ -5,6 +5,7 @@ import type {JsonRpcNode, Transport} from "src/transport"; import type {MetaTransaction, OnChainIdentifierParamsType, StateOverrideSet, UserOperationV7,} from "../../types"; import {SafeAccount} from "./SafeAccount"; +import {SafeMultiChainSigAccountV1} from "./SafeMultiChainSigAccount"; import type { CreateUserOperationV7Overrides, InitCodeOverrides, @@ -409,6 +410,48 @@ export class SafeAccountV0_3_0 extends SafeAccount { options, }); } + + /** + * Create the MetaTransactions that migrate this DEPLOYED Safe from EntryPoint + * v0.7 (this account's `Safe4337Module`) to EntryPoint v0.9 + * (`SafeMultiChainSigAccountV1`'s `Safe4337MultiChainSignatureModule`). + * + * The returned batch must be sent as a UserOperation FROM THIS v0.7 account + * (it is validated/executed by the v0.7 module on the v0.7 EntryPoint). After + * it lands, attach the same account address to `SafeMultiChainSigAccountV1` to + * operate on EntryPoint v0.9. Both modules are stateless, so no storage + * clearing is required. See {@link createModuleMigrationMetaTransactions}. + * + * @param nodeRpcUrl - The JSON-RPC API url for the target chain + * @param overrides - override the source/target module addresses or module lookup + * @returns a promise of [disableV07, enableV09, setFallbackHandler] MetaTransactions + */ + public async createMigrateToSafeMultiChainSigAccountV1MetaTransactions( + nodeRpcUrl: string | Transport | JsonRpcNode, + overrides: { + safeV07ModuleAddress?: string; + safeV09ModuleAddress?: string; + prevModuleAddress?: string; + modulesStart?: string; + modulesPageSize?: bigint; + } = {}, + ): Promise { + const moduleV07Address = overrides.safeV07ModuleAddress ?? this.safe4337ModuleAddress; + const moduleV09Address = + overrides.safeV09ModuleAddress ?? + SafeMultiChainSigAccountV1.DEFAULT_SAFE_4337_MODULE_ADDRESS; + + return this.createModuleMigrationMetaTransactions( + nodeRpcUrl, + moduleV07Address, + moduleV09Address, + { + prevModuleAddress: overrides.prevModuleAddress, + modulesStart: overrides.modulesStart, + modulesPageSize: overrides.modulesPageSize, + }, + ); + } } /** diff --git a/src/constants.ts b/src/constants.ts index 3c388fba..45f8383b 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -12,6 +12,14 @@ export const ENTRYPOINT_V7 = "0x0000000071727De22E5E9d8BAf0edAc6f37da032"; /** EntryPoint v0.6 contract address */ export const ENTRYPOINT_V6 = "0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789"; +/** + * Safe fallback-handler storage slot: keccak256("fallback_manager.handler.address"). + * The address of a Safe's fallback handler (which is the 4337 module) is stored + * in the lower 20 bytes of the word at this slot. + */ +export const SAFE_FALLBACK_HANDLER_STORAGE_SLOT = + "0x6c9a6c4a39284e37ed1cf53d337577d14212a4870fb976a4366c693b939918d5"; + /** Safe L2 singleton v1.5.0 address and init hash */ export const Safe_L2_V1_5_0: SafeAccountSingleton = { singletonAddress: "0xEdd160fEBBD92E350D4D398fb636302fccd67C7e", diff --git a/src/transport/JsonRpcNode.ts b/src/transport/JsonRpcNode.ts index 7f00a35a..2becc06c 100644 --- a/src/transport/JsonRpcNode.ts +++ b/src/transport/JsonRpcNode.ts @@ -155,6 +155,32 @@ export class JsonRpcNode implements Transport { } } + /** + * `eth_getStorageAt`. Returns the 32-byte storage word at `slot` for + * `address` at the given block tag (default `"latest"`), as a hex string. + */ + async getStorageAt( + address: string, + slot: string, + blockTag: string | bigint = "latest", + options?: RequestOptions, + ): Promise { + try { + const result = await this.outbound.request( + {method: "eth_getStorageAt", params: [address, slot, blockTag]}, + options, + ); + if (typeof result !== "string") { + throw new AbstractionKitError("BAD_DATA", "eth_getStorageAt returned ill formed data", { + context: JSON.stringify(result), + }); + } + return result; + } catch (err) { + throw translateNodeError(err, "eth_getStorageAt"); + } + } + /** * `eth_call`. Executes a read-only call against `to` and returns the raw * return data as a hex string. Supports state overrides via the optional From d9cfd52e70ce7d5fa2144a2705b66fb5f9100a19 Mon Sep 17 00:00:00 2001 From: sednaoui Date: Fri, 29 May 2026 12:30:33 +0200 Subject: [PATCH 2/8] Fix prevModuleAddress wiring in v0.6->v0.7 migration helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit createMigrateToSafeAccountV0_3_0MetaTransactions passed overrides.safeV06ModuleAddress (the module being disabled) as prevModuleAddress (the linked-list predecessor). When a caller set safeV06ModuleAddress explicitly, this produced disableModule(prev=module, module) — claiming the module precedes itself. Default callers were unaffected (undefined -> on-chain predecessor lookup). Add a proper prevModuleAddress override and stop mis-wiring it; the predecessor is still looked up on-chain when not provided. --- src/account/Safe/SafeAccountV0_2_0.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/account/Safe/SafeAccountV0_2_0.ts b/src/account/Safe/SafeAccountV0_2_0.ts index 1e7705f4..519cd953 100644 --- a/src/account/Safe/SafeAccountV0_2_0.ts +++ b/src/account/Safe/SafeAccountV0_2_0.ts @@ -369,6 +369,7 @@ export class SafeAccountV0_2_0 extends SafeAccount { overrides: { safeV06ModuleAddress?: string; safeV07ModuleAddress?: string; + prevModuleAddress?: string; pageSize?: bigint; modulesStart?: string; } = {}, @@ -384,7 +385,9 @@ export class SafeAccountV0_2_0 extends SafeAccount { moduleV06Address, moduleV07Address, { - prevModuleAddress: overrides.safeV06ModuleAddress, + // previous module in the linked list (skips the RPC lookup when set); + // left undefined by default so the predecessor is fetched on-chain. + prevModuleAddress: overrides.prevModuleAddress, modulesPageSize: overrides.pageSize, modulesStart: overrides.modulesStart, }, From 9d608abf7504a46ad5fed779345b0425cafc4b3b Mon Sep 17 00:00:00 2001 From: sednaoui Date: Fri, 29 May 2026 12:45:40 +0200 Subject: [PATCH 3/8] Add tests for module-migration helpers, fallback reader, and getStorageAt Unit (offline, deterministic): - test/safe/moduleMigration.test.js: createModuleMigrationMetaTransactions (selectors/targets/order, explicit predecessor, and on-chain predecessor lookup via mock transport), createMigrateToSafeMultiChainSigAccountV1- MetaTransactions (defaults + overrides), getFallbackHandler, and a regression for the v0.6->v0.7 prevModuleAddress wiring (prev != module). - test/transport/JsonRpcNode.test.js: getStorageAt params, block tag, and BAD_DATA on non-string. Integration (e2e, cross-EntryPoint): - test/integration/e2e/migrate-safe-v07-to-v09/migrate.test.js: deploy a SafeAccountV0_3_0 on EP v0.7, migrate to the v0.9 multi-chain module, assert the on-chain module/fallback-handler swap, then execute a userop through SafeMultiChainSigAccountV1 on EP v0.9. --- .../migrate-safe-v07-to-v09/migrate.test.js | 97 ++++++++++ test/safe/moduleMigration.test.js | 183 ++++++++++++++++++ test/transport/JsonRpcNode.test.js | 30 +++ 3 files changed, 310 insertions(+) create mode 100644 test/integration/e2e/migrate-safe-v07-to-v09/migrate.test.js create mode 100644 test/safe/moduleMigration.test.js diff --git a/test/integration/e2e/migrate-safe-v07-to-v09/migrate.test.js b/test/integration/e2e/migrate-safe-v07-to-v09/migrate.test.js new file mode 100644 index 00000000..42716047 --- /dev/null +++ b/test/integration/e2e/migrate-safe-v07-to-v09/migrate.test.js @@ -0,0 +1,97 @@ +// e2e: migrate a DEPLOYED Safe from EntryPoint v0.7 (SafeAccountV0_3_0) to +// EntryPoint v0.9 (SafeMultiChainSigAccountV1) by swapping the 4337 module + +// fallback handler, then prove the upgraded account operates on v0.9. +// +// Cross-EntryPoint: the deploy + migrate ops are validated by the v0.7 module +// and routed through the v7 bundler; the post-migration op runs through the v9 +// bundler against the same anvil node. Self-funded via anvil_setBalance — no +// paymaster needed. + +const { Wallet } = require('ethers'); +const { + SafeAccountV0_3_0, + SafeMultiChainSigAccountV1, + sendJsonRpcRequest, +} = require('../../../../dist/index.cjs'); +const { runnable, unrunnable, nodeUrl, bundlerUrl } = require('../../_runnable.cjs'); + +jest.setTimeout(300000); + +const TEN_ETH_HEX = '0x8AC7230489E80000'; +const V07_MODULE = SafeAccountV0_3_0.DEFAULT_SAFE_4337_MODULE_ADDRESS; +const V09_MODULE = SafeMultiChainSigAccountV1.DEFAULT_SAFE_4337_MODULE_ADDRESS; + +describe.each(runnable)('Safe v0.7 -> v0.9 migration e2e: $name (chainId $chainId)', (chain) => { + const node = nodeUrl(chain); + const v07Bundler = bundlerUrl({ ...chain, entrypoint: 'v7' }); + const v09Bundler = bundlerUrl({ ...chain, entrypoint: 'v9' }); + const chainId = BigInt(chain.chainId); + + const owner = Wallet.createRandom(); + + let oldAccount; // SafeAccountV0_3_0 view (EP v0.7) + let accountAddress; + + async function sendUserOp(account, bundler, transactions) { + const userOp = await account.createUserOperation(transactions, node, bundler, { + verificationGasLimitPercentageMultiplier: 200, + }); + userOp.signature = account.signUserOperation(userOp, [owner.privateKey], chainId); + return (await account.sendUserOperation(userOp, bundler)).included(); + } + + beforeAll(async () => { + oldAccount = SafeAccountV0_3_0.initializeNewAccount([owner.address]); + accountAddress = oldAccount.accountAddress; + await sendJsonRpcRequest(node, 'anvil_setBalance', [accountAddress, TEN_ETH_HEX]); + }); + + test('deploy the Safe on EntryPoint v0.7', async () => { + const receipt = await sendUserOp(oldAccount, v07Bundler, [ + { to: owner.address, value: 0n, data: '0x' }, + ]); + expect(receipt?.success).toBe(true); + + const code = await sendJsonRpcRequest(node, 'eth_getCode', [accountAddress, 'latest']); + expect(code).not.toBe('0x'); + // The v0.7 module is the active module on the fresh Safe. + expect(await oldAccount.isModuleEnabled(node, V07_MODULE)).toBe(true); + }); + + test('migrate to the v0.9 multi-chain module (validated by the v0.7 module)', async () => { + oldAccount = new SafeAccountV0_3_0(accountAddress); + const batch = await oldAccount.createMigrateToSafeMultiChainSigAccountV1MetaTransactions(node); + expect(batch).toHaveLength(3); + + const receipt = await sendUserOp(oldAccount, v07Bundler, batch); + expect(receipt?.success).toBe(true); + }); + + test('on-chain state reflects the upgrade', async () => { + expect(await oldAccount.isModuleEnabled(node, V09_MODULE)).toBe(true); + expect(await oldAccount.isModuleEnabled(node, V07_MODULE)).toBe(false); + + const fallbackHandler = await oldAccount.getFallbackHandler(node); + expect(fallbackHandler.toLowerCase()).toBe(V09_MODULE.toLowerCase()); + }); + + test('the upgraded account executes a UserOperation on EntryPoint v0.9', async () => { + const newAccount = new SafeMultiChainSigAccountV1(accountAddress); + const recipient = Wallet.createRandom().address; + const value = 1_000_000_000_000_000n; + + const receipt = await sendUserOp(newAccount, v09Bundler, [ + { to: recipient, value, data: '0x' }, + ]); + expect(receipt?.success).toBe(true); + + const bal = await sendJsonRpcRequest(node, 'eth_getBalance', [recipient, 'latest']); + expect(BigInt(bal)).toBe(value); + }); +}); + +if (unrunnable.length > 0) { + describe.skip.each(unrunnable)('Safe v0.7 -> v0.9 migration e2e: $name (setup failed)', () => { + test('skipped', () => {}); + }); +} diff --git a/test/safe/moduleMigration.test.js b/test/safe/moduleMigration.test.js new file mode 100644 index 00000000..20079227 --- /dev/null +++ b/test/safe/moduleMigration.test.js @@ -0,0 +1,183 @@ +// Unit tests for the Safe 4337 module-migration helpers and the fallback-handler +// reader. Deterministic and offline: a hand-rolled mock Transport stands in for +// the node, and when a previous module is supplied the disable path skips RPC +// entirely. + +const { AbiCoder, getAddress } = require('ethers'); +const { + SafeAccountV0_2_0, + SafeAccountV0_3_0, + SafeMultiChainSigAccountV1, + SAFE_FALLBACK_HANDLER_STORAGE_SLOT, +} = require('../../dist/index.cjs'); + +const DISABLE_MODULE = '0xe009cfde'; // disableModule(address,address) +const ENABLE_MODULE = '0x610b5925'; // enableModule(address) +const SET_FALLBACK_HANDLER = '0xf08a0323'; // setFallbackHandler(address) +const SENTINEL = '0x0000000000000000000000000000000000000001'; + +const ACCOUNT = '0x1111111111111111111111111111111111111111'; +const V07_MODULE = SafeAccountV0_3_0.DEFAULT_SAFE_4337_MODULE_ADDRESS; +const V09_MODULE = SafeMultiChainSigAccountV1.DEFAULT_SAFE_4337_MODULE_ADDRESS; +const V06_MODULE = SafeAccountV0_2_0.DEFAULT_SAFE_4337_MODULE_ADDRESS; + +// Decode the i-th 32-byte address argument (after the 4-byte selector). +function addrArg(data, i) { + const start = 10 + i * 64; + return getAddress('0x' + data.slice(start, start + 64).slice(-40)); +} + +// A Transport whose request handler is supplied per-test. `calls` records every +// request so method/params can be asserted. +function mockTransport(handler) { + const calls = []; + return { + request: async (args) => { + calls.push(args); + return handler(args); + }, + calls, + }; +} + +describe('SafeAccount.createModuleMigrationMetaTransactions', () => { + test('returns [disable, enable, setFallbackHandler] with correct selectors and targets', async () => { + const account = new SafeAccountV0_3_0(ACCOUNT); + const batch = await account.createModuleMigrationMetaTransactions( + 'https://unused.invalid', + V07_MODULE, + V09_MODULE, + { prevModuleAddress: SENTINEL }, // skip the on-chain predecessor lookup + ); + + expect(batch).toHaveLength(3); + for (const tx of batch) { + expect(getAddress(tx.to)).toBe(getAddress(ACCOUNT)); + expect(tx.value).toBe(0n); + } + + // 1. disableModule(prev, oldModule) + expect(batch[0].data.slice(0, 10)).toBe(DISABLE_MODULE); + expect(addrArg(batch[0].data, 0)).toBe(getAddress(SENTINEL)); + expect(addrArg(batch[0].data, 1)).toBe(getAddress(V07_MODULE)); + + // 2. enableModule(newModule) + expect(batch[1].data.slice(0, 10)).toBe(ENABLE_MODULE); + expect(addrArg(batch[1].data, 0)).toBe(getAddress(V09_MODULE)); + + // 3. setFallbackHandler(newModule) + expect(batch[2].data.slice(0, 10)).toBe(SET_FALLBACK_HANDLER); + expect(addrArg(batch[2].data, 0)).toBe(getAddress(V09_MODULE)); + }); + + test('looks up the predecessor on-chain when prevModuleAddress is omitted', async () => { + // getModulesPaginated returns the old module at index 0, so the + // predecessor is the sentinel. + const transport = mockTransport(({ method }) => { + if (method === 'eth_call') { + return AbiCoder.defaultAbiCoder().encode( + ['address[]', 'address'], + [[V07_MODULE], SENTINEL], + ); + } + throw new Error(`unexpected method ${method}`); + }); + + const account = new SafeAccountV0_3_0(ACCOUNT); + const batch = await account.createModuleMigrationMetaTransactions( + transport, + V07_MODULE, + V09_MODULE, + ); + + expect(transport.calls.some((c) => c.method === 'eth_call')).toBe(true); + expect(batch[0].data.slice(0, 10)).toBe(DISABLE_MODULE); + expect(addrArg(batch[0].data, 0)).toBe(getAddress(SENTINEL)); // predecessor + expect(addrArg(batch[0].data, 1)).toBe(getAddress(V07_MODULE)); // module disabled + }); +}); + +describe('SafeAccountV0_3_0.createMigrateToSafeMultiChainSigAccountV1MetaTransactions', () => { + test('disables the v0.7 module and enables/sets the v0.9 module', async () => { + const account = new SafeAccountV0_3_0(ACCOUNT); + const batch = await account.createMigrateToSafeMultiChainSigAccountV1MetaTransactions( + 'https://unused.invalid', + { prevModuleAddress: SENTINEL }, + ); + + expect(batch).toHaveLength(3); + expect(addrArg(batch[0].data, 1)).toBe(getAddress(V07_MODULE)); // disable v0.7 + expect(addrArg(batch[1].data, 0)).toBe(getAddress(V09_MODULE)); // enable v0.9 + expect(addrArg(batch[2].data, 0)).toBe(getAddress(V09_MODULE)); // fallback -> v0.9 + }); + + test('honors explicit module overrides', async () => { + const customOld = '0x00000000000000000000000000000000000000a7'; + const customNew = '0x00000000000000000000000000000000000000a9'; + const account = new SafeAccountV0_3_0(ACCOUNT); + const batch = await account.createMigrateToSafeMultiChainSigAccountV1MetaTransactions( + 'https://unused.invalid', + { safeV07ModuleAddress: customOld, safeV09ModuleAddress: customNew, prevModuleAddress: SENTINEL }, + ); + expect(addrArg(batch[0].data, 1)).toBe(getAddress(customOld)); + expect(addrArg(batch[1].data, 0)).toBe(getAddress(customNew)); + expect(addrArg(batch[2].data, 0)).toBe(getAddress(customNew)); + }); +}); + +describe('SafeAccountV0_2_0.createMigrateToSafeAccountV0_3_0MetaTransactions (prevModuleAddress regression)', () => { + test('defaults to migrating the v0.6 module to the v0.7 module', async () => { + const transport = mockTransport(({ method }) => { + if (method === 'eth_call') { + return AbiCoder.defaultAbiCoder().encode( + ['address[]', 'address'], + [[V06_MODULE], SENTINEL], + ); + } + throw new Error(`unexpected method ${method}`); + }); + const account = new SafeAccountV0_2_0(ACCOUNT); + const batch = await account.createMigrateToSafeAccountV0_3_0MetaTransactions(transport); + + expect(addrArg(batch[0].data, 1)).toBe(getAddress(V06_MODULE)); // disable v0.6 + expect(addrArg(batch[1].data, 0)).toBe(getAddress(V07_MODULE)); // enable v0.7 + expect(addrArg(batch[2].data, 0)).toBe(getAddress(V07_MODULE)); // fallback -> v0.7 + }); + + test('explicit prevModuleAddress is used as the predecessor, NOT the module being disabled', async () => { + // Regression: previously safeV06ModuleAddress was mis-wired into + // prevModuleAddress, producing disableModule(prev=module, module). + const predecessor = '0x00000000000000000000000000000000000000aa'; + const moduleToDisable = '0x00000000000000000000000000000000000000bb'; + const account = new SafeAccountV0_2_0(ACCOUNT); + const batch = await account.createMigrateToSafeAccountV0_3_0MetaTransactions( + 'https://unused.invalid', + { safeV06ModuleAddress: moduleToDisable, prevModuleAddress: predecessor }, + ); + + expect(batch[0].data.slice(0, 10)).toBe(DISABLE_MODULE); + expect(addrArg(batch[0].data, 0)).toBe(getAddress(predecessor)); // prev + expect(addrArg(batch[0].data, 1)).toBe(getAddress(moduleToDisable)); // module + // The bug would have made these equal: + expect(addrArg(batch[0].data, 0)).not.toBe(addrArg(batch[0].data, 1)); + }); +}); + +describe('SafeAccount.getFallbackHandler', () => { + test('reads the fallback-handler storage slot and returns a checksummed address', async () => { + const stored = '0x' + + '0'.repeat(24) + + V09_MODULE.slice(2).toLowerCase(); // 32-byte word: left-padded address + const transport = mockTransport(({ method, params }) => { + expect(method).toBe('eth_getStorageAt'); + expect(params[0]).toBe(ACCOUNT); + expect(params[1]).toBe(SAFE_FALLBACK_HANDLER_STORAGE_SLOT); + expect(params[2]).toBe('latest'); + return stored; + }); + + const account = new SafeAccountV0_3_0(ACCOUNT); + const handler = await account.getFallbackHandler(transport); + expect(handler).toBe(getAddress(V09_MODULE)); + }); +}); diff --git a/test/transport/JsonRpcNode.test.js b/test/transport/JsonRpcNode.test.js index ea05eb2c..c33e3085 100644 --- a/test/transport/JsonRpcNode.test.js +++ b/test/transport/JsonRpcNode.test.js @@ -95,6 +95,36 @@ describe("JsonRpcNode", () => { expect(typeof code).toBe("string"); }); + test("getStorageAt() sends eth_getStorageAt with the expected params", async () => { + const slot = "0x6c9a6c4a39284e37ed1cf53d337577d14212a4870fb976a4366c693b939918d5"; + const mock = tx(({ method, params }) => { + expect(method).toBe("eth_getStorageAt"); + expect(params).toEqual(["0xdead", slot, "latest"]); + return "0x" + "0".repeat(24) + "22939e839e3c0f479b713eaf95e0df128554aead"; + }); + const node = new ak.JsonRpcNode(mock); + const word = await node.getStorageAt("0xdead", slot); + expect(typeof word).toBe("string"); + expect(word.endsWith("22939e839e3c0f479b713eaf95e0df128554aead")).toBe(true); + }); + + test("getStorageAt() forwards an explicit block tag", async () => { + const mock = tx(({ params }) => { + expect(params[2]).toBe("0x10"); + return "0x" + "0".repeat(64); + }); + const node = new ak.JsonRpcNode(mock); + await node.getStorageAt("0xdead", "0x0", "0x10"); + }); + + test("getStorageAt() throws BAD_DATA when the transport returns a non-string", async () => { + const mock = tx(() => 12345); + const node = new ak.JsonRpcNode(mock); + await expect(node.getStorageAt("0xdead", "0x0")).rejects.toMatchObject({ + code: "BAD_DATA", + }); + }); + test("call() sends eth_call without state overrides", async () => { const mock = tx(({ method, params }) => { expect(method).toBe("eth_call"); From 7d15ab023a386636518a51ef479525925ae820e2 Mon Sep 17 00:00:00 2001 From: sednaoui Date: Mon, 1 Jun 2026 14:37:18 +0200 Subject: [PATCH 4/8] Add opt-out preflight + getSafeVersion to module migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before building a module-migration batch, verify on-chain that the account is actually a Safe running the old 4337 module — the module is enabled AND is the current fallback handler — and that its Safe version meets the module minimum (>= 1.4.1). This turns a cryptic on-chain AA23/AA24 into a clear up-front error. Opt out with { skipPreflight: true }. - SafeAccount.getSafeVersion(node): reads VERSION(). - SafeAccount.createModuleMigrationMetaTransactions: runs the preflight unless skipped; threaded through the v0.6->v0.7 and v0.7->v0.9 wrappers. - Note: an equality check against the target class's singleton would be wrong (it would reject valid 1.5.0 Safes); the modules only need >= 1.4.1, so the check is a minimum, not a match. Tests cover the preflight pass/fail cases, skipPreflight bypass, getSafeVersion, and getFallbackHandler. --- src/account/Safe/SafeAccount.ts | 113 ++++++++++++++++- src/account/Safe/SafeAccountV0_2_0.ts | 2 + src/account/Safe/SafeAccountV0_3_0.ts | 2 + test/safe/moduleMigration.test.js | 174 ++++++++++++++++++-------- 4 files changed, 236 insertions(+), 55 deletions(-) diff --git a/src/account/Safe/SafeAccount.ts b/src/account/Safe/SafeAccount.ts index 60acca93..fbc8add8 100644 --- a/src/account/Safe/SafeAccount.ts +++ b/src/account/Safe/SafeAccount.ts @@ -2759,6 +2759,24 @@ export class SafeAccount extends SmartAccount { return getAddress("0x" + word.slice(-40)); } + /** + * read the Safe's version string via `VERSION()` (e.g. "1.4.1"). + * @param nodeRpcUrl - The JSON-RPC API url for the target chain + * @returns a promise of the Safe singleton version string + */ + public async getSafeVersion( + nodeRpcUrl: string | Transport | JsonRpcNode, + ): Promise { + const callData = createCallData(getFunctionSelector("VERSION()"), [], []); + const result = await JsonRpcNode.from(nodeRpcUrl).call( + { to: this.accountAddress, data: callData }, + "latest", + ); + const abiCoder = AbiCoder.defaultAbiCoder(); + const [version] = abiCoder.decode(["string"], result); + return version; + } + /** * create a list of dummy signer/signature pairs for gas estimation based on the expected signers. * @param expectedSigners - signers whose signatures will be produced at sign time @@ -3045,11 +3063,17 @@ export class SafeAccount extends SmartAccount { * by the OLD module on the OLD EntryPoint; disabling that module mid-batch is * safe because validation has already completed. * + * Unless `skipPreflight` is set, this verifies on-chain that the account is + * actually a Safe running `oldModuleAddress` (the module is enabled AND is the + * current fallback handler) and that its Safe version meets the module minimum + * (>= 1.4.1) — turning a would-be cryptic on-chain `AA23`/`AA24` into a clear + * up-front error. + * * @param nodeRpcUrl - The JSON-RPC API url for the target chain (used to find - * the previous module in the linked list when not provided) + * the previous module in the linked list when not provided, and for preflight) * @param oldModuleAddress - the currently-enabled 4337 module to disable * @param newModuleAddress - the 4337 module to enable and set as fallback handler - * @param overrides - overrides for finding the previous module + * @param overrides - previous-module lookup overrides and `skipPreflight` * @returns a promise of [disableOld, enableNew, setFallbackHandler] MetaTransactions */ public async createModuleMigrationMetaTransactions( @@ -3060,8 +3084,13 @@ export class SafeAccount extends SmartAccount { prevModuleAddress?: string; modulesStart?: string; modulesPageSize?: bigint; + skipPreflight?: boolean; } = {}, ): Promise { + if (overrides.skipPreflight !== true) { + await this.assertMigratableFromModule(nodeRpcUrl, oldModuleAddress); + } + const disableOldModule = await this.createDisableModuleMetaTransaction( nodeRpcUrl, oldModuleAddress, @@ -3087,6 +3116,86 @@ export class SafeAccount extends SmartAccount { return [disableOldModule, enableNewModule, setFallbackHandler]; } + /** + * Minimum Safe singleton version required by the ERC-4337 modules. + */ + private static readonly MIN_SAFE_4337_VERSION = "1.4.1"; + + /** + * Assert that this account is a deployed Safe currently running `oldModuleAddress` + * as both its enabled module and its fallback handler, on a Safe version that + * meets the 4337 module minimum. Throws a descriptive `BAD_DATA` error otherwise. + */ + private async assertMigratableFromModule( + nodeRpcUrl: string | Transport | JsonRpcNode, + oldModuleAddress: string, + ): Promise { + const node = JsonRpcNode.from(nodeRpcUrl); + + // The migration UserOperation is validated through the Safe's fallback + // handler on the old EntryPoint, so the old module must be the fallback + // handler for the migration to be processable at all. + let fallbackHandler: string; + try { + fallbackHandler = await this.getFallbackHandler(node); + } catch (err) { + throw new AbstractionKitError( + "BAD_DATA", + `Could not read the fallback handler of ${this.accountAddress} — is it a deployed Safe? ` + + "Pass { skipPreflight: true } to bypass this check.", + { cause: ensureError(err) }, + ); + } + if (fallbackHandler.toLowerCase() !== oldModuleAddress.toLowerCase()) { + throw new AbstractionKitError( + "BAD_DATA", + `Safe ${this.accountAddress} fallback handler is ${fallbackHandler}, expected the ` + + `old 4337 module ${oldModuleAddress}. This account is not a Safe running that ` + + "module; pass { skipPreflight: true } to bypass.", + ); + } + + // The old module must also be enabled so execTransactionFromModule (which + // executes the migration batch) is authorized. + if (!(await this.isModuleEnabled(node, oldModuleAddress))) { + throw new AbstractionKitError( + "BAD_DATA", + `The 4337 module ${oldModuleAddress} is not enabled on Safe ${this.accountAddress}. ` + + "Pass { skipPreflight: true } to bypass.", + ); + } + + // Both modules require Safe >= 1.4.1. + const version = await this.getSafeVersion(node); + if (!SafeAccount.isVersionAtLeast(version, SafeAccount.MIN_SAFE_4337_VERSION)) { + throw new AbstractionKitError( + "BAD_DATA", + `Safe version ${version} is below the minimum ${SafeAccount.MIN_SAFE_4337_VERSION} ` + + "required by the 4337 modules. Pass { skipPreflight: true } to bypass.", + ); + } + } + + /** + * Compare dotted version strings numerically (ignoring any "+suffix"), e.g. + * isVersionAtLeast("1.4.1", "1.4.1") === true, ("1.5.0", "1.4.1") === true. + */ + private static isVersionAtLeast(version: string, minimum: string): boolean { + const parse = (v: string): number[] => + v + .split("+")[0] + .split(".") + .map((n) => parseInt(n, 10) || 0); + const a = parse(version); + const b = parse(minimum); + for (let i = 0; i < Math.max(a.length, b.length); i++) { + const x = a[i] ?? 0; + const y = b[i] ?? 0; + if (x !== y) return x > y; + } + return true; + } + /** * Simulate the encoded calldata for this account on Tenderly and optionally return a share link. * When `isInit` isn't provided, nonce is fetched via `nodeRpcUrl` to decide whether to include diff --git a/src/account/Safe/SafeAccountV0_2_0.ts b/src/account/Safe/SafeAccountV0_2_0.ts index 519cd953..a588cb17 100644 --- a/src/account/Safe/SafeAccountV0_2_0.ts +++ b/src/account/Safe/SafeAccountV0_2_0.ts @@ -372,6 +372,7 @@ export class SafeAccountV0_2_0 extends SafeAccount { prevModuleAddress?: string; pageSize?: bigint; modulesStart?: string; + skipPreflight?: boolean; } = {}, ): Promise { const moduleV06Address = @@ -390,6 +391,7 @@ export class SafeAccountV0_2_0 extends SafeAccount { prevModuleAddress: overrides.prevModuleAddress, modulesPageSize: overrides.pageSize, modulesStart: overrides.modulesStart, + skipPreflight: overrides.skipPreflight, }, ); } diff --git a/src/account/Safe/SafeAccountV0_3_0.ts b/src/account/Safe/SafeAccountV0_3_0.ts index 5bd763a2..369749dc 100644 --- a/src/account/Safe/SafeAccountV0_3_0.ts +++ b/src/account/Safe/SafeAccountV0_3_0.ts @@ -434,6 +434,7 @@ export class SafeAccountV0_3_0 extends SafeAccount { prevModuleAddress?: string; modulesStart?: string; modulesPageSize?: bigint; + skipPreflight?: boolean; } = {}, ): Promise { const moduleV07Address = overrides.safeV07ModuleAddress ?? this.safe4337ModuleAddress; @@ -449,6 +450,7 @@ export class SafeAccountV0_3_0 extends SafeAccount { prevModuleAddress: overrides.prevModuleAddress, modulesStart: overrides.modulesStart, modulesPageSize: overrides.modulesPageSize, + skipPreflight: overrides.skipPreflight, }, ); } diff --git a/test/safe/moduleMigration.test.js b/test/safe/moduleMigration.test.js index 20079227..33b4e980 100644 --- a/test/safe/moduleMigration.test.js +++ b/test/safe/moduleMigration.test.js @@ -1,7 +1,8 @@ -// Unit tests for the Safe 4337 module-migration helpers and the fallback-handler -// reader. Deterministic and offline: a hand-rolled mock Transport stands in for -// the node, and when a previous module is supplied the disable path skips RPC -// entirely. +// Unit tests for the Safe 4337 module-migration helpers, the migration preflight, +// and the fallback-handler / version readers. Deterministic and offline: a +// hand-rolled mock Transport stands in for the node. Builder-shape tests pass +// `skipPreflight: true` so they exercise only calldata construction; the preflight +// is covered by its own block. const { AbiCoder, getAddress } = require('ethers'); const { @@ -9,6 +10,7 @@ const { SafeAccountV0_3_0, SafeMultiChainSigAccountV1, SAFE_FALLBACK_HANDLER_STORAGE_SLOT, + getFunctionSelector, } = require('../../dist/index.cjs'); const DISABLE_MODULE = '0xe009cfde'; // disableModule(address,address) @@ -21,14 +23,19 @@ const V07_MODULE = SafeAccountV0_3_0.DEFAULT_SAFE_4337_MODULE_ADDRESS; const V09_MODULE = SafeMultiChainSigAccountV1.DEFAULT_SAFE_4337_MODULE_ADDRESS; const V06_MODULE = SafeAccountV0_2_0.DEFAULT_SAFE_4337_MODULE_ADDRESS; +const VERSION_SELECTOR = getFunctionSelector('VERSION()'); +const IS_MODULE_ENABLED_SELECTOR = getFunctionSelector('isModuleEnabled(address)'); + // Decode the i-th 32-byte address argument (after the 4-byte selector). function addrArg(data, i) { const start = 10 + i * 64; return getAddress('0x' + data.slice(start, start + 64).slice(-40)); } -// A Transport whose request handler is supplied per-test. `calls` records every -// request so method/params can be asserted. +function leftPadAddress(address) { + return '0x' + '0'.repeat(24) + address.toLowerCase().replace(/^0x/, '').padStart(40, '0'); +} + function mockTransport(handler) { const calls = []; return { @@ -40,14 +47,34 @@ function mockTransport(handler) { }; } -describe('SafeAccount.createModuleMigrationMetaTransactions', () => { +// A transport simulating a deployed Safe: fallback handler, module-enabled +// answer, version string, and the module list for predecessor lookups. +function safeMock({ fallback = V07_MODULE, moduleEnabled = true, version = '1.4.1', modules = [V07_MODULE] } = {}) { + return mockTransport(({ method, params }) => { + if (method === 'eth_getStorageAt') return leftPadAddress(fallback); + if (method === 'eth_call') { + const data = params[0].data; + if (data.startsWith(VERSION_SELECTOR)) { + return AbiCoder.defaultAbiCoder().encode(['string'], [version]); + } + if (data.startsWith(IS_MODULE_ENABLED_SELECTOR)) { + return AbiCoder.defaultAbiCoder().encode(['bool'], [moduleEnabled]); + } + // getModulesPaginated(address,uint256) + return AbiCoder.defaultAbiCoder().encode(['address[]', 'address'], [modules, SENTINEL]); + } + throw new Error(`unexpected method ${method}`); + }); +} + +describe('SafeAccount.createModuleMigrationMetaTransactions (builder shape)', () => { test('returns [disable, enable, setFallbackHandler] with correct selectors and targets', async () => { const account = new SafeAccountV0_3_0(ACCOUNT); const batch = await account.createModuleMigrationMetaTransactions( 'https://unused.invalid', V07_MODULE, V09_MODULE, - { prevModuleAddress: SENTINEL }, // skip the on-chain predecessor lookup + { prevModuleAddress: SENTINEL, skipPreflight: true }, ); expect(batch).toHaveLength(3); @@ -56,38 +83,25 @@ describe('SafeAccount.createModuleMigrationMetaTransactions', () => { expect(tx.value).toBe(0n); } - // 1. disableModule(prev, oldModule) expect(batch[0].data.slice(0, 10)).toBe(DISABLE_MODULE); expect(addrArg(batch[0].data, 0)).toBe(getAddress(SENTINEL)); expect(addrArg(batch[0].data, 1)).toBe(getAddress(V07_MODULE)); - // 2. enableModule(newModule) expect(batch[1].data.slice(0, 10)).toBe(ENABLE_MODULE); expect(addrArg(batch[1].data, 0)).toBe(getAddress(V09_MODULE)); - // 3. setFallbackHandler(newModule) expect(batch[2].data.slice(0, 10)).toBe(SET_FALLBACK_HANDLER); expect(addrArg(batch[2].data, 0)).toBe(getAddress(V09_MODULE)); }); test('looks up the predecessor on-chain when prevModuleAddress is omitted', async () => { - // getModulesPaginated returns the old module at index 0, so the - // predecessor is the sentinel. - const transport = mockTransport(({ method }) => { - if (method === 'eth_call') { - return AbiCoder.defaultAbiCoder().encode( - ['address[]', 'address'], - [[V07_MODULE], SENTINEL], - ); - } - throw new Error(`unexpected method ${method}`); - }); - + const transport = safeMock(); const account = new SafeAccountV0_3_0(ACCOUNT); const batch = await account.createModuleMigrationMetaTransactions( transport, V07_MODULE, V09_MODULE, + { skipPreflight: true }, ); expect(transport.calls.some((c) => c.method === 'eth_call')).toBe(true); @@ -102,13 +116,13 @@ describe('SafeAccountV0_3_0.createMigrateToSafeMultiChainSigAccountV1MetaTransac const account = new SafeAccountV0_3_0(ACCOUNT); const batch = await account.createMigrateToSafeMultiChainSigAccountV1MetaTransactions( 'https://unused.invalid', - { prevModuleAddress: SENTINEL }, + { prevModuleAddress: SENTINEL, skipPreflight: true }, ); expect(batch).toHaveLength(3); - expect(addrArg(batch[0].data, 1)).toBe(getAddress(V07_MODULE)); // disable v0.7 - expect(addrArg(batch[1].data, 0)).toBe(getAddress(V09_MODULE)); // enable v0.9 - expect(addrArg(batch[2].data, 0)).toBe(getAddress(V09_MODULE)); // fallback -> v0.9 + expect(addrArg(batch[0].data, 1)).toBe(getAddress(V07_MODULE)); + expect(addrArg(batch[1].data, 0)).toBe(getAddress(V09_MODULE)); + expect(addrArg(batch[2].data, 0)).toBe(getAddress(V09_MODULE)); }); test('honors explicit module overrides', async () => { @@ -117,7 +131,7 @@ describe('SafeAccountV0_3_0.createMigrateToSafeMultiChainSigAccountV1MetaTransac const account = new SafeAccountV0_3_0(ACCOUNT); const batch = await account.createMigrateToSafeMultiChainSigAccountV1MetaTransactions( 'https://unused.invalid', - { safeV07ModuleAddress: customOld, safeV09ModuleAddress: customNew, prevModuleAddress: SENTINEL }, + { safeV07ModuleAddress: customOld, safeV09ModuleAddress: customNew, prevModuleAddress: SENTINEL, skipPreflight: true }, ); expect(addrArg(batch[0].data, 1)).toBe(getAddress(customOld)); expect(addrArg(batch[1].data, 0)).toBe(getAddress(customNew)); @@ -127,21 +141,14 @@ describe('SafeAccountV0_3_0.createMigrateToSafeMultiChainSigAccountV1MetaTransac describe('SafeAccountV0_2_0.createMigrateToSafeAccountV0_3_0MetaTransactions (prevModuleAddress regression)', () => { test('defaults to migrating the v0.6 module to the v0.7 module', async () => { - const transport = mockTransport(({ method }) => { - if (method === 'eth_call') { - return AbiCoder.defaultAbiCoder().encode( - ['address[]', 'address'], - [[V06_MODULE], SENTINEL], - ); - } - throw new Error(`unexpected method ${method}`); - }); const account = new SafeAccountV0_2_0(ACCOUNT); - const batch = await account.createMigrateToSafeAccountV0_3_0MetaTransactions(transport); - - expect(addrArg(batch[0].data, 1)).toBe(getAddress(V06_MODULE)); // disable v0.6 - expect(addrArg(batch[1].data, 0)).toBe(getAddress(V07_MODULE)); // enable v0.7 - expect(addrArg(batch[2].data, 0)).toBe(getAddress(V07_MODULE)); // fallback -> v0.7 + const batch = await account.createMigrateToSafeAccountV0_3_0MetaTransactions( + safeMock({ fallback: V06_MODULE, modules: [V06_MODULE] }), + { skipPreflight: true }, + ); + expect(addrArg(batch[0].data, 1)).toBe(getAddress(V06_MODULE)); + expect(addrArg(batch[1].data, 0)).toBe(getAddress(V07_MODULE)); + expect(addrArg(batch[2].data, 0)).toBe(getAddress(V07_MODULE)); }); test('explicit prevModuleAddress is used as the predecessor, NOT the module being disabled', async () => { @@ -152,32 +159,93 @@ describe('SafeAccountV0_2_0.createMigrateToSafeAccountV0_3_0MetaTransactions (pr const account = new SafeAccountV0_2_0(ACCOUNT); const batch = await account.createMigrateToSafeAccountV0_3_0MetaTransactions( 'https://unused.invalid', - { safeV06ModuleAddress: moduleToDisable, prevModuleAddress: predecessor }, + { safeV06ModuleAddress: moduleToDisable, prevModuleAddress: predecessor, skipPreflight: true }, ); expect(batch[0].data.slice(0, 10)).toBe(DISABLE_MODULE); expect(addrArg(batch[0].data, 0)).toBe(getAddress(predecessor)); // prev expect(addrArg(batch[0].data, 1)).toBe(getAddress(moduleToDisable)); // module - // The bug would have made these equal: expect(addrArg(batch[0].data, 0)).not.toBe(addrArg(batch[0].data, 1)); }); }); -describe('SafeAccount.getFallbackHandler', () => { - test('reads the fallback-handler storage slot and returns a checksummed address', async () => { - const stored = '0x' - + '0'.repeat(24) - + V09_MODULE.slice(2).toLowerCase(); // 32-byte word: left-padded address +describe('migration preflight', () => { + test('passes when the old module is the fallback handler, enabled, and version >= 1.4.1', async () => { + const account = new SafeAccountV0_3_0(ACCOUNT); + const batch = await account.createMigrateToSafeMultiChainSigAccountV1MetaTransactions( + safeMock({ fallback: V07_MODULE, moduleEnabled: true, version: '1.4.1' }), + ); + expect(batch).toHaveLength(3); + }); + + test('passes on a newer Safe version (1.5.0)', async () => { + const account = new SafeAccountV0_3_0(ACCOUNT); + const batch = await account.createMigrateToSafeMultiChainSigAccountV1MetaTransactions( + safeMock({ version: '1.5.0' }), + ); + expect(batch).toHaveLength(3); + }); + + test('throws when the fallback handler is not the old module', async () => { + const account = new SafeAccountV0_3_0(ACCOUNT); + await expect( + account.createMigrateToSafeMultiChainSigAccountV1MetaTransactions( + safeMock({ fallback: '0x00000000000000000000000000000000000000cc' }), + ), + ).rejects.toMatchObject({ code: 'BAD_DATA' }); + }); + + test('throws when the old module is not enabled', async () => { + const account = new SafeAccountV0_3_0(ACCOUNT); + await expect( + account.createMigrateToSafeMultiChainSigAccountV1MetaTransactions( + safeMock({ fallback: V07_MODULE, moduleEnabled: false }), + ), + ).rejects.toMatchObject({ code: 'BAD_DATA' }); + }); + + test('throws when the Safe version is below 1.4.1', async () => { + const account = new SafeAccountV0_3_0(ACCOUNT); + await expect( + account.createMigrateToSafeMultiChainSigAccountV1MetaTransactions( + safeMock({ version: '1.3.0' }), + ), + ).rejects.toMatchObject({ code: 'BAD_DATA' }); + }); + + test('skipPreflight bypasses all checks (no storage read, builds anyway)', async () => { + // A mock that would FAIL preflight (wrong fallback) still produces a batch + // and never reads the fallback-handler slot. + const transport = safeMock({ fallback: '0x00000000000000000000000000000000000000cc' }); + const account = new SafeAccountV0_3_0(ACCOUNT); + const batch = await account.createMigrateToSafeMultiChainSigAccountV1MetaTransactions( + transport, + { prevModuleAddress: SENTINEL, skipPreflight: true }, + ); + expect(batch).toHaveLength(3); + expect(transport.calls.some((c) => c.method === 'eth_getStorageAt')).toBe(false); + }); +}); + +describe('SafeAccount readers', () => { + test('getFallbackHandler returns a checksummed address from the slot', async () => { const transport = mockTransport(({ method, params }) => { expect(method).toBe('eth_getStorageAt'); expect(params[0]).toBe(ACCOUNT); expect(params[1]).toBe(SAFE_FALLBACK_HANDLER_STORAGE_SLOT); - expect(params[2]).toBe('latest'); - return stored; + return leftPadAddress(V09_MODULE); }); + const account = new SafeAccountV0_3_0(ACCOUNT); + expect(await account.getFallbackHandler(transport)).toBe(getAddress(V09_MODULE)); + }); + test('getSafeVersion decodes the VERSION() string', async () => { + const transport = mockTransport(({ method, params }) => { + expect(method).toBe('eth_call'); + expect(params[0].data.startsWith(VERSION_SELECTOR)).toBe(true); + return AbiCoder.defaultAbiCoder().encode(['string'], ['1.4.1']); + }); const account = new SafeAccountV0_3_0(ACCOUNT); - const handler = await account.getFallbackHandler(transport); - expect(handler).toBe(getAddress(V09_MODULE)); + expect(await account.getSafeVersion(transport)).toBe('1.4.1'); }); }); From 4957737085dbab6ae0697fa3aa6754b7e7dae8e7 Mon Sep 17 00:00:00 2001 From: sednaoui Date: Mon, 1 Jun 2026 14:41:23 +0200 Subject: [PATCH 5/8] Use decodeAbiParameters in getSafeVersion after dev merge dev's ethers-minimization dropped the AbiCoder import from SafeAccount.ts; getSafeVersion now decodes VERSION() via the local decodeAbiParameters helper (matching isModuleEnabled/getModules), fixing a runtime ReferenceError that the bundler build did not catch. --- src/account/Safe/SafeAccount.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/account/Safe/SafeAccount.ts b/src/account/Safe/SafeAccount.ts index a7b5499f..0a46c96a 100644 --- a/src/account/Safe/SafeAccount.ts +++ b/src/account/Safe/SafeAccount.ts @@ -2772,8 +2772,7 @@ export class SafeAccount extends SmartAccount { { to: this.accountAddress, data: callData }, "latest", ); - const abiCoder = AbiCoder.defaultAbiCoder(); - const [version] = abiCoder.decode(["string"], result); + const [version] = decodeAbiParameters<[string]>(["string"], result); return version; } From 99bf825f877d99ce7e8461308e60828daf311129 Mon Sep 17 00:00:00 2001 From: sednaoui Date: Mon, 1 Jun 2026 16:11:38 +0200 Subject: [PATCH 6/8] Normalize preflight errors from module/version checks to BAD_DATA The migration preflight already wrapped getFallbackHandler, but a raw RPC/ABI error from isModuleEnabled or getSafeVersion (e.g. a reverting VERSION() on a non-Safe), or an empty/invalid version string, could leak through unnormalized. Wrap both calls in try/catch and treat empty/invalid versions as a failed preflight, rethrowing a clear AbstractionKitError("BAD_DATA", ...) that names the Safe, the module, and the skipPreflight hint. --- src/account/Safe/SafeAccount.ts | 39 ++++++++++++++++++++++++----- test/safe/moduleMigration.test.js | 41 +++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/src/account/Safe/SafeAccount.ts b/src/account/Safe/SafeAccount.ts index 0a46c96a..67971781 100644 --- a/src/account/Safe/SafeAccount.ts +++ b/src/account/Safe/SafeAccount.ts @@ -3155,7 +3155,18 @@ export class SafeAccount extends SmartAccount { // The old module must also be enabled so execTransactionFromModule (which // executes the migration batch) is authorized. - if (!(await this.isModuleEnabled(node, oldModuleAddress))) { + let moduleEnabled: boolean; + try { + moduleEnabled = await this.isModuleEnabled(node, oldModuleAddress); + } catch (err) { + throw new AbstractionKitError( + "BAD_DATA", + `Could not check whether module ${oldModuleAddress} is enabled on Safe ` + + `${this.accountAddress} — is it a deployed Safe? Pass { skipPreflight: true } to bypass.`, + { cause: ensureError(err) }, + ); + } + if (!moduleEnabled) { throw new AbstractionKitError( "BAD_DATA", `The 4337 module ${oldModuleAddress} is not enabled on Safe ${this.accountAddress}. ` + @@ -3163,13 +3174,29 @@ export class SafeAccount extends SmartAccount { ); } - // Both modules require Safe >= 1.4.1. - const version = await this.getSafeVersion(node); - if (!SafeAccount.isVersionAtLeast(version, SafeAccount.MIN_SAFE_4337_VERSION)) { + // Both modules require Safe >= 1.4.1. Treat read/decode failures and + // empty/invalid version strings as a failed preflight. + let version: string; + try { + version = await this.getSafeVersion(node); + } catch (err) { + throw new AbstractionKitError( + "BAD_DATA", + `Could not read the Safe version (VERSION()) of ${this.accountAddress} — is it a ` + + `deployed Safe running module ${oldModuleAddress}? Pass { skipPreflight: true } to bypass.`, + { cause: ensureError(err) }, + ); + } + if ( + typeof version !== "string" || + version.trim() === "" || + !SafeAccount.isVersionAtLeast(version, SafeAccount.MIN_SAFE_4337_VERSION) + ) { throw new AbstractionKitError( "BAD_DATA", - `Safe version ${version} is below the minimum ${SafeAccount.MIN_SAFE_4337_VERSION} ` + - "required by the 4337 modules. Pass { skipPreflight: true } to bypass.", + `Safe ${this.accountAddress} reported version "${version}", which does not meet the ` + + `minimum ${SafeAccount.MIN_SAFE_4337_VERSION} required by the 4337 modules ` + + `(module ${oldModuleAddress}). Pass { skipPreflight: true } to bypass.`, ); } } diff --git a/test/safe/moduleMigration.test.js b/test/safe/moduleMigration.test.js index 33b4e980..76a4cf3e 100644 --- a/test/safe/moduleMigration.test.js +++ b/test/safe/moduleMigration.test.js @@ -225,6 +225,47 @@ describe('migration preflight', () => { expect(batch).toHaveLength(3); expect(transport.calls.some((c) => c.method === 'eth_getStorageAt')).toBe(false); }); + + test('normalizes a raw error from the module-enabled check to BAD_DATA', async () => { + // fallback handler passes, but the isModuleEnabled eth_call reverts. + const transport = mockTransport(({ method, params }) => { + if (method === 'eth_getStorageAt') return leftPadAddress(V07_MODULE); + if (method === 'eth_call' && params[0].data.startsWith(IS_MODULE_ENABLED_SELECTOR)) { + throw new Error('execution reverted'); + } + return AbiCoder.defaultAbiCoder().encode(['address[]', 'address'], [[V07_MODULE], SENTINEL]); + }); + const account = new SafeAccountV0_3_0(ACCOUNT); + await expect( + account.createMigrateToSafeMultiChainSigAccountV1MetaTransactions(transport), + ).rejects.toMatchObject({ name: 'AbstractionKitError', code: 'BAD_DATA' }); + }); + + test('normalizes a raw error from the VERSION() read to BAD_DATA', async () => { + const transport = mockTransport(({ method, params }) => { + if (method === 'eth_getStorageAt') return leftPadAddress(V07_MODULE); + if (method === 'eth_call') { + if (params[0].data.startsWith(VERSION_SELECTOR)) throw new Error('execution reverted'); + if (params[0].data.startsWith(IS_MODULE_ENABLED_SELECTOR)) { + return AbiCoder.defaultAbiCoder().encode(['bool'], [true]); + } + } + return AbiCoder.defaultAbiCoder().encode(['address[]', 'address'], [[V07_MODULE], SENTINEL]); + }); + const account = new SafeAccountV0_3_0(ACCOUNT); + await expect( + account.createMigrateToSafeMultiChainSigAccountV1MetaTransactions(transport), + ).rejects.toMatchObject({ name: 'AbstractionKitError', code: 'BAD_DATA' }); + }); + + test('treats an empty VERSION() string as a failed preflight', async () => { + const account = new SafeAccountV0_3_0(ACCOUNT); + await expect( + account.createMigrateToSafeMultiChainSigAccountV1MetaTransactions( + safeMock({ version: '' }), + ), + ).rejects.toMatchObject({ code: 'BAD_DATA' }); + }); }); describe('SafeAccount readers', () => { From b7fd8051ad26df04771b01c3da3e4c4c44ad3c41 Mon Sep 17 00:00:00 2001 From: sednaoui Date: Mon, 1 Jun 2026 16:19:03 +0200 Subject: [PATCH 7/8] Make createModuleMigrationMetaTransactions protected It's the shared implementation behind the version-specific migration helpers, which pin the correct module addresses. Marking it protected steers developers to those wrappers instead of supplying raw old/new module addresses directly (easy to get wrong). Subclass wrappers still call it via `this`. Tests now exercise the shape and predecessor lookup through the public wrapper. --- src/account/Safe/SafeAccount.ts | 8 +++++++- test/safe/moduleMigration.test.js | 32 ++++++++----------------------- 2 files changed, 15 insertions(+), 25 deletions(-) diff --git a/src/account/Safe/SafeAccount.ts b/src/account/Safe/SafeAccount.ts index 67971781..03a8a7bc 100644 --- a/src/account/Safe/SafeAccount.ts +++ b/src/account/Safe/SafeAccount.ts @@ -3073,8 +3073,14 @@ export class SafeAccount extends SmartAccount { * @param newModuleAddress - the 4337 module to enable and set as fallback handler * @param overrides - previous-module lookup overrides and `skipPreflight` * @returns a promise of [disableOld, enableNew, setFallbackHandler] MetaTransactions + * + * @remarks Shared implementation behind the version-specific migration helpers + * (e.g. {@link SafeAccountV0_3_0.createMigrateToSafeMultiChainSigAccountV1MetaTransactions}). + * It is `protected` on purpose: those wrappers pin the correct module addresses, + * so callers reach migration through them rather than supplying raw module + * addresses directly. */ - public async createModuleMigrationMetaTransactions( + protected async createModuleMigrationMetaTransactions( nodeRpcUrl: string | Transport | JsonRpcNode, oldModuleAddress: string, newModuleAddress: string, diff --git a/test/safe/moduleMigration.test.js b/test/safe/moduleMigration.test.js index 76a4cf3e..52fe8aaa 100644 --- a/test/safe/moduleMigration.test.js +++ b/test/safe/moduleMigration.test.js @@ -67,13 +67,13 @@ function safeMock({ fallback = V07_MODULE, moduleEnabled = true, version = '1.4. }); } -describe('SafeAccount.createModuleMigrationMetaTransactions (builder shape)', () => { +// createModuleMigrationMetaTransactions is protected; its shape and predecessor +// lookup are exercised through the public version-specific wrapper. +describe('SafeAccountV0_3_0.createMigrateToSafeMultiChainSigAccountV1MetaTransactions', () => { test('returns [disable, enable, setFallbackHandler] with correct selectors and targets', async () => { const account = new SafeAccountV0_3_0(ACCOUNT); - const batch = await account.createModuleMigrationMetaTransactions( + const batch = await account.createMigrateToSafeMultiChainSigAccountV1MetaTransactions( 'https://unused.invalid', - V07_MODULE, - V09_MODULE, { prevModuleAddress: SENTINEL, skipPreflight: true }, ); @@ -83,13 +83,14 @@ describe('SafeAccount.createModuleMigrationMetaTransactions (builder shape)', () expect(tx.value).toBe(0n); } + // 1. disableModule(prev, oldModule = v0.7) expect(batch[0].data.slice(0, 10)).toBe(DISABLE_MODULE); expect(addrArg(batch[0].data, 0)).toBe(getAddress(SENTINEL)); expect(addrArg(batch[0].data, 1)).toBe(getAddress(V07_MODULE)); - + // 2. enableModule(newModule = v0.9) expect(batch[1].data.slice(0, 10)).toBe(ENABLE_MODULE); expect(addrArg(batch[1].data, 0)).toBe(getAddress(V09_MODULE)); - + // 3. setFallbackHandler(newModule = v0.9) expect(batch[2].data.slice(0, 10)).toBe(SET_FALLBACK_HANDLER); expect(addrArg(batch[2].data, 0)).toBe(getAddress(V09_MODULE)); }); @@ -97,10 +98,8 @@ describe('SafeAccount.createModuleMigrationMetaTransactions (builder shape)', () test('looks up the predecessor on-chain when prevModuleAddress is omitted', async () => { const transport = safeMock(); const account = new SafeAccountV0_3_0(ACCOUNT); - const batch = await account.createModuleMigrationMetaTransactions( + const batch = await account.createMigrateToSafeMultiChainSigAccountV1MetaTransactions( transport, - V07_MODULE, - V09_MODULE, { skipPreflight: true }, ); @@ -109,21 +108,6 @@ describe('SafeAccount.createModuleMigrationMetaTransactions (builder shape)', () expect(addrArg(batch[0].data, 0)).toBe(getAddress(SENTINEL)); // predecessor expect(addrArg(batch[0].data, 1)).toBe(getAddress(V07_MODULE)); // module disabled }); -}); - -describe('SafeAccountV0_3_0.createMigrateToSafeMultiChainSigAccountV1MetaTransactions', () => { - test('disables the v0.7 module and enables/sets the v0.9 module', async () => { - const account = new SafeAccountV0_3_0(ACCOUNT); - const batch = await account.createMigrateToSafeMultiChainSigAccountV1MetaTransactions( - 'https://unused.invalid', - { prevModuleAddress: SENTINEL, skipPreflight: true }, - ); - - expect(batch).toHaveLength(3); - expect(addrArg(batch[0].data, 1)).toBe(getAddress(V07_MODULE)); - expect(addrArg(batch[1].data, 0)).toBe(getAddress(V09_MODULE)); - expect(addrArg(batch[2].data, 0)).toBe(getAddress(V09_MODULE)); - }); test('honors explicit module overrides', async () => { const customOld = '0x00000000000000000000000000000000000000a7'; From 0fd4d4dcd4aa392291f5363ad15d4acc5a7579ff Mon Sep 17 00:00:00 2001 From: sednaoui Date: Mon, 1 Jun 2026 16:21:47 +0200 Subject: [PATCH 8/8] docs(changelog): add module-migration helpers, readers, and v0.6->v0.7 fix Document under [UNRELEASED]: the Safe v0.7->v0.9 migration helper with opt-out preflight, the getFallbackHandler/getSafeVersion/getStorageAt readers and the SAFE_FALLBACK_HANDLER_STORAGE_SLOT export, and the prevModuleAddress fix in the v0.6->v0.7 migration helper. --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a9e9828..c71fd364 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ ## [UNRELEASED] +### New Features + +- **Safe ERC-4337 module migration helpers.** `SafeAccountV0_3_0.createMigrateToSafeMultiChainSigAccountV1MetaTransactions(nodeRpcUrl, overrides?)` builds the `disableModule` + `enableModule` + `setFallbackHandler` batch that migrates a deployed Safe from the EntryPoint v0.7 module to the v0.9 `Safe4337MultiChainSignatureModule`. Both modules are stateless, so no storage clearing is required. Unless `{ skipPreflight: true }` is passed, it first verifies on-chain that the account is actually a Safe running the old module (the module is enabled **and** is the current fallback handler) on a Safe version `>= 1.4.1`, turning a would-be cryptic on-chain `AA23`/`AA24` into a clear up-front error. +- **New Safe / transport readers.** `SafeAccount.getFallbackHandler(nodeRpcUrl)` (the active 4337 module), `SafeAccount.getSafeVersion(nodeRpcUrl)` (reads `VERSION()`), `JsonRpcNode.getStorageAt(address, slot, blockTag?)`, and the exported `SAFE_FALLBACK_HANDLER_STORAGE_SLOT` constant. + +### Bug Fixes + +- **`SafeAccountV0_2_0.createMigrateToSafeAccountV0_3_0MetaTransactions` predecessor wiring.** When `safeV06ModuleAddress` was set explicitly, the v0.6 → v0.7 migration passed the module being disabled as its own linked-list predecessor, producing `disableModule(prev = module, module)`. It now exposes a dedicated `prevModuleAddress` override (defaulting to the on-chain lookup) and shares the generic migration implementation. Default callers are unaffected. + ## 0.3.8 ### New Features