lnrpc+lncli: deprecate sat_per_byte and add sat_per_vbyte

This commit deprecates/replaces the old field `sat_per_byte` with
`sat_per_vbyte`. While the old field suggests sat per byte, it’s
actually using sat per virtual byte. We use the Hidden param to hide all
the deprecated flags. These flags won't show up in help menu onwards,
while stay valid that can be passed from cli. Thus bash scripts
referencing these fields won't be broken.

cmd/lncli/cmd_open_channel.go

@@ -84,7 +84,7 @@ var openChannelCommand = cli.Command{
 	another address.
 
 	One can manually set the fee to be used for the funding transaction via either
-	the --conf_target or --sat_per_byte arguments. This is optional.`,
+	the --conf_target or --sat_per_vbyte arguments. This is optional.`,
 	ArgsUsage: "node-key local-amt push-amt",
 	Flags: []cli.Flag{
 		cli.StringFlag{
@@ -119,7 +119,12 @@ var openChannelCommand = cli.Command{
 				"used for fee estimation",
 		},
 		cli.Int64Flag{
-			Name: "sat_per_byte",
+			Name:   "sat_per_byte",
+			Usage:  "Deprecated, use sat_per_vbyte instead.",
+			Hidden: true,
+		},
+		cli.Int64Flag{
+			Name: "sat_per_vbyte",
 			Usage: "(optional) a manual fee expressed in " +
 				"sat/vbyte that should be used when crafting " +
 				"the transaction",
@@ -214,10 +219,19 @@ func openChannel(ctx *cli.Context) error {
 		return nil
 	}
 
+	// Check that only the field sat_per_vbyte or the deprecated field
+	// sat_per_byte is used.
+	feeRateFlag, err := checkNotBothSet(
+		ctx, "sat_per_vbyte", "sat_per_byte",
+	)
+	if err != nil {
+		return err
+	}
+
 	minConfs := int32(ctx.Uint64("min_confs"))
 	req := &lnrpc.OpenChannelRequest{
 		TargetConf:                 int32(ctx.Int64("conf_target")),
-		SatPerByte:                 ctx.Int64("sat_per_byte"),
+		SatPerVbyte:                ctx.Uint64(feeRateFlag),
 		MinHtlcMsat:                ctx.Int64("min_htlc_msat"),
 		RemoteCsvDelay:             uint32(ctx.Uint64("remote_csv_delay")),
 		MinConfs:                   minConfs,
@@ -730,7 +744,7 @@ func checkPsbtFlags(req *lnrpc.OpenChannelRequest) error {
 		return fmt.Errorf("specifying minimum confirmations for PSBT " +
 			"funding is not supported")
 	}
-	if req.TargetConf != 0 || req.SatPerByte != 0 {
+	if req.TargetConf != 0 || req.SatPerByte != 0 || req.SatPerVbyte != 0 {
 		return fmt.Errorf("setting fee estimation parameters not " +
 			"supported for PSBT funding")
 	}

cmd/lncli/commands.go

@@ -225,7 +225,7 @@ var sendCoinsCommand = cli.Command{
 	Send amt coins in satoshis to the base58 or bech32 encoded bitcoin address addr.
 
 	Fees used when sending the transaction can be specified via the --conf_target, or
-	--sat_per_byte optional flags.
+	--sat_per_vbyte optional flags.
 
 	Positional arguments and flags can be used interchangeably but not at the same time!
 	`,
@@ -253,7 +253,12 @@ var sendCoinsCommand = cli.Command{
 				"used for fee estimation",
 		},
 		cli.Int64Flag{
-			Name: "sat_per_byte",
+			Name:   "sat_per_byte",
+			Usage:  "Deprecated, use sat_per_vbyte instead.",
+			Hidden: true,
+		},
+		cli.Int64Flag{
+			Name: "sat_per_vbyte",
 			Usage: "(optional) a manual fee expressed in " +
 				"sat/vbyte that should be used when crafting " +
 				"the transaction",
@@ -284,9 +289,20 @@ func sendCoins(ctx *cli.Context) error {
 		return nil
 	}
 
-	if ctx.IsSet("conf_target") && ctx.IsSet("sat_per_byte") {
-		return fmt.Errorf("either conf_target or sat_per_byte should be " +
-			"set, but not both")
+	// Check that only the field sat_per_vbyte or the deprecated field
+	// sat_per_byte is used.
+	feeRateFlag, err := checkNotBothSet(
+		ctx, "sat_per_vbyte", "sat_per_byte",
+	)
+	if err != nil {
+		return err
+	}
+
+	// Only fee rate flag or conf_target should be set, not both.
+	if _, err := checkNotBothSet(
+		ctx, feeRateFlag, "conf_target",
+	); err != nil {
+		return err
 	}
 
 	switch {
@@ -324,7 +340,7 @@ func sendCoins(ctx *cli.Context) error {
 		Addr:             addr,
 		Amount:           amt,
 		TargetConf:       int32(ctx.Int64("conf_target")),
-		SatPerByte:       ctx.Int64("sat_per_byte"),
+		SatPerVbyte:      ctx.Uint64(feeRateFlag),
 		SendAll:          ctx.Bool("sweepall"),
 		Label:            ctx.String(txLabelFlag.Name),
 		MinConfs:         minConfs,
@@ -464,7 +480,7 @@ var sendManyCommand = cli.Command{
 	Name:      "sendmany",
 	Category:  "On-chain",
 	Usage:     "Send bitcoin on-chain to multiple addresses.",
-	ArgsUsage: "send-json-string [--conf_target=N] [--sat_per_byte=P]",
+	ArgsUsage: "send-json-string [--conf_target=N] [--sat_per_vbyte=P]",
 	Description: `
 	Create and broadcast a transaction paying the specified amount(s) to the passed address(es).
 
@@ -480,9 +496,15 @@ var sendManyCommand = cli.Command{
 				"confirm in, will be used for fee estimation",
 		},
 		cli.Int64Flag{
-			Name: "sat_per_byte",
-			Usage: "(optional) a manual fee expressed in sat/vbyte that should be " +
-				"used when crafting the transaction",
+			Name:   "sat_per_byte",
+			Usage:  "Deprecated, use sat_per_vbyte instead.",
+			Hidden: true,
+		},
+		cli.Int64Flag{
+			Name: "sat_per_vbyte",
+			Usage: "(optional) a manual fee expressed in " +
+				"sat/vbyte that should be used when crafting " +
+				"the transaction",
 		},
 		cli.Uint64Flag{
 			Name: "min_confs",
@@ -505,9 +527,20 @@ func sendMany(ctx *cli.Context) error {
 		return err
 	}
 
-	if ctx.IsSet("conf_target") && ctx.IsSet("sat_per_byte") {
-		return fmt.Errorf("either conf_target or sat_per_byte should be " +
-			"set, but not both")
+	// Check that only the field sat_per_vbyte or the deprecated field
+	// sat_per_byte is used.
+	feeRateFlag, err := checkNotBothSet(
+		ctx, "sat_per_vbyte", "sat_per_byte",
+	)
+	if err != nil {
+		return err
+	}
+
+	// Only fee rate flag or conf_target should be set, not both.
+	if _, err := checkNotBothSet(
+		ctx, feeRateFlag, "conf_target",
+	); err != nil {
+		return err
 	}
 
 	client, cleanUp := getClient(ctx)
@@ -517,7 +550,7 @@ func sendMany(ctx *cli.Context) error {
 	txid, err := client.SendMany(ctxc, &lnrpc.SendManyRequest{
 		AddrToAmount:     amountToAddr,
 		TargetConf:       int32(ctx.Int64("conf_target")),
-		SatPerByte:       ctx.Int64("sat_per_byte"),
+		SatPerVbyte:      ctx.Uint64(feeRateFlag),
 		Label:            ctx.String(txLabelFlag.Name),
 		MinConfs:         minConfs,
 		SpendUnconfirmed: minConfs == 0,
@@ -651,7 +684,7 @@ var closeChannelCommand = cli.Command{
 
 	In the case of a cooperative closure, one can manually set the fee to
 	be used for the closing transaction via either the --conf_target or
-	--sat_per_byte arguments. This will be the starting value used during
+	--sat_per_vbyte arguments. This will be the starting value used during
 	fee negotiation. This is optional.
 
 	In the case of a cooperative closure, one can manually set the address
@@ -690,7 +723,12 @@ var closeChannelCommand = cli.Command{
 				"lnd config will be used.",
 		},
 		cli.Int64Flag{
-			Name: "sat_per_byte",
+			Name:   "sat_per_byte",
+			Usage:  "Deprecated, use sat_per_vbyte instead.",
+			Hidden: true,
+		},
+		cli.Int64Flag{
+			Name: "sat_per_vbyte",
 			Usage: "(optional) a manual fee expressed in " +
 				"sat/vbyte that should be used when crafting " +
 				"the transaction",
@@ -717,6 +755,15 @@ func closeChannel(ctx *cli.Context) error {
 		return nil
 	}
 
+	// Check that only the field sat_per_vbyte or the deprecated field
+	// sat_per_byte is used.
+	feeRateFlag, err := checkNotBothSet(
+		ctx, "sat_per_vbyte", "sat_per_byte",
+	)
+	if err != nil {
+		return err
+	}
+
 	channelPoint, err := parseChannelPoint(ctx)
 	if err != nil {
 		return err
@@ -727,7 +774,7 @@ func closeChannel(ctx *cli.Context) error {
 		ChannelPoint:    channelPoint,
 		Force:           ctx.Bool("force"),
 		TargetConf:      int32(ctx.Int64("conf_target")),
-		SatPerByte:      ctx.Int64("sat_per_byte"),
+		SatPerVbyte:     ctx.Uint64(feeRateFlag),
 		DeliveryAddress: ctx.String("delivery_addr"),
 	}
 
@@ -824,7 +871,7 @@ var closeAllChannelsCommand = cli.Command{
 
 	In the case of cooperative closures, one can manually set the fee to
 	be used for the closing transactions via either the --conf_target or
-	--sat_per_byte arguments. This will be the starting value used during
+	--sat_per_vbyte arguments. This will be the starting value used during
 	fee negotiation. This is optional.`,
 	Flags: []cli.Flag{
 		cli.BoolFlag{
@@ -843,7 +890,12 @@ var closeAllChannelsCommand = cli.Command{
 				"used for fee estimation",
 		},
 		cli.Int64Flag{
-			Name: "sat_per_byte",
+			Name:   "sat_per_byte",
+			Usage:  "Deprecated, use sat_per_vbyte instead.",
+			Hidden: true,
+		},
+		cli.Int64Flag{
+			Name: "sat_per_vbyte",
 			Usage: "(optional) a manual fee expressed in " +
 				"sat/vbyte that should be used when crafting " +
 				"the closing transactions",
@@ -857,6 +909,15 @@ func closeAllChannels(ctx *cli.Context) error {
 	client, cleanUp := getClient(ctx)
 	defer cleanUp()
 
+	// Check that only the field sat_per_vbyte or the deprecated field
+	// sat_per_byte is used.
+	feeRateFlag, err := checkNotBothSet(
+		ctx, "sat_per_vbyte", "sat_per_byte",
+	)
+	if err != nil {
+		return err
+	}
+
 	listReq := &lnrpc.ListChannelsRequest{}
 	openChannels, err := client.ListChannels(ctxc, listReq)
 	if err != nil {
@@ -983,9 +1044,9 @@ func closeAllChannels(ctx *cli.Context) error {
 					},
 					OutputIndex: uint32(index),
 				},
-				Force:      !channel.GetActive(),
-				TargetConf: int32(ctx.Int64("conf_target")),
-				SatPerByte: ctx.Int64("sat_per_byte"),
+				Force:       !channel.GetActive(),
+				TargetConf:  int32(ctx.Int64("conf_target")),
+				SatPerVbyte: ctx.Uint64(feeRateFlag),
 			}
 
 			txidChan := make(chan string, 1)

cmd/lncli/main.go

@@ -240,6 +240,23 @@ func extractPathArgs(ctx *cli.Context) (string, string, error) {
 	return tlsCertPath, macPath, nil
 }
 
+// checkNotBothSet accepts two flag names, a and b, and checks that only flag a
+// or flag b can be set, but not both. It returns the name of the flag or an
+// error.
+func checkNotBothSet(ctx *cli.Context, a, b string) (string, error) {
+	if ctx.IsSet(a) && ctx.IsSet(b) {
+		return "", fmt.Errorf(
+			"either %s or %s should be set, but not both", a, b,
+		)
+	}
+
+	if ctx.IsSet(a) {
+		return a, nil
+	}
+
+	return b, nil
+}
+
 func main() {
 	app := cli.NewApp()
 	app.Name = "lncli"

cmd/lncli/walletrpc_active.go

@@ -88,8 +88,8 @@ func pendingSweeps(ctx *cli.Context) error {
 
 	// Sort them in ascending fee rate order for display purposes.
 	sort.Slice(resp.PendingSweeps, func(i, j int) bool {
-		return resp.PendingSweeps[i].SatPerByte <
-			resp.PendingSweeps[j].SatPerByte
+		return resp.PendingSweeps[i].SatPerVbyte <
+			resp.PendingSweeps[j].SatPerVbyte
 	})
 
 	var pendingSweepsResp = struct {
@@ -134,7 +134,7 @@ var bumpFeeCommand = cli.Command{
 	fee transaction that is under the control of the wallet.
 
 	A fee preference must be provided, either through the conf_target or
-	sat_per_byte parameters.
+	sat_per_vbyte parameters.
 
 	Note that this command currently doesn't perform any validation checks
 	on the fee preference being provided. For now, the responsibility of
@@ -153,7 +153,12 @@ var bumpFeeCommand = cli.Command{
 				"be swept on-chain within",
 		},
 		cli.Uint64Flag{
-			Name: "sat_per_byte",
+			Name:   "sat_per_byte",
+			Usage:  "Deprecated, use sat_per_vbyte instead.",
+			Hidden: true,
+		},
+		cli.Uint64Flag{
+			Name: "sat_per_vbyte",
 			Usage: "a manual fee expressed in sat/vbyte that " +
 				"should be used when sweeping the output",
 		},
@@ -174,6 +179,15 @@ func bumpFee(ctx *cli.Context) error {
 		return cli.ShowCommandHelp(ctx, "bumpfee")
 	}
 
+	// Check that only the field sat_per_vbyte or the deprecated field
+	// sat_per_byte is used.
+	feeRateFlag, err := checkNotBothSet(
+		ctx, "sat_per_vbyte", "sat_per_byte",
+	)
+	if err != nil {
+		return err
+	}
+
 	// Validate and parse the relevant arguments/flags.
 	protoOutPoint, err := NewProtoOutPoint(ctx.Args().Get(0))
 	if err != nil {
@@ -184,10 +198,10 @@ func bumpFee(ctx *cli.Context) error {
 	defer cleanUp()
 
 	resp, err := client.BumpFee(ctxc, &walletrpc.BumpFeeRequest{
-		Outpoint:   protoOutPoint,
-		TargetConf: uint32(ctx.Uint64("conf_target")),
-		SatPerByte: uint32(ctx.Uint64("sat_per_byte")),
-		Force:      ctx.Bool("force"),
+		Outpoint:    protoOutPoint,
+		TargetConf:  uint32(ctx.Uint64("conf_target")),
+		SatPerVbyte: ctx.Uint64(feeRateFlag),
+		Force:       ctx.Bool("force"),
 	})
 	if err != nil {
 		return err
@@ -216,7 +230,12 @@ var bumpCloseFeeCommand = cli.Command{
 				"be swept on-chain within",
 		},
 		cli.Uint64Flag{
-			Name: "sat_per_byte",
+			Name:   "sat_per_byte",
+			Usage:  "Deprecated, use sat_per_vbyte instead.",
+			Hidden: true,
+		},
+		cli.Uint64Flag{
+			Name: "sat_per_vbyte",
 			Usage: "a manual fee expressed in sat/vbyte that " +
 				"should be used when sweeping the output",
 		},
@@ -233,9 +252,18 @@ func bumpCloseFee(ctx *cli.Context) error {
 		return cli.ShowCommandHelp(ctx, "bumpclosefee")
 	}
 
+	// Check that only the field sat_per_vbyte or the deprecated field
+	// sat_per_byte is used.
+	feeRateFlag, err := checkNotBothSet(
+		ctx, "sat_per_vbyte", "sat_per_byte",
+	)
+	if err != nil {
+		return err
+	}
+
 	// Validate the channel point.
 	channelPoint := ctx.Args().Get(0)
-	_, err := NewProtoOutPoint(channelPoint)
+	_, err = NewProtoOutPoint(channelPoint)
 	if err != nil {
 		return err
 	}
@@ -291,10 +319,10 @@ func bumpCloseFee(ctx *cli.Context) error {
 			sweepTxID, sweep.Outpoint.OutputIndex)
 
 		_, err = walletClient.BumpFee(ctxc, &walletrpc.BumpFeeRequest{
-			Outpoint:   sweep.Outpoint,
-			TargetConf: uint32(ctx.Uint64("conf_target")),
-			SatPerByte: uint32(ctx.Uint64("sat_per_byte")),
-			Force:      true,
+			Outpoint:    sweep.Outpoint,
+			TargetConf:  uint32(ctx.Uint64("conf_target")),
+			SatPerVbyte: ctx.Uint64(feeRateFlag),
+			Force:       true,
 		})
 		if err != nil {
 			return err
@@ -595,7 +623,7 @@ func fundPsbt(ctx *cli.Context) error {
 
 	case ctx.Uint64("sat_per_vbyte") > 0:
 		req.Fees = &walletrpc.FundPsbtRequest_SatPerVbyte{
-			SatPerVbyte: uint32(ctx.Uint64("sat_per_vbyte")),
+			SatPerVbyte: ctx.Uint64("sat_per_vbyte"),
 		}
 	}
 

cmd/lncli/walletrpc_types.go

@@ -5,29 +5,29 @@ import "github.com/lightningnetwork/lnd/lnrpc/walletrpc"
 // PendingSweep is a CLI-friendly type of the walletrpc.PendingSweep proto. We
 // use this to show more useful string versions of byte slices and enums.
 type PendingSweep struct {
-	OutPoint            OutPoint `json:"outpoint"`
-	WitnessType         string   `json:"witness_type"`
-	AmountSat           uint32   `json:"amount_sat"`
-	SatPerByte          uint32   `json:"sat_per_byte"`
-	BroadcastAttempts   uint32   `json:"broadcast_attempts"`
-	NextBroadcastHeight uint32   `json:"next_broadcast_height"`
-	RequestedSatPerByte uint32   `json:"requested_sat_per_byte"`
-	RequestedConfTarget uint32   `json:"requested_conf_target"`
-	Force               bool     `json:"force"`
+	OutPoint             OutPoint `json:"outpoint"`
+	WitnessType          string   `json:"witness_type"`
+	AmountSat            uint32   `json:"amount_sat"`
+	SatPerVByte          uint32   `json:"sat_per_vbyte"`
+	BroadcastAttempts    uint32   `json:"broadcast_attempts"`
+	NextBroadcastHeight  uint32   `json:"next_broadcast_height"`
+	RequestedSatPerVByte uint32   `json:"requested_sat_per_vbyte"`
+	RequestedConfTarget  uint32   `json:"requested_conf_target"`
+	Force                bool     `json:"force"`
 }
 
 // NewPendingSweepFromProto converts the walletrpc.PendingSweep proto type into
 // its corresponding CLI-friendly type.
 func NewPendingSweepFromProto(pendingSweep *walletrpc.PendingSweep) *PendingSweep {
 	return &PendingSweep{
-		OutPoint:            NewOutPointFromProto(pendingSweep.Outpoint),
-		WitnessType:         pendingSweep.WitnessType.String(),
-		AmountSat:           pendingSweep.AmountSat,
-		SatPerByte:          pendingSweep.SatPerByte,
-		BroadcastAttempts:   pendingSweep.BroadcastAttempts,
-		NextBroadcastHeight: pendingSweep.NextBroadcastHeight,
-		RequestedSatPerByte: pendingSweep.RequestedSatPerByte,
-		RequestedConfTarget: pendingSweep.RequestedConfTarget,
-		Force:               pendingSweep.Force,
+		OutPoint:             NewOutPointFromProto(pendingSweep.Outpoint),
+		WitnessType:          pendingSweep.WitnessType.String(),
+		AmountSat:            pendingSweep.AmountSat,
+		SatPerVByte:          uint32(pendingSweep.SatPerVbyte),
+		BroadcastAttempts:    pendingSweep.BroadcastAttempts,
+		NextBroadcastHeight:  pendingSweep.NextBroadcastHeight,
+		RequestedSatPerVByte: uint32(pendingSweep.RequestedSatPerVbyte),
+		RequestedConfTarget:  pendingSweep.RequestedConfTarget,
+		Force:                pendingSweep.Force,
 	}
 }