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

DLC Mutual Close #170

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

matthewjablack
Copy link
Contributor

This PR adds new close_dlc as per the discussion regarding #161 which enables a way to request to close a DLC before cet_locktime with mutual consent

Protocol.md Outdated Show resolved Hide resolved
Protocol.md Outdated Show resolved Hide resolved
Protocol.md Outdated Show resolved Hide resolved
Protocol.md Outdated Show resolved Hide resolved
Protocol.md Outdated Show resolved Hide resolved
Protocol.md Outdated Show resolved Hide resolved
Protocol.md Outdated Show resolved Hide resolved
Protocol.md Outdated Show resolved Hide resolved
Protocol.md Outdated Show resolved Hide resolved
Protocol.md Show resolved Hide resolved
@benthecarman
Copy link
Contributor

imo this would be better if it was more generalized. It would be great if this could be used to inside of any transaction. That way we could close our DLC inside of a regular transaction or in a batch with many other people, or even inside of a coinjoin.

Copy link
Contributor

@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.

I agree with Ben, it could be better to uplift this scheme as an application on top of LN splice-in/splice-out. See BOLT specification draft : lightning/bolts#863

This message contains information about mutual close, which allows the counterparty
to broadcast a mutual closing transaction.

1. type: ? (`close_dlc_v0`)
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use something in the LN experimental range : 32768 - 65535 and mark it as reserved in the waiting room : lightning/bolts#716

Copy link
Member

Choose a reason for hiding this comment

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

I guess ideally the best would be to use 42784 to follow after the signing message.

Choose a reason for hiding this comment

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

42784 seems to be already used by contract_descriptor_v1

Copy link
Member

Choose a reason for hiding this comment

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

Yes, so until #171 or similar is merged it is taken, but after all the inner structures will use context specific type numbers.

Copy link
Contributor

Choose a reason for hiding this comment

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

what number was ended up being chosen here?

Copy link
Member

Choose a reason for hiding this comment

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

At the end I think we decided not to touch message type numbers in #163 so I guess whatever is ok?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point, added to TODO section at end of PR

Protocol.md Outdated
`fund_input_serial_id` is a randomly chosen number which uniquely identifies the funding output to be spent.
Inputs in the closing transaction will be sorted by `fund_input_serial_id` and `input_serial_id` in `funding_inputs`.

`funding_inputs` are extra inputs to mutual close to enable the sending party to nullify the transaction by double spending the inputs.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure you can really guarantee the double-spend without further checks on the payouts outputs witnessScripts. I think your counterparty might start to spend its payouts, even honestly, as a start of a new DLC or LN, thus falsifying your fee-estimation to successfully double-spend.

One mitigation could be to require to reveal witnssScripts and enforce there is a 1 OP_CSV to ensure children can be accepted as final in the network mempools.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what is your concern, but the point of being able to double spend is in case your counter party doesn't broadcast the closing TX, so I feel if he would be spending the closing TX output it means that it's already broadcast and thus you don't need to worry about double spending?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree with @Tibo-lg. Curious to know if there's a case that's not accounted for here.

Generally the initiator should double-spend the funding_inputs in the case that the closing tx is not created in a timely manner (very dependent on the type of contract of course, and market conditions). This should be done shortly after sending the dlc close message, in the case that the counterparty doesn't mutually close the dlc.

In the case that the counterparty does mutually close the dlc in a timely manner, then nullifying the transaction would no longer be a concern.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to be clear, what do you qualify by "extra inputs", the funding outputs of a DLC funding transaction or inputs beyond the scope of the protocol ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These extra inputs are beyond the scope of the protocol. Just utxos that exist in a users wallet

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks I see, though I think my point still hold.

Let's say you have a mutual closing transaction between Alice-Bob composed of 2-inputs and 2-outputs.

First input is the outpoint of the funding transaction output (the 2-of-2 P2WSH)
Second input is an input from Alice wallet.
First output is Alice's payout.
Second output is Bob's payout.

The mutual close is broadcasted by Alice with an absolute fee of 1000 sats.
Bob spends his own payout output with a child transaction paying an absolute fee of 1000 sats.
Alice attempts to double-spend the mutual close by spending her wallet input with a transaction paying an absolute fee of 1500 sats.

According BIP125, rule 3, a replacement transactions must pay an absolute fee higher than the replaced transactions. Alice attempts should fail as the fee is lower than the mutual close + Bob's child one ?

I think if we want to guarantee the replacement of the mutual close with an unilateral CET, I think we should ensure that

  1. the payout outputs are encumbered by a 1 CSV (to expel any child from the network mempools)
  2. the absolute fee, the feerate and replacement fee penalty of any CET transaction is higher than the mutual close one

However, I think fulfilling the second requirement is in contradiction with the goal of this proposal, namely fast-close in period of fees higher than the feerate with which CET have been counter-signed.

Let me know if you think the description is correct.

So I think a better behavior, instead of double-spending the mutual close with wallet inputs could be to CPFP the mutual close ? And if it doesn't work, assumes the CET has been broadcast across network mempools and CPFP the CET ?

Or we can swallow the bullet for now, ship it as it is, mark it as an issue in #132 and wait for package-relay simplifying that kind of issue :) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I see the confusion. The initiator has a wallet input, but the reciprocator (the one broadcasting) does not.

So I think a better behavior, instead of double-spending the mutual close with wallet inputs could be to CPFP the mutual close ? And if it doesn't work, assumes the CET has been broadcast across network mempools and CPFP the CET ?

Additionally, the purpose of the wallet input is to nullify the mutual close in the case that the mutual close has not been broadcast at all. So it would not be possible to CPFP the mutual close if it has not been broadcast in the first place.

The mutual close is broadcasted by Alice with an absolute fee of 1000 sats.
Bob spends his own payout output with a child transaction paying an absolute fee of 1000 sats.
Alice attempts to double-spend the mutual close by spending her wallet input with a transaction paying an absolute fee of 1500 sats.

In the DLC Close spec proposed, only one party has a wallet input, and that would be the initiator. So if Alice is broadcasting the closing tx, she would be the reciprocator, not the initiator, and she would not have her own wallet input as part of the closing tx. Bob however would have a wallet input.

So a feasible scenario similar to what you've proposed might look like this (assuming Alice is the initiator and Bob is the reciprocator and Alice is spending her wallet input because Bob did not broadcast mutual close in a timely manner):

First input is the outpoint of the funding transaction output (the 2-of-2 P2WSH)
Second input is an input from Alice wallet.
First output is Alice's payout.
Second output is Bob's payout.

Alice spends her wallet input with a transaction paying an absolute fee of 1500 sats.
Bob attempts to double-spend Alice's wallet input by broadcasting the mutual close with an absolute fee of 1000 sats.
Bob continues the attempt to double spend Alice's wallet input by spending his own payout output with a child transaction paying an absolute fee of 1000 sats.

In this instance, Alice would be spending her wallet input since Bob did not broadcast in a timely manner.

What's not clear to me is whether BIP 125 rule 3 would make Bob's double-spend using the mutual close tx invalid since the absolute fees would be lower than Alice's original spend of her wallet input.

But regardless of whether Bob's broadcast of the mutual close is successful, Alice has either successfully spent her wallet input, nullifying the mutual close, or has forced Bob's hand to actual broadcast the mutual close in a timely manner, which is why Alice was spending her wallet input in the first place.

In a real-world scenario, one could imagine a futures contract that Alice and Bob have entered into for a DLC. After some time inside the contract, Alice wishes to exit the position with Bob, and waits only 1 minute after passing a DlcClose message to him. After 1 minute, Alice does not detect the mutual close tx in the mempool, so Alice spends her wallet input and either she succeeds in nullifying the mutual close tx, or Bob successfully double-spends her wallet input with the mutual close tx which achieves Alice's goal in the first place.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, now I think we have the same model in mind.

What's not clear to me is whether BIP 125 rule 3 would make Bob's double-spend using the mutual close tx invalid since the absolute fees would be lower than Alice's original spend of her wallet input.

Currently yes, BIP 125 rule 3 would prevent Bob's double spend Alice input. Though if package relay is deployed Bob would be able to broadcast a mutual_close + CPFP and if mempool fee spikes at the same time, blocks Alice wallet input spending. Of course, Alice could override assuming she is able to observe Bob mutual_close + CPFP broadcast (an attacker could prevent this by broadcasting low-fees M1 inside Alice's mempool and pinning M2 across the rest of the network).

I think that case is good to know and to monitor when package relay is deployed.

But regardless of whether Bob's broadcast of the mutual close is successful, Alice has either successfully spent her wallet input, nullifying the mutual close, or has forced Bob's hand to actual broadcast the mutual close in a timely manner, which is why Alice was spending her wallet input in the first place.

I think that's correct and it would be a good rational to add to the PR. Further, I think it could be good to specify an implementation-defined nullifying_point for Alice to enforce, similar in spirit to the security_point from Non-Interactive-Protocol.md. Eg, value could be dynamic in function of mempool fees changes nullifying the economic equilibrium of the DLC.

In a real-world scenario, one could imagine a futures contract that Alice and Bob have entered into for a DLC. After some time inside the contract, Alice wishes to exit the position with Bob, and waits only 1 minute after passing a DlcClose message to him. After 1 minute, Alice does not detect the mutual close tx in the mempool, so Alice spends her wallet input and either she succeeds in nullifying the mutual close tx, or Bob successfully double-spends her wallet input with the mutual close tx which achieves Alice's goal in the first place.

Really agree on the long-term design that we should have a bunch of options, even negotiated with fee premiums to update or exit the DLC in function of other changes, such as mempool fee spikes or lightning routing fees ones :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Glad we're on the same page :)

Currently yes, BIP 125 rule 3 would prevent Bob's double spend Alice input. Though if package relay is deployed Bob would be able to broadcast a mutual_close + CPFP and if mempool fee spikes at the same time, blocks Alice wallet input spending. Of course, Alice could override assuming she is able to observe Bob mutual_close + CPFP broadcast (an attacker could prevent this by broadcasting low-fees M1 inside Alice's mempool and pinning M2 across the rest of the network).

Got it, so essentially as it stands, BIP 125 would favor Alice, and package relay would favor Bob. Either way, she achieves her goal.

I think that's correct and it would be a good rational to add to the PR. Further, I think it could be good to specify an implementation-defined nullifying_point for Alice to enforce, similar in spirit to the security_point from Non-Interactive-Protocol.md. Eg, value could be dynamic in function of mempool fees changes nullifying the economic equilibrium of the DLC.

Great point, I've added nullifying_point and rationale to Non-Interactive-Protocol.md explaining the reasoning for spending funding-inputs.

Really agree on the long-term design that we should have a bunch of options, even negotiated with fee premiums to update or exit the DLC in function of other changes, such as mempool fee spikes or lightning routing fees ones :)

💯 I'm sure there's lots more that can be done and improved with future iterations of closing messages!

@matthewjablack
Copy link
Contributor Author

I realized funding_signatures for funding_inputs was missing, which are required sigs to broadcast and finalize close dlc. This has been added to close_dlc specification in Protocol.md

@matthewjablack
Copy link
Contributor Author

Based on feedback from @ariard I've added nullifying_point and rationale to Non-Interactive-Protocol.md to explain why funding-inputs are necessary, and introduce a point where the initiator should nullify the close signature by spending funding-inputs as the outcome of the close transaction become increasingly unfavorable.


`payout_spk` and `payout_serial_id` from `offer_dlc` as well as `payout_spk` and `payout_serial_id` from `accept_dlc` should be used for constructing the close transaction

`fund_input_serial_id` is a randomly chosen number which uniquely identifies the funding output to be spent.
Copy link
Contributor

Choose a reason for hiding this comment

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

calling this fund_input_serial_id is a bit confusing when we call the other inputs funding_inputs however I can't think of a better name

Copy link
Contributor Author

@matthewjablack matthewjablack Nov 10, 2021

Choose a reason for hiding this comment

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

in Transactions.md we call the DLC output the Funding Output but not sure if funding_output_serial_id would be appropriate, since it's an input in this case

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah :/ I'm not sure what a better name is

Copy link
Member

Choose a reason for hiding this comment

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

I think what should be changed is the funding_inputs naming. It conflict with the inputs of the funding transactions and is IMHO quite confusing. What about something like extra_inputs or cancellable_inputs?

Protocol.md Outdated Show resolved Hide resolved
- if the `close_signature` is incorrect:
- MUST ignore the message.
- on receipt of a valid `close_dlc`:
- SHOULD broadcast the closing transaction.
Copy link
Contributor

Choose a reason for hiding this comment

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

they need to at least sign the tx too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in Protocol.md it only mentions SHOULD broadcast the funding transaction not actually signing it. Is signing mentioned somewhere else for this one?

Transactions.md Outdated Show resolved Hide resolved
Copy link
Member

@Tibo-lg Tibo-lg left a comment

Choose a reason for hiding this comment

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

It might be nice to consider defining a feature flag for this as well.


`payout_spk` and `payout_serial_id` from `offer_dlc` as well as `payout_spk` and `payout_serial_id` from `accept_dlc` should be used for constructing the close transaction

`fund_input_serial_id` is a randomly chosen number which uniquely identifies the funding output to be spent.
Copy link
Member

Choose a reason for hiding this comment

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

I think what should be changed is the funding_inputs naming. It conflict with the inputs of the funding transactions and is IMHO quite confusing. What about something like extra_inputs or cancellable_inputs?

@matthewjablack
Copy link
Contributor Author

matthewjablack commented Dec 7, 2021

It might be nice to consider defining a feature flag for this as well.

I agree that a contract flag could make sense to ensure one party does not waste sats sending a DlcClose message to a peer that is not going to accept it. Perhaps adding contract flag definition would make sense in a subsequent PR.

I think what should be changed is the funding_inputs naming. It conflict with the inputs of the funding transactions and is IMHO quite confusing. What about something like extra_inputs or cancellable_inputs?

I changed funding_inputs => extra_inputs and funding_signatures => extra_signatures to reduce confusion. I considered cancellable_inputs and cancellable_signatures however the concept of cancellable_signatures seemed very strange and confusing at face value.

@matthewjablack
Copy link
Contributor Author

I also added close_locktime to help prevent fee sniping. It should be set to less than or equal to current timestamp/block.

Copy link
Member

@Tibo-lg Tibo-lg left a comment

Choose a reason for hiding this comment

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

I checked on how this will be affected by #163, and it seems the only things to do is change the field num_extra_inputs from u16 to bigsize.

Protocol.md Outdated
* [`u64`:`fund_input_serial_id`]
* [`u16`:`num_extra_inputs`]
* [`num_extra_inputs*extra_input`:`extra_inputs`]
* [`extra_signatures`:`extra_signatures`]
Copy link
Member

Choose a reason for hiding this comment

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

This is not a defined type. I guess you wanted to do the same as for funding_signatures, but note that funding_signatures is actually defined in Messaging.md. Maybe the best would be to change the type name of funding_signatures so that it's more abstract and reuse that (if the goal is to have the exact same structure). I guess originally there was only a single use for it which is why it was named like that.

Copy link
Contributor Author

@matthewjablack matthewjablack Jan 2, 2022

Choose a reason for hiding this comment

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

This is a great point. I agree that funding_signatures should be abstracted. I think funding_inputs should also be abstracted since it's used with DlcClose as well.

Renamed funding_input to input and funding_signatures to input_signatures to abstract them from funding transaction and allow them to be re-used for DlcClose messages.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! I might have gone with tx_input instead of just input but we can change it later if the meaning is not clear enough.

@matthewjablack
Copy link
Contributor Author

matthewjablack commented Jan 2, 2022

I checked on how this will be affected by #163, and it seems the only things to do is change the field num_extra_inputs from u16 to bigsize.

Fantastic 👌

TODO:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants