Skip to content

refactor(billing): cleans up top up master wallets#2246

Merged
ygrishajev merged 1 commit intomainfrom
feature/billing-master-wallets
Nov 18, 2025
Merged

refactor(billing): cleans up top up master wallets#2246
ygrishajev merged 1 commit intomainfrom
feature/billing-master-wallets

Conversation

@ygrishajev
Copy link
Contributor

@ygrishajev ygrishajev commented Nov 18, 2025

Summary by CodeRabbit

  • Refactor
    • Consolidated wallet configuration to use a unified managed wallet approach for all balance top-ups.

@ygrishajev ygrishajev requested a review from a team as a code owner November 18, 2025 12:27
@ygrishajev ygrishajev force-pushed the feature/billing-master-wallets branch from 60031fd to af059b2 Compare November 18, 2025 12:28
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 18, 2025

Walkthrough

This PR removes UAKT and USDC top-up master wallet support throughout the billing and deployment services, including environment variables, provider registrations, wallet type definitions, and related services. The system transitions to a managed-wallet-only architecture for handling deployments.

Changes

Cohort / File(s) Summary
Environment Configuration
apps/api/env/.env.functional.test, apps/api/env/.env.local.sample, apps/api/env/.env.unit.test
Removed UAKT_TOP_UP_MASTER_WALLET_MNEMONIC and USDC_TOP_UP_MASTER_WALLET_MNEMONIC environment variable definitions across test and sample configuration files.
Billing Configuration
apps/api/src/billing/config/env.config.ts
Removed schema validation entries for UAKT_TOP_UP_MASTER_WALLET_MNEMONIC and USDC_TOP_UP_MASTER_WALLET_MNEMONIC from envSchema.
Billing Providers
apps/api/src/billing/providers/signing-client.provider.ts, apps/api/src/billing/providers/wallet.provider.ts
Removed exported constants/tokens and container registrations for UAKT_TOP_UP_MASTER_SIGNING_CLIENT, USDC_TOP_UP_MASTER_SIGNING_CLIENT, UAKT_TOP_UP_MASTER_WALLET, and USDC_TOP_UP_MASTER_WALLET.
Type Definitions
apps/api/src/billing/types/wallet.type.ts
Removed TopUpMasterWalletType type; simplified MasterWalletType from "MANAGED" | TopUpMasterWalletType to exclusively "MANAGED".
Top-up Services
apps/api/src/deployment/services/top-up-custodial-balance/top-up-custodial-balance.service.ts, apps/api/src/deployment/services/top-up-tools/top-up-tools.service.ts
Removed entire TopUpCustodialBalanceService class and Balances interface; removed entire TopUpToolsService class including wallet/client mappings and helper methods.
Top-up Managed Deployments
apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.ts, apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.ts
Removed injected managedMasterWallet dependency and associated @InjectWallet decorator from service constructor; updated tests accordingly.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Removed public API surfaces: Multiple exported tokens, types, and classes across billing and deployment layers require verification that all dependent imports and usages have been addressed.
  • Service deletions: Full removal of TopUpToolsService and TopUpCustodialBalanceService should confirm no orphaned references exist elsewhere in the codebase.
  • Type simplification: MasterWalletType narrowing from union type to literal may impact type-checking in consumer code; verify downstream usages align.
  • Multi-layer refactoring: Changes span configuration, dependency injection, type system, and business logic across different domains, requiring careful cross-layer verification.

Possibly related PRs

  • akash-network/console#2151: Modifies signing-client.provider.ts UAKT/USDC top-up master signing client registrations and factories.
  • akash-network/console#2215: Converts master wallet constants into InjectionToken exports; affects same wallet provider API surface being removed here.
  • akash-network/console#1488: Modifies TopUpCustodialBalanceService.recordTx logic and types in the same service being deleted.

Suggested reviewers

  • baktun14
  • stalniy

Poem

🐰 Top-up wallets fade away,
UAKT, USDC see their day,
Managed masters stand alone,
Simpler code, a cleaner throne! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'refactor(billing): cleans up top up master wallets' clearly and concisely describes the main change: removal of top-up master wallet infrastructure from the billing module.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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/billing-master-wallets

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
apps/api/src/billing/providers/signing-client.provider.ts (1)

6-24: Managed‑only wiring looks consistent; InjectSigningClient is now effectively constant

The import cleanup to only MANAGED_MASTER_WALLET and the remaining MANAGED_MASTER_SIGNING_CLIENT registration are consistent with dropping the top‑up master wallets; DI wiring here still looks sound.

Given MasterWalletType is now just "MANAGED", InjectSigningClient effectively always resolves the same token. That’s fine and keeps the API future‑proof, but if you don’t anticipate re‑adding more master wallet types, you could simplify this to a plain inject(MANAGED_MASTER_SIGNING_CLIENT) and adjust call sites in a follow‑up to reduce indirection. Not blocking for this PR.

apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.ts (1)

166-189: “Master wallet” naming may now be misleading after moving to managed wallets

The log event MASTER_WALLET_INSUFFICIENT_FUNDS and chainErrorService.isMasterWalletInsufficientFundsError still refer to a “master wallet”, while this PR’s goal is to move to a managed‑wallet‑only model. Functionally this likely still works (error classification by content), but the naming can confuse future readers about where funds actually come from.

I’d leave behavior as‑is for this PR and track a separate follow‑up to rename these to something like MANAGED_WALLET_INSUFFICIENT_FUNDS / isManagedWalletInsufficientFundsError (and update any shared enums/metrics) so terminology matches the new architecture. Based on learnings.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a750da4 and af059b2.

📒 Files selected for processing (11)
  • apps/api/env/.env.functional.test (0 hunks)
  • apps/api/env/.env.local.sample (0 hunks)
  • apps/api/env/.env.unit.test (0 hunks)
  • apps/api/src/billing/config/env.config.ts (0 hunks)
  • apps/api/src/billing/providers/signing-client.provider.ts (1 hunks)
  • apps/api/src/billing/providers/wallet.provider.ts (0 hunks)
  • apps/api/src/billing/types/wallet.type.ts (1 hunks)
  • apps/api/src/deployment/services/top-up-custodial-balance/top-up-custodial-balance.service.ts (0 hunks)
  • apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.ts (0 hunks)
  • apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.ts (1 hunks)
  • apps/api/src/deployment/services/top-up-tools/top-up-tools.service.ts (0 hunks)
💤 Files with no reviewable changes (8)
  • apps/api/src/billing/providers/wallet.provider.ts
  • apps/api/src/deployment/services/top-up-custodial-balance/top-up-custodial-balance.service.ts
  • apps/api/env/.env.functional.test
  • apps/api/src/deployment/services/top-up-tools/top-up-tools.service.ts
  • apps/api/src/billing/config/env.config.ts
  • apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.ts
  • apps/api/env/.env.local.sample
  • apps/api/env/.env.unit.test
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: baktun14
Repo: akash-network/console PR: 1725
File: apps/api/src/utils/constants.ts:5-5
Timestamp: 2025-07-24T17:00:52.361Z
Learning: In the Akash Network Console project, when cross-cutting concerns or broader refactoring issues are identified during PR review, the preferred approach is to create a separate GitHub issue to track the work rather than expanding the scope of the current PR. This maintains focus and allows for proper planning of architectural improvements.
⏰ 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). (3)
  • GitHub Check: validate / validate-app
  • GitHub Check: test-build
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
apps/api/src/billing/types/wallet.type.ts (1)

1-1: Type‑level narrowing to managed‑only is aligned with the refactor

Restricting MasterWalletType to "MANAGED" matches the removal of top‑up master wallets and gives you nice compile‑time guarantees that no other master wallet types are used anymore. Looks good and consistent with the DI/provider changes.

apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.ts (1)

1-8: Barrel import for billing services looks good; just watch for module cycles

Switching to @src/billing/services for DepositDeploymentMsgOptions and RpcMessageService helps centralize billing exports and keeps this service clean. Please just double‑check that the billing services barrel doesn’t import anything back from @src/deployment/..., to avoid introducing a subtle circular dependency between billing and deployment modules.

@ygrishajev ygrishajev merged commit 2e24537 into main Nov 18, 2025
55 of 59 checks passed
@ygrishajev ygrishajev deleted the feature/billing-master-wallets branch November 18, 2025 13:18
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