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

interactive-tx: Add dual-funding flow, using the interactive tx protocol (feature 28/29) #851

Merged
merged 21 commits into from
Feb 13, 2024

Conversation

niftynei
Copy link
Collaborator

@niftynei niftynei commented Mar 2, 2021

This commit adds the interactive transaction construction protocol, 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.

The easiest way to understand the gist of this PR is this video, which explains the prior protocol and overviews this one.

https://youtu.be/i_GxmNZjwhk

@rustyrussell
Copy link
Collaborator

26/27 already taken by #814 (at least, I proposed it move there to avoid clash)? Try 28/29?

@niftynei niftynei changed the title interactive-tx: Add dual-funding flow, using the interactive tx protocol (feature 26/27) interactive-tx: Add dual-funding flow, using the interactive tx protocol (feature 28/29) Mar 2, 2021
02-peer-protocol.md Outdated Show resolved Hide resolved
02-peer-protocol.md Outdated Show resolved Hide resolved
02-peer-protocol.md Outdated Show resolved Hide resolved
02-peer-protocol.md Outdated Show resolved Hide resolved
02-peer-protocol.md Outdated Show resolved Hide resolved
02-peer-protocol.md Outdated Show resolved Hide resolved
02-peer-protocol.md Outdated Show resolved Hide resolved
02-peer-protocol.md Outdated Show resolved Hide resolved
02-peer-protocol.md Outdated Show resolved Hide resolved
02-peer-protocol.md Outdated Show resolved Hide resolved
Copy link

@ariard ariard left a comment

Choose a reason for hiding this comment

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

Ongoing review, awesome work!

02-peer-protocol.md Outdated Show resolved Hide resolved
02-peer-protocol.md Show resolved Hide resolved
02-peer-protocol.md Show resolved Hide resolved
@Kixunil
Copy link

Kixunil commented Apr 8, 2021

Would it make sense to add optional auth_token field (up to 33 bytes?) so that one could only allow dual-funded channels for authorized peers? Or maybe to identify how much the other side wants to put into the channel.

Can be done by node ID too but this may lead to less UX hassle as the ID doesn't need to be transferred back-and-forth.

@niftynei
Copy link
Collaborator Author

niftynei commented Apr 8, 2021 via email

@Kixunil
Copy link

Kixunil commented Apr 9, 2021

Yes, I mean perhaps it'd be beneficial to have a standardized field so that clients implement the interface of filling that in. Would be real shame if only LND could open authenticated channel to LND, Elair to Ecliar and c-lightning to c-lightning...

Basically have a way for the server to provide hex/base64 string that user fills in to get double-funded channel. This will be easier if all impls agree on the field being used.

Copy link

@ariard ariard left a comment

Choose a reason for hiding this comment

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

Left few more comments, 3 high-level remarks in mind :)

  • Should we recommend to verify finalized interactive transaction against client's full-node transaction standardness verifier (i.e if you're using Core's testmempoolaccept) ? Even if we precise most of the well-known policy checks, we still have edge cases like interactive transaction's parent being a non-mature coinbase transaction. Likely impossible to handle for mobile clients, without assuming full-validation, for them detection of wrongdoing might be only a best-effort.

  • Should we enrich failure modes ? Right now the only one detailed is broadcasting the commitment transaction thus stopping the cooperation with the counterparty. This might not work in adversarial settings, where your counterparty is pinning the interactive transaction to drive crazy your fee-bumping logic or DoS your committed coins or whatever. In that case of scenario, honest double-spend of one's inputs after a timeout might be wiser.

  • Should we introduce the interactive transaction section in its own BOLT ? As the 2 above points underscore it, multi-party funded transaction in the context of trust-minimized counterparty is far harder to get right than single-funded transaction. We migh land first this chunk, and from then enrich/harden the spec with best practices as we discover more edge cases and base layer support improves and we might add poodle or an equivalent solution to the privacy issues. Further, I think this interaction transaction protocol is best thing we have in the ecosystem which look like the specification of a coinjoin ? I can definitively see non-ln projects implementing it or seeking compatibility or extending it to achieve fancier stuff like cut-through.

02-peer-protocol.md Outdated Show resolved Hide resolved
02-peer-protocol.md Outdated Show resolved Hide resolved
02-peer-protocol.md Show resolved Hide resolved
added by the sending node
- the `txid` does not match the txid of the transaction
- MUST fail the channel if:
- the `witness_stack` weight lowers the effective `feerate`
Copy link

Choose a reason for hiding this comment

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

FYI, this one is quite easy to bypass by an adversarial counterparty. Let's announce first a witnessScript with alternative script branches though diverging witnesses's weights. Branch A weight is 60 wu, branch B weight is 120 wu. You provide witness A to your honest counterparty, it satisfies the negotiated transaction feerate so honest signatures/witnesses are answered back. Now, you broadcast transaction with witness B, thus lowering transaction feerate.

Ideally, I think you could sanitize counterparty's witness through miniscript to learn if it's the most economical spending.

02-peer-protocol.md Outdated Show resolved Hide resolved
02-peer-protocol.md Show resolved Hide resolved
02-peer-protocol.md Outdated Show resolved Hide resolved
A `fee_step` 2 would be `1.25^2 * 512` (or 640 + 640 / 4), 800 sat/kw.

If a valid `funding_locked` message is received in the middle of an
RBF attempt, the attempt MUST be abandoned.
Copy link

Choose a reason for hiding this comment

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

Should we recommend to enforce bip125 rules 3 and 4 ? It's quite easy to increase the feerate by diminishing the number of inputs/outputs though paying less in absolute fee and thus hitting a rejection of the replacement ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ideally yes, but in practice it's impossible to pre-commit to a feerate that is guaranteed to surpass the last tx's absolute fee, given that the input set can be reconfigured.

Open to suggestions on how to enforce this however.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As currently implemented, in some cases you have to repeatedly RBF (and increment the feerate) until you hit upon a tx that works -- really not ideal but it works.


It's recommended that a peer, rather than fail the RBF negotiation due to
a large feerate change, instead sets their `funding_satoshis` to zero,
and decline to participate further in the channel funding.
Copy link

Choose a reason for hiding this comment

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

Do you have ambiguity there ? You might be interested to participate further in the interactive transaction confirmation but not in this specific channel as all its value is swallowed by the bumping feerate ?

Should init_rbf/ack_rbf be transaction-level messages and not channel-level ones ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should init_rbf/ack_rbf be transaction-level messages and not channel-level ones ?

I'm not sure I follow what you mean by this -- as described they re-initiate the tx-construction phase of the protocol.

02-peer-protocol.md Outdated Show resolved Hide resolved
@t-bast
Copy link
Collaborator

t-bast commented Jun 7, 2021

I think this proposal is a great opportunity to provide a standard way of paying for liquidity (to help with the initial inbound liquidity problem). Maybe it's worth a separate PR that will enrich the interactive-tx protocol (and can be merged separately), I'll work on that once I've done a more thorough review of this PR.

The high-level idea is to simply introduce new TLVs to allow participants to:

  • announce the fee they would charge to provide liquidity (funds on their side) -> probably in node_announcement
  • request the remote participants to add X sats by paying that fee -> in the interactive-tx messages

Deducing the fee from the initial main outputs would then be deterministic and could be verified before finalizing the funding tx, which is great. It would remove the hackish uses of push_msat in trusted liquidity providers.

It would also provide an incentive for nodes to add funds to channels where they're not the initiator. Because without such a fee, it will be hard to automate the addition of funds when we're fundee, because we most likely don't want to add some of our funds to every channel that remote nodes open to us (otherwise they could easily deplete our utxo pool).

Any thought on this high-level proposal before I start diving into technical details?

@niftynei
Copy link
Collaborator Author

niftynei commented Jun 7, 2021

I think this proposal is a great opportunity to provide a standard way of paying for liquidity (to help with the initial inbound liquidity problem). Maybe it's worth a separate PR that will enrich the interactive-tx protocol (and can be merged separately), I'll work on that once I've done a more thorough review of this PR.

The high-level idea is to simply introduce new TLVs to allow participants to:

  • announce the fee they would charge to provide liquidity (funds on their side) -> probably in node_announcement
  • request the remote participants to add X sats by paying that fee -> in the interactive-tx messages

Deducing the fee from the initial main outputs would then be deterministic and could be verified before finalizing the funding tx, which is great. It would remove the hackish uses of push_msat in trusted liquidity providers.

It would also provide an incentive for nodes to add funds to channels where they're not the initiator. Because without such a fee, it will be hard to automate the addition of funds when we're fundee, because we most likely don't want to add some of our funds to every channel that remote nodes open to us (otherwise they could easily deplete our utxo pool).

Any thought on this high-level proposal before I start diving into technical details?

I think this is a great idea! So great in fact that I've already written up a spec draft and have started working on implementing it. I just pushed up the draft version #878, and would love to collaborate, get your feedback on it.

@niftynei
Copy link
Collaborator Author

niftynei commented Jun 8, 2021

Should we recommend to verify finalized interactive transaction against client's full-node transaction standardness verifier.

Hm. Ideally we would specify all of the "standardness" rules here so as not to be dependent on the 'standardness verifier' implementation, however that list seems to fairly changeable. Better to specify what to do in the case that the broadcast fails? (double spend the inputs/attempt another RBF).

Should we enrich failure modes ? ... In that case of scenario, honest double-spend of one's inputs after a timeout might be wiser.

Yeah, the 'double spending of an input to cancel an open' is a bit implicit here, it's probably worthwhile to call it out explicitly.

Should we introduce the interactive transaction section in its own BOLT ? As the 2 above points underscore it, multi-party funded transaction in the context of trust-minimized counterparty is far harder to get right than single-funded transaction. We migh land first this chunk, and from then enrich/harden the spec with best practices as we discover more edge cases and base layer support improves and we might add poodle or an equivalent solution to the privacy issues.

Yeah, I think this is a good idea, and something that we can do naturally/easily after this is merged (and more sections of the spec adopt it/use it)

@niftynei
Copy link
Collaborator Author

Most recent push is whitespace only changes.

Copy link
Collaborator

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

ACK 024df99

@t-bast
Copy link
Collaborator

t-bast commented Dec 5, 2023

During yesterday's spec meeting, we discussed griefing attacks and pinning again, and suggested re-evaluating the option of making the opener send tx_signatures first regardless of who contributed more funds (see niftynei#9 for historical discussions).

There are three pinning vectors that apply to interactive-tx transactions:

  1. Unconfirmed inputs that have a low ancestor feerate
  2. Chaining low-feerate descendants to a change output of the transaction
  3. Inputs for which the witness may be arbitrarily inflated to increase the weight of the transaction

Nodes can avoid the first one by setting the require_confirmed_inputs TLV.

The second one can be fixed by either constraining change outputs (to have a CSV 1 for example) or using v3 transactions when available. In practice though, as long as nodes ensure that the transaction has a reasonable feerate, it is very likely that it will confirm: it isn't that simple to reliably pin it.

The third one can be fixed by either sending tx_signatures last, or restricting what inputs may be used (e.g. to p2wpkh or key-path-only p2tr, or some template scripts). The main issue of changing the order of tx_signatures is that it breaks batching and adds a non-deterministic ordering rule to the protocol.

I'd like to highlight a few things to help guide the decision. First of all, if we keep the existing tx_signatures ordering, the attacker needs to lock more funds than the attacked node to be able to sign last. Also, this attack only works when the non-initiator decides to add funds to the dual-funded channel, which in practice will only happen if the initiator is paying for a liquidity ads, and the seller will apply some rate-limits based on who the buyer is (whether they have other channels in the network, if those channels are old, if the seller still has enough liquidity to spare, etc). When selling liquidity, you will need to score the buyer somehow to try to make the most yield, so that's simply another risk factor to take into account.

My personal opinion is that it's not worth introducing the tx_signatures ordering option, or at least not right now. It's unclear whether it will be necessary in practice, so I'd rather keep the protocol simpler, and implement policies on the liquidity ads seller side (restricting input types and who we sell liquidity to) if griefing becomes a widespread issue. If we later discover that we absolutely need to fix that, we can easily introduce this with a new feature bit. By that time, I hope that other solutions (mostly improvements to the bitcoin mempool policies) will give us better guarantees against pinning attacks as a whole.

@morehouse @niftynei what do you think?

@morehouse
Copy link
Contributor

During yesterday's spec meeting, we discussed griefing attacks and pinning again, and suggested re-evaluating the option of making the opener send tx_signatures first regardless of who contributed more funds (see niftynei#9 for historical discussions).

There are three pinning vectors that apply to interactive-tx transactions:

1. Unconfirmed inputs that have a low ancestor feerate

2. Chaining low-feerate descendants to a change output of the transaction

3. Inputs for which the witness may be arbitrarily inflated to increase the weight of the transaction

AFAICT, vector 2 only works in combination with vector 3.

There's also another category of pinning, where mempools get partitioned -- a low-feerate transaction that conflicts with the funding tx can be pinned in some mempools while the funding tx is in the victim's mempool.

Nodes can avoid the first one by setting the require_confirmed_inputs TLV.

Perhaps we make this the default instead of a TLV?

The second one can be fixed by either constraining change outputs (to have a CSV 1 for example) or using v3 transactions when available.

I'm not sure these are magic bullets either. Either one can require the victim to spend some extra fees to CPFP or double-spend the funding tx. Under sustained griefing, the victim's funds could be slowly burned as fees.

In practice though, as long as nodes ensure that the transaction has a reasonable feerate, it is very likely that it will confirm: it isn't that simple to reliably pin it.

Right, vector 3 is what enables the feerate to be reduced so that pinning can occur.

The third one can be fixed by either sending tx_signatures last, or restricting what inputs may be used (e.g. to p2wpkh or key-path-only p2tr, or some template scripts).

I think p2tr doesn't work, because even key-path spends can be inflated via the annex.

The main issue of changing the order of tx_signatures is that it breaks batching and adds a non-deterministic ordering rule to the protocol.

I'd like to highlight a few things to help guide the decision. First of all, if we keep the existing tx_signatures ordering, the attacker needs to lock more funds than the attacked node to be able to sign last.

Sort of, but those funds don't need to actually be the attacker's. If Mallory targets 2 (or more) LSPs at once, she can use the LSPs' inputs against each other, ensuring she sends tx_signatures last. Mallory only needs to lock enough funds to pay the liquidity ads.

Also, this attack only works when the non-initiator decides to add funds to the dual-funded channel, which in practice will only happen if the initiator is paying for a liquidity ads, and the seller will apply some rate-limits based on who the buyer is (whether they have other channels in the network, if those channels are old, if the seller still has enough liquidity to spare, etc). When selling liquidity, you will need to score the buyer somehow to try to make the most yield, so that's simply another risk factor to take into account.

My personal opinion is that it's not worth introducing the tx_signatures ordering option, or at least not right now. It's unclear whether it will be necessary in practice, so I'd rather keep the protocol simpler, and implement policies on the liquidity ads seller side (restricting input types and who we sell liquidity to) if griefing becomes a widespread issue. If we later discover that we absolutely need to fix that, we can easily introduce this with a new feature bit. By that time, I hope that other solutions (mostly improvements to the bitcoin mempool policies) will give us better guarantees against pinning attacks as a whole.

Yeah, I'm not sure either. I made a Discourse post discussing liquidity griefing in depth. It would be great to get some discussion going over there.

@t-bast
Copy link
Collaborator

t-bast commented Dec 7, 2023

Sort of, but those funds don't need to actually be the attacker's. If Mallory targets 2 (or more) LSPs at once, she can use the LSPs' inputs against each other, ensuring she sends tx_signatures last. Mallory only needs to lock enough funds to pay the liquidity ads.

Right, that's a good point!

Perhaps we make this the default instead of a TLV?

In the longer term, we should be able to get rid of this restriction, so I'd rather keep it opt-in. We discussed this a while ago, there are scenarios where it makes sense to use unconfirmed inputs to optimize liquidity usage (high fees and chains of unconfirmed txs), and if the other party is honest, this doesn't create any issue.

I'm not sure these are magic bullets either. Either one can require the victim to spend some extra fees to CPFP or double-spend the funding tx. Under sustained griefing, the victim's funds could be slowly burned as fees.

True, this can create a slow drain on the victim's funds if the attack is sustained. But that's only under the assumption that the transactions never confirm on their own, which they sometimes should if the feerate is reasonable, in which case it costs fees for the attacker, not the victim. If the feerate was too low and we're using v3, then a double-spend isn't necessarily costing anything to the victim, that double-spend would just pay the expected current feerate (and if this is double-spent in another liquidity ads attempt, it won't be the seller who pays the on-chain fees).

@morehouse
Copy link
Contributor

morehouse commented Dec 7, 2023

In the longer term, we should be able to get rid of this restriction, so I'd rather keep it opt-in.

How will we do this?

I'm not sure these are magic bullets either. Either one can require the victim to spend some extra fees to CPFP or double-spend the funding tx. Under sustained griefing, the victim's funds could be slowly burned as fees.

True, this can create a slow drain on the victim's funds if the attack is sustained. But that's only under the assumption that the transactions never confirm on their own, which they sometimes should if the feerate is reasonable, in which case it costs fees for the attacker, not the victim. If the feerate was too low and we're using v3, then a double-spend isn't necessarily costing anything to the victim, that double-spend would just pay the expected current feerate (and if this is double-spent in another liquidity ads attempt, it won't be the seller who pays the on-chain fees).

Good points. I did a little math to see what a typical griefing might cost an LSP.

Some recent mempool feerates:

  • Minimum relay: 10 sat/vB
  • 1 week confirmation: 20 sat/vB
  • 6 hour confirmation: 80 sat/vB

Let's say we use 80 sat/vB for a funding transaction and the LSP's portion of the transaction is 250 vB, so they allocate 80 * 250 = 20,000 sat for the transaction fee. Let's say the total transaction is 500 vB, so the total fee for the transaction is 40,000 sat.

The attacker inflates their witnesses to reduce the feerate to 10 sat/vB and also chains a 1,000 vB descendant transaction (the max allowed by v3 policy) with a fee of 10,000 sat (10 sat/vB). This package is unlikely to confirm for a very long time.

If the LSP wishes to double-spend their griefed UTXOs, they must now spend 40,000 + 10,000 = 50,000 sat. If they use another 500 vB dual-funded transaction to double spend, the required feerate for that transaction would be 50,000 sat / 500 vB = 100 sat/vB This is 25% more than the expected current feerate.

Suppose further that the attacker is doing repeated griefing and this new dual-funded transaction is actually with another node controlled by the attacker. The attacker can grief this transaction as well, causing the double-spend fee to increase to 50,000 + 10,000 = 60,000 sat (120 sat/vB). And so on.

@t-bast
Copy link
Collaborator

t-bast commented Dec 7, 2023

How will we do this?

If an attacker lowers the effective feerate by using low-feerate ancestors, or double-spends one of the ancestor transactions, this is free to double-spend: a competing transaction can use the same feerate without any additional cost. The only cumbersome part is how long you wait before figuring out that something is fishy with that transaction and you should unlock its utxos. For that, you need a heuristic that "this tx seems to have a good enough feerate, but it hasn't confirmed for X blocks, so let's mark it as malicious".

The attack becomes more painful when coupled with a descendant chain, and IMO the only way to fix this that doesn't have annoying drawbacks is to fix mempool policies ™️ 🤞 (v3 txs / cluster mempool).

If we don't need to care about descendant pinning, this is thus not so much of an issue and confirmed inputs shouldn't be the default (as they create inefficient liquidity usage). I've detailed that point a bit more on your delving bitcoin post, maybe we should keep the discussion over there where it will be accessible to more readers than on this PR?

Good points. I did a little math to see what a typical griefing might cost an LSP.

Nice work! But this pinning attack does not reliably work though, so I believe it's not the end of the world. To be able to effectively lower the feerate of the funding transaction, the attacker needs to have low-feerate ancestors. But since those ancestors are low-feerate, they will be dropped from a fraction of the network's mempools, which will allow an honest transaction at the normal feerate to go through (because in those mempools, that honest transaction isn't a double-spend, the conflicting transactions have been evicted). That's what @TheBlueMatt mentioned a few times, that in practice today, based on his experiments, even low-ish feerate transactions are still able to propagate and confirm.

Make `require_confirmed_inputs` explicit for RBF regnegotiation.

Requested-By: @t-bast
Copy link
Collaborator

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

ACK 27ffef4, I think this is ready to be merged! 🎉

Let's discuss this during the next spec meeting (early january) and hopefully this will be our first merged PR in 2024!

@t-bast
Copy link
Collaborator

t-bast commented Jan 22, 2024

Since this PR hasn't been merged yet, I'd like to get everyone's feedback on adding yet another change before integrating this: niftynei#18

After having hands-on experience doing support for users who are affected by this, I think it would be best to immediately provide the tools to get rid of this issue entirely now that taproot is a thing. The PR linked contains two design options to achieve that, let me know what you think is best. Even though we have running code using the current messages, I'm fully open to making backwards-incompatible changes to the spec if necessary to ensure a cleaner protocol in the future without the need for additional feature bits/negotiation.

@niftynei @morehouse @dunxen @ddustin

@t-bast
Copy link
Collaborator

t-bast commented Feb 5, 2024

After figuring out the attack vectors in more details, I've retracted niftynei#18 (more details directly on that PR).

So I'm stilli ACKing 27ffef4 and I think we should consider merging this PR!

@t-bast
Copy link
Collaborator

t-bast commented Feb 13, 2024

As discussed in yesterday's spec meeting, this PR has widespread ACKs so we can merge it whenever you want @niftynei 🎉 🎉

The only remaining open comment is this one: #851 (comment), up to you to decide what you want to do here (and we can also re-work the wording or rationale in a follow-up PR if necessary).

@niftynei niftynei merged commit 0bdaa8b into lightning:master Feb 13, 2024
@niftynei
Copy link
Collaborator Author

Thanks @t-bast for shepherding this through for the last few months (years?). Would not have happened without his tireless focus on getting this done!

Now we splice?? :)

@t-bast
Copy link
Collaborator

t-bast commented Feb 14, 2024

not my final form

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.