Skip to content
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.

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

rpc: anchor reserve check for SendOutputs #7631

Merged
merged 4 commits into from
May 4, 2023
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
6 changes: 6 additions & 0 deletions docs/release-notes/release-notes-0.17.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -23,4 +28,5 @@ unlock or create.

* Daniel McNally
* Elle Mouton
* hieblmi
* Jordi Montes
92 changes: 92 additions & 0 deletions itest/lnd_onchain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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
guggero marked this conversation as resolved.
Show resolved Hide resolved
// 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.
//
Expand Down
17 changes: 17 additions & 0 deletions lnrpc/walletrpc/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package walletrpc

import "errors"

guggero marked this conversation as resolved.
Show resolved Hide resolved
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")
)
55 changes: 42 additions & 13 deletions lnrpc/walletrpc/walletkit_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
guggero marked this conversation as resolved.
Show resolved Hide resolved
)
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,
Copy link
Collaborator

Choose a reason for hiding this comment

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

🙏

label,
)
if err != nil {
return nil, err
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down
15 changes: 15 additions & 0 deletions lntest/rpc/wallet_kit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}