Merge pull request #7631 from hieblmi/sendoutputs-reserve

rpc: anchor reserve check for `SendOutputs`

docs/release-notes/release-notes-0.17.0.md

@@ -13,6 +13,11 @@
   the entire retribution struct. This reduces the amount of data that needs to 
   be held in memory. 
 
+## RPC
+
+* [SendOutputs](https://github.com/lightningnetwork/lnd/pull/7631) now adheres
+  to the anchor channel reserve requirement.
+
 ## Misc
 
 * [Generate default macaroons
@@ -23,4 +28,5 @@ unlock or create.
 
 * Daniel McNally
 * Elle Mouton
+* hieblmi
 * Jordi Montes

itest/lnd_onchain_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/btcsuite/btcd/wire"
 	"github.com/lightningnetwork/lnd/lnrpc"
 	"github.com/lightningnetwork/lnd/lnrpc/chainrpc"
+	"github.com/lightningnetwork/lnd/lnrpc/signrpc"
 	"github.com/lightningnetwork/lnd/lnrpc/walletrpc"
 	"github.com/lightningnetwork/lnd/lntest"
 	"github.com/lightningnetwork/lnd/lntest/node"
@@ -26,6 +27,7 @@ func testChainKit(ht *lntest.HarnessTest) {
 	// avoid the need to start separate nodes.
 	testChainKitGetBlock(ht)
 	testChainKitGetBlockHash(ht)
+	testChainKitSendOutputsAnchorReserve(ht)
 }
 
 // testChainKitGetBlock ensures that given a block hash, the RPC endpoint
@@ -74,6 +76,96 @@ func testChainKitGetBlockHash(ht *lntest.HarnessTest) {
 	require.Equal(ht, expected, actual)
 }
 
+// testChainKitSendOutputsAnchorReserve checks if the SendOutputs rpc prevents
+// our wallet balance to drop below the required anchor channel reserve amount.
+func testChainKitSendOutputsAnchorReserve(ht *lntest.HarnessTest) {
+	// Start two nodes supporting anchor channels.
+	args := lntest.NodeArgsForCommitType(lnrpc.CommitmentType_ANCHORS)
+
+	// NOTE: we cannot reuse the standby node here as the test requires the
+	// node to start with no UTXOs.
+	charlie := ht.NewNode("Charlie", args)
+	bob := ht.Bob
+	ht.RestartNodeWithExtraArgs(bob, args)
+
+	// We'll start the test by sending Charlie some coins.
+	fundingAmount := btcutil.Amount(100_000)
+	ht.FundCoins(fundingAmount, charlie)
+
+	// Before opening the channel we ensure that the nodes are connected.
+	ht.EnsureConnected(charlie, bob)
+
+	// We'll get the anchor reserve that is required for a single channel.
+	reserve := charlie.RPC.RequiredReserve(
+		&walletrpc.RequiredReserveRequest{
+			AdditionalPublicChannels: 1,
+		},
+	)
+
+	// Charlie opens an anchor channel and keeps twice the amount of the
+	// anchor reserve in her wallet.
+	chanAmt := fundingAmount - 2*btcutil.Amount(reserve.RequiredReserve)
+	outpoint := ht.OpenChannel(charlie, bob, lntest.OpenChannelParams{
+		Amt:            chanAmt,
+		CommitmentType: lnrpc.CommitmentType_ANCHORS,
+		SatPerVByte:    1,
+	})
+
+	// Now we obtain a taproot address from bob which Charlie will use to
+	// send coins to him via the SendOutputs rpc.
+	address := bob.RPC.NewAddress(&lnrpc.NewAddressRequest{
+		Type: lnrpc.AddressType_TAPROOT_PUBKEY,
+	})
+	decodedAddr := ht.DecodeAddress(address.Address)
+	addrScript := ht.PayToAddrScript(decodedAddr)
+
+	// First she will try to send Bob an amount that would undershoot her
+	// reserve requirement by one satoshi.
+	balance := charlie.RPC.WalletBalance()
+	utxo := &wire.TxOut{
+		Value:    balance.TotalBalance - reserve.RequiredReserve + 1,
+		PkScript: addrScript,
+	}
+	req := &walletrpc.SendOutputsRequest{
+		Outputs: []*signrpc.TxOut{{
+			Value:    utxo.Value,
+			PkScript: utxo.PkScript,
+		}},
+		SatPerKw: 2400,
+		MinConfs: 1,
+	}
+
+	// We try to send the reserve violating transaction and expect it to
+	// fail.
+	_, err := charlie.RPC.WalletKit.SendOutputs(ht.Context(), req)
+	require.ErrorContains(ht, err, walletrpc.ErrInsufficientReserve.Error())
+
+	ht.MineBlocksAndAssertNumTxes(1, 0)
+
+	// Next she will try to send Bob an amount that just leaves enough
+	// reserves in her wallet.
+	utxo = &wire.TxOut{
+		Value:    balance.TotalBalance - reserve.RequiredReserve,
+		PkScript: addrScript,
+	}
+	req = &walletrpc.SendOutputsRequest{
+		Outputs: []*signrpc.TxOut{{
+			Value:    utxo.Value,
+			PkScript: utxo.PkScript,
+		}},
+		SatPerKw: 2400,
+		MinConfs: 1,
+	}
+
+	// This second transaction should be published correctly.
+	charlie.RPC.SendOutputs(req)
+
+	ht.MineBlocksAndAssertNumTxes(1, 1)
+
+	// Clean up our test setup.
+	ht.CloseChannel(charlie, outpoint)
+}
+
 // testCPFP ensures that the daemon can bump an unconfirmed  transaction's fee
 // rate by broadcasting a Child-Pays-For-Parent (CPFP) transaction.
 //

lnrpc/walletrpc/errors.go

@@ -0,0 +1,17 @@
+package walletrpc
+
+import "errors"
+
+var (
+
+	// ErrZeroLabel is returned when an attempt is made to label a
+	// transaction with an empty label.
+	ErrZeroLabel = errors.New("cannot label transaction with empty " +
+		"label")
+
+	// ErrInsufficientReserve is returned when SendOutputs wouldn't leave
+	// enough funds in the wallet to cover for the anchor reserve.
+	ErrInsufficientReserve = errors.New("the outputs to be sent " +
+		"would leave insufficient reserves for anchor channels in " +
+		"the wallet")
+)

lnrpc/walletrpc/walletkit_server.go

@@ -222,10 +222,6 @@ var (
 	}
 )
 
-// ErrZeroLabel is returned when an attempt is made to label a transaction with
-// an empty label.
-var ErrZeroLabel = errors.New("cannot label transaction with empty label")
-
 // ServerShell is a shell struct holding a reference to the actual sub-server.
 // It is used to register the gRPC sub-server with the root server before we
 // have the necessary dependencies to populate the actual sub-server.
@@ -394,8 +390,8 @@ func (w *WalletKit) ListUnspent(ctx context.Context,
 	// Force min_confs and max_confs to be zero if unconfirmed_only is
 	// true.
 	if req.UnconfirmedOnly && (req.MinConfs != 0 || req.MaxConfs != 0) {
-		return nil, fmt.Errorf("min_confs and max_confs must be zero if " +
-			"unconfirmed_only is true")
+		return nil, fmt.Errorf("min_confs and max_confs must be zero " +
+			"if unconfirmed_only is true")
 	}
 
 	// When unconfirmed_only is inactive and max_confs is zero (default
@@ -666,30 +662,58 @@ func (w *WalletKit) SendOutputs(ctx context.Context,
 	// Before we can request this transaction to be created, we'll need to
 	// amp the protos back into the format that the internal wallet will
 	// recognize.
+	var totalOutputValue int64
 	outputsToCreate := make([]*wire.TxOut, 0, len(req.Outputs))
 	for _, output := range req.Outputs {
 		outputsToCreate = append(outputsToCreate, &wire.TxOut{
 			Value:    output.Value,
 			PkScript: output.PkScript,
 		})
+		totalOutputValue += output.Value
 	}
 
 	// Then, we'll extract the minimum number of confirmations that each
 	// output we use to fund the transaction should satisfy.
-	minConfs, err := lnrpc.ExtractMinConfs(req.MinConfs, req.SpendUnconfirmed)
+	minConfs, err := lnrpc.ExtractMinConfs(
+		req.MinConfs, req.SpendUnconfirmed,
+	)
 	if err != nil {
 		return nil, err
 	}
 
+	// Before sending out funds we need to ensure that the remainder of our
+	// wallet funds would cover for the anchor reserve requirement. We'll
+	// also take unconfirmed funds into account.
+	walletBalance, err := w.cfg.Wallet.ConfirmedBalance(
+		0, lnwallet.DefaultAccountName,
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	// We'll get the currently required reserve amount.
+	reserve, err := w.RequiredReserve(ctx, &RequiredReserveRequest{})
+	if err != nil {
+		return nil, err
+	}
+
+	// Then we check if our current wallet balance undershoots the required
+	// reserve if we'd send out the outputs specified in the request.
+	if int64(walletBalance)-totalOutputValue < reserve.RequiredReserve {
+		return nil, ErrInsufficientReserve
+	}
+
 	label, err := labels.ValidateAPI(req.Label)
 	if err != nil {
 		return nil, err
 	}
 
-	// Now that we have the outputs mapped, we can request that the wallet
-	// attempt to create this transaction.
+	// Now that we have the outputs mapped and checked for the reserve
+	// requirement, we can request that the wallet attempts to create this
+	// transaction.
 	tx, err := w.cfg.Wallet.SendOutputs(
-		outputsToCreate, chainfee.SatPerKWeight(req.SatPerKw), minConfs, label,
+		outputsToCreate, chainfee.SatPerKWeight(req.SatPerKw), minConfs,
+		label,
 	)
 	if err != nil {
 		return nil, err
@@ -923,7 +947,10 @@ func (w *WalletKit) BumpFee(ctx context.Context,
 			err)
 	}
 
-	inp := input.NewBaseInput(op, witnessType, signDesc, uint32(currentHeight))
+	inp := input.NewBaseInput(
+		op, witnessType, signDesc, uint32(currentHeight),
+	)
+
 	sweepParams := sweep.Params{Fee: feePreference}
 	if _, err = w.cfg.Sweeper.SweepInput(inp, sweepParams); err != nil {
 		return nil, err
@@ -1524,7 +1551,8 @@ func (w *WalletKit) ListAccounts(ctx context.Context,
 		keyScopeFilter = &keyScope
 
 	default:
-		return nil, fmt.Errorf("unhandled address type %v", req.AddressType)
+		return nil, fmt.Errorf("unhandled address type %v",
+			req.AddressType)
 	}
 
 	accounts, err := w.cfg.Wallet.ListAccounts(req.Name, keyScopeFilter)
@@ -1638,7 +1666,8 @@ func parseAddrType(addrType AddressType,
 	switch addrType {
 	case AddressType_UNKNOWN:
 		if required {
-			return nil, errors.New("an address type must be specified")
+			return nil, fmt.Errorf("an address type must be " +
+				"specified")
 		}
 		return nil, nil
 

lntest/rpc/wallet_kit.go

@@ -290,3 +290,18 @@ func (h *HarnessRPC) ImportTapscript(
 
 	return resp
 }
+
+// RequiredReserve makes a RPC call to the node's WalletKitClient and asserts.
+//
+//nolint:lll
+func (h *HarnessRPC) RequiredReserve(
+	req *walletrpc.RequiredReserveRequest) *walletrpc.RequiredReserveResponse {
+
+	ctxt, cancel := context.WithTimeout(h.runCtx, DefaultTimeout)
+	defer cancel()
+
+	resp, err := h.WalletKit.RequiredReserve(ctxt, req)
+	h.NoError(err, "RequiredReserve")
+
+	return resp
+}