Merge pull request #4908 from halseth/anchors-reserve-utxo

[anchors] Reserve wallet balance for anchor fee bumping

lntest/itest/lnd_onchain_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/btcsuite/btcd/txscript"
 	"github.com/btcsuite/btcutil"
@@ -11,7 +12,9 @@ import (
 	"github.com/lightningnetwork/lnd/lnrpc/walletrpc"
 	"github.com/lightningnetwork/lnd/lntest"
 	"github.com/lightningnetwork/lnd/lntest/wait"
+	"github.com/lightningnetwork/lnd/lnwallet"
 	"github.com/lightningnetwork/lnd/sweep"
+	"github.com/stretchr/testify/require"
 )
 
 // testCPFP ensures that the daemon can bump an unconfirmed  transaction's fee
@@ -22,7 +25,7 @@ func testCPFP(net *lntest.NetworkHarness, t *harnessTest) {
 	// Skip this test for neutrino, as it's not aware of mempool
 	// transactions.
 	if net.BackendCfg.Name() == "neutrino" {
-		t.Skipf("skipping reorg test for neutrino backend")
+		t.Skipf("skipping CPFP test for neutrino backend")
 	}
 
 	// We'll start the test by sending Alice some coins, which she'll use to
@@ -160,3 +163,198 @@ func testCPFP(net *lntest.NetworkHarness, t *harnessTest) {
 		t.Fatalf(err.Error())
 	}
 }
+
+// testAnchorReservedValue tests that we won't allow sending transactions when
+// that would take the value we reserve for anchor fee bumping out of our
+// wallet.
+func testAnchorReservedValue(net *lntest.NetworkHarness, t *harnessTest) {
+	// Start two nodes supporting anchor channels.
+	args := commitTypeAnchors.Args()
+	alice, err := net.NewNode("Alice", args)
+	require.NoError(t.t, err)
+
+	defer shutdownAndAssert(net, t, alice)
+
+	bob, err := net.NewNode("Bob", args)
+	require.NoError(t.t, err)
+
+	defer shutdownAndAssert(net, t, bob)
+
+	ctxb := context.Background()
+	ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
+	err = net.ConnectNodes(ctxt, alice, bob)
+	require.NoError(t.t, err)
+
+	// Send just enough coins for Alice to open a channel without a change output.
+	const (
+		chanAmt = 1000000
+		feeEst  = 8000
+	)
+
+	ctxt, _ = context.WithTimeout(context.Background(), defaultTimeout)
+	err = net.SendCoins(ctxt, chanAmt+feeEst, alice)
+	require.NoError(t.t, err)
+
+	// Alice opens a channel that would consume all the funds in her
+	// wallet, without a change output. This should not be allowed.
+	resErr := lnwallet.ErrReservedValueInvalidated.Error()
+
+	ctxt, _ = context.WithTimeout(context.Background(), defaultTimeout)
+	_, err = net.OpenChannel(
+		ctxt, alice, bob,
+		lntest.OpenChannelParams{
+			Amt: chanAmt,
+		},
+	)
+	if err == nil || !strings.Contains(err.Error(), resErr) {
+		t.Fatalf("expected failure, got: %v", err)
+	}
+
+	// Alice opens a smaller channel. This works since it will have a
+	// change output.
+	ctxt, _ = context.WithTimeout(context.Background(), defaultTimeout)
+	aliceChanPoint := openChannelAndAssert(
+		ctxt, t, net, alice, bob,
+		lntest.OpenChannelParams{
+			Amt: chanAmt / 2,
+		},
+	)
+
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+	err = alice.WaitForNetworkChannelOpen(ctxt, aliceChanPoint)
+	require.NoError(t.t, err)
+
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+	err = bob.WaitForNetworkChannelOpen(ctxt, aliceChanPoint)
+	require.NoError(t.t, err)
+
+	// Alice tries to send all coins to an internal address. This is
+	// allowed, since the final wallet balance will still be above the
+	// reserved value.
+	addrReq := &lnrpc.NewAddressRequest{
+		Type: lnrpc.AddressType_WITNESS_PUBKEY_HASH,
+	}
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+	resp, err := alice.NewAddress(ctxt, addrReq)
+	require.NoError(t.t, err)
+
+	sweepReq := &lnrpc.SendCoinsRequest{
+		Addr:    resp.Address,
+		SendAll: true,
+	}
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+	_, err = alice.SendCoins(ctxt, sweepReq)
+	require.NoError(t.t, err)
+
+	block := mineBlocks(t, net, 1, 1)[0]
+
+	// The sweep transaction should have exactly one input, the change from
+	// the previous SendCoins call.
+	sweepTx := block.Transactions[1]
+	if len(sweepTx.TxIn) != 1 {
+		t.Fatalf("expected 1 inputs instead have %v", len(sweepTx.TxIn))
+	}
+
+	// It should have a single output.
+	if len(sweepTx.TxOut) != 1 {
+		t.Fatalf("expected 1 output instead have %v", len(sweepTx.TxOut))
+	}
+
+	// Wait for Alice to see her balance as confirmed.
+	waitForConfirmedBalance := func() int64 {
+		var balance int64
+		err := wait.NoError(func() error {
+			req := &lnrpc.WalletBalanceRequest{}
+			ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+			resp, err := alice.WalletBalance(ctxt, req)
+			if err != nil {
+				return err
+			}
+
+			if resp.TotalBalance == 0 {
+				return fmt.Errorf("no balance")
+			}
+
+			if resp.UnconfirmedBalance > 0 {
+				return fmt.Errorf("unconfirmed balance")
+			}
+
+			balance = resp.TotalBalance
+			return nil
+		}, defaultTimeout)
+		require.NoError(t.t, err)
+
+		return balance
+	}
+
+	_ = waitForConfirmedBalance()
+
+	// Alice tries to send all funds to an external address, the reserved
+	// value must stay in her wallet.
+	minerAddr, err := net.Miner.NewAddress()
+	require.NoError(t.t, err)
+
+	sweepReq = &lnrpc.SendCoinsRequest{
+		Addr:    minerAddr.String(),
+		SendAll: true,
+	}
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+	_, err = alice.SendCoins(ctxt, sweepReq)
+	require.NoError(t.t, err)
+
+	// We'll mine a block which should include the sweep transaction we
+	// generated above.
+	block = mineBlocks(t, net, 1, 1)[0]
+
+	// The sweep transaction should have exactly one inputs as we only had
+	// the the single output from above in the wallet.
+	sweepTx = block.Transactions[1]
+	if len(sweepTx.TxIn) != 1 {
+		t.Fatalf("expected 1 inputs instead have %v", len(sweepTx.TxIn))
+	}
+
+	// It should have two outputs, one being the miner address, the other
+	// one being the reserve going back to our wallet.
+	if len(sweepTx.TxOut) != 2 {
+		t.Fatalf("expected 2 outputs instead have %v", len(sweepTx.TxOut))
+	}
+
+	// The reserved value is now back in Alice's wallet.
+	aliceBalance := waitForConfirmedBalance()
+
+	// Alice closes channel, should now be allowed to send everything to an
+	// external address.
+	closeChannelAndAssert(ctxt, t, net, alice, aliceChanPoint, false)
+
+	newBalance := waitForConfirmedBalance()
+	if newBalance <= aliceBalance {
+		t.Fatalf("Alice's balance did not increase after channel close")
+	}
+
+	// We'll wait for the balance to reflect that the channel has been
+	// closed and the funds are in the wallet.
+
+	sweepReq = &lnrpc.SendCoinsRequest{
+		Addr:    minerAddr.String(),
+		SendAll: true,
+	}
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+	_, err = alice.SendCoins(ctxt, sweepReq)
+	require.NoError(t.t, err)
+
+	// We'll mine a block which should include the sweep transaction we
+	// generated above.
+	block = mineBlocks(t, net, 1, 1)[0]
+
+	// The sweep transaction should have two inputs, the change output from
+	// the previous sweep, and the output from the coop closed channel.
+	sweepTx = block.Transactions[1]
+	if len(sweepTx.TxIn) != 2 {
+		t.Fatalf("expected 2 inputs instead have %v", len(sweepTx.TxIn))
+	}
+
+	// It should have a single output.
+	if len(sweepTx.TxOut) != 1 {
+		t.Fatalf("expected 1 output instead have %v", len(sweepTx.TxOut))
+	}
+}

lntest/itest/lnd_test_list_on_test.go

@@ -230,6 +230,10 @@ var allTestCases = []*testCase{
 		name: "cpfp",
 		test: testCPFP,
 	},
+	{
+		name: "anchors reserved value",
+		test: testAnchorReservedValue,
+	},
 	{
 		name: "macaroon authentication",
 		test: testMacaroonAuthentication,

lntest/itest/log_error_whitelist.txt

@@ -251,3 +251,6 @@
 <time> [ERR] UTXN: Failed to sweep first-stage HTLC (CLTV-delayed) output <chan_point>
 <time> [ERR] UTXN: Notification chan closed, can't advance output <chan_point>
 <time> [ERR] DISC: Unable to rebroadcast stale announcements: unable to retrieve outgoing channels: channel from self node has no policy
+<time> [ERR] RPCS: [/lnrpc.Lightning/OpenChannel]: reserved wallet balance invalidated
+<time> [ERR] RPCS: [/lnrpc.Lightning/SendCoins]: reserved wallet balance invalidated
+<time> [ERR] RPCS: unable to open channel to NodeKey(<hex>): reserved wallet balance invalidated

lnwallet/chanfunding/assembler.go

@@ -103,6 +103,16 @@ type Intent interface {
 	// change.
 	LocalFundingAmt() btcutil.Amount
 
+	// Inputs returns all inputs to the final funding transaction that we
+	// know about. Note that there might be more, but we are not (yet)
+	// aware of.
+	Inputs() []wire.OutPoint
+
+	// Outputs returns all outputs of the final funding transaction that we
+	// know about. Note that there might be more, but we are not (yet)
+	// aware of.
+	Outputs() []*wire.TxOut
+
 	// Cancel allows the caller to cancel a funding Intent at any time.
 	// This will return any resources such as coins back to the eligible
 	// pool to be used in order channel fundings.

lnwallet/chanfunding/canned_assembler.go

@@ -98,6 +98,30 @@ func (s *ShimIntent) ThawHeight() uint32 {
 	return s.thawHeight
 }
 
+// Inputs returns all inputs to the final funding transaction that we
+// know about. For the ShimIntent this will always be none, since it is funded
+// externally.
+func (s *ShimIntent) Inputs() []wire.OutPoint {
+	return nil
+}
+
+// Outputs returns all outputs of the final funding transaction that we
+// know about. Since this is an externally funded channel, the channel output
+// is the only known one.
+func (s *ShimIntent) Outputs() []*wire.TxOut {
+	_, txOut, err := s.FundingOutput()
+	if err != nil {
+		log.Warnf("Unable to find funding output for shim intent: %v",
+			err)
+
+		// Failed finding funding output, return empty list of known
+		// outputs.
+		return nil
+	}
+
+	return []*wire.TxOut{txOut}
+}
+
 // FundingKeys couples our multi-sig key along with the remote party's key.
 type FundingKeys struct {
 	// LocalKey is our multi-sig key.

lnwallet/chanfunding/psbt_assembler.go

@@ -392,6 +392,53 @@ func (i *PsbtIntent) Cancel() {
 	i.ShimIntent.Cancel()
 }
 
+// Inputs returns all inputs to the final funding transaction that we know
+// about. These are only known after the PSBT has been verified.
+func (i *PsbtIntent) Inputs() []wire.OutPoint {
+	var inputs []wire.OutPoint
+
+	switch i.State {
+
+	// We return the inputs to the pending psbt.
+	case PsbtVerified:
+		for _, in := range i.PendingPsbt.UnsignedTx.TxIn {
+			inputs = append(inputs, in.PreviousOutPoint)
+		}
+
+	// We return the inputs to the final funding tx.
+	case PsbtFinalized, PsbtFundingTxCompiled:
+		for _, in := range i.FinalTX.TxIn {
+			inputs = append(inputs, in.PreviousOutPoint)
+		}
+
+	// In all other states we cannot know the inputs to the funding tx, and
+	// return an empty list.
+	default:
+	}
+
+	return inputs
+}
+
+// Outputs returns all outputs of the final funding transaction that we
+// know about. These are only known after the PSBT has been verified.
+func (i *PsbtIntent) Outputs() []*wire.TxOut {
+	switch i.State {
+
+	// We return the outputs of the pending psbt.
+	case PsbtVerified:
+		return i.PendingPsbt.UnsignedTx.TxOut
+
+	// We return the outputs of the final funding tx.
+	case PsbtFinalized, PsbtFundingTxCompiled:
+		return i.FinalTX.TxOut
+
+	// In all other states we cannot know the final outputs, and return an
+	// empty list.
+	default:
+		return nil
+	}
+}
+
 // PsbtAssembler is a type of chanfunding.Assembler wherein the funding
 // transaction is constructed outside of lnd by using partially signed bitcoin
 // transactions (PSBT).

lnwallet/chanfunding/wallet_assembler.go

@@ -152,6 +152,28 @@ func (f *FullIntent) CompileFundingTx(extraInputs []*wire.TxIn,
 	return fundingTx, nil
 }
 
+// Inputs returns all inputs to the final funding transaction that we
+// know about. Since this funding transaction is created all from our wallet,
+// it will be all inputs.
+func (f *FullIntent) Inputs() []wire.OutPoint {
+	var ins []wire.OutPoint
+	for _, coin := range f.InputCoins {
+		ins = append(ins, coin.OutPoint)
+	}
+
+	return ins
+}
+
+// Outputs returns all outputs of the final funding transaction that we
+// know about. This will be the funding output and the change outputs going
+// back to our wallet.
+func (f *FullIntent) Outputs() []*wire.TxOut {
+	outs := f.ShimIntent.Outputs()
+	outs = append(outs, f.ChangeOutputs...)
+
+	return outs
+}
+
 // Cancel allows the caller to cancel a funding Intent at any time.  This will
 // return any resources such as coins back to the eligible pool to be used in
 // order channel fundings.

lnwallet/wallet.go

@@ -5,6 +5,7 @@ import (
 	"crypto/sha256"
 	"errors"
 	"fmt"
+	"math"
 	"net"
 	"sync"
 	"sync/atomic"
@@ -32,6 +33,12 @@ const (
 	// The size of the buffered queue of requests to the wallet from the
 	// outside word.
 	msgBufferSize = 100
+
+	// anchorChanReservedValue is the amount we'll keep around in the
+	// wallet in case we have to fee bump anchor channels on force close.
+	// TODO(halseth): update constant to target a specific commit size at
+	// set fee rate.
+	anchorChanReservedValue = btcutil.Amount(10_000)
 )
 
 var (
@@ -39,6 +46,12 @@ var (
 	// contribution handling process if the process should be paused for
 	// the construction of a PSBT outside of lnd's wallet.
 	ErrPsbtFundingRequired = errors.New("PSBT funding required")
+
+	// ErrReservedValueInvalidated is returned if we try to publish a
+	// transaction that would take the walletbalance below what we require
+	// to keep around to fee bump our open anchor channels.
+	ErrReservedValueInvalidated = errors.New("reserved wallet balance " +
+		"invalidated")
 )
 
 // PsbtFundingRequired is a type that implements the error interface and
@@ -303,7 +316,12 @@ type LightningWallet struct {
 	// monotonically integer. All requests concerning the channel MUST
 	// carry a valid, active funding ID.
 	fundingLimbo map[uint64]*ChannelReservation
-	limboMtx     sync.RWMutex
+
+	// reservationIDs maps a pending channel ID to the reservation ID used
+	// as key in the fundingLimbo map. Used to easily look up a channel
+	// reservation given a pending channel ID.
+	reservationIDs map[[32]byte]uint64
+	limboMtx       sync.RWMutex
 
 	// lockedOutPoints is a set of the currently locked outpoint. This
 	// information is kept in order to provide an easy way to unlock all
@@ -334,6 +352,7 @@ func NewLightningWallet(Cfg Config) (*LightningWallet, error) {
 		msgChan:          make(chan interface{}, msgBufferSize),
 		nextFundingID:    0,
 		fundingLimbo:     make(map[uint64]*ChannelReservation),
+		reservationIDs:   make(map[[32]byte]uint64),
 		lockedOutPoints:  make(map[wire.OutPoint]struct{}),
 		fundingIntents:   make(map[[32]byte]chanfunding.Intent),
 		quit:             make(chan struct{}),
@@ -404,6 +423,7 @@ func (l *LightningWallet) LockedOutpoints() []*wire.OutPoint {
 func (l *LightningWallet) ResetReservations() {
 	l.nextFundingID = 0
 	l.fundingLimbo = make(map[uint64]*ChannelReservation)
+	l.reservationIDs = make(map[[32]byte]uint64)
 
 	for outpoint := range l.lockedOutPoints {
 		l.UnlockOutpoint(outpoint)
@@ -510,16 +530,16 @@ func (l *LightningWallet) RegisterFundingIntent(expectedID [32]byte,
 // PsbtFundingVerify looks up a previously registered funding intent by its
 // pending channel ID and tries to advance the state machine by verifying the
 // passed PSBT.
-func (l *LightningWallet) PsbtFundingVerify(pid [32]byte,
+func (l *LightningWallet) PsbtFundingVerify(pendingChanID [32]byte,
 	packet *psbt.Packet) error {
 
 	l.intentMtx.Lock()
 	defer l.intentMtx.Unlock()
 
-	intent, ok := l.fundingIntents[pid]
+	intent, ok := l.fundingIntents[pendingChanID]
 	if !ok {
 		return fmt.Errorf("no funding intent found for "+
-			"pendingChannelID(%x)", pid[:])
+			"pendingChannelID(%x)", pendingChanID[:])
 	}
 	psbtIntent, ok := intent.(*chanfunding.PsbtIntent)
 	if !ok {
@@ -530,7 +550,49 @@ func (l *LightningWallet) PsbtFundingVerify(pid [32]byte,
 		return fmt.Errorf("error verifying PSBT: %v", err)
 	}
 
-	return nil
+	// Get the channel reservation for that corresponds to this pending
+	// channel ID.
+	l.limboMtx.Lock()
+	pid, ok := l.reservationIDs[pendingChanID]
+	if !ok {
+		l.limboMtx.Unlock()
+		return fmt.Errorf("no channel reservation found for "+
+			"pendingChannelID(%x)", pendingChanID[:])
+	}
+
+	pendingReservation, ok := l.fundingLimbo[pid]
+	l.limboMtx.Unlock()
+
+	if !ok {
+		return fmt.Errorf("no channel reservation found for "+
+			"reservation ID %v", pid)
+	}
+
+	// Now the the PSBT has been verified, we can again check whether the
+	// value reserved for anchor fee bumping is respected.
+	numAnchors, err := l.currentNumAnchorChans()
+	if err != nil {
+		return err
+	}
+
+	// If this commit type is an anchor channel we add that to our counter,
+	// but only if we are contributing funds to the channel. This is done
+	// to still allow incoming channels even though we have no UTXOs
+	// available, as in bootstrapping phases.
+	if pendingReservation.partialState.ChanType.HasAnchors() &&
+		intent.LocalFundingAmt() > 0 {
+		numAnchors++
+	}
+
+	// We check the reserve value again, this should already have been
+	// checked for regular FullIntents, but now the PSBT intent is also
+	// populated.
+	return l.WithCoinSelectLock(func() error {
+		_, err := l.CheckReservedValue(
+			intent.Inputs(), intent.Outputs(), numAnchors,
+		)
+		return err
+	})
 }
 
 // PsbtFundingFinalize looks up a previously registered funding intent by its
@@ -714,6 +776,46 @@ func (l *LightningWallet) handleFundingReserveRequest(req *InitFundingReserveMsg
 		thawHeight = shimIntent.ThawHeight()
 	}
 
+	// Now that we have a funding intent, we'll check whether funding a
+	// channel using it would violate our reserved value for anchor channel
+	// fee bumping. We first get our current number of anchor channels.
+	numAnchors, err := l.currentNumAnchorChans()
+	if err != nil {
+		fundingIntent.Cancel()
+
+		req.err <- err
+		req.resp <- nil
+		return
+	}
+
+	// If this commit type is an anchor channel we add that to our counter,
+	// but only if we are contributing funds to the channel. This is done
+	// to still allow incoming channels even though we have no UTXOs
+	// available, as in bootstrapping phases.
+	if req.CommitType == CommitmentTypeAnchorsZeroFeeHtlcTx &&
+		fundingIntent.LocalFundingAmt() > 0 {
+		numAnchors++
+	}
+
+	// Check the reserved value using the inputs and outputs given by the
+	// intent. Not that for the PSBT intent type we don't yet have the
+	// funding tx ready, so this will always pass. We'll do another check
+	// when the PSBT has been verified.
+	err = l.WithCoinSelectLock(func() error {
+		_, err := l.CheckReservedValue(
+			fundingIntent.Inputs(), fundingIntent.Outputs(),
+			numAnchors,
+		)
+		return err
+	})
+	if err != nil {
+		fundingIntent.Cancel()
+
+		req.err <- err
+		req.resp <- nil
+		return
+	}
+
 	// The total channel capacity will be the size of the funding output we
 	// created plus the remote contribution.
 	capacity := localFundingAmt + remoteFundingAmt
@@ -748,6 +850,7 @@ func (l *LightningWallet) handleFundingReserveRequest(req *InitFundingReserveMsg
 	// request.
 	l.limboMtx.Lock()
 	l.fundingLimbo[id] = reservation
+	l.reservationIDs[req.PendingChanID] = id
 	l.limboMtx.Unlock()
 
 	// Funding reservation request successfully handled. The funding inputs
@@ -757,6 +860,153 @@ func (l *LightningWallet) handleFundingReserveRequest(req *InitFundingReserveMsg
 	req.err <- nil
 }
 
+// currentNumAnchorChans returns the current number of anchor channels the
+// wallet should be ready to fee bump if needed.
+func (l *LightningWallet) currentNumAnchorChans() (int, error) {
+	// Count all anchor channels that are open or pending
+	// open, or waiting close.
+	chans, err := l.Cfg.Database.FetchAllChannels()
+	if err != nil {
+		return 0, err
+	}
+
+	var numAnchors int
+	for _, c := range chans {
+		if c.ChanType.HasAnchors() {
+			numAnchors++
+		}
+	}
+
+	// We also count pending close channels.
+	pendingClosed, err := l.Cfg.Database.FetchClosedChannels(
+		true,
+	)
+	if err != nil {
+		return 0, err
+	}
+
+	for _, c := range pendingClosed {
+		c, err := l.Cfg.Database.FetchHistoricalChannel(
+			&c.ChanPoint,
+		)
+		if err != nil {
+			// We don't have a guarantee that all channels re found
+			// in the historical channels bucket, so we continue.
+			walletLog.Warnf("Unable to fetch historical "+
+				"channel: %v", err)
+			continue
+		}
+
+		if c.ChanType.HasAnchors() {
+			numAnchors++
+		}
+	}
+
+	return numAnchors, nil
+}
+
+// CheckReservedValue checks whether publishing a transaction with the given
+// inputs and outputs would violate the value we reserve in the wallet for
+// bumping the fee of anchor channels. The numAnchorChans argument should be
+// set the the number of open anchor channels controlled by the wallet after
+// the transaction has been published.
+//
+// If the reserved value is violated, the returned error will be
+// ErrReservedValueInvalidated. The method will also return the current
+// reserved value, both in case of success and in case of
+// ErrReservedValueInvalidated.
+//
+// NOTE: This method should only be run with the CoinSelectLock held.
+func (l *LightningWallet) CheckReservedValue(in []wire.OutPoint,
+	out []*wire.TxOut, numAnchorChans int) (btcutil.Amount, error) {
+
+	// Get all unspent coins in the wallet.
+	witnessOutputs, err := l.ListUnspentWitness(0, math.MaxInt32)
+	if err != nil {
+		return 0, err
+	}
+
+	ourInput := make(map[wire.OutPoint]struct{})
+	for _, op := range in {
+		ourInput[op] = struct{}{}
+	}
+
+	// When crafting a transaction with inputs from the wallet, these coins
+	// will usually be locked in the process, and not be returned when
+	// listing unspents. In this case they have already been deducted from
+	// the wallet balance. In case they haven't been properly locked, we
+	// check whether they are still listed among our unspents and deduct
+	// them.
+	var walletBalance btcutil.Amount
+	for _, in := range witnessOutputs {
+		// Spending an unlocked wallet UTXO, don't add it to the
+		// balance.
+		if _, ok := ourInput[in.OutPoint]; ok {
+			continue
+		}
+
+		walletBalance += in.Value
+	}
+
+	// Now we go through the outputs of the transaction, if any of the
+	// outputs are paying into the wallet (likely a change output), we add
+	// it to our final balance.
+	for _, txOut := range out {
+		_, addrs, _, err := txscript.ExtractPkScriptAddrs(
+			txOut.PkScript, &l.Cfg.NetParams,
+		)
+		if err != nil {
+			// Non-standard outputs can safely be skipped because
+			// they're not supported by the wallet.
+			continue
+		}
+
+		for _, addr := range addrs {
+			if !l.IsOurAddress(addr) {
+				continue
+			}
+
+			walletBalance += btcutil.Amount(txOut.Value)
+
+			// We break since we don't want to double count the output.
+			break
+		}
+	}
+
+	// We reserve a given amount for each anchor channel.
+	reserved := btcutil.Amount(numAnchorChans) * anchorChanReservedValue
+
+	if walletBalance < reserved {
+		walletLog.Debugf("Reserved value=%v above final "+
+			"walletbalance=%v with %d anchor channels open",
+			reserved, walletBalance, numAnchorChans)
+		return reserved, ErrReservedValueInvalidated
+	}
+
+	return reserved, nil
+}
+
+// CheckReservedValueTx calls CheckReservedValue with the inputs and outputs
+// from the given tx, with the number of anchor channels currently open in the
+// database.
+//
+// NOTE: This method should only be run with the CoinSelectLock held.
+func (l *LightningWallet) CheckReservedValueTx(tx *wire.MsgTx) (btcutil.Amount,
+	error) {
+
+	numAnchors, err := l.currentNumAnchorChans()
+	if err != nil {
+		return 0, err
+	}
+
+	var inputs []wire.OutPoint
+	for _, txIn := range tx.TxIn {
+		inputs = append(inputs, txIn.PreviousOutPoint)
+	}
+
+	return l.CheckReservedValue(inputs, tx.TxOut, numAnchors)
+}
+
 // initOurContribution initializes the given ChannelReservation with our coins
 // and change reserved for the channel, and derives the keys to use for this
 // channel.
@@ -890,6 +1140,7 @@ func (l *LightningWallet) handleFundingCancelRequest(req *fundingReserveCancelMs
 	delete(l.fundingLimbo, req.pendingFundingID)
 
 	pid := pendingReservation.pendingChanID
+	delete(l.reservationIDs, pid)
 
 	l.intentMtx.Lock()
 	if intent, ok := l.fundingIntents[pid]; ok {
@@ -1440,6 +1691,7 @@ func (l *LightningWallet) handleFundingCounterPartySigs(msg *addCounterPartySigs
 	// Funding complete, this entry can be removed from limbo.
 	l.limboMtx.Lock()
 	delete(l.fundingLimbo, res.reservationID)
+	delete(l.reservationIDs, res.pendingChanID)
 	l.limboMtx.Unlock()
 
 	l.intentMtx.Lock()

rpcserver.go

@@ -1056,7 +1056,25 @@ func (r *rpcServer) sendCoinsOnChain(paymentMap map[string]int64,
 		return nil, err
 	}
 
-	tx, err := r.server.cc.Wallet.SendOutputs(outputs, feeRate, minconf, label)
+	// We first do a dry run, to sanity check we won't spend our wallet
+	// balance below the reserved amount.
+	authoredTx, err := r.server.cc.Wallet.CreateSimpleTx(
+		outputs, feeRate, true,
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	_, err = r.server.cc.Wallet.CheckReservedValueTx(authoredTx.Tx)
+	if err != nil {
+		return nil, err
+	}
+
+	// If that checks out, we're failry confident that creating sending to
+	// these outputs will keep the wallet balance above the reserve.
+	tx, err := r.server.cc.Wallet.SendOutputs(
+		outputs, feeRate, minconf, label,
+	)
 	if err != nil {
 		return nil, err
 	}
@@ -1253,10 +1271,12 @@ func (r *rpcServer) SendCoins(ctx context.Context,
 		// With the sweeper instance created, we can now generate a
 		// transaction that will sweep ALL outputs from the wallet in a
 		// single transaction. This will be generated in a concurrent
-		// safe manner, so no need to worry about locking.
+		// safe manner, so no need to worry about locking. The tx will
+		// pay to the change address created above if we needed to
+		// reserve any value, the rest will go to targetAddr.
 		sweepTxPkg, err := sweep.CraftSweepAllTx(
 			feePerKw, lnwallet.DefaultDustLimit(),
-			uint32(bestHeight), targetAddr, wallet,
+			uint32(bestHeight), nil, targetAddr, wallet,
 			wallet.WalletController, wallet.WalletController,
 			r.server.cc.FeeEstimator, r.server.cc.Signer,
 		)
@@ -1264,6 +1284,75 @@ func (r *rpcServer) SendCoins(ctx context.Context,
 			return nil, err
 		}
 
+		// Before we publish the transaction we make sure it won't
+		// violate our reserved wallet value.
+		var reservedVal btcutil.Amount
+		err = wallet.WithCoinSelectLock(func() error {
+			var err error
+			reservedVal, err = wallet.CheckReservedValueTx(
+				sweepTxPkg.SweepTx,
+			)
+			return err
+		})
+
+		// If sending everything to this address would invalidate our
+		// reserved wallet balance, we create a new sweep tx, where
+		// we'll send the reserved value back to our wallet.
+		if err == lnwallet.ErrReservedValueInvalidated {
+			sweepTxPkg.CancelSweepAttempt()
+
+			rpcsLog.Debugf("Reserved value %v not satisfied after "+
+				"send_all, trying with change output",
+				reservedVal)
+
+			// We'll request a change address from the wallet,
+			// where we'll send this reserved value back to. This
+			// ensures this is an address the wallet knows about,
+			// allowing us to pass the reserved value check.
+			changeAddr, err := r.server.cc.Wallet.NewAddress(
+				lnwallet.WitnessPubKey, true,
+			)
+			if err != nil {
+				return nil, err
+			}
+
+			// Send the reserved value to this change address, the
+			// remaining funds will go to the targetAddr.
+			outputs := []sweep.DeliveryAddr{
+				{
+					Addr: changeAddr,
+					Amt:  reservedVal,
+				},
+			}
+
+			sweepTxPkg, err = sweep.CraftSweepAllTx(
+				feePerKw, lnwallet.DefaultDustLimit(),
+				uint32(bestHeight), outputs, targetAddr, wallet,
+				wallet.WalletController, wallet.WalletController,
+				r.server.cc.FeeEstimator, r.server.cc.Signer,
+			)
+			if err != nil {
+				return nil, err
+			}
+
+			// Sanity check the new tx by re-doing the check.
+			err = wallet.WithCoinSelectLock(func() error {
+				_, err := wallet.CheckReservedValueTx(
+					sweepTxPkg.SweepTx,
+				)
+				return err
+			})
+			if err != nil {
+				sweepTxPkg.CancelSweepAttempt()
+
+				return nil, err
+			}
+		} else if err != nil {
+			sweepTxPkg.CancelSweepAttempt()
+
+			return nil, err
+		}
+
 		rpcsLog.Debugf("Sweeping all coins from wallet to addr=%v, "+
 			"with tx=%v", in.Addr, spew.Sdump(sweepTxPkg.SweepTx))
 

sweep/sweeper.go

@@ -1192,8 +1192,8 @@ func (s *UtxoSweeper) sweep(inputs inputSet, feeRate chainfee.SatPerKWeight,
 
 	// Create sweep tx.
 	tx, err := createSweepTx(
-		inputs, s.currentOutputScript, uint32(currentHeight), feeRate,
+		inputs, nil, s.currentOutputScript, uint32(currentHeight),
-		dustLimit(s.relayFeeRate), s.cfg.Signer,
+		feeRate, dustLimit(s.relayFeeRate), s.cfg.Signer,
 	)
 	if err != nil {
 		return fmt.Errorf("create sweep tx: %v", err)
@@ -1487,7 +1487,7 @@ func (s *UtxoSweeper) CreateSweepTx(inputs []input.Input, feePref FeePreference,
 	}
 
 	return createSweepTx(
-		inputs, pkScript, currentBlockHeight, feePerKw,
+		inputs, nil, pkScript, currentBlockHeight, feePerKw,
 		dustLimit(s.relayFeeRate), s.cfg.Signer,
 	)
 }

sweep/sweeper_test.go

@@ -354,7 +354,7 @@ func assertTxFeeRate(t *testing.T, tx *wire.MsgTx,
 	outputAmt := tx.TxOut[0].Value
 
 	fee := btcutil.Amount(inputAmt - outputAmt)
-	_, estimator := getWeightEstimate(inputs, 0)
+	_, estimator := getWeightEstimate(inputs, nil, 0)
 	txWeight := estimator.weight()
 
 	expectedFee := expectedFeeRate.FeeForWeight(int64(txWeight))

sweep/txgenerator.go

@@ -131,12 +131,14 @@ func generateInputPartitionings(sweepableInputs []txInput,
 	return sets, nil
 }
 
-// createSweepTx builds a signed tx spending the inputs to a the output script.
+// createSweepTx builds a signed tx spending the inputs to the given outputs,
-func createSweepTx(inputs []input.Input, outputPkScript []byte,
+// sending any leftover change to the change script.
-	currentBlockHeight uint32, feePerKw chainfee.SatPerKWeight,
+func createSweepTx(inputs []input.Input, outputs []*wire.TxOut,
-	dustLimit btcutil.Amount, signer input.Signer) (*wire.MsgTx, error) {
+	changePkScript []byte, currentBlockHeight uint32,
+	feePerKw chainfee.SatPerKWeight, dustLimit btcutil.Amount,
+	signer input.Signer) (*wire.MsgTx, error) {
 
-	inputs, estimator := getWeightEstimate(inputs, feePerKw)
+	inputs, estimator := getWeightEstimate(inputs, outputs, feePerKw)
 	txFee := estimator.fee()
 
 	var (
@@ -210,6 +212,16 @@ func createSweepTx(inputs []input.Input, outputPkScript []byte,
 		totalInput += btcutil.Amount(o.SignDesc().Output.Value)
 	}
 
+	// Add the outputs given, if any.
+	for _, o := range outputs {
+		sweepTx.AddTxOut(o)
+		requiredOutput += btcutil.Amount(o.Value)
+	}
+
+	if requiredOutput+txFee > totalInput {
+		return nil, fmt.Errorf("insufficient input to create sweep tx")
+	}
+
 	// The value remaining after the required output and fees, go to
 	// change. Not that this fee is what we would have to pay in case the
 	// sweep tx has a change output.
@@ -219,7 +231,7 @@ func createSweepTx(inputs []input.Input, outputPkScript []byte,
 	// above.
 	if changeAmt >= dustLimit {
 		sweepTx.AddTxOut(&wire.TxOut{
-			PkScript: outputPkScript,
+			PkScript: changePkScript,
 			Value:    int64(changeAmt),
 		})
 	} else {
@@ -287,8 +299,8 @@ func createSweepTx(inputs []input.Input, outputPkScript []byte,
 
 // getWeightEstimate returns a weight estimate for the given inputs.
 // Additionally, it returns counts for the number of csv and cltv inputs.
-func getWeightEstimate(inputs []input.Input, feeRate chainfee.SatPerKWeight) (
+func getWeightEstimate(inputs []input.Input, outputs []*wire.TxOut,
-	[]input.Input, *weightEstimator) {
+	feeRate chainfee.SatPerKWeight) ([]input.Input, *weightEstimator) {
 
 	// We initialize a weight estimator so we can accurately asses the
 	// amount of fees we need to pay for this sweep transaction.
@@ -297,13 +309,19 @@ func getWeightEstimate(inputs []input.Input, feeRate chainfee.SatPerKWeight) (
 	// be more efficient on-chain.
 	weightEstimate := newWeightEstimator(feeRate)
 
-	// Our sweep transaction will pay to a single segwit p2wkh address,
+	// Our sweep transaction will always pay to the given set of outputs.
-	// ensure it contributes to our weight estimate. If the inputs we add
+	for _, o := range outputs {
-	// have required TxOuts, then this will be our change address. Note
+		weightEstimate.addOutput(o)
-	// that if we have required TxOuts, we might end up creating a sweep tx
+	}
-	// without a change output. It is okay to add the change output to the
+
-	// weight estimate regardless, since the estimated fee will just be
+	// If there is any leftover change after paying to the given outputs
-	// subtracted from this already dust output, and trimmed.
+	// and required outputs, it will go to a single segwit p2wkh address.
+	// This will be our change address, so ensure it contributes to our
+	// weight estimate. Note that if we have other outputs, we might end up
+	// creating a sweep tx without a change output. It is okay to add the
+	// change output to the weight estimate regardless, since the estimated
+	// fee will just be subtracted from this already dust output, and
+	// trimmed.
 	weightEstimate.addP2WKHOutput()
 
 	// For each output, use its witness type to determine the estimate

sweep/txgenerator_test.go

@@ -39,7 +39,7 @@ func TestWeightEstimate(t *testing.T) {
 		))
 	}
 
-	_, estimator := getWeightEstimate(inputs, 0)
+	_, estimator := getWeightEstimate(inputs, nil, 0)
 	weight := int64(estimator.weight())
 	if weight != expectedWeight {
 		t.Fatalf("unexpected weight. expected %d but got %d.",

sweep/walletsweep.go

@@ -148,13 +148,24 @@ type WalletSweepPackage struct {
 	CancelSweepAttempt func()
 }
 
+// DeliveryAddr is a pair of (address, amount) used to craft a transaction
+// paying to more than one specified address.
+type DeliveryAddr struct {
+	// Addr is the address to pay to.
+	Addr btcutil.Address
+
+	// Amt is the amount to pay to the given address.
+	Amt btcutil.Amount
+}
+
 // CraftSweepAllTx attempts to craft a WalletSweepPackage which will allow the
-// caller to sweep ALL outputs within the wallet to a single UTXO, as specified
+// caller to sweep ALL outputs within the wallet to a list of outputs. Any
-// by the delivery address. The sweep transaction will be crafted with the
+// leftover amount after these outputs and transaction fee, is sent to a single
-// target fee rate, and will use the utxoSource and outpointLocker as sources
+// output, as specified by the change address. The sweep transaction will be
-// for wallet funds.
+// crafted with the target fee rate, and will use the utxoSource and
+// outpointLocker as sources for wallet funds.
 func CraftSweepAllTx(feeRate chainfee.SatPerKWeight, dustLimit btcutil.Amount,
-	blockHeight uint32, deliveryAddr btcutil.Address,
+	blockHeight uint32, deliveryAddrs []DeliveryAddr, changeAddr btcutil.Address,
 	coinSelectLocker CoinSelectionLocker, utxoSource UtxoSource,
 	outpointLocker OutpointLocker, feeEstimator chainfee.Estimator,
 	signer input.Signer) (*WalletSweepPackage, error) {
@@ -261,9 +272,25 @@ func CraftSweepAllTx(feeRate chainfee.SatPerKWeight, dustLimit btcutil.Amount,
 		inputsToSweep = append(inputsToSweep, &input)
 	}
 
-	// Next, we'll convert the delivery addr to a pkScript that we can use
+	// Create a list of TxOuts from the given delivery addresses.
+	var txOuts []*wire.TxOut
+	for _, d := range deliveryAddrs {
+		pkScript, err := txscript.PayToAddrScript(d.Addr)
+		if err != nil {
+			unlockOutputs()
+
+			return nil, err
+		}
+
+		txOuts = append(txOuts, &wire.TxOut{
+			PkScript: pkScript,
+			Value:    int64(d.Amt),
+		})
+	}
+
+	// Next, we'll convert the change addr to a pkScript that we can use
 	// to create the sweep transaction.
-	deliveryPkScript, err := txscript.PayToAddrScript(deliveryAddr)
+	changePkScript, err := txscript.PayToAddrScript(changeAddr)
 	if err != nil {
 		unlockOutputs()
 
@@ -273,7 +300,7 @@ func CraftSweepAllTx(feeRate chainfee.SatPerKWeight, dustLimit btcutil.Amount,
 	// Finally, we'll ask the sweeper to craft a sweep transaction which
 	// respects our fee preference and targets all the UTXOs of the wallet.
 	sweepTx, err := createSweepTx(
-		inputsToSweep, deliveryPkScript, blockHeight, feeRate,
+		inputsToSweep, txOuts, changePkScript, blockHeight, feeRate,
 		dustLimit, signer,
 	)
 	if err != nil {

sweep/walletsweep_test.go

@@ -288,7 +288,8 @@ func TestCraftSweepAllTxCoinSelectFail(t *testing.T) {
 	utxoLocker := newMockOutpointLocker()
 
 	_, err := CraftSweepAllTx(
-		0, 100, 10, nil, coinSelectLocker, utxoSource, utxoLocker, nil, nil,
+		0, 100, 10, nil, nil, coinSelectLocker, utxoSource,
+		utxoLocker, nil, nil,
 	)
 
 	// Since we instructed the coin select locker to fail above, we should
@@ -313,7 +314,8 @@ func TestCraftSweepAllTxUnknownWitnessType(t *testing.T) {
 	utxoLocker := newMockOutpointLocker()
 
 	_, err := CraftSweepAllTx(
-		0, 100, 10, nil, coinSelectLocker, utxoSource, utxoLocker, nil, nil,
+		0, 100, 10, nil, nil, coinSelectLocker, utxoSource,
+		utxoLocker, nil, nil,
 	)
 
 	// Since passed in a p2wsh output, which is unknown, we should fail to
@@ -347,8 +349,8 @@ func TestCraftSweepAllTx(t *testing.T) {
 	utxoLocker := newMockOutpointLocker()
 
 	sweepPkg, err := CraftSweepAllTx(
-		0, 100, 10, deliveryAddr, coinSelectLocker, utxoSource, utxoLocker,
+		0, 100, 10, nil, deliveryAddr, coinSelectLocker, utxoSource,
-		feeEstimator, signer,
+		utxoLocker, feeEstimator, signer,
 	)
 	if err != nil {
 		t.Fatalf("unable to make sweep tx: %v", err)