ark-cli improvements #780
ark-cli improvements #780
Conversation
WalkthroughThis PR updates module dependencies across the project and adds new functionality to the ark-cli package, including Nostr nsec key support, a new vtxos subcommand for listing wallet VTXOs, and enhanced redeem flow with optional outpoint filtering. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes span new feature implementation with moderate logic density (Nostr key handling, VTXO querying, JSON formatting), though the core additions are straightforward. Dependency updates are extensive but largely homogeneous version bumps, which reduces per-file review complexity. The primary review focus should be verifying the Nostr integration correctness, VTXO filtering logic, and compatibility implications of the transitive dependency updates. Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumpkg/ark-cli/go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
go.mod(1 hunks)pkg/ark-cli/go.mod(3 hunks)pkg/ark-cli/main.go(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pkg/ark-cli/main.go (2)
api-spec/protobuf/gen/ark/v1/types.pb.go (3)
Outpoint(24-30)Outpoint(43-43)Outpoint(58-60)internal/core/application/types.go (2)
Outpoint(163-163)VOut(117-117)
⏰ 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). (3)
- GitHub Check: Build and Scan
- GitHub Check: integration tests
- GitHub Check: unit tests
🔇 Additional comments (9)
pkg/ark-cli/main.go (7)
10-11: LGTM! New imports are properly utilized.The new imports support the nsec handling, outpoint parsing, and string manipulation features added in this PR.
Also applies to: 18-18
150-166: LGTM! Flag definitions are well-structured.The new flags follow existing conventions and have sensible defaults. The outpoint format specification in the usage text is helpful for users.
270-278: LGTM! New vtxos command is well-defined.The command structure follows existing patterns and includes appropriate flags for filtering and authentication.
460-476: LGTM! Enhanced force redemption flow with outpoint filtering.The implementation correctly parses optional outpoint filters and returns JSON-formatted outputs, aligning with the updated SDK behavior mentioned in the PR description.
642-662: LGTM! Outpoint parsing is correctly implemented.The function properly validates the format and parses the vout value. While txid format isn't validated here, allowing the server to handle that validation is a reasonable design choice.
664-684: LGTM! VTXO listing logic is correct.The function properly filters the response based on user-specified flags, providing a clean API for viewing spendable and/or spent VTXOs.
228-247: LGTM! Flag groupings improved for readability.The explicit array formatting makes the flag lists clearer, and the addition of
outpointsFlagto the redeem command aligns with the new filtering functionality.go.mod (1)
24-24: LGTM! Dependency updates support new CLI features.The updated versions of ark-lib and go-sdk provide the necessary API changes for the new vtxos command and enhanced redeem functionality.
Also applies to: 30-30
pkg/ark-cli/go.mod (1)
10-12: LGTM! Dependencies updated to support new features.The addition of go-nostr and updates to ark-lib and go-sdk enable the nsec handling and enhanced VTXO management features introduced in this PR.
| // handle nsec | ||
| seed := ctx.String(privateKeyFlag.Name) | ||
| if strings.HasPrefix(seed, "nsec") { | ||
| // Decode nsec using nip19 to get the raw private key | ||
| _, decoded, err := nip19.Decode(seed) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to decode nsec: %v", err) | ||
| } | ||
|
|
||
| // The decoded value is already a hex string, so use it directly | ||
| seed = decoded.(string) | ||
| } | ||
|
|
There was a problem hiding this comment.
Add type safety check for nip19 decode result.
The type assertion on line 310 could panic if nip19.Decode doesn't return a string. While it should return a string for "nsec" prefixes, adding a safety check would prevent potential runtime panics.
Apply this diff to add safe type checking:
// handle nsec
seed := ctx.String(privateKeyFlag.Name)
if strings.HasPrefix(seed, "nsec") {
// Decode nsec using nip19 to get the raw private key
_, decoded, err := nip19.Decode(seed)
if err != nil {
return fmt.Errorf("failed to decode nsec: %v", err)
}
- // The decoded value is already a hex string, so use it directly
- seed = decoded.(string)
+ // The decoded value should be a hex string
+ hexSeed, ok := decoded.(string)
+ if !ok {
+ return fmt.Errorf("decoded nsec is not a string")
+ }
+ seed = hexSeed
}🤖 Prompt for AI Agents
In pkg/ark-cli/main.go around lines 300 to 312, the code type-asserts
decoded.(string) from nip19.Decode which can panic if the decoded value is not a
string; change this to safely check the type (e.g., v, ok := decoded.(string))
and return a descriptive error if ok is false, so the function fails gracefully
instead of panicking, preserving the existing flow of replacing seed with the
decoded hex string when valid.
| vtxosCommand = cli.Command{ | ||
| Name: "vtxos", | ||
| Usage: "List VTXOs (coins) in the wallet", | ||
| Flags: []cli.Flag{passwordFlag, expiryDetailsFlag, spendableFlag, spentFlag}, |
There was a problem hiding this comment.
passwordFlag and expiryDetailsFlag seems to be unused by listVtxos()
2 participants
This PR is extending the CLI with :
initvtxoscommand listing the wallet coinsredeem --forceaccording to improve Unroll method go-sdk#46@altafan please review
Summary by CodeRabbit
New Features
vtxoscommand to list wallet VTXOs with filtering options (spendable/spent)Chores