lncli: fix misleading default values in updatechanpolicy help

[yashbhutwala]

Summary

This PR fixes issue #1523 where the updatechanpolicy command displayed misleading "(default: 0)" text for required numeric flags.

• Converts base_fee_msat, fee_rate_ppm, time_lock_delta to StringFlag
• Adds [required] prefix and example values to help text
• Removes misleading "(default: 0)" text
• No behavior changes, just improved UX

Related PR

• Phase 2: multi: make updatechanpolicy fields optional with *_specified flags #10500 - Makes fields truly optional with *_specified proto flags (more invasive but complete solution)
• This PR (Phase 1) is a standalone fix that can be merged immediately
• If Phase 2 is merged, Phase 1 becomes unnecessary (but Phase 1 provides value on its own if Phase 2 takes longer to review/merge)

Problem

cli.Int64Flag and cli.Uint64Flag automatically append "(default: 0)" to help text, which confused users because:

1. These fields are actually required, not optional
2. For time_lock_delta, 0 is not even a valid value (minimum is 18)

Solution

Convert numeric flags to StringFlag (following the existing pattern used by fee_rate which was already a StringFlag), and add clear [required] labels.

Changes

• Convert base_fee_msat from Int64Flag to StringFlag
• Convert fee_rate_ppm from Uint64Flag to StringFlag
• Convert time_lock_delta from Uint64Flag to StringFlag
• Add [required] prefix to usage text for required fields
• Add example values and valid ranges to help users
• Update parsing logic to use explicit strconv functions

Before vs After

Before:

--base_fee_msat value    the base fee in milli-satoshis... (default: 0)
--time_lock_delta value  the CLTV delta... (default: 0)

After:

--base_fee_msat value    [required] the base fee in milli-satoshis... (e.g., 1000)
--time_lock_delta value  [required] the CLTV delta... Must be between 18 and 65535 (e.g., 80)

Why StringFlag?

1. Existing precedent: fee_rate was already a StringFlag in this same command
2. Option 2 (Value + ctx.IsSet) cannot work without protocol changes: The RPC server validates time_lock_delta >= 18, so sending 0 to mean "don't change" would fail
3. Directly addresses user feedback: As suggested in lnd updatechanpolicy has confusing --help #1523, "it should just say this is a required argument"
4. No breaking changes: Behavior remains identical, only help text improves

See detailed analysis of previous attempts in PR #6762 and #8673.

Test Plan

• go build ./cmd/lncli - compiles successfully
• go test ./cmd/commands/... - all tests pass
• go vet ./cmd/commands/... - no issues
• Verified help output shows correct text without "(default: 0)"

Fixes #1523

[gemini-code-assist]

Summary of Changes

Hello @yashbhutwala, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a long-standing usability issue in the lncli updatechanpolicy command where required numeric parameters were incorrectly displayed with a "(default: 0)" suffix in the help text, leading to user confusion. The solution involves refactoring these parameters to use StringFlag types, aligning with existing patterns, and significantly improving the help messages to clearly indicate required fields, provide examples, and specify valid ranges. This change enhances the command-line interface's clarity and user experience without altering its core behavior.

Highlights

• Improved Help Text Clarity: The updatechanpolicy command's help text now accurately reflects required numeric flags, removing misleading "(default: 0)" messages.
• Flag Type Conversion: Several numeric flags (base_fee_msat, fee_rate_ppm, time_lock_delta) have been converted from Int64Flag/Uint64Flag to StringFlag.
• Enhanced Usage Information: Help messages for affected flags now include [required] labels, example values, and valid ranges where applicable.
• Robust Parsing Logic: The command's internal parsing logic has been updated to explicitly use strconv functions for converting string inputs to their respective numeric types, ensuring correct handling.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request effectively addresses a usability issue in the updatechanpolicy command by replacing numeric flags with string flags. This change prevents the display of misleading (default: 0) help text for required fields, leading to a more intuitive command-line experience. The implementation is solid, with appropriate updates to flag definitions and parsing logic. I have included a couple of minor suggestions to align code formatting with the repository's style guide for function calls, enhancing readability and maintainability.

[hieblmi]

Hi @yashbhutwala, thanks for your fixes!

In my opinion we should correctly display the default values rather then removing them.
Defaults are shown across other cli tools as well.

I think we could read defaults from the config file, if they are set?

[yashbhutwala]

In my opinion we should correctly display the default values rather then removing them.
Defaults are shown across other cli tools as well.

I think we could read defaults from the config file, if they are set?

Thanks for the feedback @hieblmi! I appreciate the suggestion, but I think showing defaults doesn't quite fit the semantics of updatechanpolicy for a few reasons:

1. Semantic mismatch: updatechanpolicy is an UPDATE operation, not CREATE. For updates, the natural "default" is "preserve current value", not "use config default". Showing (default: 1000) for base_fee_msat could mislead users into thinking that's their current channel's value, or that omitting the field will use 1000.

2. Technical limitation: lncli is a separate binary from lnd and doesn't have access to the user's config file. We can only access hardcoded constants (chainreg.DefaultBitcoinBaseFeeMSat, etc.), which may differ from what the user has configured.

3. Current behavior mismatch: Even if we showed defaults, the current RPC behavior doesn't use them - omitting a field sends 0, which either changes the value to 0 (invalid for time_lock_delta < 18) or causes validation failure.

The proper solution is in the companion PR #10500, which:

• Adds *_specified boolean fields to the protocol
• Uses chainreg.Default* as Value: with ctx.IsSet() checks
• When a field is not specified, the current channel value is preserved

This makes defaults meaningful - if you omit time_lock_delta, the current value stays unchanged.

This PR (#10499) is a minimal fix that accurately reflects the current behavior: these fields ARE required today. Once #10500 is merged, the fields become truly optional with sensible defaults.

Does that make sense?

[gemini-code-assist]

Code Review

This pull request effectively addresses issue #1523 by fixing misleading default values in the updatechanpolicy help text. The conversion of base_fee_msat, fee_rate_ppm, and time_lock_delta to StringFlags, along with the updated usage messages, significantly improves the user experience by accurately reflecting that these fields are required and providing helpful examples. The corresponding changes to the parsing logic using strconv functions are correctly implemented with appropriate error handling. The release notes have also been updated to reflect these changes. No critical, high, or medium severity issues were found in this review.

[yashbhutwala]

@hieblmi can you please review my comment? 🙏

[lightninglabs-deploy]

@yashbhutwala, remember to re-request review from reviewers when ready