lnd: use change output index from authoring TX to check reserved value #5665
Conversation
rpcserver.go
Outdated
| // If the reserved value was invalidated we make sure to explicitly | ||
| // check if a change output was undetected but that would leave | ||
| // sufficient funds for the reserved value. | ||
| case lnwallet.ErrReservedValueInvalidated: |
There was a problem hiding this comment.
What about moving this into CheckReservedValueTx (passing the full authored tx, etc?). This way we can more easily write additional unit tests for this, and also ensure that all other callers also get this fix.
Thinking about it more, we can likely put this behind an additional function (or modify the existing one) that is used to check if a transaction contains a change address or not.
There was a problem hiding this comment.
The function CheckReservedValueTx now also takes an optional parameter ChangeIndex to explicitly define a change output (in addition to the check for internal wallet addresses that are being watched).
This check is currently also used when sweeping coins from the wallet. However, in that case a change address is created by calling NewAddress, which will immediately add it as being watched regardless if the sweeping being successful or not. This could now potentially be re-factored to instead add a change without adding it to the wallet, see if it succeeds by explicitly setting the change index, and then retrospectively adding it the wallet. This would be similar to how sending coins works right now.
bjarnemagnussen
force-pushed
the
upstream/feat/single-utxo-reserved
branch
2 times, most recently
from
13e2250
to
df7731e
Compare
|
Also needs a release notes update |
bjarnemagnussen
force-pushed
the
upstream/feat/single-utxo-reserved
branch
from
df7731e
to
a8bb3ed
Compare
|
Thanks @Roasbeef, I have addressed your last issues! |
|
Needs a rebase then this should be g2g, thanks for working on this! |
bjarnemagnussen
force-pushed
the
upstream/feat/single-utxo-reserved
branch
from
a8bb3ed
to
1d84c1d
Compare
|
Rebased |
Description
Currently a whole UTXO is reserved when anchor-channels is used. This means that if the node has a single UTXO, it cannot be spend even if a change output would leave sufficient funds for the reserve in the wallet.
This PR makes use of the returned
txauthor.AuthoredTxstruct that contains a field for an eventual change index. The reason that this is not automatically detected by theCheckReservedValueTxfunction is because the authored transaction is created in dry-run, meaning it does not add the used change output to the wallet to watch for.This PR hence assumes that the change index from the authored transaction returned by
WalletController.CreateSimpleTxwill indeed be a change output controlled by the wallet.Fixes #5648