Skip to content

multi: make updatechanpolicy fields optional with *_specified flags #10500

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.

Sign up for GitHub

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

Draft
yashbhutwala wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

multi: make updatechanpolicy fields optional with *_specified flags #10500

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 50 additions & 28 deletions cmd/commands/commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/btcsuite/btcd/wire"
"github.com/jessevdk/go-flags"
"github.com/lightningnetwork/lnd"
"github.com/lightningnetwork/lnd/chainreg"
"github.com/lightningnetwork/lnd/lnrpc"
"github.com/lightningnetwork/lnd/lnwire"
"github.com/lightningnetwork/lnd/routing"
Expand Down Expand Up @@ -2525,7 +2526,9 @@ var updateChannelPolicyCommand = cli.Command{
Name: "base_fee_msat",
Usage: "the base fee in milli-satoshis that will be " +
"charged for each forwarded HTLC, regardless " +
"of payment size",
"of payment size. If unset, the existing " +
"base fee is retained",
Value: int64(chainreg.DefaultBitcoinBaseFeeMSat),
},
cli.StringFlag{
Name: "fee_rate",
Expand All @@ -2534,7 +2537,8 @@ var updateChannelPolicyCommand = cli.Command{
"forwarded HTLC, the lowest possible rate is " +
"0 with a granularity of 0.000001 " +
"(millionths). Can not be set at the same " +
"time as fee_rate_ppm",
"time as fee_rate_ppm. If unset, the " +
"existing fee rate is retained",
},
cli.Uint64Flag{
Name: "fee_rate_ppm",
Expand All @@ -2543,7 +2547,9 @@ var updateChannelPolicyCommand = cli.Command{
"value of each forwarded HTLC, the lowest " +
"possible rate is 0 with a granularity of " +
"0.000001 (millionths). Can not be set at " +
"the same time as fee_rate",
"the same time as fee_rate. If unset, the " +
"existing fee rate is retained",
Value: uint64(chainreg.DefaultBitcoinFeeRate),
},
cli.Int64Flag{
Name: "inbound_base_fee_msat",
Expand Down Expand Up @@ -2573,7 +2579,10 @@ var updateChannelPolicyCommand = cli.Command{
cli.Uint64Flag{
Name: "time_lock_delta",
Usage: "the CLTV delta that will be applied to all " +
"forwarded HTLCs",
"forwarded HTLCs. Must be between 18 and " +
"65535. If unset, the existing time lock " +
"delta is retained",
Value: uint64(chainreg.DefaultBitcoinTimeLockDelta),
},
cli.Uint64Flag{
Name: "min_htlc_msat",
Expand Down Expand Up @@ -2660,62 +2669,69 @@ func updateChannelPolicy(ctx *cli.Context) error {
defer cleanUp()

var (
baseFee int64
feeRate float64
feeRatePpm uint64
timeLockDelta uint16
err error
baseFee int64
baseFeeMsatSpecified bool
feeRate float64
feeRatePpm uint64
feeRateSpecified bool
timeLockDelta uint32
timeLockDeltaSpecified bool
err error
)
args := ctx.Args()

// Parse base_fee_msat. If explicitly set, mark as specified.
switch {
case ctx.IsSet("base_fee_msat"):
baseFee = ctx.Int64("base_fee_msat")
baseFeeMsatSpecified = true
case args.Present():
baseFee, err = strconv.ParseInt(args.First(), 10, 64)
if err != nil {
return fmt.Errorf("unable to decode base_fee_msat: %w",
err)
}
baseFeeMsatSpecified = true
args = args.Tail()
default:
return fmt.Errorf("base_fee_msat argument missing")
}

// Parse fee_rate or fee_rate_ppm. If explicitly set, mark as specified.
switch {
case ctx.IsSet("fee_rate") && ctx.IsSet("fee_rate_ppm"):
return fmt.Errorf("fee_rate or fee_rate_ppm can not both be set")
case ctx.IsSet("fee_rate"):
feeRate = ctx.Float64("fee_rate")
feeRateSpecified = true
case ctx.IsSet("fee_rate_ppm"):
feeRatePpm = ctx.Uint64("fee_rate_ppm")
feeRateSpecified = true
case args.Present():
feeRate, err = strconv.ParseFloat(args.First(), 64)
if err != nil {
return fmt.Errorf("unable to decode fee_rate: %w", err)
}

feeRateSpecified = true
args = args.Tail()
default:
return fmt.Errorf("fee_rate or fee_rate_ppm argument missing")
}

// Parse time_lock_delta. If explicitly set, mark as specified.
switch {
case ctx.IsSet("time_lock_delta"):
timeLockDeltaStr := ctx.String("time_lock_delta")
timeLockDelta, err = parseTimeLockDelta(timeLockDeltaStr)
tld, err := parseTimeLockDelta(timeLockDeltaStr)
if err != nil {
return err
}
timeLockDelta = uint32(tld)
timeLockDeltaSpecified = true
case args.Present():
timeLockDelta, err = parseTimeLockDelta(args.First())
tld, err := parseTimeLockDelta(args.First())
if err != nil {
return err
}

timeLockDelta = uint32(tld)
timeLockDeltaSpecified = true
args = args.Tail()
default:
return fmt.Errorf("time_lock_delta argument missing")
}
Comment on lines +2683 to 2735
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The argument parsing logic for base_fee_msat, fee_rate*, and time_lock_delta is a bit repetitive across the three switch blocks. This also violates the style guide which requires comments for each case and spacing between them.

Consider refactoring this into a series of if/else if blocks to reduce code duplication and improve readability and style guide adherence.

For example, the time_lock_delta parsing could be simplified to something like this:

// Parse time_lock_delta. If explicitly set, mark as specified.
var timeLockDeltaStr string
var timeLockDeltaSet bool
if ctx.IsSet("time_lock_delta") {
    timeLockDeltaStr = ctx.String("time_lock_delta")
    timeLockDeltaSet = true
} else if args.Present() {
    timeLockDeltaStr = args.First()
    timeLockDeltaSet = true
    args = args.Tail()
}

if timeLockDeltaSet {
    tld, err := parseTimeLockDelta(timeLockDeltaStr)
    if err != nil {
        return err
    }
    timeLockDelta = uint32(tld)
    timeLockDeltaSpecified = true
}

A similar refactoring can be applied to the other argument parsing blocks.

References
  1. The style guide recommends adding a brief comment for each case in a switch statement and separating case blocks with a newline for better readability. The current implementation does not follow this. (link)


var (
Expand Down Expand Up @@ -2771,11 +2787,14 @@ func updateChannelPolicy(ctx *cli.Context) error {
createMissingEdge := ctx.Bool("create_missing_edge")

req := &lnrpc.PolicyUpdateRequest{
BaseFeeMsat: baseFee,
TimeLockDelta: uint32(timeLockDelta),
MaxHtlcMsat: ctx.Uint64("max_htlc_msat"),
InboundFee: inboundFee,
CreateMissingEdge: createMissingEdge,
BaseFeeMsat: baseFee,
BaseFeeMsatSpecified: baseFeeMsatSpecified,
TimeLockDelta: timeLockDelta,
TimeLockDeltaSpecified: timeLockDeltaSpecified,
FeeRateSpecified: feeRateSpecified,
MaxHtlcMsat: ctx.Uint64("max_htlc_msat"),
InboundFee: inboundFee,
CreateMissingEdge: createMissingEdge,
}

if ctx.IsSet("min_htlc_msat") {
Expand All @@ -2793,10 +2812,13 @@ func updateChannelPolicy(ctx *cli.Context) error {
}
}

if feeRate != 0 {
req.FeeRate = feeRate
} else if feeRatePpm != 0 {
req.FeeRatePpm = uint32(feeRatePpm)
// Set fee rate values if specified.
if feeRateSpecified {
if feeRate != 0 {
req.FeeRate = feeRate
} else if feeRatePpm != 0 {
req.FeeRatePpm = uint32(feeRatePpm)
}
}

resp, err := client.UpdateChannelPolicy(ctxc, req)
Expand Down
7 changes: 7 additions & 0 deletions docs/release-notes/release-notes-0.21.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,13 @@

## RPC Updates

* [Added `*_specified` boolean fields to `PolicyUpdateRequest`](https://github.com/lightningnetwork/lnd/pull/10500)
for `base_fee_msat`, `fee_rate`, and `time_lock_delta`. This enables truly
optional updates to channel policies - when a `*_specified` field is false,
the corresponding parameter is not modified, allowing users to update only
specific policy fields while preserving others. Previously, omitting a field
would reset it to zero (which was invalid for `time_lock_delta`).

## lncli Updates

## Breaking Changes
Expand Down
40 changes: 37 additions & 3 deletions lnrpc/lightning.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 19 additions & 3 deletions lnrpc/lightning.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4737,18 +4737,34 @@ message PolicyUpdateRequest {
}

// The base fee charged regardless of the number of milli-satoshis sent.
// Only applied if base_fee_msat_specified is true.
int64 base_fee_msat = 3;

// If true, base_fee_msat is applied. If false, the existing base fee is
// retained.
bool base_fee_msat_specified = 12;

// The effective fee rate in milli-satoshis. The precision of this value
// goes up to 6 decimal places, so 1e-6.
// goes up to 6 decimal places, so 1e-6. Only applied if fee_rate_specified
// is true.
double fee_rate = 4;

// The effective fee rate in micro-satoshis (parts per million).
// The effective fee rate in micro-satoshis (parts per million). Only
// applied if fee_rate_specified is true.
uint32 fee_rate_ppm = 9;

// The required timelock delta for HTLCs forwarded over the channel.
// If true, fee_rate or fee_rate_ppm is applied. If false, the existing fee
// rate is retained.
bool fee_rate_specified = 13;

// The required timelock delta for HTLCs forwarded over the channel. Only
// applied if time_lock_delta_specified is true.
uint32 time_lock_delta = 5;

// If true, time_lock_delta is applied. If false, the existing time lock
// delta is retained.
bool time_lock_delta_specified = 14;

// If set, the maximum HTLC size in milli-satoshis. If unset, the maximum
// HTLC will be unchanged.
uint64 max_htlc_msat = 6;
Expand Down
19 changes: 13 additions & 6 deletions routing/localchans/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,11 +372,15 @@ func (r *Manager) updateEdge(chanPoint wire.OutPoint,
return err
}

// Update forwarding fee scheme and required time lock delta.
edge.FeeBaseMSat = newSchema.BaseFee
edge.FeeProportionalMillionths = lnwire.MilliSatoshi(
newSchema.FeeRate,
)
// Update forwarding fee scheme if specified.
if newSchema.BaseFee != nil {
edge.FeeBaseMSat = *newSchema.BaseFee
}
if newSchema.FeeRate != nil {
edge.FeeProportionalMillionths = lnwire.MilliSatoshi(
*newSchema.FeeRate,
)
}

// If inbound fees are set, we update the edge with them.
err = fn.MapOptionZ(newSchema.InboundFee,
Expand All @@ -392,7 +396,10 @@ func (r *Manager) updateEdge(chanPoint wire.OutPoint,
return err
}

edge.TimeLockDelta = uint16(newSchema.TimeLockDelta)
// Update time lock delta if specified.
if newSchema.TimeLockDelta != nil {
edge.TimeLockDelta = uint16(*newSchema.TimeLockDelta)
}

// Retrieve negotiated channel htlc amt limits.
amtMin, amtMax, err := r.getHtlcAmtLimits(channel)
Expand Down
21 changes: 12 additions & 9 deletions routing/localchans/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,15 @@
localMultisigKey := localMultisigPrivKey.PubKey()
remoteMultisigPrivKey, _ := btcec.NewPrivateKey()
remoteMultisigKey := remoteMultisigPrivKey.PubKey()
baseFee := lnwire.MilliSatoshi(100)
feeRate := uint32(200)
timeLockDelta := uint32(80)
newPolicy := routing.ChannelPolicy{
FeeSchema: routing.FeeSchema{
BaseFee: 100,
FeeRate: 200,
BaseFee: &baseFee,
FeeRate: &feeRate,
},
TimeLockDelta: 80,
TimeLockDelta: &timeLockDelta,
MaxHTLC: 5000,
}

Expand All @@ -80,13 +83,13 @@
}

policy := chanPolicies[chanPointValid]
if policy.TimeLockDelta != newPolicy.TimeLockDelta {
if policy.TimeLockDelta != *newPolicy.TimeLockDelta {
t.Fatal("unexpected time lock delta")
}
if policy.BaseFee != newPolicy.BaseFee {
if policy.BaseFee != *newPolicy.BaseFee {
t.Fatal("unexpected base fee")
}
if uint32(policy.FeeRate) != newPolicy.FeeRate {
if uint32(policy.FeeRate) != *newPolicy.FeeRate {
t.Fatal("unexpected base fee")
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The error message here seems to be a copy-paste error. It should probably say 'unexpected fee rate' instead of 'unexpected base fee' to match the check being performed.

Suggested change
t.Fatal("unexpected base fee")
t.Fatal("unexpected fee rate")

}
if policy.MaxHTLC != newPolicy.MaxHTLC {
Expand All @@ -108,13 +111,13 @@
if !policy.MessageFlags.HasMaxHtlc() {
t.Fatal("expected max htlc flag")
}
if policy.TimeLockDelta != uint16(newPolicy.TimeLockDelta) {
if policy.TimeLockDelta != uint16(*newPolicy.TimeLockDelta) {

Check failure on line 114 in routing/localchans/manager_test.go

View workflow job for this annotation

GitHub Actions / Lint code 

the line is 85 characters long, which exceeds the maximum of 80 characters. (ll)
t.Fatal("unexpected time lock delta")
}
if policy.FeeBaseMSat != newPolicy.BaseFee {
if policy.FeeBaseMSat != *newPolicy.BaseFee {
t.Fatal("unexpected base fee")
}
if uint32(policy.FeeProportionalMillionths) != newPolicy.FeeRate {
if uint32(policy.FeeProportionalMillionths) != *newPolicy.FeeRate {

Check failure on line 120 in routing/localchans/manager_test.go

View workflow job for this annotation

GitHub Actions / Lint code 

the line is 91 characters long, which exceeds the maximum of 80 characters. (ll)
t.Fatal("unexpected base fee")
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The error message here seems to be a copy-paste error. It should probably say 'unexpected fee rate' instead of 'unexpected base fee' to match the check being performed.

Suggested change
t.Fatal("unexpected base fee")
t.Fatal("unexpected fee rate")

}
if policy.MaxHTLC != newPolicy.MaxHTLC {
Expand Down
Loading
Loading