Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 25 additions & 2 deletions cmd/lncli/cmd_open_channel.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,13 @@ var openChannelCommand = cli.Command{
Usage: "the number of satoshis the wallet should " +
"commit to the channel",
},
cli.BoolFlag{
Name: "fundmax",
Usage: "if set, the wallet will attempt to commit " +
"the maximum possible local amount to the " +
"channel. This must not be set at the same " +
"time as local_amt",
},
cli.Uint64Flag{
Name: "base_fee_msat",
Usage: "the base fee in milli-satoshis that will " +
Expand Down Expand Up @@ -292,6 +299,7 @@ func openChannel(ctx *cli.Context) error {
ZeroConf: ctx.Bool("zero_conf"),
ScidAlias: ctx.Bool("scid_alias"),
RemoteChanReserveSat: ctx.Uint64("remote_reserve_sats"),
FundMax: ctx.Bool("fundmax"),
}

switch {
Expand Down Expand Up @@ -350,8 +358,23 @@ func openChannel(ctx *cli.Context) error {
return fmt.Errorf("unable to decode local amt: %v", err)
}
args = args.Tail()
default:
return fmt.Errorf("local amt argument missing")
case !ctx.Bool("fundmax"):
return fmt.Errorf("either local_amt or fundmax must be " +
"specified")
}

// The fundmax flag is NOT allowed to be combined with local_amt above.
// It is allowed to be combined with push_amt, but only if explicitly
// set.
if ctx.Bool("fundmax") && req.LocalFundingAmount != 0 {
return fmt.Errorf("local amount cannot be set if attempting " +
"to commit the maximum amount out of the wallet")
}

// The fundmax flag is NOT allowed to be combined with the psbt flag.
if ctx.Bool("fundmax") && ctx.Bool("psbt") {
return fmt.Errorf("psbt cannot be set if attempting " +
"to commit the maximum amount out of the wallet")
}

if ctx.IsSet("push_amt") {
Expand Down
5 changes: 5 additions & 0 deletions docs/release-notes/release-notes-0.16.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
restore single channels](https://github.com/lightningnetwork/lnd/pull/7437)
to and from a file on disk.

* [Add a `fundmax` flag to `openchannel` to allow for the allocation of all
funds in a wallet](https://github.com/lightningnetwork/lnd/pull/6903) towards
a new channel opening.

## Watchtowers

* [Fix Address iterator
Expand Down Expand Up @@ -66,6 +70,7 @@ available](https://github.com/lightningnetwork/lnd/pull/7529).

* ardevd
* Elle Mouton
* hieblmi
* Oliver Gugger
* Tommy Volk
* Yong Yu
Expand Down
46 changes: 29 additions & 17 deletions funding/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,15 @@ type InitFundingMsg struct {
// peer.
MaxLocalCsv uint16

// FundUpToMaxAmt is the maximum amount to try to commit to. If set, the
// MinFundAmt field denotes the acceptable minimum amount to commit to,
// while trying to commit as many coins as possible up to this value.
FundUpToMaxAmt btcutil.Amount

// MinFundAmt must be set iff FundUpToMaxAmt is set. It denotes the
// minimum amount to commit to.
MinFundAmt btcutil.Amount

// ChanFunder is an optional channel funder that allows the caller to
// control exactly how the channel funding is carried out. If not
// specified, then the default chanfunding.WalletAssembler will be
Expand Down Expand Up @@ -4079,23 +4088,26 @@ func (f *Manager) handleInitFundingMsg(msg *InitFundingMsg) {
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: regarding the commit message, it feels like it could get some love :)


req := &lnwallet.InitFundingReserveMsg{
ChainHash: &msg.ChainHash,
PendingChanID: chanID,
NodeID: peerKey,
NodeAddr: msg.Peer.Address(),
SubtractFees: msg.SubtractFees,
LocalFundingAmt: localAmt,
RemoteFundingAmt: 0,
CommitFeePerKw: commitFeePerKw,
FundingFeePerKw: msg.FundingFeePerKw,
PushMSat: msg.PushAmt,
Flags: channelFlags,
MinConfs: msg.MinConfs,
CommitType: commitType,
ChanFunder: msg.ChanFunder,
ZeroConf: zeroConf,
OptionScidAlias: scid,
ScidAliasFeature: scidFeatureVal,
ChainHash: &msg.ChainHash,
PendingChanID: chanID,
NodeID: peerKey,
NodeAddr: msg.Peer.Address(),
SubtractFees: msg.SubtractFees,
LocalFundingAmt: localAmt,
RemoteFundingAmt: 0,
FundUpToMaxAmt: msg.FundUpToMaxAmt,
MinFundAmt: msg.MinFundAmt,
RemoteChanReserve: chanReserve,
CommitFeePerKw: commitFeePerKw,
FundingFeePerKw: msg.FundingFeePerKw,
PushMSat: msg.PushAmt,
Flags: channelFlags,
MinConfs: msg.MinConfs,
CommitType: commitType,
ChanFunder: msg.ChanFunder,
ZeroConf: zeroConf,
OptionScidAlias: scid,
ScidAliasFeature: scidFeatureVal,
}

reservation, err := f.cfg.Wallet.InitChannelReservation(req)
Expand Down
196 changes: 191 additions & 5 deletions funding/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ func openChannel(t *testing.T, alice, bob *testNode, localFundingAmt,
*wire.OutPoint, *wire.MsgTx) {

publ := fundChannel(
t, alice, bob, localFundingAmt, pushAmt, false, numConfs,
t, alice, bob, localFundingAmt, pushAmt, false, 0, 0, numConfs,
updateChan, announceChan, nil,
)
fundingOutPoint := &wire.OutPoint{
Expand All @@ -723,7 +723,8 @@ func openChannel(t *testing.T, alice, bob *testNode, localFundingAmt,
// fundChannel takes the funding process to the point where the funding
// transaction is confirmed on-chain. Returns the funding tx.
func fundChannel(t *testing.T, alice, bob *testNode, localFundingAmt,
pushAmt btcutil.Amount, subtractFees bool, numConfs uint32,
pushAmt btcutil.Amount, subtractFees bool, fundUpToMaxAmt,
minFundAmt btcutil.Amount, numConfs uint32, //nolint:unparam
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q: so this unparam allows us to not specify all function parameters when calling this function ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also the fundMax seems to be not used in this function ?

Copy link
Collaborator Author

@hieblmi hieblmi Mar 6, 2023

Choose a reason for hiding this comment

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

nolint:unparam was in place for numConfs which isn't used in this function either. But it seems like the linter isn't flagging it anymore so it could be removed together with the parameter in a separate commit.
I've removed the fundMax flag.

updateChan chan *lnrpc.OpenStatusUpdate, announceChan bool,
chanType *lnwire.ChannelType) *wire.MsgTx {

Expand All @@ -734,6 +735,8 @@ func fundChannel(t *testing.T, alice, bob *testNode, localFundingAmt,
TargetPubkey: bob.privKey.PubKey(),
ChainHash: *fundingNetParams.GenesisHash,
SubtractFees: subtractFees,
FundUpToMaxAmt: fundUpToMaxAmt,
MinFundAmt: minFundAmt,
LocalFundingAmt: localFundingAmt,
PushAmt: lnwire.NewMSatFromSatoshis(pushAmt),
FundingFeePerKw: 1000,
Expand Down Expand Up @@ -3730,7 +3733,7 @@ func TestFundingManagerFundAll(t *testing.T) {
// Initiate a fund channel, and inspect the funding tx.
pushAmt := btcutil.Amount(0)
fundingTx := fundChannel(
t, alice, bob, test.spendAmt, pushAmt, true, 1,
t, alice, bob, test.spendAmt, pushAmt, true, 0, 0, 1,
updateChan, true, nil,
)

Expand Down Expand Up @@ -3761,6 +3764,157 @@ func TestFundingManagerFundAll(t *testing.T) {
}
}

// TestFundingManagerFundMax tests that we can initiate a funding request to use
// the maximum allowed funds remaining in the wallet.
func TestFundingManagerFundMax(t *testing.T) {
t.Parallel()

// Helper function to create a test utxos
constructTestUtxos := func(values ...btcutil.Amount) []*lnwallet.Utxo {
var utxos []*lnwallet.Utxo
for _, value := range values {
utxos = append(utxos, &lnwallet.Utxo{
AddressType: lnwallet.WitnessPubKey,
Value: value,
PkScript: mock.CoinPkScript,
OutPoint: wire.OutPoint{
Hash: chainhash.Hash{},
Index: 0,
},
})
}

return utxos
}

tests := []struct {
name string
coins []*lnwallet.Utxo
fundUpToMaxAmt btcutil.Amount
minFundAmt btcutil.Amount
pushAmt btcutil.Amount
change bool
}{
{
// We will spend all the funds in the wallet, and expect
// no change output due to the dust limit.
coins: constructTestUtxos(
MaxBtcFundingAmount + 1,
),
fundUpToMaxAmt: MaxBtcFundingAmount,
minFundAmt: MinChanFundingSize,
pushAmt: 0,
change: false,
},
{
// We spend less than the funds in the wallet, so a
// change output should be created.
coins: constructTestUtxos(
2 * MaxBtcFundingAmount,
),
fundUpToMaxAmt: MaxBtcFundingAmount,
minFundAmt: MinChanFundingSize,
pushAmt: 0,
change: true,
},
{
// We spend less than the funds in the wallet when
// setting a smaller channel size, so a change output
// should be created.
coins: constructTestUtxos(
MaxBtcFundingAmount,
),
fundUpToMaxAmt: MaxBtcFundingAmount / 2,
minFundAmt: MinChanFundingSize,
pushAmt: 0,
change: true,
},
{
// We are using the entirety of two inputs for the
// funding of a channel, hence expect no change output.
coins: constructTestUtxos(
MaxBtcFundingAmount/2, MaxBtcFundingAmount/2,
),
fundUpToMaxAmt: MaxBtcFundingAmount,
minFundAmt: MinChanFundingSize,
pushAmt: 0,
change: false,
},
{
// We are using a fraction of two inputs for the funding
// of our channel, hence expect a change output.
coins: constructTestUtxos(
MaxBtcFundingAmount/2, MaxBtcFundingAmount/2,
),
fundUpToMaxAmt: MaxBtcFundingAmount / 2,
minFundAmt: MinChanFundingSize,
pushAmt: 0,
change: true,
},
{
// We are funding a channel with half of the balance in
// our wallet hence expect a change output. Furthermore
// we push half of the funding amount to the remote end
// which we expect to succeed.
coins: constructTestUtxos(MaxBtcFundingAmount),
fundUpToMaxAmt: MaxBtcFundingAmount / 2,
minFundAmt: MinChanFundingSize / 4,
pushAmt: MaxBtcFundingAmount / 4,
change: true,
},
}

for _, test := range tests {
test := test

t.Run(test.name, func(t *testing.T) {
t.Parallel()
// We set up our mock wallet to control a list of UTXOs
// that sum to more than the max channel size.
addFunds := func(fundingCfg *Config) {
wc := fundingCfg.Wallet.WalletController
mockWc, ok := wc.(*mock.WalletController)
if ok {
mockWc.Utxos = test.coins
}
}
alice, bob := setupFundingManagers(t, addFunds)
defer tearDownFundingManagers(t, alice, bob)

// We will consume the channel updates as we go, so no
// buffering is needed.
updateChan := make(chan *lnrpc.OpenStatusUpdate)

// Initiate a fund channel, and inspect the funding tx.
pushAmt := test.pushAmt
fundingTx := fundChannel(
t, alice, bob, 0, pushAmt, false,
test.fundUpToMaxAmt, test.minFundAmt, 1,
updateChan, true, nil,
)

// Check whether the expected change output is present.
if test.change {
require.EqualValues(t, 2, len(fundingTx.TxOut))
}

if !test.change {
require.EqualValues(t, 1, len(fundingTx.TxOut))
}

// Inputs should be all funds in the wallet.
require.Equal(t, len(test.coins), len(fundingTx.TxIn))

for i, txIn := range fundingTx.TxIn {
require.Equal(
t, test.coins[i].OutPoint,
txIn.PreviousOutPoint,
)
}
})
}
}

// TestGetUpfrontShutdown tests different combinations of inputs for getting a
// shutdown script. It varies whether the peer has the feature set, whether
// the user has provided a script and our local configuration to test that
Expand Down Expand Up @@ -4212,8 +4366,8 @@ func TestFundingManagerZeroConf(t *testing.T) {

// Call fundChannel with the zero-conf ChannelType.
fundingTx := fundChannel(
t, alice, bob, fundingAmt, pushAmt, false, 1, updateChan, true,
&channelType,
t, alice, bob, fundingAmt, pushAmt, false, 0, 0, 1, updateChan,
true, &channelType,
)
fundingOp := &wire.OutPoint{
Hash: fundingTx.TxHash(),
Expand Down Expand Up @@ -4309,3 +4463,35 @@ func TestFundingManagerZeroConf(t *testing.T) {
// have been deleted from the database, as the channel is announced.
assertNoFwdingPolicy(t, alice, bob, fundingOp)
}

// TestCommitmentTypeFundmaxSanityCheck was introduced as a way of reminding
// developers of new channel commitment types to also consider the channel
// opening behavior with a specified fundmax flag. To give a hypothetical
// example, if ANCHOR types had been introduced after the fundmax flag had been
// activated, the developer would have had to code for the anchor reserve in the
// funding manager in the context of public and private channels. Otherwise
// inconsistent bahvior would have resulted when specifying fundmax for
// different types of channel openings.
// To ensure consistency this test compares a map of locally defined channel
// commitment types to the list of channel types that are defined in the proto
// files. It fails if the proto files contain additional commitment types. Once
// the developer considered the new channel type behavior it can be added in
// this test to the map `allCommitmentTypes`.
func TestCommitmentTypeFundmaxSanityCheck(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know if this test is that useful?

Copy link
Collaborator Author

@hieblmi hieblmi Mar 2, 2023

Choose a reason for hiding this comment

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

I added this test-case because it was envisaged in the original PR: #4029 (comment)

As the test documentation lays out it helps developers of new commitment types be reminded to consider the 'fundmax' behavior of their implementation, e.g. provide additional reserves or similar constraints.

Does that make sense?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think its a good idea to have such a check, or we include a reserve for all types except the one we no don't need one ?

t.Parallel()
allCommitmentTypes := map[string]int{
"UNKNOWN_COMMITMENT_TYPE": 0,
"LEGACY": 1,
"STATIC_REMOTE_KEY": 2,
"ANCHORS": 3,
"SCRIPT_ENFORCED_LEASE": 4,
}

for commitmentType := range lnrpc.CommitmentType_value {
if _, ok := allCommitmentTypes[commitmentType]; !ok {
t.Fatalf("Commitment type %s hasn't been considered "+
"in the context of the --fundmax flag for "+
"channel openings.", commitmentType)
}
}
}
4 changes: 4 additions & 0 deletions itest/list_on_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -519,4 +519,8 @@ var allTestCases = []*lntest.TestCase{
Name: "watchtower session management",
TestFunc: testWatchtowerSessionManagement,
},
{
Name: "channel fundmax",
TestFunc: testChannelFundMax,
},
}
Loading