-
Notifications
You must be signed in to change notification settings - Fork 35
feat: utils address command #419
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughAdds a new shared address utilities module and re-exports it; introduces a CLI "utils" command group with an "address" subcommand that uses the new utilities; updates task code to use the shared hexToBech32 helper. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as toolkit utils address
participant Utils as utils.convertAddressAll
participant Libs as bech32 / ethers
User->>CLI: toolkit utils address <input> [--json]
CLI->>Utils: convertAddressAll(input)
Utils->>Libs: parse/validate (hex | bech32 | byte array)
Libs-->>Utils: normalized 20-byte address
Utils-->>CLI: UnifiedAddressFormats (checksummed hex, hexUppercase, bech32 acc/valoper/valcons, raw bytes)
alt --json
CLI-->>User: JSON output
else
CLI-->>User: formatted table output
end
Note over CLI,Utils: On error -> log and exit 1
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (8)
packages/tasks/src/account.ts (1)
145-148
: Bech32 output uses default HRPUsing hexToBech32(address) (default "zeta") is appropriate. If you ever need valoper/valcons here, consider exposing them similarly or using convertAddressAll.
utils/address.ts (5)
17-26
: Stricter Bech32 validation (limit HRP and payload length)Currently any bech32 string passes, deferring failure to bech32ToHex. Restrict to expected Zeta HRPs and 20‑byte payloads to fail fast and reduce false positives.
export const isBech32Address = (value: string): boolean => { try { - const decoded = bech32.decode(value); - return ( - !!decoded && Array.isArray(decoded.words) && decoded.words.length > 0 - ); + const decoded = bech32.decode(value); + const allowed = new Set([ZETA_HRP, ZETA_VALOPER_HRP, ZETA_VALCONS_HRP]); + // Validate HRP and decoded payload size (20 bytes expected) + const bytes = bech32.fromWords(decoded.words); + return allowed.has(decoded.prefix) && Array.isArray(decoded.words) && bytes.length === 20; } catch { return false; } };
38-48
: Validate HRP in bech32ToHex for clearer errorsDecoding addresses with the wrong HRP currently proceeds if 20 bytes long. Validate HRP and throw a helpful error.
export const bech32ToHex = (bech32Addr: string): string => { - const decoded = bech32.decode(bech32Addr); - const bytes = Buffer.from(bech32.fromWords(decoded.words)); + const decoded = bech32.decode(bech32Addr); + const allowed = new Set([ZETA_HRP, ZETA_VALOPER_HRP, ZETA_VALCONS_HRP]); + if (!allowed.has(decoded.prefix)) { + throw new Error( + `Unsupported HRP '${decoded.prefix}'. Expected one of: ${Array.from(allowed).join(", ")}` + ); + } + const bytes = Buffer.from(bech32.fromWords(decoded.words)); if (bytes.length !== 20) { throw new Error( `Invalid address bytes length ${bytes.length}, expected 20` ); } const hex = `0x${bytes.toString("hex")}`; return ethers.getAddress(hex); };
104-113
: Fix misaligned comments in UnifiedAddressFormatsComments appear next to the wrong fields; this can confuse consumers.
export interface UnifiedAddressFormats { - // EIP-55, with 0x bech32Acc: string; bech32Valcons: string; bech32Valoper: string; bytes: number[]; - // uppercase, without 0x prefix - checksumHex: string; - hex: string; + // EIP-55, with 0x + checksumHex: string; + // UPPERCASE hex, without 0x prefix + hex: string; }
28-36
: Optional: avoid Buffer to improve portabilityYou can drop Node Buffer usage and rely on Uint8Array/ethers helpers to make this module more environment‑agnostic.
// hexToBech32 - const words = bech32.toWords(Buffer.from(addressBytes)); + const words = bech32.toWords(addressBytes); // convertAddressAll - const hexLower = Buffer.from(bytes).toString("hex"); + const hexLower = ethers.hexlify(bytes).slice(2).toLowerCase();Also applies to: 126-130
118-142
: Output ordering (nit)If consumers rely on property iteration for display, consider ordering fields Acc → Valoper → Valcons to match examples. Otherwise, ignore.
packages/commands/src/utils/address.ts (2)
22-25
: Consider consistent key naming in JSON output.The JSON output uses inconsistent naming conventions:
bech32Con
andbech32Val
are abbreviatedbech32Acc
is also abbreviated- But the source uses full names:
bech32Valcons
,bech32Valoper
Consider using either full names consistently (
bech32Account
,bech32Validator
,bech32Consensus
) or keeping the current abbreviated style—just ensure it matches any API contracts or documentation.
41-44
: Type the error parameter.The catch block uses an untyped
error
parameter. While the current usage is safe, explicitly typing it improves code clarity.Apply this diff:
- } catch (error) { + } catch (error: unknown) { console.error("Failed to convert address:", error);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
packages/commands/src/index.ts
(1 hunks)packages/commands/src/program.ts
(2 hunks)packages/commands/src/utils/address.ts
(1 hunks)packages/commands/src/utils/index.ts
(1 hunks)packages/tasks/src/account.ts
(2 hunks)utils/address.ts
(1 hunks)utils/index.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: fadeev
PR: zeta-chain/toolkit#346
File: packages/commands/src/ton/depositAndCall.ts:0-0
Timestamp: 2025-06-13T15:33:54.781Z
Learning: fadeev prefers responses in English.
🧬 Code graph analysis (4)
packages/commands/src/utils/index.ts (1)
packages/commands/src/utils/address.ts (1)
addressCommand
(6-45)
packages/commands/src/utils/address.ts (1)
utils/address.ts (1)
convertAddressAll
(118-142)
packages/commands/src/program.ts (1)
packages/commands/src/utils/index.ts (1)
utilsCommand
(5-8)
packages/tasks/src/account.ts (1)
utils/address.ts (1)
hexToBech32
(28-36)
🔇 Additional comments (8)
packages/tasks/src/account.ts (1)
14-14
: Correct utility importSwitching to shared hexToBech32 centralizes logic and removes duplicate bech32 code. Good change.
packages/commands/src/index.ts (1)
9-9
: Publicly exposing utils is appropriateThis makes the utils command group available to consumers. No issues.
utils/index.ts (1)
2-2
: Expose address utilities via utilsGood centralization; consistent with CLI integration.
packages/commands/src/program.ts (1)
13-13
: Wires utils command group correctlyutilsCommand imported and added as a subcommand; consistent with existing structure.
Also applies to: 28-28
packages/commands/src/utils/index.ts (1)
1-8
: LGTM! Clean command group setup.The command group structure follows Commander.js best practices. The imports are correct, and disabling the automatic help command is appropriate for a command group that will be integrated into a parent command structure.
packages/commands/src/utils/address.ts (3)
6-14
: LGTM! Clear command definition.The command definition is well-structured with:
- Clear summary describing the functionality
- Properly typed argument with helpful description
- Optional JSON output flag for programmatic usage
15-45
: LGTM! Solid error handling and dual output formats.The action handler implementation is clean and functional:
- Proper error handling with informative messages
- Support for both human-readable table and machine-readable JSON output
- Appropriate use of
process.exit(1)
for CLI error scenarios
4-4
: Import path is valid. The relative import (../../../../utils/address
) correctly resolves toutils/address.ts
at the repository root; no changes required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested ACK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/commands/src/utils/address.ts (1)
6-13
: Consider clarifying the summary regarding bytes output.The summary mentions "Show all address formats (bytes, hex, bech32...)" but the actual output (lines 18-41) does not include the
bytes
field from the result. This creates a minor expectation mismatch. Either update the summary to remove the mention of bytes, or include the bytes in the output if it's intended to be shown.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/commands/src/utils/address.ts
(1 hunks)utils/address.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- utils/address.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: fadeev
PR: zeta-chain/toolkit#346
File: packages/commands/src/ton/depositAndCall.ts:0-0
Timestamp: 2025-06-13T15:33:54.781Z
Learning: fadeev prefers responses in English.
🧬 Code graph analysis (1)
packages/commands/src/utils/address.ts (1)
utils/address.ts (1)
convertAddressAll
(118-142)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
packages/commands/src/utils/address.ts (2)
15-46
: LGTM!The action handler is well-structured with proper error handling, clear separation between JSON and table output modes, and consistent field selection across both formats. The use of try-catch with process.exit(1) on error is appropriate for a CLI command.
4-4
: Address the fragile deep relative import path for long-term maintainability.The import
../../../../utils/address
traverses four directory levels and works correctly but is fragile for refactoring. Relative imports are functional but difficult to maintain in larger workspaces; TypeScript path aliases can create custom paths for imports, making the codebase easier to navigate and refactor. Consider this as part of establishing path aliases across the monorepo (e.g.,@/utils/address
) when infrastructure is in place, or restructure to reduce coupling between deeply nested packages.
Summary by CodeRabbit
New Features
Refactor