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

Alternative proposal for dual-funding tx_signatures ordering #9

Closed
wants to merge 39 commits into from

Conversation

t-bast
Copy link

@t-bast t-bast commented Oct 20, 2022

The peer that sends tx_signatures first is taking a risk when they are contributing to the channel funding: the other peer may never send their tx_signatures, and the signer will need to double-spend themselves to ensure that their funds are not locked.

The accepting node is more at risk here, as they don't choose who tries to open channels to them, whereas an opening node who detects a malicious accepting node can just go somewhere else to open a channel.

Thus by default, the opener should send their tx_signatures first, unless they explicitly request the accepting node to go first because they are trying to coordinate multiparty tx construction.

In my opinion, there are only two cases where the accepting node would contribute to a channel being opened to them:

  • when they are using liquidity ads and are getting paid for it: in that case, they may be willing to take a risk and sign first (with policies to restrict how often / how much they are risking)
  • when the opening node is in some kind of pre-configured whitelist of nodes the accepting node is interested in, in which case the node operator has manually selected those nodes and is probably willing to take the risk (at least once)

Credits to @morehouse who laid this out in lightning#851 (comment)

@morehouse @niftynei let me know what you think. I'm not convinced that this is the right way, but at least it's an alternative to consider.

NB: this builds on top of #7, please only look at the last commit

niftynei and others added 30 commits August 3, 2022 16:52
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
So that it knows what open_channel2 is responding to.

Spotted-By: @rustyrussell
Remove the fee_step in favor of using an explicit bump feerate, with
the constraint that it must be at least 1/64th greater than the last
sent rate.

Suggested-By: @rustyrussell
As @t-bast points out, we define 'point' in 01, which covers the
encoding.
"Failing the channel" may not be the right action in all cases where the
interactive construction protocol is used (e.g. closes), so we move
reference to it as a mitigation technique specifically to the usage of
the interactive protocol in the v2 open sequence.

Suggested-By: @t-bast
For open_channel2 + accept_channel2 we use the opener's basepoint and a
zeroed out point for the accepter.  This is because we don't know what
the accepter's basepoint is yet. (It's transmitted in accept_channel2)

Pointed-Out-By: @t-bast
P2SH-wrapped inputs are segwit inputs and should be eligible for
spending, however the spec as written didn't permit them

Reported-by: @SomberNight
P2SH outputs have larger dust requirements than native SegWit (and
Taproot) scripts; we disallow the creation of them on funding txs as a
they're problematic for relay when allowing dust limits below 546 sats
(see lightning#894)

Suggested-by: @t-bast
input_count of 1 times min witness weight of 110, is 110. not 107

the sum of 612 is correct, however.

Reported-By: @SimonVrouwe
* Move RBF inside the interactive-tx protocol
* Use same tlvs as open_channel / accept_channel
* Require native segwit
* Fix a few typos and whitespaces issues (thanks markdown linter)
And add `channel_type` tlv.
t-bast and others added 5 commits August 3, 2022 17:01
We use 1/24 instead of 1/64 to produce larger steps in fee-bumping.
Add signature data, remove wrapped segwit inputs, update calculations
to be more explicit about how to figure fee
If both peers contribute an equal amount to a interactive tx,
peer with "lower" pubkey sends first
The weight of a witness stack for a p2wpkh low-r signature is 107 bytes.
Don't show up in the CLN generator w/o this
Copy link

@morehouse morehouse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me!

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.
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.
@niftynei
Copy link
Owner

I think I've come around to being ok with a flag for "peer must be first" flag. The griefing usecase for a large LSP is a "real and present" danger as you've pointed out. While we might be able to mitigate this in the future with PODLEs, right now it's a good thing to have so we should add it.

I would suggest that we leave the "biggest amount goes last" as the default (as this nominally helps with batched/multiparty opens... even though I haven't coded it up yet, currently we send sigs as soon as we have them). What do you think @t-bast ?

Also apologies for moving the discussion, I can't figure out where the last comments ended up...

@t-bast
Copy link
Author

t-bast commented Oct 27, 2022

I would suggest that we leave the "biggest amount goes last" as the default (as this nominally helps with batched/multiparty opens... even though I haven't coded it up yet, currently we send sigs as soon as we have them). What do you think @t-bast ?

Just to make sure I understand this correctly, instead of "opener goes first but can request the other peer to start", you would like to have:

  • If remote_tx_signatures_first is not included, "smallest amount contributed signs first"
  • Either the opening node or the accepting node may send remote_tx_signatures_first (respectively in open_channel2 or accept_channel2) to override that default ordering
  • If the opening node sent remote_tx_signatures_first, the accepting node cannot send it and must send an error if they don't want to sign first

I believe this is worse than "opener goes first but can request the other peer to start", because in practice what will happen is that liquidity providers (accepters) will always set remote_tx_signatures_first to protect themselves by default (that's the most rational choice to maximize the yield on their liquidity). This means that opening nodes will have to send remote_tx_signatures_first whenever they want to do batching, and that the default ordering of "smallest amount contributed signs first" will actually never be used, so why even specify (and implement) it? That only adds unnecessary complexity, doesn't it?

this nominally helps with batched/multiparty opens

Why does it help though? If you're batching you just have to signal remote_tx_signatures_first and then you know that the batching will just work.

Also apologies for moving the discussion, I can't figure out where the last comments ended up...

That's what happens when your PRs are too popular, too many people are interested and make great comments on it 😆

The peer that sends `tx_signatures` first is taking a risk when they are
contributing to the channel funding: the other peer may never send their
`tx_signatures`, and the signer will need to double-spend themselves
to ensure that their funds are not locked.

The accepting node is more at risk here, as they don't choose who tries
to open channels to them, whereas an opening node who detects a
malicious accepting node can just go somewhere else to open a channel.

Thus by default, the opener should send their `tx_signatures` first, unless
they explicitly request the accepting node to go first because they are
trying to coordinate multiparty tx construction.
@t-bast t-bast force-pushed the tx-signatures-ordering branch from 3f3b512 to 1e03d44 Compare October 27, 2022 10:05
@morehouse
Copy link

I would suggest that we leave the "biggest amount goes last" as the default (as this nominally helps with batched/multiparty opens... even though I haven't coded it up yet, currently we send sigs as soon as we have them). What do you think @t-bast ?

Leaving the naive dual-funding user open to DoS seems like a poor default.

I agree with @t-bast, and regardless of the "default" in the spec, for LND I intend to implement remote_tx_signatures_first as default for acceptors.

@t-bast
Copy link
Author

t-bast commented Nov 23, 2022

I don't think this change is useful anymore, as I think the strategy outlined here gets rid of the griefing attack entirely with close to no downside. I'll close this PR soon unless we find an issue with the anti-griefing strategy.

@t-bast t-bast closed this Dec 2, 2022
@t-bast t-bast deleted the tx-signatures-ordering branch December 2, 2022 11:33
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