Skip to content

feat: refactor service classes - #14

Merged
digitalSloth merged 4 commits into
mainfrom
feature/refactor-service-classes
Jun 19, 2026
Merged

feat: refactor service classes#14
digitalSloth merged 4 commits into
mainfrom
feature/refactor-service-classes

Conversation

@digitalSloth

Copy link
Copy Markdown
Owner

Uniform service-layer architecture: ChainService base + Tier-B convention

Summary

Makes the src/core/ service layer architecturally uniform before we add unit tests. Previously the services shared a singleton pattern but diverged in constructor shape; the planned test-friendly refactor would have left them in three different shapes. This collapses that inconsistency into a documented two-tier convention.

  • Tier A — chain services: the 7 blockchain services share one base (ChainService) and one DI seam (an injected ZenonService).
  • Tier B — everything else: services with genuinely different dependencies follow one documented rule — public constructor injecting its own deps, defaulted to the production wiring; the singleton/module-const is just the prod wrapper.

Net result: −43 lines (56 insertions / 99 deletions), no behavior change, and every service is now constructable with fakes for the upcoming test suite.

Changes

1. Add ChainService abstract base
New src/core/chain-service.tsprotected zenon, a public constructor injecting ZenonService (defaulted to the prod singleton), and ensureInitialized(). The ctor is public so subclasses stay test-constructable; abstract is what prevents direct instantiation.

2. Migrate the 7 network services onto it
account / transaction / stake / plasma / pillar / rewards / token now extends ChainService, dropping their duplicated zenon/zenonService fields, private constructor, and ensureInitialized(). Each keeps its own static instance + getInstance() (deliberately not hoisted, to avoid a TS strict static-typing pitfall). No RPC method body changed.

3. WalletService Tier-B ctor + document the convention

  • wallet-service.ts: public defaulted-DI ctor (constructor(private storage: StorageAdapter = storageService)); getInstance() wiring unchanged.
  • Comment-only docs of the two-tier rule in chain-service.ts (canonical home) plus one-line pointers on session-manager.ts and storage-service.ts.

Behavior preservation

Fully behavior-preserving. Each service's getInstance() still returns the same singleton wired to the same ZenonService/storageService; the defaulted ctor params reproduce the previous explicit wiring exactly. No composable, background.ts, main.ts, or src/core/index.ts was touched — the ~16 getInstance() call sites are unchanged.

Out of scope

ZenonService (the injected dependency itself), PlasmaBotService (already self-documents its exemption), SessionManager/StorageService, and the storage adapters are intentionally left as-is under the Tier-B convention. No tests are added here — that's the follow-up.

Verification

  • npm run typecheck — 0 errors
  • npm run lint — 0 errors (6 pre-existing warnings)
  • npm run build and npm run build:extension — both pass

Phase 1 of the service-architecture uniformity refactor. Adds the abstract
ChainService base (protected zenon + injected ZenonService, public ctor) that
the 7 network services will extend in Phase 2. No consumers yet.
Phase 2 of the service-architecture uniformity refactor. The 7 network
services (account/transaction/stake/plasma/pillar/rewards/token) now extend
ChainService, dropping their duplicated zenon/zenonService fields, private
constructor, and ensureInitialized() in favour of the shared base. Singleton
getInstance() machinery and all RPC bodies are unchanged; behaviour preserved.
Phase 3+4 of the service-architecture uniformity refactor.
- wallet-service.ts: public defaulted-DI ctor (drop private, default storage to
storageService); getInstance() wiring and behaviour unchanged.
- chain-service.ts / session-manager.ts / storage-service.ts: document the
two-tier convention (Tier A = ChainService base; Tier B = public defaulted-DI
ctor, module-const is the prod wrapper). Comments only, no behaviour change.
@0x3639

Copy link
Copy Markdown
Collaborator

Findings

  • [P2] src/core/chain-service.ts:25 still types the injected dependency as concrete ZenonService, so plain test fakes are rejected by TypeScript. ZenonService has private members and a private constructor, making it nominally typed. A virtual test module with new AccountService(fakeZenonService) fails with: “missing properties from type 'ZenonService': zenon, isInitialized, initializePromise…” Use a small interface or Pick<ZenonService, 'getZenon' | 'ensureInitialized'> for the constructor type.

Checks

  • Reviewed [PR #14](feat: refactor service classes #14) at head e52bcd43617d85910b2bbccb0c61093f479f97af.
  • npm run typecheck: passed.
  • eslint . under bundled Node 24: passed with 6 existing warnings.
  • Web and extension production builds: passed.

No production behavior regression jumped out; the main issue is that the refactor does not fully deliver the claimed fake-injectable service layer yet.

@0x3639

Copy link
Copy Markdown
Collaborator

Looks good to go. No additional comments!

@0x3639
0x3639 self-requested a review June 17, 2026 23:47
@digitalSloth
digitalSloth merged commit e1e33bb into mainJun 19, 2026
0x3639 added a commit to 0x3639/nom-webwallet that referenced this pull request Jun 20, 2026
@0x36390x3639 mentioned this pull request Jun 20, 2026
@edgepillar

Copy link
Copy Markdown

Hi — I’m considering a small tests-only follow-up to #14 focused on SessionManager.

Proposed scope:

  • add Vitest and replace the placeholder npm test script;
  • add deterministic unit tests for unlock and retrieval, selective and global locking, timeout eviction, and filtering expired sessions;
  • update the existing documentation that states no automated tests are configured.

The tests would use opaque fake keystore objects. There would be no production-code changes, mnemonic or wallet fixtures, browser-storage tests, or CI workflow changes.

I did not find an open issue, pull request, or public branch implementing this scope. Is anyone already working on the test-suite follow-up mentioned in #14? If not, would this be a useful first PR?

@digitalSloth

Copy link
Copy Markdown
OwnerAuthor

@edgepillar Yes please, that would be great if you want to start working on the unit tests for the repo

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@digitalSloth@0x3639@edgepillar