[v0.21.x-branch] Backport #10815: lnwallet/rpcwallet: keep zero-value prev outputs in remote-sign PSBT#10822
Merged
Conversation
…ign prep Before forwarding a SignOutputRaw request to the remote signer instance, remoteSign rebuilds a PSBT from the unsigned transaction and annotates every input with a WitnessUtxo (so the downstream walletkit.SignPsbt call accepts it — taproot sighash computation requires the prev output of every input, not just the one being signed). For non-signed inputs the prep stage first asks the watch-only wallet about the outpoint via FetchOutpointInfo, then — when the wallet does not own or track the outpoint — falls back to the sign descriptor's PrevOutputFetcher. The fallback previously required `utxo.Value != 0`, which silently dropped legitimate zero-value entries on the floor and left the corresponding PSBT input bare. The walletkit.SignPsbt entry point on the remote signer then rejected the PSBT with "input (index=N) doesn't specify any UTXO info" because input N had neither a WitnessUtxo nor a NonWitnessUtxo annotation. BIP-322 (signing virtual transactions for message attestation) is the canonical hitter: its to_spend output is mandated by the BIP to be exactly value=0 with the message commitment as pk_script, and that output is referenced as input 0 of every BIP-322 to_sign transaction. Any caller that drives a BIP-322 sign through a remote-signer LND deployment was failing for this reason. The validation we actually want is that the fetched prev output is representable as a usable WitnessUtxo: non-nil and with a non-empty pk_script. Drop the Value check; the zero-value case is well-formed and the resulting PSBT input will serialize cleanly. The fetched-but- empty-pk_script case continues to be rejected (a WitnessUtxo with empty PkScript is malformed at PSBT serialization), and the warning log when no fallback resolves the outpoint is preserved verbatim. Lift the WitnessUtxo-population loop out of remoteSign into a package-level helper so the resolution policy is unit-testable without spinning up a real wallet + remote signer pair. The helper takes a fetchOutpointInfoFn callback that mirrors lnwallet.WalletController.FetchOutpointInfo. No behavior change for the wallet-owns-it path or the no-fallback path. (cherry picked from commit 9f31668)
Cover the four resolution branches plus the BIP-322 regression case:
- wallet-owns-it: FetchOutpointInfo returns a Utxo, helper writes
the matching WitnessUtxo into the PSBT input.
- external-fallback: wallet returns ErrNotMine, helper writes the
WitnessUtxo from the sign descriptor's PrevOutputFetcher.
- zero-value-fallback: same as above with the fetched entry's Value
set to zero. This is the BIP-322 to_spend shape (input 0 of every
BIP-322 to_sign references a virtual prev whose Value is mandated
to be zero); the helper must populate the WitnessUtxo rather than
silently skip it.
- no-fallback: wallet returns ErrNotMine and no PrevOutputFetcher
is provided; the helper leaves the input bare and the warning log
fires (asserted only by absence of a populated WitnessUtxo).
- empty-pk_script-fallback: the fetcher returns a non-nil entry
with an empty PkScript; the helper rejects it as unusable (the
PSBT WitnessUtxo serializer requires a non-empty script) and
leaves the input bare.
The signed input (signDesc.InputIndex) is intentionally left untouched
by the helper — that input is the one the caller's main path will
populate later — and the tests cross-check that invariant on the
wallet-owns-it case.
(cherry picked from commit 6bb2c6f)
Add the bug-fix entry under 0.21.0 with a link to the PR. (cherry picked from commit 44f2ef7)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #10815
In this PR, we fix a sharp edge in
RPCKeyRing.remoteSignthat silentlybroke any signing flow whose
to_signtx references a zero-value prevoutput. The canonical hitter is BIP-322 message signing, which mandates
that the virtual
to_spendoutput is exactlyvalue=0with the messagecommitment as
pk_script, and that output is referenced as input 0 ofevery
to_signtx handed toSignOutputRaw.Before forwarding the request,
remoteSignrebuilds a PSBT from theunsigned tx and annotates every non-signed input with a
WitnessUtxo,so the downstream
walletkit.SignPsbtcall can proceed (taproot sighashneeds every prev output, not just the one being signed). When the watch-
only wallet doesn't know an outpoint, the prep stage falls back to the
sign descriptor's
PrevOutputFetcher. That fallback used to requireutxo.Value != 0, which silently dropped legitimate zero-value entrieson the floor. The remote signer would then reject the resulting PSBT
with
input (index=N) doesn't specify any UTXO info, raised inwalletkit.SignPsbtprecisely because input N had neither aWitnessUtxonor aNonWitnessUtxo.The fix
We drop the
Value != 0guard. The only condition that actually mattersis that the fetched entry is usable as a
WitnessUtxo: non-nil with anon-empty
PkScript. A zero-valueWitnessUtxowith a well-formedPkScriptserializes cleanly into a PSBT, so there's nothing else tovalidate at this layer.
We also lift the population loop out of
remoteSigninto a package-level helper,
populateNonSignedInputWitnessUtxos, so the resolutionpolicy can be exercised in unit tests without standing up a full wallet
fetchOutpointInfoFncallbackthat mirrors
lnwallet.WalletController.FetchOutpointInfo. No behaviorchange for the wallet-owns-it path or the no-fallback path.
Tests
New unit tests in
lnwallet/rpcwallet/rpcwallet_test.gowalk througheach resolution branch. The wallet-owns-it case exercises the
FetchOutpointInfohappy path and cross-checks that the signed inputitself is left untouched. The external-fallback case has the wallet
return
ErrNotMineand resolves the prev via the fetcher. The zero-value-fallback case is the BIP-322 regression: same shape as external-
fallback, but the fetcher entry has
Value=0, and the test assertsthat
WitnessUtxolands on the PSBT instead of being silently skipped.Two more cover the unhappy paths: no fetcher attached (input left bare,
warning logged) and a fetcher entry with an empty
PkScript(rejectedas unusable, since a PSBT
WitnessUtxowith empty script is malformedat serialization).
Compat
No behavior change for the wallet-owns-it path, the no-fallback path,
or the non-zero-value fallback path. The only new behavior is that
legitimately zero-value entries from the supplied
PrevOutputFetcherland in the PSBT instead of being silently skipped.