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

With an anchor-channel open having only a single UTXO cannot be selected for SendCoins/SendMany #5648

Closed
bjarnemagnussen opened this issue Aug 21, 2021 · 11 comments · Fixed by #5665
Labels
anchors bug Unintended code behaviour validation wallet The wallet (lnwallet) which LND uses

Comments

@bjarnemagnussen
Copy link
Contributor

bjarnemagnussen commented Aug 21, 2021

Background

When using anchor-channels a minimum reserved wallet balance is required (10_000 sats/channel; maximum 100_000 sats).

When the node has some anchor-channel open and only a single UTXO in the wallet, then calling SendCoins or SendMany to send some on-chain funds out is impossible as it returns the error reserved wallet balance invalidated, even though we send out an amount that would leave sufficient funds on the node.

However, it is still possible to call OpenChannel to open a channel as long as we fund it with an amount that leaves enough on-chain funds for the reserved wallet balance.

As a work-around it is possible to request a new address on the node and then use SendCoins or SendMany to actively send an amount equal to (or greater than) the required reserved wallet balance to your own address. This splits the UTXO and makes it possible to now send funds out again.

It therefore seems that the problem is that the check for reserved wallet balance is done without knowledge of a change output using SendCoins/SendMany, while this is known when opening a new channel with OpenChannel?

Your environment

  • version of lnd: 0.13.0/0.13.1
  • which operating system (uname -a on *Nix): linux

Steps to reproduce

  1. Create a new node.
  2. Generate a new address and send funds to this address.
  3. From the node open an anchor-channel to some remote. Now all of the on-chain funds are in a single UTXO, which are un-spendable using SendCoins/SendMany when trying to send to an external address (even though the transaction would leave sufficient funds for the wallet reserve on the node).
  4. It is still possible to open channels and to split the UTXO when actively sending funds to your own address requested via newaddress.

Expected behaviour

It should be possible to detect that the change output of a transaction from SendCoins/SendMany would leave sufficient wallet reserve funds.

Actual behaviour

The error reserved wallet balance invalidated is returned.

@bjarnemagnussen bjarnemagnussen changed the title With an anchor-channel open Having only a single UTXO cannot be selected for SendCoins With an anchor-channel open having only a single UTXO cannot be selected for SendCoins/SendMany Aug 21, 2021
@Roasbeef
Copy link
Member

It should be possible to detect that the change output of a transaction from SendCoins/SendMany would leave sufficient wallet reserve funds.

An attempt to verify this is here: https://github.com/lightningnetwork/lnd/blob/master/lnwallet/wallet.go#L1004

So you have a reproducible scenario where you aren't trying to send the entire balance of the wallet out that triggers this assertion?

@bjarnemagnussen
Copy link
Contributor Author

bjarnemagnussen commented Aug 21, 2021

Exactly, I am only trying to send an amount that would leave (more than) enough for the reserved value in the wallet.

Just from a real quick debugging of the check that you link to I can see that both the external address but also the "automatically" created change address are NOT considered as belonging to the wallet in that check.

@Roasbeef
Copy link
Member

One way to start to pinpoint this would be to write a new unit tests in the wallet that creates an address and ensure that the IsOurAddress call returns true. FWIW, this is used in a few other areas as is.

@bjarnemagnussen
Copy link
Contributor Author

It seems the problem is that when sendcoins is called it will as a sanity-check first create a transaction in dry-run mode and then directly thereafter make the check for reserved wallet funds by indirectly checking if there is a change output:

lnd/rpcserver.go

Lines 983 to 995 in 4487acc

// 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, minConfs, true,
)
if err != nil {
return nil, err
}
_, err = r.server.cc.Wallet.CheckReservedValueTx(authoredTx.Tx)
if err != nil {
return nil, err
}

Since the transaction was made in dry-run mode it did not commit anything to the db, hence also not that it created a new change address, which is therefore not picked up from the check.

The actual logic for the dry-run happens inside the btcwallet dependency here:
https://github.com/btcsuite/btcwallet/blob/9b5a201c344c63be44705afafa974948e86b047b/wallet/createtx.go#L109

As expected when calling l.NewAddress(WitnessPubKey, true, DefaultAccountName) inside CheckReservedValue then the exact same change address is derived as was used in the dry-run and added as our address, which means it now passes the test. This however is obviously not a solution.

A better approach may be to actually make use of the ChangeIndex field returned by CreateSimpleTx and "force" it as a change address inside the CheckReserveValue check?

@Roasbeef Roasbeef added anchors bug Unintended code behaviour validation wallet The wallet (lnwallet) which LND uses labels Aug 25, 2021
@Roasbeef
Copy link
Member

A better approach may be to actually make use of the ChangeIndex field returned by CreateSimpleTx and "force" it as a change address inside the CheckReserveValue check?

You mean manually create that change address before we call CheckReserveValue again? I guess we could just derive that key using the method that don't advance the change address counter, that manually pass it into CheckReservedValueTx?

@bjarnemagnussen
Copy link
Contributor Author

Is there a method that allows deriving the next internal/change key, without advancing the counter? Because I couldn't find one.

My question was more if it would be acceptable to make use of the txauthor.AuthoredTx struct returned by WalletController.CreateSimpleTx. As this struct has a field ChangeIndex that is only positive or zero when CreateSimpleTx did create a change output (also in dry-run), we could simply check the amount inside this output if the CheckReservedValueTx returned the ErrReservedValueInvalidated error before deciding if the reserved value was indeed invalidated?

I have opened a PR with a minimal change that makes use of it, as it is probably easier to reason about this in code :)

@Roasbeef
Copy link
Member

Is there a method that allows deriving the next internal/change key, without advancing the counter? Because I couldn't find one.

Yes: https://github.com/lightningnetwork/lnd/blob/master/keychain/derivation.go#L163

@Kixunil
Copy link
Contributor

Kixunil commented Aug 31, 2021

Could this be related to #5539?

@bjarnemagnussen
Copy link
Contributor Author

bjarnemagnussen commented Sep 2, 2021

@Kixunil If I understand #5539 correctly, then the wallet reserve invalidated error is happening immediately when wanting to open a channel interactively utilizing PSBT (with e.g. an empty internal wallet), before actually providing the change/funding output for the reserved wallet balance yet? In that case I don't think it is directly related, as this issue is at a later stage where actually all information on change outputs would be available.

@musdom
Copy link

musdom commented Sep 23, 2021

Facing this same issue with a new signet v0.13.1 node with one UTXO and one anchor channel. Couldn't send any amount to my electrum wallet. Had to send to own address, and then to electrum.

@benthecarman
Copy link
Contributor

I can reproduce this issue as well. On a regtest node I have 3 utxos totaling 6 btc, I cannot start a psbt funded channel open with all 3 utxos even though the channel size is only 100k sats.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
anchors bug Unintended code behaviour validation wallet The wallet (lnwallet) which LND uses
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants