-
Notifications
You must be signed in to change notification settings - Fork 76
fix(billing): only check for deployment allowance tx when create #1804
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe deployment allowance assertion is now executed only when a message of type "/akash.deployment.v1beta3.MsgCreateDeployment" is present. For non-deployment messages, deployment allowance validation is skipped. Other validations, including fee allowance checks, remain unchanged. No public signatures were modified. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant ManagedSigner as ManagedSignerService
participant MsgInspector as Message Inspector
participant Allowance as AllowanceValidator
participant Chain as Akash Chain
Client->>ManagedSigner: sign(messages, userWallet)
ManagedSigner->>MsgInspector: contains MsgCreateDeployment?
MsgInspector-->>ManagedSigner: yes/no
alt contains deployment message
ManagedSigner->>Allowance: assert deploymentAllowance > 0
else
Note over ManagedSigner: Skip deployment allowance check
end
ManagedSigner->>Allowance: assert feeAllowance >= required
Allowance-->>ManagedSigner: validations pass/fail
alt validations pass
ManagedSigner->>Chain: submit signed tx
Chain-->>ManagedSigner: tx result
else
ManagedSigner-->>Client: throw validation error
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
apps/api/src/billing/services/managed-signer/managed-signer.service.ts (1)
104-106: Avoid usinganyin catch; preferunknownThis violates the repo guideline for TypeScript. Change to
unknownto maintain type safety without breaking usage.- } catch (error: any) { + } catch (error: unknown) { throw await this.chainErrorService.toAppError(error, messages); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/src/billing/services/managed-signer/managed-signer.service.ts(1 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/services/managed-signer/managed-signer.service.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/services/managed-signer/managed-signer.service.ts
🧬 Code Graph Analysis (1)
apps/api/src/billing/services/managed-signer/managed-signer.service.ts (1)
apps/indexer/drizzle/schema.ts (1)
message(521-567)
⏰ 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 (1)
apps/api/src/billing/services/managed-signer/managed-signer.service.ts (1)
74-76: Correctly scope deployment allowance check to MsgCreateDeploymentThis aligns with the PR objective and prevents blocking non-deployment messages unnecessarily.
| if (messages.some(message => message.typeUrl === "/akash.deployment.v1beta3.MsgCreateDeployment")) { | ||
| assert(userWallet.deploymentAllowance > 0, 403, "UserWallet has no deployment allowance"); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Use a shared constant or type guard for the typeUrl to avoid typos and future version drift
Hardcoding the string inline risks typos and makes future protocol bumps harder. Consider centralizing:
- Define a constant (or import one if available) for "/akash.deployment.v1beta3.MsgCreateDeployment".
- Optionally, support multiple versions if both can appear during migrations.
Example:
// e.g., constants/akash.ts
export const MSG_CREATE_DEPLOYMENT = "/akash.deployment.v1beta3.MsgCreateDeployment";
// usage
if (messages.some(m => m.typeUrl === MSG_CREATE_DEPLOYMENT)) { ... }To confirm whether other versions of the CreateDeployment msg appear in this repo (e.g., v1beta2) and whether a shared constant already exists, run:
🏁 Script executed:
#!/bin/bash
# Find occurrences of CreateDeployment messages and potential shared constants
rg -n --no-ignore-case $'/akash\\.deployment\\..*\\.(MsgCreateDeployment)' -A 2 -B 2
rg -n --no-ignore-case $'MsgCreateDeployment' -A 2 -B 2
rg -n --no-ignore-case $'typeUrl' -A 1 -B 1 | rg -n 'CreateDeployment'Length of output: 1609
🏁 Script executed:
#!/bin/bash
# Search for any MsgCreateDeployment typeUrl usages (all versions)
rg -n -e "/akash\.deployment\.[^/]*\.MsgCreateDeployment" -t ts
# Search for any existing constant definitions referencing CreateDeployment
rg -n -e "CREATE_DEPLOYMENT" -t tsLength of output: 7659
Centralize MsgCreateDeployment typeUrl into a shared constant
To avoid hard-coding the same string in multiple places and ease future version bumps, introduce a single constant (and optional type-guard) for MsgCreateDeployment and replace all inline usages in the API:
• Define in a new file, e.g. apps/api/src/constants/akash.ts:
export const MSG_CREATE_DEPLOYMENT = "/akash.deployment.v1beta3.MsgCreateDeployment";
// Optional: support older versions if needed
export const MSG_CREATE_DEPLOYMENT_VERSIONS = [
"/akash.deployment.v1beta1.MsgCreateDeployment",
"/akash.deployment.v1beta2.MsgCreateDeployment",
MSG_CREATE_DEPLOYMENT,
];
export function isMsgCreateDeployment(typeUrl: string): boolean {
return MSG_CREATE_DEPLOYMENT_VERSIONS.includes(typeUrl);
}• Replace inline checks like:
if (messages.some(m => m.typeUrl === "/akash.deployment.v1beta3.MsgCreateDeployment")) { … }with:
import { isMsgCreateDeployment } from "constants/akash";
if (messages.some(m => isMsgCreateDeployment(m.typeUrl))) { … }Locations to update in apps/api:
- billing/services/managed-signer/managed-signer.service.ts (line 74)
- billing/services/trial-validation/trial-validation.service.ts (lines 19, 29)
- billing/services/chain-error/chain-error.service.ts (line 52)
- billing/services/rpc-message-service/rpc-message.service.ts (line 168)
- billing/http-schemas/tx.schema.ts (line 10)
🤖 Prompt for AI Agents
In apps/api/src/billing/services/managed-signer/managed-signer.service.ts around
lines 74-76, the MsgCreateDeployment typeUrl is hard-coded; create a new
constant file at apps/api/src/constants/akash.ts exporting MSG_CREATE_DEPLOYMENT
(and optional MSG_CREATE_DEPLOYMENT_VERSIONS and isMsgCreateDeployment helper
that checks inclusion), then replace the inline comparison with messages.some(m
=> isMsgCreateDeployment(m.typeUrl)) and update the other listed files
(trial-validation.service.ts lines 19 & 29, chain-error.service.ts line 52,
rpc-message.service.ts line 168, and billing/http-schemas/tx.schema.ts line 10)
to import and use the constant/helper instead of the literal string.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1804 +/- ##
==========================================
- Coverage 42.48% 42.20% -0.29%
==========================================
Files 933 928 -5
Lines 26292 26116 -176
Branches 7000 6987 -13
==========================================
- Hits 11169 11021 -148
+ Misses 14769 14741 -28
Partials 354 354
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
Summary by CodeRabbit