Skip to content

add sats_per_kweight option when crafting a transaction (continue) #10067

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
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

add sats_per_kweight option when crafting a transaction (continue) #10067

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6e31518
lnrpc,lnd,lncli,itest,lntest: add sat/kw option
ziggie1984 Mar 4, 2023
6747a95
walletrpc: add sat_per_kweight option
ziggie1984 Mar 4, 2023
fb96b96
funding: add sat_per_kweight option
ziggie1984 Feb 23, 2023
7f81409
itest+lntest: add tests for sat_per_kweight option
ziggie1984 Mar 4, 2023
6fe4d00
lncli: add sat_per_kweight option
ziggie1984 Feb 23, 2023
0613a60
docs: add release-notes.
ziggie1984 Oct 31, 2023
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
53 changes: 38 additions & 15 deletions cmd/lncli/cmd_open_channel.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ var openChannelCommand = cli.Command{
close out to another address.

One can manually set the fee to be used for the funding transaction via
either the --conf_target or --sat_per_vbyte arguments. This is
optional.

--conf_target, --sat_per_vbyte or --sat_per_kweight arguments.
This is optional.
One can also specify a short string memo to record some useful
information about the channel using the --memo argument. This is stored
locally only, and is purely for reference. It has no bearing on the
Expand Down Expand Up @@ -166,17 +166,19 @@ var openChannelCommand = cli.Command{
"transaction *should* confirm in, will be " +
"used for fee estimation",
},
cli.Int64Flag{
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: "sat_per_kweight",
Usage: "(optional) a manual fee expressed in " +
"sat/kweight that should be used when " +
"crafting the transaction. As an example " +
"250 sat/kw equals 1 sat/vbyte",
},
cli.BoolFlag{
Name: "private",
Usage: "make the channel private, such that it won't " +
Expand Down Expand Up @@ -302,10 +304,10 @@ 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",
// Check that only the sat_per_vbyte or
// sat_per_kweight is used.
_, err = checkNotBothSet(
ctx, "sat_per_vbyte", "sat_per_kweight",
)
if err != nil {
return err
Expand All @@ -314,7 +316,7 @@ func openChannel(ctx *cli.Context) error {
minConfs := int32(ctx.Uint64("min_confs"))
req := &lnrpc.OpenChannelRequest{
TargetConf: int32(ctx.Int64("conf_target")),
SatPerVbyte: ctx.Uint64(feeRateFlag),
SatPerVbyte: ctx.Uint64("sat_per_vbyte"),
MinHtlcMsat: ctx.Int64("min_htlc_msat"),
RemoteCsvDelay: uint32(ctx.Uint64("remote_csv_delay")),
MinConfs: minConfs,
Expand All @@ -327,6 +329,7 @@ func openChannel(ctx *cli.Context) error {
RemoteChanReserveSat: ctx.Uint64("remote_reserve_sats"),
FundMax: ctx.Bool("fundmax"),
Memo: ctx.String("memo"),
SatPerKweight: ctx.Uint64("sat_per_kweight"),
}

switch {
Expand Down Expand Up @@ -788,7 +791,8 @@ var batchOpenChannelCommand = cli.Command{
close out to another address.

One can manually set the fee to be used for the funding transaction via
either the --conf_target or --sat_per_vbyte arguments. This is optional.
--conf_target, --sat_per_vbyte or --sat_per_kweight arguments.
This is optional.
`,
ArgsUsage: "channels-json",
Flags: []cli.Flag{
Expand All @@ -804,6 +808,13 @@ var batchOpenChannelCommand = cli.Command{
"sat/vByte that should be used when crafting " +
"the transaction",
},
cli.Uint64Flag{
Name: "sat_per_kweight",
Usage: "(optional) a manual fee expressed in " +
"sat/kweight that should be used when " +
"crafting the transaction. As an example " +
"250 sat/kw equals 1 sat/vbyte",
},
cli.Uint64Flag{
Name: "min_confs",
Usage: "(optional) the minimum number of " +
Expand Down Expand Up @@ -845,13 +856,23 @@ func batchOpenChannel(ctx *cli.Context) error {
return nil
}

// Check that not both the sat_per_kweight or sat_per_vbyte field
// is used.
_, err := checkNotBothSet(
ctx, "sat_per_vbyte", "sat_per_kweight",
)
if err != nil {
return err
}
Comment on lines +861 to +866
Copy link

Choose a reason for hiding this comment

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

medium

Similar to the openchannel command, the batchopenchannel command should also validate that conf_target is not used at the same time as sat_per_vbyte or sat_per_kweight to avoid ambiguity in fee specification.

// Make sure only one fee preference is specified.
	feeRate, err := checkNotBothSet(
		ctx, "conf_target", "sat_per_vbyte",
	)
	if err != nil {
		return err
	}
	_, err = checkNotBothSet(
		ctx, feeRate, "sat_per_kweight",
	)
	if err != nil {
		return err
	}


minConfs := int32(ctx.Uint64("min_confs"))
req := &lnrpc.BatchOpenChannelRequest{
TargetConf: int32(ctx.Int64("conf_target")),
SatPerVbyte: int64(ctx.Uint64("sat_per_vbyte")),
MinConfs: minConfs,
SpendUnconfirmed: minConfs == 0,
Label: ctx.String("label"),
SatPerKweight: ctx.Uint64("sat_per_kweight"),
}

// Let's try and parse the JSON part of the CLI now. Fortunately we can
Expand Down Expand Up @@ -1051,7 +1072,9 @@ 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 || req.SatPerVbyte != 0 { // nolint:staticcheck
if req.TargetConf != 0 || req.SatPerKweight != 0 ||
req.SatPerVbyte != 0 {

return fmt.Errorf("setting fee estimation parameters not " +
"supported for PSBT funding")
}
Expand Down
86 changes: 48 additions & 38 deletions cmd/lncli/commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,8 @@ var sendCoinsCommand = cli.Command{
Description: `
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_vbyte optional flags.
Fees used when sending the transaction can be specified via the
--conf_target, --sat_per_vbyte or --sat_per_kweight optional flags.

Positional arguments and flags can be used interchangeably but not at the same time!
`,
Expand All @@ -287,17 +287,19 @@ var sendCoinsCommand = cli.Command{
"transaction *should* confirm in, will be " +
"used for fee estimation",
},
cli.Int64Flag{
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: "sat_per_kweight",
Usage: "(optional) a manual fee expressed in " +
"sat/kweight that should be used when " +
"crafting the transaction. As an example " +
"250 sat/kw equals 1 sat/vbyte",
},
cli.Uint64Flag{
Name: "min_confs",
Usage: "(optional) the minimum number of confirmations " +
Expand Down Expand Up @@ -332,10 +334,10 @@ func sendCoins(ctx *cli.Context) error {
return nil
}

// Check that only the field sat_per_vbyte or the deprecated field
// sat_per_byte is used.
// Check that only the field sat_per_vbyte or the field
// sat_per_kweight is used.
feeRateFlag, err := checkNotBothSet(
ctx, "sat_per_vbyte", "sat_per_byte",
ctx, "sat_per_vbyte", "sat_per_kweight",
)
if err != nil {
return err
Expand Down Expand Up @@ -417,6 +419,7 @@ func sendCoins(ctx *cli.Context) error {
Label: ctx.String(txLabelFlag.Name),
MinConfs: minConfs,
SpendUnconfirmed: minConfs == 0,
SatPerKweight: ctx.Uint64("sat_per_kweight"),
}
txid, err := client.SendCoins(ctxc, req)
if err != nil {
Expand Down Expand Up @@ -549,10 +552,11 @@ func listUnspent(ctx *cli.Context) error {
}

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_vbyte=P]",
Name: "sendmany",
Category: "On-chain",
Usage: "Send bitcoin on-chain to multiple addresses.",
ArgsUsage: "send-json-string [--conf_target=N] [--sat_per_vbyte=P] " +
"[--sat_per_kweight=R]",
Description: `
Create and broadcast a transaction paying the specified amount(s) to the passed address(es).

Expand All @@ -567,17 +571,19 @@ var sendManyCommand = cli.Command{
Usage: "(optional) the number of blocks that the transaction *should* " +
"confirm in, will be used for fee estimation",
},
cli.Int64Flag{
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: "sat_per_kweight",
Usage: "(optional) a manual fee expressed in " +
"sat/kweight that should be used when " +
"crafting the transaction. As an example " +
"250 sat/kw equals 1 sat/vbyte",
},
cli.Uint64Flag{
Name: "min_confs",
Usage: "(optional) the minimum number of confirmations " +
Expand All @@ -599,10 +605,10 @@ func sendMany(ctx *cli.Context) error {
return err
}

// Check that only the field sat_per_vbyte or the deprecated field
// sat_per_byte is used.
// Check that only the field sat_per_vbyte or the field
// sat_per_kweight is used.
feeRateFlag, err := checkNotBothSet(
ctx, "sat_per_vbyte", "sat_per_byte",
ctx, "sat_per_vbyte", "sat_per_kweight",
)
if err != nil {
return err
Expand All @@ -626,6 +632,7 @@ func sendMany(ctx *cli.Context) error {
Label: ctx.String(txLabelFlag.Name),
MinConfs: minConfs,
SpendUnconfirmed: minConfs == 0,
SatPerKweight: ctx.Uint64("sat_per_kweight"),
})
if err != nil {
return err
Expand Down Expand Up @@ -756,12 +763,12 @@ var closeChannelCommand = cli.Command{
funds will be time locked for a few blocks before they can be spent.

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_vbyte arguments. This will be the starting value used during
fee negotiation. This is optional. The parameter --max_fee_rate in
comparison is the end boundary of the fee negotiation, if not specified
it's always x3 of the starting value. Increasing this value increases
the chance of a successful negotiation.
be used for the closing transaction via either the --conf_target,
--sat_per_kweight or --sat_per_vbyte arguments. This will be the
starting value used during fee negotiation. This is optional. The
parameter --max_fee_rate in comparison is the end boundary of the fee
negotiation, if not specified it's always x3 of the starting value.
Increasing this value increases the chance of a successful negotiation.

In the case of a cooperative closure, one can manually set the address
to deliver funds to upon closure. This is optional, and may only be used
Expand Down Expand Up @@ -804,18 +811,20 @@ var closeChannelCommand = cli.Command{
"then the conf-target value set in the main " +
"lnd config will be used.",
},
cli.Int64Flag{
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; default is a conf-target " +
"of 6 blocks",
},
cli.Uint64Flag{
Name: "sat_per_kweight",
Usage: "(optional) a manual fee expressed in " +
"sat/kweight that should be used when " +
"crafting the transaction. As an example " +
"250 sat/kw equals 1 sat/vbyte",
},
cli.StringFlag{
Name: "delivery_addr",
Usage: "(optional) an address to deliver funds " +
Expand Down Expand Up @@ -846,10 +855,10 @@ 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",
// Check that only the field sat_per_vbyte or the field
// sat_per_kweight is used.
_, err := checkNotBothSet(
ctx, "sat_per_vbyte", "sat_per_kweight",
)
if err != nil {
return err
Expand All @@ -865,7 +874,8 @@ func closeChannel(ctx *cli.Context) error {
ChannelPoint: channelPoint,
Force: ctx.Bool("force"),
TargetConf: int32(ctx.Int64("conf_target")),
SatPerVbyte: ctx.Uint64(feeRateFlag),
SatPerVbyte: ctx.Uint64("sat_per_vbyte"),
SatPerKweight: ctx.Uint64("sat_per_kweight"),
DeliveryAddress: ctx.String("delivery_addr"),
MaxFeePerVbyte: ctx.Uint64("max_fee_rate"),
}
Expand Down
Loading