fix: make initializeNewAccount polymorphic so subclasses get their own runtime type

[Sednaoui]

Fixes #116

Red/green verified: test/safe/initializeNewAccountRuntimeType.test.js reproduces instanceof SafeAccountV1_5_0_M_0_3_0 === false on pre-fix dev (2 failed) and passes with the fix. createAccountAddress/createFactoryAddressAndData mentioned in the issue return strings, not instances, so they were unaffected.

Follow-up: detached calls (const init = SafeAccountV0_3_0.initializeNewAccount; init([owner]), where strict-mode this is undefined) would have thrown TypeError: this is not a constructor with the polymorphic factory. They now fall back to constructing the base class, preserving pre-PR behavior for plain-JS callers; covered by two detached-call regression tests (red/green verified against the previous commit of this branch).

Summary by CodeRabbit

• Bug Fixes

  • Account creation factories now preserve the actual subclass type at runtime (so subclass behavior and instanceof checks work as expected), while still falling back to previous plain-JS behavior for detached/bare-function calls.

• Tests

  • Added tests verifying runtime type correctness of account creation across versions, subclasses, and detached-call scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b5f66cff-ff29-4a95-8a10-6a1e048ce5ec

📥 Commits

Reviewing files that changed from the base of the PR and between db6c606 and 2bd6608.

📒 Files selected for processing (1)

• CHANGELOG.md

✅ Files skipped from review due to trivial changes (1)

• CHANGELOG.md

---

📝 Walkthrough

Walkthrough

SafeAccountV0_3_0.initializeNewAccount is converted to a polymorphic generic static factory (explicit this: T, returns InstanceType) that instantiates via the calling ctor (with a SafeAccountV0_3_0 fallback for detached calls). SafeAccountV1_5_0_M_0_3_0 delegates to the polymorphic base factory; tests and a changelog entry validate and document the runtime behavior.

Changes

Polymorphic Static Factory Fix

Layer / File(s)	Summary
Base factory polymorphic signature and instantiation src/account/Safe/SafeAccountV0_3_0.ts	initializeNewAccount becomes generic with this: T and InstanceType<T> return type; implementation chooses `ctor = this
Subclass method signature and super delegation src/account/Safe/SafeAccountV1_5_0_M_0_3_0.ts	initializeNewAccount signature updated to this: T / InstanceType<T> and implementation delegates to super.initializeNewAccount(...) cast to InstanceType<T> so construction dispatches through the calling subclass.
Runtime type validation tests test/safe/initializeNewAccountRuntimeType.test.js	Adds Jest tests verifying v1 factory returns v1 instances, v0 factory preserves v0 instances, subclass polymorphism returns subclass instances, and detached-function calls fall back to base-class construction while applying derived address defaults.
Changelog documentation CHANGELOG.md	UNRELEASED Bug Fixes entry documents the runtime-type correctness fix, the polymorphic new this(...) behavior, and the detached-call fallback.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• sherifahmed990
• andrewwahid

Poem

🐇 A little factory hopped with delight,
it learned to call forward, not bind to the night.
With generics and this it returns what you seek,
subclass or base, the right type will speak.
Tested and tidy — the rabbit says "peek!"

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: making initializeNewAccount polymorphic so subclasses get their own runtime type, which directly addresses issue #116.
Description check	✅ Passed	The description covers the issue reference, test validation approach, and the detached-call fallback behavior. All critical aspects are explained despite not following the template structure.
Linked Issues check	✅ Passed	The pull request fully addresses issue #116 by making initializeNewAccount polymorphic using generic static factories with InstanceType return types and new this(...) construction, which was the proposed solution (#1 from the issue).
Out of Scope Changes check	✅ Passed	All changes are narrowly scoped to fixing the runtime type issue in initializeNewAccount for SafeAccountV0_3_0 and SafeAccountV1_5_0_M_0_3_0 with comprehensive test coverage, with no unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8274c1c8f8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Handle unbound factory calls

When JavaScript consumers cache or destructure this public factory (for example, const init = SafeAccountV0_3_0.initializeNewAccount; init([owner])), the static this value is undefined, so new this(...) now throws TypeError: this is not a constructor. The previous hard-coded constructor path worked in that scenario, so this is a runtime breaking change for existing JS/CommonJS callers that use the factory as a callback or extracted function.

Useful? React with 👍 / 👎.

[sherifahmed990]

LGTM