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

Bolt 2: Make interactive-tx explicitly use SIGHASH_ALL #16

Open
wants to merge 17 commits into
base: nifty/dual-fund
Choose a base branch
from

Conversation

ddustin
Copy link

@ddustin ddustin commented Apr 7, 2023

This was previously assumed but adding it to the spec makes it explicit, should we ever want to change it in the future.

niftynei and others added 17 commits February 7, 2023 13:38
This commit adds the interactive transaction construction protcol, as
well as the first practical example of using it, v2 of channel
establishment.

Note that for v2 we also update the channel_id, which now uses the hash
of the revocation_basepoints. We move away from using the funding
transaction id, as the introduction of RBF* makes it such that a single
channel may have many funding transaction id's over the course of
its lifetime.

*Later, also splicing
If both peers contribute an equal amount to a interactive tx,
peer with "lower" pubkey sends first
This commit contains many (small) changes to address most of the pending
comments:

- removed feature dependency on anchor outputs
- the "zerod_channel_id" created confusion, we instead explicitly detail
  the open and accept cases
- "fail the negotiation" was unclear in interactive-tx
- clarified field names in `tx_signatures`
- many nits from PR comments

Thanks to @morehouse, @ariard and @antonilol for the helpful comments.
Repeat the second commit point in the initial openchannel2 message.

Suggested-By: @pm47
Prior versions of the v2 dual-funding protocol assumed a 'minimum fee'
payment for any witness stack of any input, as a way to simplify fee
checks.

The suggested min feerate didn't make sense for taproot spend paths etc;
instead we remove this check entirely.
This lets any side of the protocol require the other side to use confirmed
inputs, to avoid paying the fees of a low feerate unconfirmed ancestor.
This allows us to filter messages until we know the peer has ack'd our
abort, allowing us to return to "prior" negotiation state w/o fear of
encountering stale messages in the queue.

Suggested-by: @ddustin
Same argument as for including it in open_channel2
Don't assume that option-static-remotekey is set
This issue is non-trivial and worth mentioning, otherwise implementations
may forget to handle this which would result in an easy way of attacking
node's on-chain liquidity, creating a large opportunity cost.
This was previously assumed but adding it to the spec makes it explicit, should we ever want to change it in the future.
@niftynei
Copy link
Owner

niftynei commented May 4, 2023

is this more of a suggestion than a MUST?

rationale is that if they do any other sighash, it'll still be valid.

@niftynei niftynei force-pushed the nifty/dual-fund branch 2 times, most recently from d3c99ca to 091397f Compare June 30, 2023 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants