Repository navigation
channeld: verify the peer's input witnesses before signing a splice - #45
Merged
Merged
Conversation
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
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.
channeld now checks the witness the peer sends for each of its own splice inputs as its
tx_signaturesarrive, before any of ours go out, and fails the channel if one is invalid. This covers every path that receives them: the accepter, the initiator (whose signatures normally go second, in a later call), reconnection and RBF.Without this, a peer that signs first can send a valid
shared_input_signaturebut a bad witness for its own input. We accept it, send our signatures and broadcast a transaction bitcoind rejects, while the peer holds a valid copy with the same txid that it can broadcast whenever it likes, or never. Our wallet inputs stay committed in the meantime.A peer witness is accepted if it is a valid spend of:
SIGHASH_DEFAULT) or 0x01 signature, or 0x21 whereoption_unified_sigsapplies,to_remote, whose input's sequence must also satisfy its CSV.Anything else fails the channel. ECDSA signatures may use 0x01, or 0x21 where
option_unified_sigsapplies.The transaction is built once for all of the peer's inputs, with each spent output attached, so a peer adding many inputs with large prevtxs does not make the check clone the PSBT or rehash its prevtxs per input. The check is against the spent outputs in the PSBT: whether they exist, are confirmed and unspent, and whether a relative lock is already satisfied, is not known here.
A new unit test,
common/test/run-psbt_input_witness.c, checks the legacy taproot digest against the BIP341 key path test vectors, and signs and checks P2WPKH (0x01, and 0x21 only whereoption_unified_sigsapplies), P2WSH 2-of-2 (key order, NULLDUMMY), P2WSHto_remotewith CSVs from 1 to 32767 (sequences below the CSV, or with the disable or type flag, are refused) and an unrecognized P2WSH script.Two dev options support the tests:
--dev-splice-corrupt-input-witnesssendstx_signaturesfirst with a corrupted witness for each of our inputs, and--dev-splice-sign-secondnever sendstx_signaturesfirst. Neither changes the order ofcommitment_signed.New tests in
tests/test_splicing.py:test_splice_bad_input_witness: the bad witness is caught, and when we sign second ourtx_signaturesnever go out; when we signed first, the channel still resolves on chain.test_splice_peer_p2wsh_to_remote_input: ato_remoteinput is accepted, and a corrupted one caught before we sign.test_splice_peer_p2tr_input: a P2TR key path input is accepted.Without the check in channeld, the three negative cases fail. A CLN accepter adds no splice inputs, so two CLN nodes only reach the check on the accepter's side; the initiator runs the same check, at the same point, when its peer does add inputs.
Changelog-None