Skip to content

fix(billing): makes master wallets to be singletons#2151

Merged
ygrishajev merged 1 commit intomainfrom
feature/di
Nov 4, 2025
Merged

fix(billing): makes master wallets to be singletons#2151
ygrishajev merged 1 commit intomainfrom
feature/di

Conversation

@ygrishajev
Copy link
Contributor

@ygrishajev ygrishajev commented Nov 4, 2025

Summary by CodeRabbit

  • Refactor
    • Optimized internal service registration and caching mechanisms to improve resource efficiency and container performance.

@ygrishajev ygrishajev requested a review from a team as a code owner November 4, 2025 10:52
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 4, 2025

Walkthrough

The change introduces per-container caching for three BatchSigningClientService registrations in the DI container by wrapping existing factory functions with instancePerContainerCachingFactory. The underlying construction logic and dependency injection remain unchanged.

Changes

Cohort / File(s) Summary
DI Container Caching Enhancement
apps/api/src/billing/providers/signing-client.provider.ts
Added instancePerContainerCachingFactory import from tsyringe and wrapped all three service registrations (MANAGED_MASTER_SIGNING_CLIENT, UAKT_TOP_UP_MASTER_SIGNING_CLIENT, USDC_TOP_UP_MASTER_SIGNING_MASTER_SIGNING_CLIENT) with the caching factory to enable per-container instance reuse.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify that instancePerContainerCachingFactory is correctly applied to all three service registrations
  • Confirm that the inner factory function logic and dependency injection parameters are preserved without modification
  • Validate that the caching behavior aligns with the intended service lifecycle management

Possibly related PRs

Poem

🐰 Three signing clients now cached so fine,
Per container they elegantly align,
No factory duplication, just one clever way,
Efficiency hops stronger every day!

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: implementing singleton pattern for master wallets using caching factory.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/di

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 23d3c1e and 850cf8a.

📒 Files selected for processing (1)
  • apps/api/src/billing/providers/signing-client.provider.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

Never use type any or cast to type any. Always define the proper TypeScript types.

Files:

  • apps/api/src/billing/providers/signing-client.provider.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

**/*.{js,ts,tsx}: Never use deprecated methods from libraries.
Don't add unnecessary comments to the code

Files:

  • apps/api/src/billing/providers/signing-client.provider.ts
🧠 Learnings (1)
📚 Learning: 2025-08-21T04:03:23.490Z
Learnt from: stalniy
Repo: akash-network/console PR: 1827
File: apps/api/src/console.ts:146-154
Timestamp: 2025-08-21T04:03:23.490Z
Learning: In the Akash Network console project, services like JobQueueService implement dispose() methods for resource cleanup, and the tsyringe container has been extended with a dispose() method that iterates over all registered instances and calls their dispose() methods, enabling proper shutdown orchestration.

Applied to files:

  • apps/api/src/billing/providers/signing-client.provider.ts
🧬 Code graph analysis (1)
apps/api/src/billing/providers/signing-client.provider.ts (4)
apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts (1)
  • BatchSigningClientService (62-367)
apps/api/src/billing/providers/wallet.provider.ts (3)
  • MANAGED_MASTER_WALLET (9-9)
  • UAKT_TOP_UP_MASTER_WALLET (12-12)
  • USDC_TOP_UP_MASTER_WALLET (15-15)
apps/api/src/billing/providers/type-registry.provider.ts (1)
  • TYPE_REGISTRY (34-34)
apps/api/src/billing/lib/sync-signing-stargate-client/sync-signing-stargate-client.ts (1)
  • SyncSigningStargateClient (20-42)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
🔇 Additional comments (2)
apps/api/src/billing/providers/signing-client.provider.ts (2)

1-1: LGTM! Import added for caching functionality.

The addition of instancePerContainerCachingFactory is necessary for the singleton pattern implementation below.


12-54: The singleton pattern implementation is correct and test-compatible.

The codebase uses instancePerContainerCachingFactory from tsyringe, which caches one instance per container lifecycle. This is the appropriate pattern for BatchSigningClientService because:

  1. Stateful components require singleton: The service maintains DataLoader batching, semaphore serialization, and memoized wallet/chain data—all requiring a single instance per wallet to function correctly.

  2. Test isolation is strong: Tests create fresh service instances directly with mocked dependencies, bypassing the factory entirely. Each test gets fresh mocks configured in the setup() helper, ensuring no cross-test state contamination.

  3. Production design is sound: Each container gets one cached instance per token (MANAGED_MASTER_SIGNING_CLIENT, UAKT_TOP_UP_MASTER_SIGNING_CLIENT, USDC_TOP_UP_MASTER_SIGNING_CLIENT), preventing sequence number conflicts for each wallet.


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

@codecov
Copy link

codecov bot commented Nov 4, 2025

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.48%. Comparing base (23d3c1e) to head (850cf8a).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...i/src/billing/providers/signing-client.provider.ts 50.00% 2 Missing ⚠️

❌ Your patch status has failed because the patch coverage (50.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2151      +/-   ##
==========================================
- Coverage   46.83%   46.48%   -0.36%     
==========================================
  Files        1015     1005      -10     
  Lines       28734    28385     -349     
  Branches     7456     7397      -59     
==========================================
- Hits        13458    13195     -263     
+ Misses      14090    14013      -77     
+ Partials     1186     1177       -9     
Flag Coverage Δ *Carryforward flag
api 82.07% <50.00%> (ø)
deploy-web 25.25% <ø> (ø) Carriedforward from 23d3c1e
log-collector ?
notifications 88.11% <ø> (ø) Carriedforward from 23d3c1e
provider-console 81.48% <ø> (ø) Carriedforward from 23d3c1e
provider-proxy 85.28% <ø> (ø) Carriedforward from 23d3c1e

*This pull request uses carry forward flags. Click here to find out more.

Files with missing lines Coverage Δ
...i/src/billing/providers/signing-client.provider.ts 88.23% <50.00%> (ø)

... and 10 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ygrishajev ygrishajev merged commit 8e0e7bb into main Nov 4, 2025
62 checks passed
@ygrishajev ygrishajev deleted the feature/di branch November 4, 2025 11:08
Sign up for free to 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.

2 participants

Comments