feat: add channel fee estimation parameters and update related functions#9
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the CLI to align with kaleido-sdk==0.1.3, including new LSP channel fee estimation request/response models and updated trading-pair model types used by swap/market flows.
Changes:
- Bump
kaleido-sdkdependency from0.1.2to0.1.3(and refreshuv.lockaccordingly). - Update trading-pair utilities to use
TradableAssetResponseModel/TradingPairResponseModeland layer-specific asset-id resolution. - Rework
channel order estimate-feesto call the new SDK fee-estimation API shape and introduceChannelFeeEstimateParams; update node init prompts/help text.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Locks kaleido-sdk to 0.1.3 artifacts/hashes. |
| pyproject.toml | Bumps kaleido-sdk pinned version to 0.1.3. |
| kaleido_cli/utils/pairs.py | Adapts helpers to new SDK response-model types and asset id usage. |
| kaleido_cli/commands/node.py | Improves wording for wallet password creation prompts/options. |
| kaleido_cli/commands/channel.py | Adds fee-estimation params/model wiring and updates estimate-fees command/options/examples. |
Comments suppressed due to low confidence (1)
kaleido_cli/commands/channel.py:905
channel order estimate-feesno longer accepts the positionalclient_pubkeyargument. This is a breaking CLI change for any existing scripts that still pass a pubkey as the first argument. If the underlying API no longer needs it, consider keeping an optional/deprecated positional argument (ignored or validated) to preserve backward compatibility, or explicitly error when an unexpected argument is provided with a clear migration message.
)
def channel_estimate_fees(
lsp_balance_sat: Annotated[
int | None,
typer.Option("--lsp-balance", help="LSP's balance in the channel (satoshis)."),
] = None,
client_balance_sat: Annotated[
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else: | ||
| lsp_asset_amount = None | ||
| client_asset_amount = None | ||
|
|
There was a problem hiding this comment.
In interactive mode, this branch resets lsp_asset_amount/client_asset_amount to None whenever asset_id is unset. If a user supplies --lsp-asset-amount/--client-asset-amount but forgets --asset-id, their explicit inputs get silently discarded (and the later validation won’t trigger). Consider detecting this case and either (a) prompting for --asset-id, or (b) failing fast with the same error used in non-interactive mode.
There was a problem hiding this comment.
Will be done as enhancement in the future!
|
LGTM |
No description provided.