Conversation
WalkthroughRefactors type registry construction to combine Cosmos default and IBC GeneratedTypes with Akash types, exposes the registry via a DI provider, switches several Coin type imports to the chain-sdk private types, and updates tests to use deployment owners as top-up signers. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant DI as DI Container
participant RegistryBuilder as TypeRegistry Builder
participant Consumer as Service/Module
rect rgba(60,160,120,0.06)
Note over RegistryBuilder: Build defaultRegistryTypes\n(from Cosmos v1/v1beta1/v1alpha1/v2alpha1)\n+ ibcTypes (from stargate defaults)
RegistryBuilder->>RegistryBuilder: normalize "$type" -> "/<type>"
RegistryBuilder->>RegistryBuilder: collect akashTypes
RegistryBuilder->>RegistryBuilder: combined = [...defaultRegistryTypes, ...akashTypes]
end
RegistryBuilder->>DI: register TYPE_REGISTRY (combined)
App->>DI: InjectTypeRegistry()
DI->>Consumer: provide TYPE_REGISTRY
Consumer->>TYPE_REGISTRY: use GeneratedTypes for protobuf encoding/decoding
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2132 +/- ##
==========================================
- Coverage 46.45% 46.10% -0.35%
==========================================
Files 1016 1006 -10
Lines 28709 28370 -339
Branches 7463 7425 -38
==========================================
- Hits 13336 13081 -255
+ Misses 14969 14894 -75
+ Partials 404 395 -9
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/deploy-web/src/types/balances.ts (1)
1-1: Same private-types import path concern.This file has the same
private-typesimport path usage asapps/deploy-web/src/utils/priceUtils.ts. Please ensure this is the intended and stable import path across the codebase.
🧹 Nitpick comments (1)
apps/indexer/src/shared/utils/protobuf.ts (1)
15-20: Consider extracting duplicated registry construction logic.The logic for defining
DefaultRegistryType, extractingibcTypes, and constructingdefaultRegistryTypesis duplicated across multiple files:
apps/api/src/utils/protobuf.tsapps/api/src/billing/providers/type-registry.provider.tsapps/deploy-web/src/utils/customRegistry.tsConsider extracting this shared logic into a common utility module to reduce duplication and ensure consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/api/src/billing/providers/type-registry.provider.ts(1 hunks)apps/api/src/utils/protobuf.ts(1 hunks)apps/deploy-web/src/types/balances.ts(1 hunks)apps/deploy-web/src/utils/customRegistry.ts(1 hunks)apps/deploy-web/src/utils/priceUtils.ts(1 hunks)apps/indexer/src/shared/utils/protobuf.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/deploy-web/src/types/balances.tsapps/api/src/billing/providers/type-registry.provider.tsapps/api/src/utils/protobuf.tsapps/deploy-web/src/utils/priceUtils.tsapps/deploy-web/src/utils/customRegistry.tsapps/indexer/src/shared/utils/protobuf.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/deploy-web/src/types/balances.tsapps/api/src/billing/providers/type-registry.provider.tsapps/api/src/utils/protobuf.tsapps/deploy-web/src/utils/priceUtils.tsapps/deploy-web/src/utils/customRegistry.tsapps/indexer/src/shared/utils/protobuf.ts
🧬 Code graph analysis (1)
apps/api/src/billing/providers/type-registry.provider.ts (1)
apps/deploy-web/src/utils/customRegistry.ts (1)
registry(28-28)
⏰ 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). (5)
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
🔇 Additional comments (4)
apps/deploy-web/src/utils/customRegistry.ts (1)
12-28: Registry construction looks correct.The implementation properly:
- Extracts IBC types from stargate defaults
- Includes null checks in the filter (
.filter(x => x && "$type" in x))- Includes
cosmosv2alpha1types- Combines default registry types with Akash-specific types
This is consistent with the intended changes.
apps/api/src/billing/providers/type-registry.provider.ts (1)
34-37: Good addition of dependency injection support.Exposing the type registry through a DI provider (
TYPE_REGISTRYtoken andInjectTypeRegistry()) is a good practice that improves testability and allows for easier mocking in unit tests.apps/api/src/utils/protobuf.ts (1)
34-46: LGTM - proper error handling for type lookup.The
decodeMsgfunction correctly:
- Creates a registry with all required types
- Validates that the type exists before decoding
- Ensures the type is ts-proto generated
- Returns the decoded message
apps/deploy-web/src/utils/priceUtils.ts (1)
1-1: No changes needed — this is the correct import path.The
private-typesimport from@akashnetwork/chain-sdkis the intended and supported way to access types from the SDK. Analysis shows 74+ files across the codebase use this same import pattern, confirming it's the standard approach. The package is currently in alpha (1.0.0-alpha.12) under active development, which means breaking changes are possible in any release—but this is a project-wide infrastructure constraint, not specific to this file. No code modifications are required.
stalniy
force-pushed
the
fix/adds-ibc-types
branch
from
afdaf8f to
d6442b5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/api/src/utils/protobuf.ts (1)
16-21: Missingcosmosv2alpha1types - critical inconsistency with other registries.The
defaultRegistryTypesconstruction is missingcosmosv2alpha1types, while other registries in this PR include them (seeapps/deploy-web/src/utils/customRegistry.tsline 18 andapps/api/src/billing/providers/type-registry.provider.tsline 22). This inconsistency will cause cosmosv2alpha1 messages to fail decoding in this context.Add the missing import and include the types:
+import * as cosmosv2alpha1 from "@akashnetwork/chain-sdk/private-types/cosmos.v2alpha1"; type DefaultRegistryType = [string, GeneratedType]; const ibcTypes: DefaultRegistryType[] = stargateDefaultRegistryTypes.filter(([type]) => type.startsWith("/ibc")); -const defaultRegistryTypes: DefaultRegistryType[] = [...Object.values(cosmosv1), ...Object.values(cosmosv1beta1), ...Object.values(cosmosv1alpha1)] +const defaultRegistryTypes: DefaultRegistryType[] = [ + ...Object.values(cosmosv1), + ...Object.values(cosmosv1beta1), + ...Object.values(cosmosv1alpha1), + ...Object.values(cosmosv2alpha1) +] .filter(x => x && "$type" in x) .map(x => ["/" + x.$type, x as unknown as GeneratedType]) .concat(ibcTypes) as DefaultRegistryType[];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/api/src/billing/providers/type-registry.provider.ts(1 hunks)apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.ts(4 hunks)apps/api/src/utils/protobuf.ts(1 hunks)apps/deploy-web/src/types/balances.ts(1 hunks)apps/deploy-web/src/utils/customRegistry.ts(1 hunks)apps/deploy-web/src/utils/priceUtils.ts(1 hunks)apps/indexer/src/shared/utils/protobuf.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/deploy-web/src/utils/priceUtils.ts
- apps/indexer/src/shared/utils/protobuf.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()to mock dependencies in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under test.
**/*.spec.{ts,tsx}: Usesetupfunction instead ofbeforeEachin test files
setupfunction must be at the bottom of the rootdescribeblock in test files
setupfunction creates an object under test and returns it
setupfunction should accept a single parameter with inline type definition
Don't use shared state insetupfunction
Don't specify return type ofsetupfunction
Files:
apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.ts
**/*.{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/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.tsapps/api/src/billing/providers/type-registry.provider.tsapps/deploy-web/src/utils/customRegistry.tsapps/deploy-web/src/types/balances.tsapps/api/src/utils/protobuf.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/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.tsapps/api/src/billing/providers/type-registry.provider.tsapps/deploy-web/src/utils/customRegistry.tsapps/deploy-web/src/types/balances.tsapps/api/src/utils/protobuf.ts
🧬 Code graph analysis (2)
apps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.ts (1)
apps/indexer/drizzle/schema.ts (1)
deployment(185-208)
apps/api/src/billing/providers/type-registry.provider.ts (1)
apps/deploy-web/src/utils/customRegistry.ts (1)
registry(28-28)
⏰ 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). (5)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
🔇 Additional comments (5)
apps/deploy-web/src/types/balances.ts (1)
1-1: LGTM! Import path updated correctly.The import path change to use the chain-sdk private types is appropriate and maintains the type-only import.
apps/deploy-web/src/utils/customRegistry.ts (1)
10-28: LGTM! Registry construction correctly includes IBC and Cosmos types.The implementation properly:
- Extracts IBC types from stargate default registry
- Includes all Cosmos type versions (v1, v1beta1, v1alpha1, v2alpha1)
- Combines default registry types with Akash types
- Maintains correct type transformations with
$typeprefixingapps/api/src/billing/providers/type-registry.provider.ts (1)
12-37: Registry construction looks good with IBC types integration.The implementation correctly:
- Filters IBC types from stargate defaults
- Includes all required Cosmos versions (v1, v1beta1, v1alpha1, v2alpha1)
- Combines default and Akash types in the registry
- Exposes the registry via DI with
InjectTypeRegistryapps/api/src/deployment/services/top-up-managed-deployments/top-up-managed-deployments.service.spec.ts (2)
119-119: LGTM! Tests correctly updated to use deployment owner as signer.The test changes reflect the proper ownership model where deployment owners sign their own top-up transactions instead of using a constant master wallet address.
Also applies to: 154-154
287-287: LGTM! Same-owner deployments correctly use owner as signer.The test properly validates that multiple deployments with the same owner are batched in a single transaction with the owner as the signer.
Also applies to: 304-304
| const newAkashTypes: ReadonlyArray<[string, GeneratedType]> = [...Object.values(v1), ...Object.values(v1beta4), ...Object.values(v1beta5)] | ||
| .filter(x => "$type" in x) | ||
| .map(x => ["/" + x.$type, x as unknown as GeneratedType]); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify null safety in newAkashTypes filter.
Line 29 filters with .filter(x => "$type" in x) without checking if x is truthy first, unlike the defaultRegistryTypes filter on line 24 which includes x &&. If any of the imported values are null or undefined, the "$type" in x check could fail.
🏁 Script executed:
#!/bin/bash
# Check if v1, v1beta4, or v1beta5 exports contain null/undefined values
ast-grep --pattern $'export const $_ = null'
ast-grep --pattern $'export const $_ = undefined'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Locate and examine the file
find . -type f -name "type-registry.provider.ts" -exec cat {} \;Length of output: 1897
🏁 Script executed:
#!/bin/bash
# Search for imports of v1, v1beta4, v1beta5 in the file
rg -n "import.*\bv1\b" apps/api/src/billing/providers/type-registry.provider.ts -A 2 -B 2Length of output: 529
🏁 Script executed:
#!/bin/bash
# Check the structure: find what v1beta4 and v1beta5 modules export (look for their files)
find . -type f \( -name "*v1beta4*" -o -name "*v1beta5*" \) | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for other usages of Object.values on these imported modules to see if null checks are needed elsewhere
rg -n "Object.values\(" apps/api/src/billing/providers/type-registry.provider.ts -B 2 -A 2Length of output: 657
🏁 Script executed:
#!/bin/bash
# Check if there are other files that import from the same chain-sdk modules
rg -n "@akashnetwork/chain-sdk/private-types" --type=ts --type=tsx -g '!node_modules' | head -20Length of output: 27
🏁 Script executed:
#!/bin/bash
# Look for patterns where Object.values is used on these modules elsewhere in the codebase
rg -n "Object\.values\(\s*(v1|v1beta4|v1beta5|cosmos)" --type=ts --type=tsx -g '!node_modules' -A 2 -B 2Length of output: 27
Add null safety check to newAkashTypes filter to match defaultRegistryTypes pattern.
Line 29 should include x && before checking "$type" in x, matching the defensive check on line 24. Currently, if Object.values() returns null or undefined, the in operator will throw a TypeError. Both filters process values from external SDK modules using the same pattern and should apply the same safety checks.
const newAkashTypes: ReadonlyArray<[string, GeneratedType]> = [...Object.values(v1), ...Object.values(v1beta4), ...Object.values(v1beta5)]
.filter(x => x && "$type" in x)
.map(x => ["/" + x.$type, x as unknown as GeneratedType]);🤖 Prompt for AI Agents
In apps/api/src/billing/providers/type-registry.provider.ts around lines 28 to
30, the filter on newAkashTypes uses `"${'$'}type" in x` without guarding for
null/undefined which can throw if Object.values yields nullish entries; update
the filter to check x truthiness first (e.g., `x && "$type" in x`) before
accessing $type so it matches the defensive pattern used by
defaultRegistryTypes, then leave the subsequent map unchanged.
1 participant
Why
Because we have ibc types inside indexer and would like to show stats. Fixes #2131
Summary by CodeRabbit
New Features
Refactor
Tests