From 903b5dde8934318ccab89192f3ec819b544d7fd5 Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Sun, 23 Apr 2023 19:57:51 +0200 Subject: [PATCH 1/4] rpc: SendOutputs considers anchor reserve --- lnrpc/walletrpc/errors.go | 17 +++++++++++++++ lnrpc/walletrpc/walletkit_server.go | 33 +++++++++++++++++++++++------ lntest/rpc/wallet_kit.go | 15 +++++++++++++ 3 files changed, 59 insertions(+), 6 deletions(-) create mode 100644 lnrpc/walletrpc/errors.go diff --git a/lnrpc/walletrpc/errors.go b/lnrpc/walletrpc/errors.go new file mode 100644 index 0000000000..367fbce424 --- /dev/null +++ b/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") +) diff --git a/lnrpc/walletrpc/walletkit_server.go b/lnrpc/walletrpc/walletkit_server.go index 287572dd08..a3fabb381f 100644 --- a/lnrpc/walletrpc/walletkit_server.go +++ b/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. @@ -666,12 +662,14 @@ 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 @@ -681,13 +679,36 @@ func (w *WalletKit) SendOutputs(ctx context.Context, 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, ) diff --git a/lntest/rpc/wallet_kit.go b/lntest/rpc/wallet_kit.go index 9a88fe6e72..95af5f92df 100644 --- a/lntest/rpc/wallet_kit.go +++ b/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 +} From 8f5910bf91b15dc5e2323dd40dc35454f166a84d Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Sun, 23 Apr 2023 20:01:11 +0200 Subject: [PATCH 2/4] rpc: fix code style --- lnrpc/walletrpc/walletkit_server.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/lnrpc/walletrpc/walletkit_server.go b/lnrpc/walletrpc/walletkit_server.go index a3fabb381f..156227bf08 100644 --- a/lnrpc/walletrpc/walletkit_server.go +++ b/lnrpc/walletrpc/walletkit_server.go @@ -390,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 @@ -674,7 +674,9 @@ func (w *WalletKit) SendOutputs(ctx context.Context, // 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 } @@ -710,7 +712,8 @@ func (w *WalletKit) SendOutputs(ctx context.Context, // 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 @@ -944,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 @@ -1545,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) @@ -1659,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 From 77322dddb87c314988fc4ac9cda38d1a48267494 Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Sun, 23 Apr 2023 20:03:55 +0200 Subject: [PATCH 3/4] itest: anchor reserve test for SendOutputs --- itest/lnd_onchain_test.go | 92 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/itest/lnd_onchain_test.go b/itest/lnd_onchain_test.go index 8d57907d59..87e0c12358 100644 --- a/itest/lnd_onchain_test.go +++ b/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. // From 79a06d19186c63f703cd0103aaa32dc2900fa760 Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Sun, 23 Apr 2023 20:04:57 +0200 Subject: [PATCH 4/4] docs: update release notes --- docs/release-notes/release-notes-0.17.0.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/release-notes/release-notes-0.17.0.md b/docs/release-notes/release-notes-0.17.0.md index 138d812b82..f9b55306f0 100644 --- a/docs/release-notes/release-notes-0.17.0.md +++ b/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