Feat: core proto ts 1.33.6 #543
Conversation
- @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected]
- @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected]
- @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected]
- @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected] - @injectivelabs/[email protected]
WalkthroughThis pull request updates a dependency version and introduces extensive changes across the SDK. New asynchronous API methods have been added and existing ones have been renamed to better reflect their functionality. Transformer classes have been refactored to centralize coin conversion logic, and many tests have been adjusted—from proto/amino formats to EIP712‐based validations. In the permissions and governance modules, message structures and test cases have been substantially reworked, including removals of legacy files and updates to namespace and role management. Finally, the proto generation script has been modified to reference updated branch names. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant A as ChainGrpc API
participant G as gRPC Server
participant T as Transformer (Common)
C->>A: Call API method (e.g., fetchDenomDecimals)
A->>G: Construct and send gRPC Query Request
G-->>A: Return raw response
A->>T: Transform response (coin conversion, mapping)
T-->>A: Return mapped data
A-->>C: Return processed result
sequenceDiagram
participant U as User
participant M as Msg Module (e.g., MsgSubmitProposalSpotMarketLaunch)
participant A as Amino Converter
participant E as EIP712 Generator
participant I as IndexerGrpcWeb3GwApi
U->>M: Prepare message parameters (with decimals, market data)
M->>A: Convert message to Amino format
A-->>M: Return Amino data
M->>E: Generate EIP712 typed data using Amino output
E->>I: Submit EIP712 request
I-->>E: Return typed data response
E-->>M: Return final EIP712 message
M-->>U: Provide message ready for Web3 signing
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
packages/sdk-ts/src/client/chain/grpc/ChainGrpcAuctionApi.spec.tsOops! Something went wrong! :( ESLint: 7.32.0 ESLint couldn't find the config "./../../eslintrc.js" to extend from. Please check that the name of the config is correct. The config "./../../eslintrc.js" was referenced from the config file in "/packages/sdk-ts/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. packages/sdk-ts/src/client/chain/grpc/ChainGrpcBankApi.tsOops! Something went wrong! :( ESLint: 7.32.0 ESLint couldn't find the config "./../../eslintrc.js" to extend from. Please check that the name of the config is correct. The config "./../../eslintrc.js" was referenced from the config file in "/packages/sdk-ts/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. packages/sdk-ts/src/client/chain/grpc/ChainGrpcAuctionApi.tsOops! Something went wrong! :( ESLint: 7.32.0 ESLint couldn't find the config "./../../eslintrc.js" to extend from. Please check that the name of the config is correct. The config "./../../eslintrc.js" was referenced from the config file in "/packages/sdk-ts/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (15)
packages/sdk-ts/src/core/modules/exchange/msgs/MsgInstantSpotMarketLaunch.spec.ts (1)
33-33: Address the TODO comment.The TODO comment should be addressed or removed if no additional implementation is needed.
Would you like me to help implement any missing test cases or remove this TODO if it's no longer needed?
packages/sdk-ts/src/core/modules/exchange/msgs/MsgInstantSpotMarketLaunch.ts (1)
38-39: Consider adding validation for decimal places.While the
Number()conversion is good for type safety, consider adding validation to ensure the decimals are non-negative integers.- message.baseDecimals = Number(params.market.baseDecimals) - message.quoteDecimals = Number(params.market.quoteDecimals) + const baseDecimals = Number(params.market.baseDecimals) + const quoteDecimals = Number(params.market.quoteDecimals) + if (!Number.isInteger(baseDecimals) || baseDecimals < 0) { + throw new Error('baseDecimals must be a non-negative integer') + } + if (!Number.isInteger(quoteDecimals) || quoteDecimals < 0) { + throw new Error('quoteDecimals must be a non-negative integer') + } + message.baseDecimals = baseDecimals + message.quoteDecimals = quoteDecimalspackages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateActorRoles.ts (1)
17-19: Enhance class documentation.The class documentation could be more descriptive about the purpose and usage of this message type.
/** * @category Messages + * @description Message class for updating actor roles in the permissions module. + * @param {Object} params - The parameters for the message. + * @param {string} params.sender - The address of the sender. + * @param {string} params.denom - The denomination. + * @param {PermissionRoleActors[]} params.roleActorsToAdd - The roles and actors to add. + * @param {PermissionRoleActors[]} params.roleActorsToRevoke - The roles and actors to revoke. */packages/sdk-ts/src/client/indexer/transformers/IndexerCommonTransformer.ts (1)
4-11: Consider refactoring the class to a standalone function.The class contains only a single static method. For better maintainability and to align with functional programming principles, consider refactoring it to a standalone function.
-export class IndexerCommonTransformer { - static grpcCoinToCoin(coin: GrpcCoin): Coin { - return { - denom: coin.denom, - amount: coin.amount, - } - } -} +export const grpcCoinToCoin = (coin: GrpcCoin): Coin => ({ + denom: coin.denom, + amount: coin.amount, +})🧰 Tools
🪛 Biome (1.9.4)
[error] 4-11: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
packages/sdk-ts/src/client/chain/grpc/ChainGrpcAuctionApi.spec.ts (1)
66-83: Enhance test robustness.The test case could be improved in the following ways:
- Add explicit failure on error instead of just logging
- Add specific property checks for the response
Consider this improved implementation:
test('fetchLastAuctionResult', async () => { try { const response = await chainGrpcAuctionApi.fetchLastAuctionResult() expect(response).toBeDefined() + // Add specific property checks + expect(response).toHaveProperty('winner') + expect(response).toHaveProperty('amount') + expect(response).toHaveProperty('round') expect(response).toEqual( expect.objectContaining< ReturnType< typeof ChainGrpcAuctionTransformer.LastAuctionResultResponseToLastAuctionResult > >(response), ) } catch (e) { console.error( 'ChainGrpcAuctionApi.fetchLastAuctionResult => ' + (e as any).message, ) + throw e // Re-throw to fail the test } })packages/sdk-ts/src/client/chain/transformers/ChainGrpcBankTransformer.ts (1)
54-56: LGTM! Consider adding type assertions for map operations.The centralization of coin transformation logic improves maintainability. For better type safety, consider adding explicit type assertions to the map operations.
- supply: balances.map(ChainGrpcCommonTransformer.grpcCoinToCoin), + supply: balances.map((coin): Coin => ChainGrpcCommonTransformer.grpcCoinToCoin(coin)), - balances: balances.map(ChainGrpcCommonTransformer.grpcCoinToCoin), + balances: balances.map((coin): Coin => ChainGrpcCommonTransformer.grpcCoinToCoin(coin)),Also applies to: 90-92
packages/sdk-ts/src/client/chain/transformers/ChainGrpcAuctionTransformer.ts (1)
59-61: Consider adding type assertion for map operation.For better type safety, consider adding an explicit type assertion to the map operation.
- amountList: response.amount.map( - ChainGrpcCommonTransformer.grpcCoinToCoin, - ), + amountList: response.amount.map((coin): Coin => + ChainGrpcCommonTransformer.grpcCoinToCoin(coin), + ),packages/sdk-ts/src/client/indexer/transformers/IndexerAccountPortfolioTransformer.ts (1)
36-38: Consider adding type assertions for map operations.For better type safety and consistency with other transformers, consider adding explicit type assertions to the map operations.
- bankBalancesList: bankBalancesList.map( - IndexerCommonTransformer.grpcCoinToCoin, - ), + bankBalancesList: bankBalancesList.map((coin): Coin => + IndexerCommonTransformer.grpcCoinToCoin(coin), + ),Apply the same change to the second map operation at lines 66-68.
Also applies to: 66-68
packages/sdk-ts/src/client/chain/transformers/ChainGrpcPermissionsTransformer.ts (2)
143-151: Rename local variable for clarity.Calling each item
roleinsideactorsByRoleResponseToActorsByRolecould be confusing, as it represents the actor addresses rather than roles. Consider renaming the variable.
153-161: Confirm consistency of data shape.
rolesByActorResponseToRolesByActortransforms each string into{ roles: role }. Verify whether the property name “roles” is correct when mapping each single item.packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi.ts (1)
26-51: Consider reducing code duplication in try/catch blocks.All methods in this class follow a similar try/catch pattern. Extracting reusable logic could improve maintainability.
packages/sdk-ts/src/client/chain/types/permissions.ts (1)
93-115: Multiple type exports (e.g.,GrpcPermissionNamespace,GrpcPermissionsNamespace) appear inconsistent.Check whether both names are needed or if there’s an accidental duplication.
packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi.spec.ts (3)
13-29: Enhance error handling in test cases.The error handling across all test cases could be more robust. Consider:
- Adding specific error type assertions
- Testing error scenarios explicitly
- Using proper test failure mechanisms instead of console.error
Here's an example of improved error handling for one test case:
test('fetchModuleParams', async () => { try { const response = await chainGrpcPermissionsApi.fetchModuleParams() expect(response).toBeDefined() expect(response).toEqual( expect.objectContaining< ReturnType< typeof ChainGrpcPermissionsTransformer.moduleParamsResponseToModuleParams > >(response), ) } catch (e) { - console.error( - 'chainGrpcPermissionsApi.fetchModuleParams => ' + (e as any).message, - ) + expect(e).toBeInstanceOf(Error) + throw e } })Also applies to: 47-51, 70-74, 89-93, 111-115, 133-137, 155-159, 178-182, 201-205, 225-230, 248-252, 271-275, 290-294
35-37: Standardize empty array handling.The empty array checks are inconsistent across test cases. Some methods log warnings while others don't handle empty arrays at all.
Consider standardizing the empty array handling:
- Use a shared utility function
- Add assertions for minimum array length where applicable
- Document expected array sizes in test descriptions
+const warnIfEmpty = (response: any[], methodName: string) => { + if (response.length === 0) { + console.warn(`${methodName}.arrayIsEmpty`) + } +} test('fetchNamespaces', async () => { try { const response = await chainGrpcPermissionsApi.fetchNamespaces() - if (response.length === 0) { - console.warn('fetchNamespaces.arrayIsEmpty') - } + warnIfEmpty(response, 'fetchNamespaces') // ... rest of the test } })Also applies to: 58-60, 166-168, 189-191, 213-215, 259-261
31-52: Add test descriptions and edge cases.The test cases lack descriptions and edge case coverage. Consider:
- Adding descriptive test names with expected behavior
- Including edge cases for each API method
- Testing with various input parameters
Example of improved test structure:
describe('fetchNamespaceDenoms', () => { it('should return all namespace denoms', async () => { // existing happy path test }) it('should handle invalid responses', async () => { // test error case }) it('should handle empty responses', async () => { // test empty response case }) })Also applies to: 77-94, 140-160, 185-206, 233-253, 255-276
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (43)
packages/sdk-ts/package.json(1 hunks)packages/sdk-ts/src/client/chain/grpc/ChainGrpcAuctionApi.spec.ts(1 hunks)packages/sdk-ts/src/client/chain/grpc/ChainGrpcAuctionApi.ts(2 hunks)packages/sdk-ts/src/client/chain/grpc/ChainGrpcBankApi.ts(3 hunks)packages/sdk-ts/src/client/chain/grpc/ChainGrpcExchangeApi.spec.ts(2 hunks)packages/sdk-ts/src/client/chain/grpc/ChainGrpcExchangeApi.ts(2 hunks)packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi.spec.ts(2 hunks)packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi.ts(1 hunks)packages/sdk-ts/src/client/chain/transformers/ChainGrpcAuctionTransformer.ts(4 hunks)packages/sdk-ts/src/client/chain/transformers/ChainGrpcBankTransformer.ts(4 hunks)packages/sdk-ts/src/client/chain/transformers/ChainGrpcCommonTransformer.ts(1 hunks)packages/sdk-ts/src/client/chain/transformers/ChainGrpcExchangeTransformer.ts(2 hunks)packages/sdk-ts/src/client/chain/transformers/ChainGrpcPermissionsTransformer.ts(2 hunks)packages/sdk-ts/src/client/chain/types/auction.ts(2 hunks)packages/sdk-ts/src/client/chain/types/exchange.ts(2 hunks)packages/sdk-ts/src/client/chain/types/permissions.ts(1 hunks)packages/sdk-ts/src/client/indexer/transformers/IndexerAccountPortfolioTransformer.ts(4 hunks)packages/sdk-ts/src/client/indexer/transformers/IndexerCampaignTransformer.ts(3 hunks)packages/sdk-ts/src/client/indexer/transformers/IndexerCommonTransformer.ts(1 hunks)packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcAuctionTransformer.ts(2 hunks)packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcMitoTransformer.ts(9 hunks)packages/sdk-ts/src/core/modules/exchange/msgs/MsgInstantSpotMarketLaunch.spec.ts(1 hunks)packages/sdk-ts/src/core/modules/exchange/msgs/MsgInstantSpotMarketLaunch.ts(3 hunks)packages/sdk-ts/src/core/modules/gov/msgs/MsgSubmitProposalPerpetualMarketLaunch.spec.ts(2 hunks)packages/sdk-ts/src/core/modules/gov/msgs/MsgSubmitProposalPerpetualMarketLaunch.ts(1 hunks)packages/sdk-ts/src/core/modules/gov/msgs/MsgSubmitProposalSpotMarketLaunch.spec.ts(1 hunks)packages/sdk-ts/src/core/modules/gov/msgs/MsgSubmitProposalSpotMarketLaunch.ts(3 hunks)packages/sdk-ts/src/core/modules/permissions/index.ts(1 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgClaimVoucher.spec.ts(1 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgCreateNamespace.spec.ts(1 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgCreateNamespace.ts(2 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgDeleteNamespace.spec.ts(0 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgDeleteNamespace.ts(0 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgRevokeNamespaceRoles.spec.ts(0 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgRevokeNamespaceRoles.ts(0 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateActorRoles.spec.ts(1 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateActorRoles.ts(1 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespace.spec.ts(1 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespace.ts(3 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespaceRoles.spec.ts(0 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespaceRoles.ts(0 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateParams.spec.ts(2 hunks)proto/core/gen.sh(1 hunks)
💤 Files with no reviewable changes (6)
- packages/sdk-ts/src/core/modules/permissions/msgs/MsgDeleteNamespace.spec.ts
- packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespaceRoles.spec.ts
- packages/sdk-ts/src/core/modules/permissions/msgs/MsgRevokeNamespaceRoles.spec.ts
- packages/sdk-ts/src/core/modules/permissions/msgs/MsgDeleteNamespace.ts
- packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespaceRoles.ts
- packages/sdk-ts/src/core/modules/permissions/msgs/MsgRevokeNamespaceRoles.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/sdk-ts/src/client/indexer/transformers/IndexerCommonTransformer.ts
[error] 4-11: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
packages/sdk-ts/src/client/chain/transformers/ChainGrpcCommonTransformer.ts
[error] 4-11: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateActorRoles.spec.ts
[error] 5-9: Do not export from a test file.
(lint/suspicious/noExportsInTest)
packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateActorRoles.ts
[error] 20-20: Shouldn't redeclare 'MsgUpdateActorRoles'. Consider to delete it or rename it.
'MsgUpdateActorRoles' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (124)
packages/sdk-ts/src/client/chain/types/exchange.ts (2)
123-131: LGTM! Well-structured interfaces for denomination handling.The new interfaces are well-designed, using string types for numerical values to handle large numbers with precision. The naming and structure are consistent with the codebase conventions.
160-162: LGTM! Consistent type mapping to gRPC types.The new type exports maintain consistency with the existing pattern of mapping chain types to gRPC types from core-proto-ts.
packages/sdk-ts/src/core/modules/exchange/msgs/MsgInstantSpotMarketLaunch.spec.ts (3)
3-4: LGTM! Import statements are properly updated.The import statements correctly reflect the shift to EIP712 functionality and follow proper module resolution with .js extensions.
11-11: LGTM! Market parameters are properly extended.The addition of
minNotionaland decimal properties from base/quote tokens maintains proper type safety and aligns with the market interface requirements.Also applies to: 16-17
26-54: LGTM! Well-structured EIP712 test implementation.The test suite effectively validates both EIP712 v1 and v2 implementations against the Web3Gw responses. The structure is clean and follows testing best practices.
packages/sdk-ts/src/core/modules/exchange/msgs/MsgInstantSpotMarketLaunch.ts (2)
18-19: LGTM! Well-structured interface extension.The addition of
baseDecimalsandquoteDecimalsproperties is well-typed and properly placed within the market configuration.
106-107: LGTM! Consistent Amino encoding implementation.The new decimal properties are correctly included in the Amino encoding, maintaining consistency with the existing pattern.
packages/sdk-ts/src/core/modules/permissions/index.ts (3)
2-2: LGTM!The new imports are correctly structured and follow the established naming conventions.
Also applies to: 5-5
9-9: LGTM!The exports are consistent with the imports and maintain alphabetical ordering.
Also applies to: 12-12
2-2:✅ Verification successful
Verify the existence of the new message files.
Let's ensure that the newly imported message files exist in the correct location.
Also applies to: 5-5
🏁 Script executed:
#!/bin/bash # Description: Check if the new message files exist fd "MsgUpdateParams.ts|MsgUpdateActorRoles.ts" --type fLength of output: 199
Files Verified: Imported Message Files Exist
- The files
packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateParams.tsandpackages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateActorRoles.tswere confirmed to be present.- The import statements in
packages/sdk-ts/src/core/modules/permissions/index.ts(lines 2 and 5) correctly reference these existing files.packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespace.spec.ts (11)
1-2: Imports look good.
No issues found with these import statements.
4-5: Additional client and EIP712 imports are correctly introduced.
These imports support the new EIP712 tests; looks consistent.
9-10: Parameter renaming is coherent.
UsingdenomandcontractHookprovides clear context for the message fields.
11-11: Roles and permissions structure is straightforward.
Defining{ name: 'admin', roleId: 1 }is explicit for referencing roles.
12-17:roleManagersblock is well-defined.
Structuring managers as an array of objects is flexible for future expansion.
18-24: Policy statuses appear consistent with proto definitions.
The use ofInjectivePermissionsV1Beta1Permissions.Action.SENDand disabling flags is properly structured.
25-32:policyManagerCapabilitiesaddition is in line with updated permissions model.
This helps manage who can seal/disable actions. No immediate issues.
38-38: Descriptive test suite name.
describe('MsgUpdateNamespace')is concise and clear.
39-43: EIP712 setup for tests is properly handled.
Leveraging the helper ensures consistent message preparation.
45-56: EIP712 v1 test coverage is good.
Checking the typed data against chain responses increases reliability.
58-66: EIP712 v2 test coverage mirrors v1.
Ensures proper compliance across both versions.packages/sdk-ts/src/core/modules/permissions/msgs/MsgCreateNamespace.spec.ts (11)
1-2: Imports align with test requirements.
The usage ofmockFactory,prepareEip712, and proto references is appropriate.
4-5: Client and EIP712 utilities imported correctly.
Matches the approach used in other EIP712-based tests.
11-11:contractHookproperty name is consistent with the new approach.
Renaming fromwasmHooktocontractHookclarifies usage.
12-12: Role permissions follow updated structure.
Using{ name, roleId, permissions }is explicit and versioned well.
13-13:actorRolesrename introduces clarity.
It better represents any actor beyond simply “addresses.”
14-19:roleManagersarray is flexible.
Managing multiple role definitions is a strong design choice.
20-26:policyStatusesintegrates well with new action-based restrictions.
Clear approach to toggling SEND or other actions.
27-33:policyManagerCapabilitiesfosters granular control.
Allowing or disallowingcanDisableandcanSealis a robust addition.
41-46: Preparation of EIP712 arguments remains consistent with other specs.
Reusing theprepareEip712utility ensures standardization.
48-56: EIP712 v1 test coverage is complete.
Comparing local typed data to chain’s Web3Gw response is a strong validation method.
58-69: EIP712 v2 test coverage parallels v1.
Maintains consistency across versions.packages/sdk-ts/src/core/modules/permissions/msgs/MsgClaimVoucher.spec.ts (6)
1-1: Imports fromtest-utilsare correctly utilized.
Provides streamlined mocking and EIP712 test setup.
3-3:IndexerGrpcWeb3GwApiimport is well-positioned.
Used for validating chain-supplied typed data.
4-4: EIP712-based utility imports are consistent.
Matches usage patterns from other specs.
14-19: Test suite structure follows the established pattern.
UsingprepareEip712to set up arguments for uniform testing.
21-32: EIP712 v1 test ensures correctness.
Comparing typed data JSON strictly withtxResponsefosters reliability.
34-42: EIP712 v2 test coverage is similarly robust.
Verifying parity with the chain’s typed data affirms schema consistency.packages/sdk-ts/src/core/modules/gov/msgs/MsgSubmitProposalSpotMarketLaunch.spec.ts (2)
6-6: LGTM! Improved test maintainability.The changes enhance maintainability by:
- Centralizing market data using
mockFactory.injUsdtSpotMarket- Using dynamic values instead of hardcoded ones
- Adding decimal precision parameters
Also applies to: 10-18
40-51: Address the skipped test.The test for EIP712 v1 is skipped with a TODO comment. Consider either implementing the test or documenting the reason for skipping.
Do you want me to help implement the test or document the reason for skipping?
packages/sdk-ts/src/core/modules/gov/msgs/MsgSubmitProposalPerpetualMarketLaunch.spec.ts (2)
6-6: LGTM! Consistent test improvements.The changes follow the same pattern as the spot market tests, improving maintainability by using dynamic values from
mockFactory.injUsdtDerivativeMarket.Also applies to: 12-21
45-57: Address the skipped test and TODO comment.The test for EIP712 v1 is skipped with a TODO comment. Consider either implementing the test or documenting the reason for skipping.
Do you want me to help implement the test or document the reason for skipping?
packages/sdk-ts/src/core/modules/gov/msgs/MsgSubmitProposalSpotMarketLaunch.ts (2)
27-28: LGTM! Added decimal precision parameters.The addition of
baseDecimalsandquoteDecimalsimproves the market configuration capabilities.
178-188: LGTM! Improved code maintainability.The refactored
toWeb3method reduces code duplication by reusing thetoAminooutput, making the code more maintainable.packages/sdk-ts/src/core/modules/gov/msgs/MsgSubmitProposalPerpetualMarketLaunch.ts (1)
208-218: LGTM! Consistent code improvements.The refactored
toWeb3method follows the same pattern as the spot market message class, reducing code duplication by reusing thetoAminooutput.packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateParams.spec.ts (2)
16-21: LGTM! Test setup is well-structured.The test setup using
prepareEip712utility with proper sequence and account number parameters is correct.
23-34: LGTM! Comprehensive EIP712 version testing.The tests properly validate both EIP712 v1 and v2 typed data generation against the chain's response.
Also applies to: 36-44
packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespace.ts (5)
2-12: Consolidated imports well-structured.
These new import statements cleanly separate proto and type definitions, ensuring the code is more maintainable. No major issues noted.
17-22: New fields align with enhanced permission model.
Introducingdenom,contractHook, and the new permission structures (rolePermissions,roleManagers,policyStatuses, andpolicyManagerCapabilities) appears consistent with the updated requirements. The optionalcontractHookis appropriately marked with a “?”.
44-49: All fields correctly assigned intoProto.
Fields onmessagematch the newParamsinterface. Ensure the proto definitions also reflect these fields.Would you like to verify proto field definitions for consistency? I can generate a script to cross-check if needed.
50-51: Conditional assignment forcontractHooklooks good.
The code properly checks forparams.contractHookfirst, preventing unwanted fields from populating the proto.
85-89: Snake-casing final message is consistent with existing patterns.
The usage ofsnakecaseKeyssuits the expected format for amino messages. No issues here.packages/sdk-ts/src/core/modules/permissions/msgs/MsgCreateNamespace.ts (6)
6-6: Imports look correct.No concerns or recommended changes here; the usage of
MsgBaseappears consistent with the rest of your code.
20-25: Ensure adequate test coverage for new properties.You introduced several new fields such as
contractHook,rolePermissions, etc. Verify that they're properly tested in the codebase to confirm correct instantiation and serialization logic.
65-73: Validate policy status enum values.Your mapping of
policyStatusesto JSON is correct. However, consider handling unexpected or invalid enum values more gracefully to avoid potential runtime issues.
75-83: Handle unexpected actions in policyManagerCapabilities.Likewise, for
policyManagerCapabilities, consider verifying if all enum values are expected or if fallback logic is needed for any new or invalid actions.
84-91: Usage ofsnakecaseKeysis consistent.Your final transformation logic for
namespaceproperties looks good and follows the established pattern.
7-13:✅ Verification successful
Verify the local import path for type definitions.
Please confirm that
'./../../../../client/chain/types'accurately points to your type definitions, ensuring that the build system can resolve it correctly.
🏁 Script executed:
#!/bin/bash # Demonstrate searching for 'client/chain/types' in the repository fd 'types' --search-path ./Length of output: 1390
Verified Correct Import Path
The import statement in
packages/sdk-ts/src/core/modules/permissions/msgs/MsgCreateNamespace.tsusing'./../../../../client/chain/types'correctly resolves to the directorypackages/sdk-ts/src/client/chain/types, as confirmed by our repository search.packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcAuctionTransformer.ts (1)
48-50: LGTM!Good job centralizing the coin conversion logic by using
IndexerCommonTransformer.grpcCoinToCoin.packages/sdk-ts/src/client/chain/types/auction.ts (2)
4-8: LGTM!The type definitions are well-structured and properly aligned with the proto definitions. The separation of concerns between different auction entities is clear and maintainable.
Also applies to: 33-37, 39-45, 64-70, 72-80
10-13:✅ Verification successful
Verify handling of optional amounts.
The interfaces
AuctionBid,AuctionLastAuctionResult,AuctionEventBid, andAuctionEventAuctionResulthave optionalamountfields. Ensure that consumers of these interfaces properly handle cases where the amount is undefined.Also applies to: 15-19, 21-25, 27-31
🏁 Script executed:
#!/bin/bash # Search for usages of these interfaces to verify proper null checks rg -A 5 "AuctionBid|AuctionLastAuctionResult|AuctionEventBid|AuctionEventAuctionResult"Length of output: 6240
Optional Amount Fields: Verified Handling
The verification confirms that the transformer functions (e.g., in
ChainGrpcAuctionTransformer.ts) properly check ifamountis defined before conversion, ensuring consumers safely handle undefined values. Please ensure that any new consumers of theAuctionBid,AuctionLastAuctionResult,AuctionEventBid, andAuctionEventAuctionResultinterfaces follow the same null-safety practices.
- In
ChainGrpcAuctionTransformer.ts, the conversion functions include guard clauses foramount.- Existing null checks in the codebase demonstrate consistent and proper handling of optional
amountfields.packages/sdk-ts/src/client/chain/grpc/ChainGrpcAuctionApi.ts (2)
56-84: LGTM!The new
fetchCurrentBasketmethod follows the established patterns for request creation, response transformation, and error handling.
116-144: LGTM!The function has been correctly renamed and updated to handle last auction results instead of current basket.
packages/sdk-ts/src/client/indexer/transformers/IndexerCampaignTransformer.ts (2)
83-86: LGTM!Good refactoring to use the centralized coin transformation from
IndexerCommonTransformer.
186-186: LGTM!Consistent use of the centralized coin transformation from
IndexerCommonTransformer.packages/sdk-ts/src/client/chain/grpc/ChainGrpcBankApi.ts (1)
184-184: LGTM!Good refactoring to use the centralized coin transformation from
ChainGrpcCommonTransformer.packages/sdk-ts/src/client/chain/transformers/ChainGrpcExchangeTransformer.ts (3)
283-287: LGTM!Simple and straightforward transformation of the minimum notional amount.
289-296: LGTM!Clean mapping of denomination decimals with proper type transformation.
298-307:✅ Verification successful
Consider adding validation for the division operation.
The method correctly transforms minimum notionals but should handle potential division by zero or invalid decimal values.
🏁 Script executed:
#!/bin/bash # Check for potential zero decimals in the codebase rg -A 2 "decimals.*0" --type tsLength of output: 363
No additional validation required for the division operation.
After reviewing the function and verifying the surrounding code, the division uses a hardcoded constant (10**18) as the divisor, which is guaranteed not to be zero. There is no evidence in the codebase (or from the executed search) of any invalid or zero values affecting the division. If the need arises from upstream changes or future dynamic inputs, input validation can be reconsidered, but for now, the current implementation is safe.packages/sdk-ts/src/client/chain/grpc/ChainGrpcExchangeApi.ts (4)
365-393: LGTM!The fetchDenomDecimal method follows the established pattern with proper error handling.
395-423: LGTM!The fetchDenomDecimals method maintains consistency with the codebase's patterns.
425-453: LGTM!The fetchDenomMinNotional method implements proper error handling and follows the class pattern.
455-483: LGTM!The fetchDenomMinNotionals method correctly implements the established patterns.
packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcMitoTransformer.ts (1)
191-191: Great refactoring to centralize coin transformation logic!The changes improve maintainability by using the centralized
IndexerCommonTransformer.grpcCoinToCoinmethod across all coin transformations.Also applies to: 215-215, 218-218, 221-221, 392-392, 395-397, 400-402, 405-407, 418-418, 436-436, 439-439, 443-445, 469-471
packages/sdk-ts/package.json (2)
125-125: Verify the version number in PR title.The PR title mentions version 1.33.6, but the actual change updates to version 1.13.6. Please verify if this is a typo in the PR title.
124-134:❓ Verification inconclusive
Verify compatibility with other proto packages.
The core-proto-ts package is being updated to version 1.13.6. Please ensure compatibility with related proto packages that are on different patch versions:
- @injectivelabs/abacus-proto-ts: 1.13.3
- @injectivelabs/mito-proto-ts: 1.13.2
- @injectivelabs/olp-proto-ts: 1.13.3
Note that indexer-proto-ts is already at 1.13.6.
Run the following script to check for any breaking changes or compatibility issues:
🏁 Script executed:
#!/bin/bash # Description: Check for breaking changes in the proto packages # Check the changelog or release notes for core-proto-ts gh release view v1.13.6 --repo injectivelabs/core-proto-ts # Check for any issues mentioning compatibility between these versions gh issue list --repo injectivelabs/core-proto-ts --search "in:title,body 1.13.6 compatibility"Length of output: 359
ACTION: Manual Compatibility Verification Required for Proto Packages
The update to
@injectivelabs/core-proto-ts(v1.13.6) still needs to be manually verified against its related proto packages. The automated check executed via the GitHub CLI did not return the expected output (repository resolution errors were encountered forinjectivelabs/core-proto-ts). As a result, please confirm the following manually:
- Verify changelogs and release notes for
@injectivelabs/core-proto-tsv1.13.6 to ensure that no breaking changes were introduced.- Manually check compatibility between:
@injectivelabs/abacus-proto-ts(v1.13.3)@injectivelabs/mito-proto-ts(v1.13.2)@injectivelabs/olp-proto-ts(v1.13.3)- Note that
@injectivelabs/indexer-proto-tsis already aligned to v1.13.6.Once verified, update the documentation or add tests as necessary to prevent any compatibility issues.
packages/sdk-ts/src/client/chain/transformers/ChainGrpcPermissionsTransformer.ts (17)
2-16: All new imports look good.
18-18: Import ChainGrpcCommonTransformer is properly added.
25-31:grpcRoleToRolemethod implementation looks correct.
33-40:grpcActorRolesToActorRolesmethod is straightforward.
42-49:grpcRoleManagerToRoleManagermethod is concise and seems correct.
51-59:grpcPolicyStatusToPolicyStatuslogic is consistent with the proto fields.
61-70:grpcPolicyManagerCapabilityToPolicyManagerCapabilitycorrectly maps the fields.
72-81:grpcAddressVoucherToAddressVoucherleveragesgrpcCoinToCoinproperly.
83-105:grpcNamespaceToNamespacetransforms nested collections correctly.
117-121:namespaceResponseToNamespacessafely handles empty namespace scenario.Also applies to: 129-133
163-173:roleManagerResponseToRoleManageris straightforward.
175-181:roleManagersResponseToRoleManagersis implemented cleanly.
183-189:policyStatusesResponseToPolicyStatusesis consistent with the approach used in the class.
191-197:policyManagerCapabilitiesResponseToPolicyManagerCapabilitiesmapping looks fine.
199-207:voucherResponseToVoucherproperly handles absence of the voucher.
209-215:vouchersResponseToVouchersconsistently maps address vouchers.
217-235:moduleStateResponseToModuleStatemerges parameters, namespaces, and vouchers coherently.packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi.ts (12)
53-79:fetchNamespaceDenomsis a clear forward GRPC request.
81-107:fetchNamespacesmethod uses the transformer consistently.
109-137:fetchNamespacegracefully handles the denom parameter before performing the request.
139-168:fetchActorsByRolelogic and error handling conform to the existing pattern.
170-199:fetchRolesByActorcoordinates well withrolesByActorResponseToRolesByActor.
201-236:fetchRoleManageris consistent in structure and transformation.
238-264:fetchRoleManagersleverages the newly introduced roleManagers transformer.
266-292:fetchPolicyStatusesappropriately uses the policy statuses transformer.
294-322:fetchPolicyManagerCapabilitiesparam usage and error handling remain consistent.
324-351:fetchVoucherproperly forwards both denom and address in the query request.
353-381:fetchVouchersextends the pattern for bulk voucher retrieval.
383-409:fetchModuleStatecompiles the entire module state in a single call.packages/sdk-ts/src/client/chain/types/permissions.ts (13)
1-5: New imports and proto references are fine.
8-10:PermissionParamsinterface introduced.
12-16:PermissionRoleinterface is concise and clear.
18-21:PermissionActorRolesinterface is straightforward.
23-26:PermissionRoleManagerinterface looks correct.
28-32:PermissionPolicyStatusfields align with the Action enum.
34-39:PermissionPolicyManagerCapabilitycovers manager actions.
41-44:PermissionRoleActorsis a useful structure for grouping actors by role.
46-49:PermissionRoleIDsintroduced (1/2).
55-62:PermissionNamespacesimplifies permissions fields in a single object.
69-69:PermissionAddressRolesinterface for mapping addresses to roles.
78-78:PermissionVoucherinterface is reintroduced for coin arrays.
88-91:PermissionGenesisStateorganizes parameters, namespaces, and vouchers.packages/sdk-ts/src/client/chain/grpc/ChainGrpcAuctionApi.spec.ts (1)
66-83: LGTM! Well-structured test case.The new test case for
fetchLastAuctionResultfollows the established pattern, includes proper error handling, and validates the response structure using TypeScript's type system.packages/sdk-ts/src/client/chain/transformers/ChainGrpcBankTransformer.ts (1)
54-56: LGTM! Good refactoring to centralize coin transformation logic.The changes effectively centralize the coin transformation logic by utilizing
ChainGrpcCommonTransformer, reducing code duplication while maintaining consistent behavior across the codebase.Also applies to: 78-78, 90-92
packages/sdk-ts/src/client/chain/transformers/ChainGrpcAuctionTransformer.ts (3)
21-42: LGTM! Well-implemented transformation methods.The new
grpcBidToBidandgrpcLastAuctionResultToLastAuctionResultmethods properly handle the transformation of auction data with appropriate null checks.
94-104: LGTM! Proper null check implementation.The
LastAuctionResultResponseToLastAuctionResultmethod correctly handles the case wherelastAuctionResultis undefined.
55-67:❓ Verification inconclusive
Verify the impact of return type change.
The return type has been changed from
CurrentBaskettoAuctionCurrentBasket. Please ensure all consumers of this method are updated to handle the new type.
🏁 Script executed:
#!/bin/bash # Search for usages of currentBasketResponseToCurrentBasket rg -A 5 "currentBasketResponseToCurrentBasket"Length of output: 82996
Action: Verify Consumers Update for New Return Type
The implementation of
currentBasketResponseToCurrentBasketnow returns an object of typeAuctionCurrentBasketrather thanCurrentBasket. However, our search output from the TypeDoc documentation still shows the return type asCurrentBasket, which indicates that downstream consumers or the documentation generation may not yet reflect this change.Please ensure that:
- All consumers of
currentBasketResponseToCurrentBaskethave been updated to handle the new object shape defined byAuctionCurrentBasket.- The type definitions and corresponding documentation are regenerated and aligned with the new return type.
- Any tests related to this function are updated to expect fields from
AuctionCurrentBasket.packages/sdk-ts/src/client/indexer/transformers/IndexerAccountPortfolioTransformer.ts (1)
36-38: LGTM! Good refactoring to use common transformer.The changes consistently use
IndexerCommonTransformerfor coin transformations, effectively reducing code duplication and maintaining consistency across the codebase.Also applies to: 66-68
| export interface PermissionRoleActors { | ||
| role: string | ||
| actors: string[] | ||
| } |
There was a problem hiding this comment.
Move interface to a types file.
The PermissionRoleActors interface should not be exported from a test file. Consider moving it to a dedicated types file.
Move the interface to packages/sdk-ts/src/core/modules/permissions/types.ts:
// packages/sdk-ts/src/core/modules/permissions/types.ts
export interface PermissionRoleActors {
role: string
actors: string[]
}Then update the import in this file:
+import { PermissionRoleActors } from '../types'
-export interface PermissionRoleActors {
- role: string
- actors: string[]
-}🧰 Tools
🪛 Biome (1.9.4)
[error] 5-9: Do not export from a test file.
(lint/suspicious/noExportsInTest)
|
|
||
| const message = MsgUpdateActorRoles.fromJSON(params) | ||
|
|
||
| describe('MsgCreateNamespace', () => { |
There was a problem hiding this comment.
Fix incorrect describe block name.
The describe block name doesn't match the message class being tested.
-describe('MsgCreateNamespace', () => {
+describe('MsgUpdateActorRoles', () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe('MsgCreateNamespace', () => { | |
| describe('MsgUpdateActorRoles', () => { |
| static fromJSON(params: MsgUpdateActorRoles.Params): MsgUpdateActorRoles { | ||
| return new MsgUpdateActorRoles(params) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation in fromJSON.
The fromJSON method should validate the input parameters before creating a new instance.
static fromJSON(params: MsgUpdateActorRoles.Params): MsgUpdateActorRoles {
+ if (!params.sender) {
+ throw new Error('sender is required')
+ }
+ if (!params.denom) {
+ throw new Error('denom is required')
+ }
+ if (!Array.isArray(params.roleActorsToAdd) || !Array.isArray(params.roleActorsToRevoke)) {
+ throw new Error('roleActorsToAdd and roleActorsToRevoke must be arrays')
+ }
return new MsgUpdateActorRoles(params)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static fromJSON(params: MsgUpdateActorRoles.Params): MsgUpdateActorRoles { | |
| return new MsgUpdateActorRoles(params) | |
| } | |
| static fromJSON(params: MsgUpdateActorRoles.Params): MsgUpdateActorRoles { | |
| if (!params.sender) { | |
| throw new Error('sender is required') | |
| } | |
| if (!params.denom) { | |
| throw new Error('denom is required') | |
| } | |
| if (!Array.isArray(params.roleActorsToAdd) || !Array.isArray(params.roleActorsToRevoke)) { | |
| throw new Error('roleActorsToAdd and roleActorsToRevoke must be arrays') | |
| } | |
| return new MsgUpdateActorRoles(params) | |
| } |
| const policyManagerCapabilities = | ||
| proto.policyManagerCapabilities.map((policyManagerCapability) => ({ | ||
| ...policyManagerCapability, | ||
| action: InjectivePermissionsV1Beta1Permissions.actionToJSON( | ||
| policyManagerCapability.action, | ||
| ), | ||
| })) || [] | ||
|
|
There was a problem hiding this comment.
Likewise, safeguard against undefined policyManagerCapabilities.
Apply the same optional array fallback to prevent .map() from throwing if omitted in the proto.
Proposed fix:
- const policyManagerCapabilities =
- proto.policyManagerCapabilities.map((policyManagerCapability) => ({
- ...policyManagerCapability,
- action: InjectivePermissionsV1Beta1Permissions.actionToJSON(
- policyManagerCapability.action,
- ),
- })) || []
+ const policyManagerCapabilities = (proto.policyManagerCapabilities || []).map((policyManagerCapability) => ({
+ ...policyManagerCapability,
+ action: InjectivePermissionsV1Beta1Permissions.actionToJSON(
+ policyManagerCapability.action,
+ ),
+ }))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const policyManagerCapabilities = | |
| proto.policyManagerCapabilities.map((policyManagerCapability) => ({ | |
| ...policyManagerCapability, | |
| action: InjectivePermissionsV1Beta1Permissions.actionToJSON( | |
| policyManagerCapability.action, | |
| ), | |
| })) || [] | |
| const policyManagerCapabilities = (proto.policyManagerCapabilities || []).map((policyManagerCapability) => ({ | |
| ...policyManagerCapability, | |
| action: InjectivePermissionsV1Beta1Permissions.actionToJSON( | |
| policyManagerCapability.action, | |
| ), | |
| })) |
|
|
||
| const policyStatuses = | ||
| proto.policyStatuses.map((policyStatus) => ({ | ||
| ...policyStatus, | ||
| action: InjectivePermissionsV1Beta1Permissions.actionToJSON( | ||
| policyStatus.action, | ||
| ), | ||
| })) || [] | ||
|
|
There was a problem hiding this comment.
Fix potential runtime error when policyStatuses is undefined.
Using .map() directly on proto.policyStatuses can throw if the array is missing.
Here's a suggested fix:
- const policyStatuses =
- proto.policyStatuses.map((policyStatus) => ({
- ...policyStatus,
- action: InjectivePermissionsV1Beta1Permissions.actionToJSON(
- policyStatus.action,
- ),
- })) || []
+ const policyStatuses = (proto.policyStatuses || []).map((policyStatus) => ({
+ ...policyStatus,
+ action: InjectivePermissionsV1Beta1Permissions.actionToJSON(
+ policyStatus.action,
+ ),
+ }))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const policyStatuses = | |
| proto.policyStatuses.map((policyStatus) => ({ | |
| ...policyStatus, | |
| action: InjectivePermissionsV1Beta1Permissions.actionToJSON( | |
| policyStatus.action, | |
| ), | |
| })) || [] | |
| const policyStatuses = (proto.policyStatuses || []).map((policyStatus) => ({ | |
| ...policyStatus, | |
| action: InjectivePermissionsV1Beta1Permissions.actionToJSON( | |
| policyStatus.action, | |
| ), | |
| })) |
| injective_core_branch=master | ||
| cosmos_sdk_branch=v0.50.x-inj | ||
| wasmd_branch=v0.50.x-inj | ||
| ibc_go_branch=v8.3.2 | ||
| wasmd_branch=v0.53.x-inj | ||
| ibc_go_branch=v8.3.x-inj |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify compatibility with the updated branch references.
The changes include:
- Moving to master branch for injective-core could introduce instability
- Updates to wasmd and ibc-go versions may affect compatibility
Please ensure:
- The master branch of injective-core is stable
- The new versions of wasmd and ibc-go are compatible with the codebase
🌐 Web query:
What are the latest stable versions of wasmd and ibc-go that are compatible with Injective?
💡 Result:
The latest stable versions of wasmd and ibc-go compatible with Injective are wasmd v0.52 (source) and ibc-go v7 (source), specifically v7 due to Injective’s integration with Cosmos SDK-based tooling optimized for its ecosystem[^9][^10].
Key Details:
- **wasmd `v0
Citations:
- 1: https://blog.injective.com/en/mainnet-chain-upgrade-smart-contracts-are-now-live-on-injective/
- 2: https://interchaininfo.zone/resources/cosmos-news-neutron-injective-paas-by-secret-network-ibc-v-7-1-keplr-2-0-icf-stride-more
- 3: https://docs.terra.money/spec/
- 4: https://docs.injective.network/developers/cosmwasm-developers/your-first-smart-contract
- 5: https://blog.injective.com/en/defi-injective-adds-solana-avalanche-polygon-interoperability-with-wormhole-integration/
- 6: https://github.com/OpenDeFiFoundation/injective-core/blob/master/go.mod
- 7: https://docs.rs/crate/injective-cosmwasm/latest
- 8: https://github.com/cosmos/ibc-go/releases
- 9: https://ibc.cosmos.network/v8/ibc/light-clients/wasm/integration/
- 10: https://docs.injective.network/developers/cosmwasm-developers/guides/local-development
Action Required: Review and update branch references for stability
In proto/core/gen.sh (lines 11–14), the branch references were updated as follows:
- injective_core_branch: Now set to
master(previouslyrelease-prod). Verify that using themasterbranch won’t introduce instability in your production environment. - wasmd_branch: Updated to
v0.53.x-inj, yet the Injective documentation indicates that the stable wasmd version isv0.52. Confirm that this higher version is intentionally used and is fully compatible. - ibc_go_branch: Changed to
v8.3.x-inj, whereas compatibility guidelines recommendibc-go v7. Please ensure this update aligns with the stable release requirements.
Adjust these branches if necessary to match the validated stable versions and avoid potential compatibility issues.
| static namespacesResponseToNamespaces( | ||
| response: InjectivePermissionsV1Beta1Query.QueryNamespacesResponse, | ||
| ) { | ||
| return response.roles.map((role) => { | ||
| return response.namespaces.map((namespace) => { | ||
| ChainGrpcPermissionsTransformer.grpcNamespaceToNamespace(namespace) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Fix missing return inside array mapping.
Currently, the callback in .map() doesn’t return the transformed namespace, so this method will return an array of undefined.
Apply this diff to fix the bug:
static namespacesResponseToNamespaces(
response: InjectivePermissionsV1Beta1Query.QueryNamespacesResponse,
) {
- return response.namespaces.map((namespace) => {
- ChainGrpcPermissionsTransformer.grpcNamespaceToNamespace(namespace)
- })
+ return response.namespaces.map((namespace) => {
+ return ChainGrpcPermissionsTransformer.grpcNamespaceToNamespace(namespace)
+ })
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static namespacesResponseToNamespaces( | |
| response: InjectivePermissionsV1Beta1Query.QueryNamespacesResponse, | |
| ) { | |
| return response.roles.map((role) => { | |
| return response.namespaces.map((namespace) => { | |
| ChainGrpcPermissionsTransformer.grpcNamespaceToNamespace(namespace) | |
| }) | |
| } | |
| static namespacesResponseToNamespaces( | |
| response: InjectivePermissionsV1Beta1Query.QueryNamespacesResponse, | |
| ) { | |
| return response.namespaces.map((namespace) => { | |
| return ChainGrpcPermissionsTransformer.grpcNamespaceToNamespace(namespace) | |
| }) | |
| } |
| export interface PermissionAddressVoucher { | ||
| address: string | ||
| voucher?: Coin | ||
| } |
There was a problem hiding this comment.
Duplicate definition of PermissionAddressVoucher.
This interface reappears on lines 82-85 with the same name and properties, risking type redefinition errors.
Propose removing one of them to avoid duplication.
| } | ||
|
|
||
| export interface RoleIDs { | ||
| export interface PermissionRoleIDs { |
There was a problem hiding this comment.
Duplicate definition of PermissionRoleIDs.
This redefines the same interface from lines 46-49.
Remove or rename one to avoid collisions.
| export interface PermissionAddressVoucher { | ||
| address: string | ||
| voucher?: Voucher | ||
| voucher?: Coin | ||
| } |
There was a problem hiding this comment.
PermissionAddressVoucher repeated definition.
Conflicts with lines 50-53.
Please unify these definitions to prevent confusion.
ThomasRalee
force-pushed
the
feat/core-proto-ts-1.33.6
branch
from
e8870fd to
f3a20a0
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
packages/sdk-ts/src/client/chain/transformers/ChainGrpcAuctionTransformer.ts (1)
55-67:⚠️ Potential issueFix potential type mismatch in highestBidder assignment.
There appears to be a bug where
highestBidAmountis incorrectly assigned tohighestBidder.Apply this diff to fix the type mismatch:
return { amountList: response.amount.map( ChainGrpcCommonTransformer.grpcCoinToCoin, ), auctionRound: parseInt(response.auctionRound, 10), auctionClosingTime: parseInt(response.auctionClosingTime, 10), - highestBidder: response.highestBidAmount, + highestBidder: response.highestBidder, highestBidAmount: response.highestBidAmount, }packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi.spec.ts (1)
11-29: 🛠️ Refactor suggestionAdd error case testing.
The tests only cover the success path. Consider adding tests for error scenarios to ensure proper error handling.
Example implementation for error testing:
test('fetchModuleParams handles GrpcWebError', async () => { const mockError = new InjectivePermissionsV1Beta1Query.GrpcWebError( 'Test error', 500, 'metadata' ) jest.spyOn(chainGrpcPermissionsApi['client'], 'Params') .mockRejectedValueOnce(mockError) await expect(chainGrpcPermissionsApi.fetchModuleParams()) .rejects .toThrow(GrpcUnaryRequestException) })
♻️ Duplicate comments (6)
packages/sdk-ts/src/client/chain/transformers/ChainGrpcCommonTransformer.ts (1)
4-11: 🛠️ Refactor suggestionRefactor static-only class into a standalone function.
The class contains only static members, which is a code smell. Since the functionality is simple and self-contained, it would be better to export it as a standalone function.
Apply this diff to refactor the class into a standalone function:
-export class ChainGrpcCommonTransformer { - static grpcCoinToCoin(coin: GrpcCoin): Coin { - return { - denom: coin.denom, - amount: coin.amount, - } - } -} +export const grpcCoinToCoin = (coin: GrpcCoin): Coin => ({ + denom: coin.denom, + amount: coin.amount, +})🧰 Tools
🪛 Biome (1.9.4)
[error] 4-11: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
packages/sdk-ts/src/client/chain/transformers/ChainGrpcAuctionTransformer.ts (1)
94-104: 🛠️ Refactor suggestionFix method naming convention.
The method name should follow camelCase convention.
Apply this diff to fix the method name:
- static LastAuctionResultResponseToLastAuctionResult( + static lastAuctionResultResponseToLastAuctionResult( response: InjectiveAuctionV1Beta1Query.QueryLastAuctionResultResponse, ) {packages/sdk-ts/src/client/chain/transformers/ChainGrpcPermissionsTransformer.ts (1)
135-141:⚠️ Potential issueFix missing
returninside array mapping.The callback in
.map()doesn't return the transformednamespace, so this method will return an array ofundefined.Apply this diff to fix the bug:
static namespacesResponseToNamespaces( response: InjectivePermissionsV1Beta1Query.QueryNamespacesResponse, ) { - return response.namespaces.map((namespace) => { - ChainGrpcPermissionsTransformer.grpcNamespaceToNamespace(namespace) - }) + return response.namespaces.map((namespace) => { + return ChainGrpcPermissionsTransformer.grpcNamespaceToNamespace(namespace) + }) }packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespace.ts (1)
128-142:⚠️ Potential issueFix array handling in toAmino method.
The same array handling issues from previous reviews are still present.
Apply this diff to fix the issues:
const policyStatuses = - proto.policyStatuses.map((policyStatus) => ({ + (proto.policyStatuses || []).map((policyStatus) => ({ ...policyStatus, action: InjectivePermissionsV1Beta1Permissions.actionToJSON( policyStatus.action, ), - })) || [] + })) const policyManagerCapabilities = - proto.policyManagerCapabilities.map((policyManagerCapability) => ({ + (proto.policyManagerCapabilities || []).map((policyManagerCapability) => ({ ...policyManagerCapability, action: InjectivePermissionsV1Beta1Permissions.actionToJSON( policyManagerCapability.action, ), - })) || [] + }))packages/sdk-ts/src/client/chain/types/permissions.ts (2)
46-49:⚠️ Potential issueRemove duplicate
PermissionRoleIDsinterface.This interface is defined twice in the file. Keep one definition to maintain clean code.
Also applies to: 74-76
50-53:⚠️ Potential issueRemove duplicate
PermissionAddressVoucherinterface.This interface is defined twice in the file. Keep one definition to maintain clean code.
Also applies to: 82-85
🧹 Nitpick comments (17)
packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateParams.ts (1)
34-39: LGTM! Improved type safety in parameter handling.The changes enhance type safety by using the proper proto message structure. Consider adding validation for
wasmHookQueryMaxGasto ensure it's a valid non-negative value.const messageParams = InjectivePermissionsV1Beta1Params.Params.create() +if (BigInt(params.params.wasmHookQueryMaxGas) < 0) { + throw new Error('wasmHookQueryMaxGas must be a non-negative value') +} messageParams.wasmHookQueryMaxGas = params.params.wasmHookQueryMaxGaspackages/sdk-ts/src/client/indexer/transformers/IndexerCommonTransformer.ts (1)
4-11: Consider refactoring the class into a utility function.Since the class only contains a single static method, it would be more idiomatic to export it as a standalone utility function. This aligns with TypeScript best practices and reduces unnecessary class overhead.
-export class IndexerCommonTransformer { - static grpcCoinToCoin(coin: GrpcCoin): Coin { +export function grpcCoinToCoin(coin: GrpcCoin): Coin { return { denom: coin.denom, amount: coin.amount, } - } -}🧰 Tools
🪛 Biome (1.9.4)
[error] 4-11: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
packages/sdk-ts/src/client/chain/types/auction.ts (1)
21-37: Consider using type composition to reduce duplication.
AuctionEventBidandAuctionEventAuctionResultshare similar properties. Consider creating a base type to reduce duplication:interface AuctionEventBase { amount?: Coin round: string } export interface AuctionEventBid extends AuctionEventBase { bidder: string } export interface AuctionEventAuctionResult extends AuctionEventBase { winner: string }packages/sdk-ts/src/client/chain/transformers/ChainGrpcBankTransformer.ts (1)
75-79: Consider adding null check for response.balance.While the implementation is correct, consider adding explicit null checking to handle edge cases more gracefully.
- return ChainGrpcCommonTransformer.grpcCoinToCoin(response.balance!) + if (!response.balance) { + throw new Error('Balance not found in response') + } + return ChainGrpcCommonTransformer.grpcCoinToCoin(response.balance)packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespace.spec.ts (1)
7-33: Consider adding more test cases for edge scenarios.While the current test parameters cover a basic case, consider adding test cases for:
- Empty arrays for rolePermissions, roleManagers
- Multiple roles per manager
- Edge cases for permission values
Would you like me to help generate additional test cases with these scenarios?
packages/sdk-ts/src/core/modules/permissions/msgs/MsgCreateNamespace.ts (1)
2-13: Consider using path aliases for deep imports.The import path
'./../../../../client/chain/types'uses multiple parent directory traversals, which can be error-prone and harder to maintain. Consider using TypeScript path aliases to make imports more maintainable.Example configuration in
tsconfig.json:{ "compilerOptions": { "baseUrl": ".", "paths": { "@client/*": ["src/client/*"], "@core/*": ["src/core/*"] } } }Then update the import:
-import { ... } from './../../../../client/chain/types' +import { ... } from '@client/chain/types'packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateParams.spec.ts (1)
15-46: LGTM! Well-structured EIP712 compatibility tests.The test suite effectively validates EIP712 compatibility with the chain for both v1 and v2 versions.
Consider enhancing test coverage and error handling.
To make the tests more robust:
- Add error handling for API calls
- Include negative test cases (e.g., invalid messages)
- Add validation of EIP712 data structure
Here's a suggested enhancement:
describe('MsgUpdateParams', () => { describe('generates proper EIP712 compared to the Web3Gw (chain)', () => { const { endpoints, eip712Args, prepareEip712Request } = prepareEip712({ sequence: 0, accountNumber: 3, messages: message, }) it('EIP712 v1', async () => { const eip712TypedData = getEip712TypedData(eip712Args) - const txResponse = await new IndexerGrpcWeb3GwApi( - endpoints.indexer, - ).prepareEip712Request({ - ...prepareEip712Request, - eip712Version: 'v1', - }) + try { + const txResponse = await new IndexerGrpcWeb3GwApi( + endpoints.indexer, + ).prepareEip712Request({ + ...prepareEip712Request, + eip712Version: 'v1', + }) + + // Validate EIP712 structure + expect(eip712TypedData).toHaveProperty('types') + expect(eip712TypedData).toHaveProperty('primaryType') + expect(eip712TypedData).toHaveProperty('domain') + expect(eip712TypedData).toHaveProperty('message') + + expect(eip712TypedData).toStrictEqual(JSON.parse(txResponse.data)) + } catch (error) { + fail(`API call failed: ${error.message}`) + } }) + it('handles invalid messages', async () => { + const invalidMessage = { ...message, params: undefined } + const { eip712Args } = prepareEip712({ + sequence: 0, + accountNumber: 3, + messages: invalidMessage, + }) + + expect(() => getEip712TypedData(eip712Args)).toThrow() + }) }) })packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateActorRoles.ts (3)
31-65: Refactor to reduce code duplication.The mapping logic for
roleActorsToAddandroleActorsToRevokeis duplicated. Consider extracting it into a helper method.public toProto() { const { params } = this const message = InjectivePermissionsV1Beta1Tx.MsgUpdateActorRoles.create() message.sender = params.sender message.denom = params.denom + const mapRoleActors = (roleActors: PermissionRoleActors) => { + const actor = InjectivePermissionsV1Beta1Permissions.RoleActors.create() + actor.role = roleActors.role + actor.actors = roleActors.actors + return actor + } + - const roleActorsToAdd = - params.roleActorsToAdd.map((roleActors) => { - const actor = InjectivePermissionsV1Beta1Permissions.RoleActors.create() - - actor.role = roleActors.role - actor.actors = roleActors.actors - - return actor - }) || [] + message.roleActorsToAdd = params.roleActorsToAdd.map(mapRoleActors) || [] - message.roleActorsToAdd = roleActorsToAdd - - const roleActorsToRevoke = - params.roleActorsToRevoke.map((roleActors) => { - const actor = InjectivePermissionsV1Beta1Permissions.RoleActors.create() - - actor.role = roleActors.role - actor.actors = roleActors.actors - - return actor - }) || [] - - message.roleActorsToRevoke = roleActorsToRevoke + message.roleActorsToRevoke = params.roleActorsToRevoke.map(mapRoleActors) || [] return InjectivePermissionsV1Beta1Tx.MsgUpdateActorRoles.fromPartial( message, ) }
67-74: Extract duplicated type string into a constant.The type string '/injective.permissions.v1beta1.MsgUpdateActorRoles' is duplicated in
toDataandtoWeb3methods.+private static TYPE_URL = '/injective.permissions.v1beta1.MsgUpdateActorRoles' + public toData() { const proto = this.toProto() return { - '@type': '/injective.permissions.v1beta1.MsgUpdateActorRoles', + '@type': MsgUpdateActorRoles.TYPE_URL, ...proto, } } public toWeb3() { const amino = this.toAmino() const { value } = amino return { - '@type': '/injective.permissions.v1beta1.MsgUpdateActorRoles', + '@type': MsgUpdateActorRoles.TYPE_URL, ...value, } }Also applies to: 89-97
89-97: Simplify unnecessary destructuring.The destructuring of
amino.valueis unnecessary since it's only used once.public toWeb3() { const amino = this.toAmino() - const { value } = amino return { '@type': '/injective.permissions.v1beta1.MsgUpdateActorRoles', - ...value, + ...amino.value, } }packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi.ts (2)
38-50: Consider abstracting error handling logic.The error handling pattern is duplicated across all methods. Consider creating a utility method to handle these common error patterns.
Here's a suggested implementation:
+ private handleGrpcError(e: unknown, context: string) { + if (e instanceof InjectivePermissionsV1Beta1Query.GrpcWebError) { + throw new GrpcUnaryRequestException(new Error(e.toString()), { + code: e.code, + context, + }) + } + + throw new GrpcUnaryRequestException(e as Error, { + code: UnspecifiedErrorCode, + context, + }) + } async fetchModuleParams() { // ... existing code ... } catch (e: unknown) { - if (e instanceof InjectivePermissionsV1Beta1Query.GrpcWebError) { - throw new GrpcUnaryRequestException(new Error(e.toString()), { - code: e.code, - context: 'Params', - }) - } - - throw new GrpcUnaryRequestException(e as Error, { - code: UnspecifiedErrorCode, - context: 'Params', - }) + this.handleGrpcError(e, 'Params') } }Also applies to: 66-78, 94-106, 124-136, 155-167, 186-198, 223-235, 251-263, 279-291, 309-321, 338-350, 368-380, 396-408
30-33: Consider adding retry configuration options.The
retrymethod is used consistently across all API calls, but it might benefit from configurable retry options (attempts, backoff) based on the specific endpoint requirements.Consider adding retry configuration:
+ interface RetryConfig { + maxAttempts?: number; + backoffMs?: number; + } async fetchModuleParams() { const request = InjectivePermissionsV1Beta1Query.QueryParamsRequest.create() try { const response = await this.retry<InjectivePermissionsV1Beta1Query.QueryParamsResponse>( () => this.client.Params(request, this.metadata), + { maxAttempts: 3, backoffMs: 1000 } )Also applies to: 58-61, 86-89, 116-119, 147-150, 178-181, 215-218, 243-246, 271-274, 301-304, 332-335, 360-363, 388-391
packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi.spec.ts (1)
31-52: Enhance test data coverage.The tests would benefit from more diverse test data to cover edge cases and boundary conditions.
Consider adding test cases with:
- Maximum/minimum values
- Special characters in strings
- Empty strings
- Large arrays
- Malformed data
Example implementation:
test('fetchNamespaceDenoms handles edge cases', async () => { const testCases = [ { input: [], expected: 'empty array' }, { input: ['a'.repeat(1000)], expected: 'long string' }, { input: ['!@#$%^&*()'], expected: 'special chars' } ] for (const { input, expected } of testCases) { // Mock response with test case // Assert expected behavior } })Also applies to: 77-94, 118-138, 162-183, 185-206, 208-231, 233-254, 255-276
packages/sdk-ts/src/client/chain/types/permissions.ts (2)
12-63: Add JSDoc comments to document interfaces.These interfaces represent core permission concepts. Consider adding JSDoc comments to document their purpose and usage.
Example:
+/** + * Represents a permission role with its associated name, ID, and permissions. + */ export interface PermissionRole { name: string roleId: number permissions: number }
1-115: Consider organizing interfaces into logical groups.The file contains many related interfaces that could benefit from better organization. Consider:
- Grouping related interfaces using namespaces or separate files (e.g., roles, policies, vouchers)
- Creating an index file to re-export all types
packages/sdk-ts/src/core/modules/exchange/msgs/MsgInstantSpotMarketLaunch.spec.ts (1)
11-11: Consider parameterizing the minNotional value.The hardcoded value of '1' for
minNotionalmight not cover all test scenarios. Consider parameterizing this value or adding test cases with different values to ensure proper validation.packages/sdk-ts/src/client/chain/grpc/ChainGrpcExchangeApi.ts (1)
425-483: Consider adding input validation for the denom parameter.While the implementation is solid, consider adding input validation for the
denomparameter infetchDenomMinNotionalto ensure it's not empty or malformed before making the gRPC call.async fetchDenomMinNotional(denom: string) { + if (!denom || denom.trim() === '') { + throw new Error('Denom parameter cannot be empty') + } const request = InjectiveExchangeV1Beta1Query.QueryDenomMinNotionalRequest.create()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (48)
packages/sdk-ts/package.json(1 hunks)packages/sdk-ts/src/client/chain/grpc/ChainGrpcAuctionApi.spec.ts(1 hunks)packages/sdk-ts/src/client/chain/grpc/ChainGrpcAuctionApi.ts(2 hunks)packages/sdk-ts/src/client/chain/grpc/ChainGrpcBankApi.ts(3 hunks)packages/sdk-ts/src/client/chain/grpc/ChainGrpcExchangeApi.spec.ts(2 hunks)packages/sdk-ts/src/client/chain/grpc/ChainGrpcExchangeApi.ts(2 hunks)packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi.spec.ts(2 hunks)packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi.ts(1 hunks)packages/sdk-ts/src/client/chain/transformers/ChainGrpcAuctionTransformer.ts(4 hunks)packages/sdk-ts/src/client/chain/transformers/ChainGrpcBankTransformer.ts(4 hunks)packages/sdk-ts/src/client/chain/transformers/ChainGrpcCommonTransformer.ts(1 hunks)packages/sdk-ts/src/client/chain/transformers/ChainGrpcExchangeTransformer.ts(2 hunks)packages/sdk-ts/src/client/chain/transformers/ChainGrpcPermissionsTransformer.ts(2 hunks)packages/sdk-ts/src/client/chain/types/auction.ts(2 hunks)packages/sdk-ts/src/client/chain/types/exchange.ts(2 hunks)packages/sdk-ts/src/client/chain/types/permissions.ts(1 hunks)packages/sdk-ts/src/client/indexer/transformers/IndexerAccountPortfolioTransformer.ts(4 hunks)packages/sdk-ts/src/client/indexer/transformers/IndexerCampaignTransformer.ts(3 hunks)packages/sdk-ts/src/client/indexer/transformers/IndexerCommonTransformer.ts(1 hunks)packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcAuctionTransformer.ts(2 hunks)packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcMitoTransformer.ts(9 hunks)packages/sdk-ts/src/core/modules/exchange/msgs/MsgInstantSpotMarketLaunch.spec.ts(1 hunks)packages/sdk-ts/src/core/modules/exchange/msgs/MsgInstantSpotMarketLaunch.ts(3 hunks)packages/sdk-ts/src/core/modules/gov/msgs/MsgSubmitProposalPerpetualMarketLaunch.spec.ts(2 hunks)packages/sdk-ts/src/core/modules/gov/msgs/MsgSubmitProposalPerpetualMarketLaunch.ts(1 hunks)packages/sdk-ts/src/core/modules/gov/msgs/MsgSubmitProposalSpotMarketLaunch.spec.ts(1 hunks)packages/sdk-ts/src/core/modules/gov/msgs/MsgSubmitProposalSpotMarketLaunch.ts(3 hunks)packages/sdk-ts/src/core/modules/permissions/index.ts(1 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgClaimVoucher.spec.ts(1 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgCreateNamespace.spec.ts(1 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgCreateNamespace.ts(3 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgDeleteNamespace.spec.ts(0 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgDeleteNamespace.ts(0 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgRevokeNamespaceRoles.spec.ts(0 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgRevokeNamespaceRoles.ts(0 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateActorRoles.spec.ts(1 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateActorRoles.ts(1 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespace.spec.ts(1 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespace.ts(3 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespaceRoles.spec.ts(0 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespaceRoles.ts(0 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateParams.spec.ts(2 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateParams.ts(2 hunks)packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgBurn.ts(2 hunks)packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgCreateDenom.ts(2 hunks)packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgMint.ts(2 hunks)packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgSetDenomMetadata.ts(2 hunks)proto/core/gen.sh(1 hunks)
💤 Files with no reviewable changes (6)
- packages/sdk-ts/src/core/modules/permissions/msgs/MsgRevokeNamespaceRoles.spec.ts
- packages/sdk-ts/src/core/modules/permissions/msgs/MsgDeleteNamespace.spec.ts
- packages/sdk-ts/src/core/modules/permissions/msgs/MsgDeleteNamespace.ts
- packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespaceRoles.ts
- packages/sdk-ts/src/core/modules/permissions/msgs/MsgRevokeNamespaceRoles.ts
- packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespaceRoles.spec.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- packages/sdk-ts/package.json
- packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcAuctionTransformer.ts
- packages/sdk-ts/src/core/modules/gov/msgs/MsgSubmitProposalPerpetualMarketLaunch.spec.ts
- packages/sdk-ts/src/client/chain/grpc/ChainGrpcExchangeApi.spec.ts
- packages/sdk-ts/src/core/modules/exchange/msgs/MsgInstantSpotMarketLaunch.ts
- packages/sdk-ts/src/core/modules/gov/msgs/MsgSubmitProposalSpotMarketLaunch.spec.ts
- packages/sdk-ts/src/core/modules/permissions/index.ts
- proto/core/gen.sh
- packages/sdk-ts/src/client/chain/types/exchange.ts
- packages/sdk-ts/src/core/modules/gov/msgs/MsgSubmitProposalPerpetualMarketLaunch.ts
- packages/sdk-ts/src/core/modules/gov/msgs/MsgSubmitProposalSpotMarketLaunch.ts
- packages/sdk-ts/src/client/chain/grpc/ChainGrpcAuctionApi.spec.ts
- packages/sdk-ts/src/client/indexer/transformers/IndexerAccountPortfolioTransformer.ts
- packages/sdk-ts/src/client/indexer/transformers/IndexerGrpcMitoTransformer.ts
- packages/sdk-ts/src/client/chain/grpc/ChainGrpcAuctionApi.ts
- packages/sdk-ts/src/client/chain/grpc/ChainGrpcBankApi.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/sdk-ts/src/client/chain/transformers/ChainGrpcCommonTransformer.ts
[error] 4-11: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateActorRoles.ts
[error] 23-23: Shouldn't redeclare 'MsgUpdateActorRoles'. Consider to delete it or rename it.
'MsgUpdateActorRoles' is defined here:
(lint/suspicious/noRedeclare)
packages/sdk-ts/src/client/indexer/transformers/IndexerCommonTransformer.ts
[error] 4-11: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateActorRoles.spec.ts
[error] 5-9: Do not export from a test file.
(lint/suspicious/noExportsInTest)
🔇 Additional comments (38)
packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateParams.ts (1)
1-6: LGTM! Import changes are well-structured.The imports are correctly organized and the addition of
InjectivePermissionsV1Beta1Paramsaligns with its usage in thetoProtomethod.packages/sdk-ts/src/client/indexer/transformers/IndexerCommonTransformer.ts (1)
4-11: LGTM! Good job centralizing the coin conversion logic.The implementation correctly maps GrpcCoin to Coin type, and centralizing this logic reduces code duplication across multiple transformers.
🧰 Tools
🪛 Biome (1.9.4)
[error] 4-11: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
packages/sdk-ts/src/client/indexer/transformers/IndexerCampaignTransformer.ts (1)
83-86: LGTM! Consistent usage of the common transformer.The changes correctly utilize the centralized
IndexerCommonTransformer.grpcCoinToCoinmethod for all coin conversions (tvlReward,volumeReward, andrewards), maintaining consistency across the codebase.Also applies to: 186-186
packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgMint.ts (1)
11-11: LGTM! Clean implementation of the receiver parameter.The optional receiver parameter is properly implemented in both the interface and the toProto method, following TypeScript best practices.
Also applies to: 42-44
packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgBurn.ts (1)
11-11: LGTM! Clean implementation of the burnFromAddress parameter.The optional burnFromAddress parameter is properly implemented in both the interface and the toProto method, following TypeScript best practices.
Also applies to: 42-44
packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgCreateDenom.ts (1)
12-12: LGTM! Precise implementation of the allowAdminBurn parameter.The optional allowAdminBurn parameter is properly implemented with a precise undefined check, following TypeScript best practices.
Also applies to: 40-42
packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgSetDenomMetadata.ts (1)
12-12: LGTM! Thorough implementation of the adminBurnDisabled parameter.The optional adminBurnDisabled parameter is properly implemented with a precise undefined check and correct message instance creation, following TypeScript best practices.
Also applies to: 54-60
packages/sdk-ts/src/client/chain/types/auction.ts (3)
2-19: LGTM! Well-structured interfaces with clear property definitions.The new interfaces are well-defined with appropriate optional properties and type annotations. The import from core-proto-ts aligns with the type definitions.
64-70: Verify type consistency in AuctionModuleStateResponse.The
auctionRoundproperty is defined asstringinAuctionModuleStateResponsebut asnumberinAuctionModuleState. This might cause type conversion issues.Consider using consistent types or document the reason for the difference if it's intentional.
72-81: LGTM! Clear and consistent type aliases.The type aliases are well-defined and follow a consistent naming convention, properly mapping to the proto definitions.
packages/sdk-ts/src/client/chain/transformers/ChainGrpcBankTransformer.ts (3)
2-3: LGTM! Clean import restructuring.The import changes align well with the architectural shift to centralize coin transformation logic in
ChainGrpcCommonTransformer.
45-58: LGTM! Consistent use of common transformer.The refactor properly delegates coin transformation to the centralized
ChainGrpcCommonTransformer.
81-94: LGTM! Clean transformation of balances array.The refactor properly delegates coin transformation to the centralized
ChainGrpcCommonTransformerwhile maintaining the original functionality.packages/sdk-ts/src/client/chain/transformers/ChainGrpcAuctionTransformer.ts (2)
21-28: LGTM! Well-structured transformation method.The
grpcBidToBidmethod is well-implemented with proper null checks and type safety.
30-42: LGTM! Clean implementation with proper null handling.The
grpcLastAuctionResultToLastAuctionResultmethod follows good practices with proper type definitions and null checks.packages/sdk-ts/src/core/modules/permissions/msgs/MsgClaimVoucher.spec.ts (1)
1-44: LGTM! Clean transition to EIP712 testing.The test implementation is well-structured and properly validates both EIP712 v1 and v2 formats against the Web3Gw chain responses.
packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateActorRoles.spec.ts (2)
6-9: Move interface to a types file.The
PermissionRoleActorsinterface should not be exported from a test file. Consider moving it to a dedicated types file.🧰 Tools
🪛 Biome (1.9.4)
[error] 5-9: Do not export from a test file.
(lint/suspicious/noExportsInTest)
20-20: Fix incorrect describe block name.The describe block name doesn't match the message class being tested.
-describe('MsgCreateNamespace', () => { +describe('MsgUpdateActorRoles', () => {packages/sdk-ts/src/core/modules/permissions/msgs/MsgCreateNamespace.spec.ts (1)
7-36: LGTM! Enhanced namespace configuration with improved role management.The updated namespace structure provides more granular control with:
- Clear role permissions with explicit role IDs
- Dedicated role managers configuration
- Policy status management with action-specific controls
- Policy manager capabilities with fine-grained permissions
packages/sdk-ts/src/client/chain/transformers/ChainGrpcPermissionsTransformer.ts (3)
1-19: LGTM! Well-organized imports.The imports are properly organized with clear separation between internal types, transformers, and external proto dependencies.
25-105: LGTM! Well-structured transformation methods.The transformation methods are well-organized, maintain type safety, and follow a consistent pattern. Each method is focused on a specific type conversion, making the code maintainable and easy to understand.
107-115: Consider safer null handling.The use of the non-null assertion operator (
!) assumesparamswill never be null. Consider adding a null check or using optional chaining for better safety.- const params = response.params! + if (!response.params) { + throw new Error('Module params not found in response') + } + const params = response.paramspackages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespace.spec.ts (1)
1-6: LGTM! Well-organized imports.The imports are properly structured and include all necessary dependencies for EIP712 testing.
packages/sdk-ts/src/core/modules/permissions/msgs/MsgCreateNamespace.ts (4)
2-5: Verify the core-proto-ts dependency version.The file references
@injectivelabs/core-proto-ts, but ensure the installed version in yourpackage.jsonis the correct one (e.g.,1.33.6) and that the library's functionalities align with your usage.#!/bin/bash # Checking '@injectivelabs/core-proto-ts' version in package.json jq '.dependencies["@injectivelabs/core-proto-ts"]?' package.json
16-27: LGTM! The Params interface has been updated with comprehensive permission controls.The namespace structure now includes granular permission controls through role-based access control (RBAC) and policy management capabilities.
43-125: LGTM! The toProto implementation is well-structured.The code effectively maps complex permission structures while maintaining readability through:
- Clear separation of mapping operations
- Proper error handling for optional arrays using
|| []- Consistent use of Proto message creation methods
136-170: LGTM! The toAmino method properly handles enum conversions.The implementation correctly converts policy actions to their JSON representation while maintaining the original structure for other fields.
packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateParams.spec.ts (1)
1-13: LGTM! Clean test setup with proper imports and type-safe message creation.The test setup follows good practices with well-organized imports and proper parameter structure.
packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateActorRoles.ts (1)
27-29: Add input validation in fromJSON.The
fromJSONmethod should validate the input parameters before creating a new instance.packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespace.ts (2)
2-12: LGTM! Well-structured interface changes.The updated imports and interface changes effectively support the transition from basic flags to a more comprehensive permission model with roles, policies, and capabilities.
Also applies to: 14-23
116-179: LGTM! Clean implementation of message transformation methods.The toData, toWeb3, toDirectSign, and toBinary methods correctly handle the message transformations.
packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi.ts (1)
1-24: Well-structured class implementation!The class is properly organized with clear inheritance, appropriate error handling imports, and proper initialization of the gRPC client.
packages/sdk-ts/src/client/chain/types/permissions.ts (1)
1-10: LGTM! Clean imports and well-defined base interface.The imports are properly organized and the
PermissionParamsinterface is focused with a clear single responsibility.packages/sdk-ts/src/core/modules/exchange/msgs/MsgInstantSpotMarketLaunch.spec.ts (3)
33-33: Address or remove the TODO comment.The TODO comment lacks context. Please either:
- Add context about what needs to be done
- Remove it if it's no longer needed
26-54: LGTM! Well-structured EIP712 tests.The test implementation is clean and comprehensive:
- Tests both v1 and v2 of EIP712
- Properly uses async/await
- Follows best practices for test structure
3-4:✅ Verification successful
Verify the necessity of the
.jsextension in imports.The
.jsextension in TypeScript imports is uncommon. Please verify if this is required for your build setup or ESM compatibility.
🏁 Script executed:
#!/bin/bash # Check if other TypeScript files in the project use .js extensions in imports rg -g '*.ts' -l '\.js['\'']' --type tsLength of output: 36180
Confirming ESM Import Consistency
The grep search across the repo shows that many TypeScript files (e.g., in packages/exceptions and packages/sdk-ts) deliberately use the “.js” extension in their imports. This indicates that the use of “.js” for ESM compatibility is an intentional project-wide pattern. No changes are needed here as long as your bundler and runtime configuration expect these extensions.
packages/sdk-ts/src/client/chain/transformers/ChainGrpcExchangeTransformer.ts (1)
283-296: LGTM!The transformation methods are well-implemented and correctly map the response data to the expected formats.
packages/sdk-ts/src/client/chain/grpc/ChainGrpcExchangeApi.ts (1)
365-423: LGTM!The methods are well-implemented with proper error handling and follow the established patterns in the class.
| describe('MsgUpdateNamespace', () => { | ||
| it('generates proper proto', () => { | ||
| const proto = message.toProto() | ||
|
|
||
| expect(proto).toStrictEqual({ | ||
| ...protoParams, | ||
| describe('generates proper EIP712 compared to the Web3Gw (chain)', () => { | ||
| const { endpoints, eip712Args, prepareEip712Request } = prepareEip712({ | ||
| sequence: 0, | ||
| accountNumber: 3, | ||
| messages: message, | ||
| }) | ||
| }) | ||
|
|
||
| it('generates proper data', () => { | ||
| const data = message.toData() | ||
| it('EIP712 v1', async () => { | ||
| const eip712TypedData = getEip712TypedData(eip712Args) | ||
|
|
||
| expect(data).toStrictEqual({ | ||
| '@type': protoType, | ||
| ...protoParams, | ||
| }) | ||
| }) | ||
|
|
||
| it('generates proper amino', () => { | ||
| const amino = message.toAmino() | ||
| const txResponse = await new IndexerGrpcWeb3GwApi( | ||
| endpoints.indexer, | ||
| ).prepareEip712Request({ | ||
| ...prepareEip712Request, | ||
| eip712Version: 'v1', | ||
| }) | ||
|
|
||
| expect(amino).toStrictEqual({ | ||
| type: protoTypeShort, | ||
| value: protoParamsAmino, | ||
| expect(eip712TypedData).toStrictEqual(JSON.parse(txResponse.data)) | ||
| }) | ||
| }) | ||
|
|
||
| it('generates proper web3', () => { | ||
| const web3 = message.toWeb3() | ||
| it('EIP712 v2', async () => { | ||
| const eip712TypedData = getEip712TypedDataV2(eip712Args) | ||
|
|
||
| const txResponse = await new IndexerGrpcWeb3GwApi( | ||
| endpoints.indexer, | ||
| ).prepareEip712Request({ ...prepareEip712Request, eip712Version: 'v2' }) | ||
|
|
||
| expect(web3).toStrictEqual({ | ||
| '@type': protoType, | ||
| ...protoParamsAmino, | ||
| expect(eip712TypedData).toStrictEqual(JSON.parse(txResponse.data)) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error scenario tests for EIP712 validation.
The current tests verify successful cases, but should also include:
- Invalid sequence numbers
- Malformed messages
- Network errors from IndexerGrpcWeb3GwApi
Example addition:
it('handles invalid sequence number', async () => {
const invalidEip712Args = {
...eip712Args,
sequence: -1, // Invalid sequence
}
await expect(
new IndexerGrpcWeb3GwApi(endpoints.indexer)
.prepareEip712Request({
...prepareEip712Request,
eip712Version: 'v1',
})
).rejects.toThrow()
})| message.denom = params.denom | ||
| message.rolePermissions = params.rolePermissions | ||
| message.roleManagers = params.roleManagers | ||
| message.policyStatuses = params.policyStatuses | ||
| message.policyManagerCapabilities = params.policyManagerCapabilities |
There was a problem hiding this comment.
Fix redundant assignments and add null checks for arrays.
There are several issues in the message creation logic:
- Arrays are assigned twice (e.g., line 45 and 69)
- The
|| []fallbacks won't prevent errors as they come after the.map()operations
Apply this diff to fix the issues:
message.sender = params.sender
message.denom = params.denom
- message.rolePermissions = params.rolePermissions
- message.roleManagers = params.roleManagers
- message.policyStatuses = params.policyStatuses
- message.policyManagerCapabilities = params.policyManagerCapabilities
if (params.contractHook) {
const contractHook =
InjectivePermissionsV1Beta1Tx.MsgUpdateNamespace_SetContractHook.create()
contractHook.newValue = params.contractHook
message.contractHook = contractHook
}
const permissions =
- params.rolePermissions.map((rolePermission) => {
+ (params.rolePermissions || []).map((rolePermission) => {
const permission = InjectivePermissionsV1Beta1Permissions.Role.create()
permission.name = rolePermission.name
permission.permissions = rolePermission.permissions
permission.roleId = rolePermission.roleId
return permission
- }) || []
+ })
message.rolePermissions = permissions
const roleManagers =
- params.roleManagers.map((roleManager) => {
+ (params.roleManagers || []).map((roleManager) => {
const manager =
InjectivePermissionsV1Beta1Permissions.RoleManager.create()
manager.roles = roleManager.roles
manager.manager = roleManager.manager
return manager
- }) || []
+ })
message.roleManagers = roleManagers
const policyStatuses =
- params.policyStatuses.map((policyStatus) => {
+ (params.policyStatuses || []).map((policyStatus) => {
const policy =
InjectivePermissionsV1Beta1Permissions.PolicyStatus.create()
policy.action = policyStatus.action
policy.isDisabled = policyStatus.isDisabled
policy.isSealed = policyStatus.isSealed
return policy
- }) || []
+ })
message.policyStatuses = policyStatuses
const policyManagerCapabilities =
- params.policyManagerCapabilities.map((capability) => {
+ (params.policyManagerCapabilities || []).map((capability) => {
const policy =
InjectivePermissionsV1Beta1Permissions.PolicyManagerCapability.create()
policy.action = capability.action
policy.canDisable = capability.canDisable
policy.canSeal = capability.canSeal
policy.manager = capability.manager
return policy
- }) || []
+ })
message.policyManagerCapabilities = policyManagerCapabilitiesAlso applies to: 58-111
| export type GrpcPermissionNamespace = | ||
| InjectivePermissionsV1Beta1Permissions.Namespace | ||
| export type GrpcPermissionsAddressRoles = | ||
| InjectivePermissionsV1Beta1Permissions.AddressRoles | ||
| export type GrpcPermissionsRole = InjectivePermissionsV1Beta1Permissions.Role | ||
| export type GrpcPermissionsRoleIDs = | ||
| export type GrpcPermissionRoleIDs = | ||
| InjectivePermissionsV1Beta1Permissions.RoleIDs | ||
| export type GrpcPermissionsAddressVoucher = | ||
| export type GrpcPermissionsNamespace = | ||
| InjectivePermissionsV1Beta1Permissions.Namespace |
There was a problem hiding this comment.
Remove redundant type export.
GrpcPermissionsNamespace and GrpcPermissionNamespace are aliases for the same type. Keep one consistent naming convention.
Remove the redundant type export:
export type GrpcPermissionNamespace =
InjectivePermissionsV1Beta1Permissions.Namespace
-export type GrpcPermissionsNamespace =
- InjectivePermissionsV1Beta1Permissions.Namespace📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type GrpcPermissionNamespace = | |
| InjectivePermissionsV1Beta1Permissions.Namespace | |
| export type GrpcPermissionsAddressRoles = | |
| InjectivePermissionsV1Beta1Permissions.AddressRoles | |
| export type GrpcPermissionsRole = InjectivePermissionsV1Beta1Permissions.Role | |
| export type GrpcPermissionsRoleIDs = | |
| export type GrpcPermissionRoleIDs = | |
| InjectivePermissionsV1Beta1Permissions.RoleIDs | |
| export type GrpcPermissionsAddressVoucher = | |
| export type GrpcPermissionsNamespace = | |
| InjectivePermissionsV1Beta1Permissions.Namespace | |
| export type GrpcPermissionNamespace = | |
| InjectivePermissionsV1Beta1Permissions.Namespace | |
| export type GrpcPermissionRoleIDs = | |
| InjectivePermissionsV1Beta1Permissions.RoleIDs |
| static denomMinNotionalsResponseToDenomMinNotionals( | ||
| response: InjectiveExchangeV1Beta1Query.QueryDenomMinNotionalsResponse, | ||
| ): ChainDenomMinNotional[] { | ||
| return response.denomMinNotionals.map((denomDecimals) => ({ | ||
| denom: denomDecimals.denom, | ||
| minNotional: new BigNumberInBase(denomDecimals.minNotional) | ||
| .dividedBy(10 ** 18) | ||
| .toFixed(), | ||
| })) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider making the decimal places configurable.
The method currently assumes all tokens use 18 decimal places (10 ** 18). This might not be true for all tokens, as different tokens can have different decimal places (e.g., USDT uses 6 decimals).
Consider:
- Using the token's actual decimal places from
denomDecimals - Or verifying if all supported tokens indeed use 18 decimal places
🌐 Web query:
What are the common decimal places used by different tokens in blockchain?
💡 Result:
Token decimals in blockchain determine the divisibility and precision of transactions, varying significantly across different cryptocurrencies and token standards. Here's an overview of common decimal configurations:
Common Decimal Standards
Bitcoin (BTC)
- Uses 8 decimal places
- Smallest unit: satoshi (0.00000001 BTC) [2][3]
Ethereum (ETH)
- Defaults to 18 decimal places
- Smallest unit: wei (0.000000000000000001 ETH)
- Gas fees often measured in gwei (1e9 wei = 0.000000001 ETH) [2][10]
ERC-20 Tokens
- Typically use 18 decimals (e.g., stablecoins like DAI)
- Configurable via smart contracts but rarely deviate from this standard [7][9]
Solana Tokens
- Range between 0–9 decimal places
- Meme coins often use 6–9 decimals for microtransactions [1]
Other Notable Examples
| Token | Decimals | Smallest Unit | Use Case |
|---|---|---|---|
| NEO | 0 | Not divisible | Whole-unit transactions |
| GAS (Neo) | 8 | 0.00000001 GAS | Fuel for Neo blockchain |
| BNB | 8 | jager (0.00000001) | Binance ecosystem utility |
| Cardano ADA | 6 | lovelace (0.000001) | Smart contract platform |
| Stellar XLM | 7 | stroop (0.0000001) | Cross-border payments |
Factors Influencing Decimal Choices
- Transaction granularity: Higher decimals (e.g., Ethereum’s 18) enable micropayments and precise DeFi calculations [2][10].
- User experience: Lower decimals simplify display/UX for high-value tokens (e.g., WBTC uses 8 decimals like Bitcoin) [3].
- Blockchain limitations: Solana’s maximum of 9 decimals balances precision with computational efficiency [1].
- Legacy compatibility: Many chains mirror Bitcoin’s 8 or Ethereum’s 18 decimals for interoperability [3][7].
For developers, choosing decimals requires balancing precision needs with storage costs and ecosystem norms—ERC-20’s default of 18 remains dominant for fungible tokens due to its flexibility in representing fractional values accurately [7][9].
Citations:
- 1: https://docs.dexlab.space/products/minting-lab/token-decimal-guide
- 2: https://www.gemini.com/cryptopedia/satoshi-value-gwei-to-ether-to-wei-converter-eth-gwei
- 3: https://linkd.academy/en-us/library/glossary/token-decimals/
- 4: https://blog.upay.best/crypto-terminology/token-decimals/
- 5: https://help.crypto.com/en/articles/5755942-tokens-import
- 6: https://semaphoreci.com/blog/erc20-token
- 7: https://docs.openzeppelin.com/contracts/3.x/erc20
- 8: https://crypto.com/en/university/what-are-token-standards
- 9: https://docs.tatum.io/docs/token-decimals-erc-20-or-compatible
- 10: https://www.bydfi.com/en/questions/what-is-the-significance-of-decimals-in-ethereum
Don't hardcode token decimals: make the divisor configurable
Right now, the method assumes that all tokens use 18 decimals by dividing by 10¹⁸. Although many ERC-20 tokens follow this standard, numerous tokens (for example, Bitcoin with 8 decimals or tokens like USDT with 6 decimals) use different configurations. Consider either:
- Dynamically retrieving the token's decimal value from the
denomDecimalsobject (if available), or - Making the divisor configurable to handle tokens with non-standard decimal places.
Summary by CodeRabbit
New Features
burnFromAddressto theMsgBurnclass.allowAdminBurnin theMsgCreateDenomclass.adminBurnDisabledin theMsgSetDenomMetadataclass.Improvements
Bug Fixes