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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Bug Fixes

- **`SafeAccountV1_5_0_M_0_3_0.initializeNewAccount` returns the right runtime type.** The subclass factory delegated to `SafeAccountV0_3_0.initializeNewAccount`, which hard-coded `new SafeAccountV0_3_0(...)`, so the returned instance failed `instanceof SafeAccountV1_5_0_M_0_3_0` and lost subclass behavior. The base factory now instantiates polymorphically through `new this(...)`, so any subclass (including consumer-defined ones) gets its own type back. Detached calls (the factory extracted into a bare function, where strict-mode `this` is undefined) fall back to constructing the base class, preserving the previous behavior for plain-JS callers. (#116)
- **Legacy transaction `v` precision for chain IDs above 2^53.** `createAndSignLegacyRawTransaction` computed the EIP-155 `v` field via `Number(chainId)`, silently rounding large chain IDs and producing a transaction whose signature recovers to the wrong sender. The computation now stays in `BigInt`. (#128)
- **`sendJsonRpcRequest` accepts Tenderly-style `simulation_results` responses again.** The transport refactor made the URL path return only when the JSON body has a `result` field, so non-standard endpoints answering with `{ simulation_results: ... }` fell through to the (undefined) `error` branch and threw a `TypeError`. The `simulation_results` branch is restored, matching the documented behavior and the `JsonRpcResponse` type. (#182)
- **Gas estimation no longer mutates the caller's UserOperation.** `baseEstimateUserOperationGas` on `SafeAccount` and `Simple7702Account` (and their public `estimateUserOperationGas` wrappers) overwrote the operation's `signature` with a dummy and zeroed `maxFeePerGas`/`maxPriorityFeePerGas` in place, restoring the fees only on success. A bundler error left the caller's operation corrupted (zeroed fees, dummy signature). Estimation now runs on an internal shallow copy and the passed operation is never touched. (#132)
Expand Down
24 changes: 19 additions & 5 deletions src/account/Safe/SafeAccountV0_3_0.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,25 @@ export class SafeAccountV0_3_0 extends SafeAccount {
* The account address is deterministically computed but not yet deployed on-chain.
* The first UserOperation sent will deploy it automatically via factory data.
*
* Instantiates through `new this(...)`, so subclasses calling this
* factory (directly or via `super`) get an instance of the subclass,
* not a plain SafeAccountV0_3_0. A detached call (the method extracted
* into a bare function, where `this` is undefined) falls back to
* constructing a SafeAccountV0_3_0, matching the pre-polymorphic
* behavior.
*
* @param owners - Array of owner signers (at least one required)
* @param overrides - Override default initialization values
* @returns A SafeAccountV0_3_0 instance with factory data set for deployment
* @returns An instance of the calling class with factory data set for deployment
*
* @example
* const smartAccount = SafeAccountV0_3_0.initializeNewAccount(["0xOwnerAddress"]);
*/
public static initializeNewAccount(
public static initializeNewAccount<T extends typeof SafeAccountV0_3_0>(
this: T,
owners: Signer[],
overrides: InitCodeOverrides = {},
): SafeAccountV0_3_0 {
): InstanceType<T> {
let isInitWebAuthn = false;
let x = 0n;
let y = 0n;
Expand All @@ -122,7 +130,13 @@ export class SafeAccountV0_3_0 extends SafeAccount {
overrides.safeModuleSetupAddress ?? SafeAccountV0_3_0.DEFAULT_SAFE_MODULE_SETUP_ADDRESS,
);

const safe = new SafeAccountV0_3_0(accountAddress, {
// Plain-JS callers may invoke this factory detached
// (`const init = SafeAccountV0_3_0.initializeNewAccount; init(...)`),
// leaving strict-mode `this` undefined; fall back to this class so
// such calls keep working instead of throwing on `new this(...)`.
// biome-ignore lint/complexity/noThisInStatic: polymorphic factory; subclasses must get their own type back
const ctor = (this as T | undefined) ?? SafeAccountV0_3_0;
const safe: SafeAccountV0_3_0 = new ctor(accountAddress, {
safe4337ModuleAddress: overrides.safe4337ModuleAddress,
entrypointAddress: overrides.entrypointAddress,
onChainIdentifierParams: overrides.onChainIdentifierParams,
Expand All @@ -137,7 +151,7 @@ export class SafeAccountV0_3_0 extends SafeAccount {
safe.y = y;
}

return safe;
return safe as InstanceType<T>;
}

/**
Expand Down
11 changes: 8 additions & 3 deletions src/account/Safe/SafeAccountV1_5_0_M_0_3_0.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,11 @@ export class SafeAccountV1_5_0_M_0_3_0 extends SafeAccountV0_3_0 {
* @example
* const smartAccount = SafeAccountV1_5_0_M_0_3_0.initializeNewAccount(["0xOwnerAddress"]);
*/
public static initializeNewAccount(
public static initializeNewAccount<T extends typeof SafeAccountV0_3_0>(
this: T,
owners: Signer[],
overrides: InitCodeOverrides = {},
): SafeAccountV1_5_0_M_0_3_0 {
): InstanceType<T> {
const modOverrides = {
...overrides,
safeAccountSingleton: overrides.safeAccountSingleton ?? Safe_L2_V1_5_0,
Expand All @@ -111,7 +112,11 @@ export class SafeAccountV1_5_0_M_0_3_0 extends SafeAccountV0_3_0 {
overrides.eip7212WebAuthnContractVerifierForSharedSigner ??
SafeAccountV1_5_0_M_0_3_0.DEFAULT_WEB_AUTHN_CONTRACT_VERIFIER,
};
return SafeAccountV0_3_0.initializeNewAccount(owners, modOverrides);
// `super` keeps `this` bound to the calling class, so the base
// factory's `new this(...)` constructs an instance of this class
// (SafeAccountV1_5_0_M_0_3_0 or a subclass), not SafeAccountV0_3_0.
// biome-ignore lint/complexity/noThisInStatic: polymorphic factory dispatch through super
return super.initializeNewAccount(owners, modOverrides) as InstanceType<T>;
}

/**
Expand Down
52 changes: 52 additions & 0 deletions test/safe/initializeNewAccountRuntimeType.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Unit tests for issue #116: SafeAccountV1_5_0_M_0_3_0.initializeNewAccount
// must return a SafeAccountV1_5_0_M_0_3_0 instance at runtime, not a plain
// SafeAccountV0_3_0. No network required.

const ak = require("../../dist/index.cjs");

const OWNER = "0x2222222222222222222222222222222222222222";

describe("initializeNewAccount runtime type (#116)", () => {
test("SafeAccountV1_5_0_M_0_3_0.initializeNewAccount returns a SafeAccountV1_5_0_M_0_3_0", () => {
const account = ak.SafeAccountV1_5_0_M_0_3_0.initializeNewAccount([OWNER]);
expect(account).toBeInstanceOf(ak.SafeAccountV1_5_0_M_0_3_0);
});

test("instance address matches the class's createAccountAddress (v1.5.0 singleton)", () => {
const account = ak.SafeAccountV1_5_0_M_0_3_0.initializeNewAccount([OWNER]);
expect(account.accountAddress).toBe(ak.SafeAccountV1_5_0_M_0_3_0.createAccountAddress([OWNER]));
});

test("SafeAccountV0_3_0.initializeNewAccount still returns a plain SafeAccountV0_3_0", () => {
const account = ak.SafeAccountV0_3_0.initializeNewAccount([OWNER]);
expect(account).toBeInstanceOf(ak.SafeAccountV0_3_0);
expect(account).not.toBeInstanceOf(ak.SafeAccountV1_5_0_M_0_3_0);
expect(account.accountAddress).toBe(ak.SafeAccountV0_3_0.createAccountAddress([OWNER]));
});

test("a consumer subclass inheriting the factory gets its own type back", () => {
class CustomSafe extends ak.SafeAccountV0_3_0 {}
const account = CustomSafe.initializeNewAccount([OWNER]);
expect(account).toBeInstanceOf(CustomSafe);
});

// Plain-JS consumers may extract the factory into a bare function,
// leaving strict-mode `this` undefined. That worked before the
// polymorphic factory and must keep working (falling back to the
// pre-polymorphic behavior of constructing the base class).
test("detached base factory call still works and returns a SafeAccountV0_3_0", () => {
const init = ak.SafeAccountV0_3_0.initializeNewAccount;
const account = init([OWNER]);
expect(account).toBeInstanceOf(ak.SafeAccountV0_3_0);
expect(account.accountAddress).toBe(ak.SafeAccountV0_3_0.createAccountAddress([OWNER]));
});

test("detached subclass factory call still works with v1.5.0 defaults applied", () => {
const init = ak.SafeAccountV1_5_0_M_0_3_0.initializeNewAccount;
const account = init([OWNER]);
// Pre-polymorphic behavior: the wrapper class degrades to the base,
// but the v1.5.0 singleton/verifier overrides still apply.
expect(account).toBeInstanceOf(ak.SafeAccountV0_3_0);
expect(account.accountAddress).toBe(ak.SafeAccountV1_5_0_M_0_3_0.createAccountAddress([OWNER]));
});
});
Loading