-
Notifications
You must be signed in to change notification settings - Fork 4
Add wallet utxos command #47
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
|
Caution Review failedThe pull request is closed. WalkthroughAdds a new Changes
Sequence DiagramsequenceDiagram
autonumber
participant User
participant CLI as "wallet utxos\ncommand"
participant Context
participant Store as "Context Store\n(wallet/provider)"
participant Provider as Provider.get_wallet_utxos()
participant Formatter as OutputFormatter
User->>CLI: run wallet utxos [--output-format table]
CLI->>Context: build/obtain Context (captures output_format_overridden)
CLI->>Store: resolve wallet & provider (with fallbacks)
Store-->>CLI: wallet, provider
CLI->>Provider: get_wallet_utxos(address)
Provider->>Provider: build CardanoQueryClient\nquery UTxOs -> Vec<AnyUtxoData>
Provider-->>CLI: Vec<AnyUtxoData>
CLI->>Formatter: format (JSON or table)
Formatter-->>CLI: formatted output
CLI-->>User: display UTxOs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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: 1
🧹 Nitpick comments (2)
src/provider/types.rs (1)
165-204: Consider extracting the predicate construction to reduce duplication.The predicate construction in lines 171-184 is duplicated across
get_balance,get_detailed_balance, and nowget_wallet_utxos. Consider extracting this into a helper method to improve maintainability.Apply this refactor:
+ fn build_address_predicate(&self, address: &Address) -> utxorpc::spec::query::UtxoPredicate { + utxorpc::spec::query::UtxoPredicate { + r#match: Some(utxorpc::spec::query::AnyUtxoPattern { + utxo_pattern: Some(UtxoPattern::Cardano( + utxorpc::spec::cardano::TxOutputPattern { + address: Some(utxorpc::spec::cardano::AddressPattern { + exact_address: address.to_vec().into(), + ..Default::default() + }), + ..Default::default() + }, + )), + }), + ..Default::default() + } + } + pub async fn get_wallet_utxos( &self, address: &Address, ) -> Result<Vec<utxorpc::spec::query::AnyUtxoData>> { let mut client: CardanoQueryClient = self.client().await?; - let predicate = utxorpc::spec::query::UtxoPredicate { - r#match: Some(utxorpc::spec::query::AnyUtxoPattern { - utxo_pattern: Some(UtxoPattern::Cardano( - utxorpc::spec::cardano::TxOutputPattern { - address: Some(utxorpc::spec::cardano::AddressPattern { - exact_address: address.to_vec().into(), - ..Default::default() - }), - ..Default::default() - }, - )), - }), - ..Default::default() - }; + let predicate = self.build_address_predicate(address);Then update
get_balance(line 89) andget_detailed_balance(line 209) to use the same helper.src/wallet/utxos.rs (1)
105-124: Consider adding integration tests for the actual UTxO retrieval flow.The current tests only verify argument parsing. Consider adding tests that verify the actual functionality, such as:
- Fetching UTxOs for a known wallet/provider combination
- Verifying output format selection based on context
- Testing error cases (missing wallet, missing provider)
This would improve confidence in the end-to-end behavior, though the current tests are sufficient for the argument parsing layer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.md(1 hunks)README.md(1 hunks)docs/wallet.mdx(1 hunks)src/main.rs(1 hunks)src/provider/types.rs(1 hunks)src/wallet/mod.rs(3 hunks)src/wallet/utxos.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/wallet/mod.rs (2)
src/provider/utxorpc.rs (1)
utxos(161-201)src/wallet/utxos.rs (1)
run(18-49)
src/provider/types.rs (1)
src/provider/utxorpc.rs (3)
client(25-37)utxos(161-201)utxo(172-186)
src/wallet/utxos.rs (2)
src/output.rs (1)
output(13-18)src/wallet/mod.rs (1)
run(45-57)
⏰ 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: integration
- GitHub Check: unit
🔇 Additional comments (10)
README.md (1)
106-112: LGTM!The documentation clearly describes the new command and its output options.
docs/wallet.mdx (1)
65-73: LGTM!The documentation accurately describes the command and its output format behavior.
CHANGELOG.md (1)
5-10: LGTM!The changelog entry accurately describes the new feature.
src/wallet/mod.rs (3)
13-13: LGTM!The module declaration follows the existing pattern.
40-41: LGTM!The command variant is properly documented and follows the existing pattern.
55-55: LGTM!The command routing is correct and consistent with other commands.
src/main.rs (2)
99-99: LGTM!The new field correctly tracks whether the user explicitly set the output format.
105-116: LGTM!The logic correctly determines both the output format and whether it was explicitly overridden. This enables commands to have different default formats while still respecting user preferences.
src/wallet/utxos.rs (2)
18-49: LGTM!The function correctly resolves wallet and provider defaults, fetches UTxOs, and handles all error cases appropriately. The special-case default to JSON format (lines 35-39) makes sense for UTxO data, which is often piped to other tools.
61-103: LGTM!The table formatter provides a useful summary with key fields, and the JSON formatter outputs the complete UTxO data. The handling of missing fields with fallback strings is appropriate.
| }; | ||
|
|
||
| let response = client | ||
| .search_utxos(predicate, None, u32::MAX) |
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.
Unbounded UTxO query could cause performance issues.
Using u32::MAX as the limit allows fetching an unbounded number of UTxOs. Wallets with many UTxOs (e.g., exchange wallets, large DeFi contracts) could trigger slow queries, high memory usage, or timeouts.
Consider either:
- Adding a configurable limit with a reasonable default (e.g., 1000)
- Implementing pagination support
- Documenting this limitation if it's intentional
- let response = client
- .search_utxos(predicate, None, u32::MAX)
- .await
- .context("failed to query utxos")?;
+ // Consider adding a reasonable limit or pagination
+ const MAX_UTXOS: u32 = 10000;
+ let response = client
+ .search_utxos(predicate, None, MAX_UTXOS)
+ .await
+ .context("failed to query utxos")?;Committable suggestion skipped: line range outside the PR's diff.
Summary
wallet utxosCLI command that streams the configured wallet's UTxOs in JSON by defaultTesting
Codex Task
Summary by CodeRabbit
New Features
wallet utxoscommand to display the current wallet's UTxO set (JSON by default) with an optional--output-format tablefor tabular display.Documentation
✏️ Tip: You can customize this high-level summary in your review settings.