Skip to content

DLC channel specifications#196

Draft
Tibo-lg wants to merge 1 commit intodiscreetlogcontracts:masterfrom
Tibo-lg:dlc-channels
Draft

DLC channel specifications#196
Tibo-lg wants to merge 1 commit intodiscreetlogcontracts:masterfrom
Tibo-lg:dlc-channels

Conversation

@Tibo-lg
Copy link
Copy Markdown
Member

@Tibo-lg Tibo-lg commented May 16, 2022

No description provided.

@Tibo-lg Tibo-lg changed the base branch from serialization-update-proposal to master June 13, 2022 02:32
Copy link
Copy Markdown

@bonomat bonomat left a comment

Choose a reason for hiding this comment

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

Sorry for the long wait for feedback. The transactions are very similar to what we have implemented in https://github.com/comit-network/maia/. The hard part to make it interoperable will be the different message types/styles.

Comment thread Protocol.md

The protocol for renewal, settlement and closing of the channel is shown in the following [state machine](#state-machine-representation).

<img src="./images/dlc_channel_settle_renew.svg">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Am I reading it correctly that if RenewOffer was not accepted (RenewOffered state), e.g. it timed out, it will be unilateral closed?

Why did you design it that way? Is it not safe to go back to Established so that RenewOffer can be sent again?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same question in regards to SettleOffer

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I see your point. My thinking was that if your peer is unresponsive, you probably want to close the channel. It is true that these two states are a bit special in that the application of the counter party would need to get approval (or refusal) from the user before responding. But it is also not necessary to have a single timeout for all states. You could have a long one for these two, and only close if you don't get a response after it. Other solutions I can think of:

  • Being able to send RenewOffer and SettleOffer while already in these states. Things become a bit tricky though, I guess we would need to add a counter to them and have the responder reference it to know which one is being accepted/rejected.
  • Being able to cancel an offer. That feels a bit easier.

Is it not safe to go back to Established so that RenewOffer can be sent again?

I might not be understanding exactly, but if you go back to Established without notifying the other peer you sort of become out of sync don't you?

Comment thread Protocol.md

Note that transitions setting the variable `Invalid` to true are omitted for clarity.

## The `offer_channel` message
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The CloseOffer message seems to be missing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think it makes sense to include a feerate_per_vb in the CloseOffer message.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks, added the message as well as the feerate_per_vb. In LN they actually use a range, which can be nice but also mean a more complex protocol (can't send the signature right away).

Comment thread Protocol.md
* [`spk`:`change_spk`]
* [`u64`:`change_serial_id`]
* [`u64`:`fund_output_serial_id`]
* [`u64`:`feerate_per_vb`]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do you intend to take the same feerate_per_vb for all transactions (FundTx, RefundTx and CloseTx)?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's what I do at the moment yes. Do you have a different suggestion? Also my thinking is to maybe have something to update the fee like in LN.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We are doing the same thing and additionally we send a new feerate_per_vb whenever we extend/rollover a contract.

For our usecase in particular an update_fee message would not be needed as either you contact the other party to rollover/extend your contract and provide a new feerate_per_vb or you closed the position with CloseOffer which also sending over a feerate_per_vb.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Very good point. Though I wonder if we should have something to indicate that we want to accept a renewal but if a different fee rate?

Comment thread Protocol.md
2. data:
* [`channel_id`:`channel_id`]
* [`contract_id`:`temporary_contract_id`]
* [`u64`:`counter_payout`]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In our usecase of CFD-DLCs we have the concept of funding fees, i.e. a fee which needs to be paid to keep a position open.
Do you intend to implicitly put this into counter_payout?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We used to have something similar in our p2pderivatives app, users could set a premium to pay to the other party which would be included in the funding transaction. Though in that case if it doesn't require a different output I think indeed it might be better to have it implicit in the counter_payout. You could then have some extra TLVs for application specific things like that (I haven't specified the extra TLV field yet but you can assume any message can have them).

Comment thread Transactions.md
### Outputs

* The settle transaction contains two outputs, paying each party their respective payout agreed during the setup of the settle state (see [here](./Protocol.md#the-settleoffer-message)).
Note that if the payout of a party is lower than dust threshold, the corresponding output is dropped.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If it is dropped it would mean it goes to the miners. Wouldn't it make sense to add this amount to the other party instead?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't have a strong opinion but that's how it's specified for the on-chain version. It would be a bit annoying to have two different rules so maybe open an issue and we can discuss if things should be changed for both?

Comment thread Transactions.md
# Channel

* [Bitcoin-S implementation](https://github.com/bitcoin-s/bitcoin-s/blob/adaptor-dlc/dlc/src/main/scala/org/bitcoins/dlc/builder)
## Channel Contract Execution Transaction
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is pretty much the same as we have in [maia](https://github.com/comit-network/maia/blob/main/maia/src/protocol/transactions.rs].

We call the Buffer Transaction CommitTransaction because you commit to the state :)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah I remember that's the naming you use. Again I don't mind that much about the naming but what I worry is that it can be confused with the LN CommitTransaction especially if we later specified DLC channels working with LN ones.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No worries, I just wanted to point it out :) I have no hard feelings about namings in general.

Comment thread Transactions.md
* The buffer transaction output script is a P2WSH defined by the following miniscript policy:

```
or(and(pk(offer_own_pk),pk(accept_own_pk)),or(and(pk(offer_own_pk),and(pk(accept_publish_pk), pk(accept_rev_pk))),and(pk(offer_own_pk),and(pk(offer_publish_pk),pk(offer_rev_pk)))))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In maia we have an additional RefundTransaction which spends from the Buffer Transaction (CommitTransaction in our case). This allows you to get your funds back if the other party disappears and the oracle never attests.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes I have that as well, just didn't mention it. Added a small note l320 let me know if it's clear enough or not.

@Tibo-lg
Copy link
Copy Markdown
Member Author

Tibo-lg commented Jul 11, 2022

Sorry for the long wait for feedback. The transactions are very similar to what we have implemented in https://github.com/comit-network/maia/. The hard part to make it interoperable will be the different message types/styles.

Thanks for the review @bonomat. I'm open for changes, though I'd like to keep things consistent with the on-chain specs where it make sense as multiple implementations already support it.

Just a side note, lately I have been considering if it would be better to share the next_per_update_point as part of the previous update. IIUC that's what LN is doing and I think it would save a message per update (since if a party already has the point they can already build and sign transactions).

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.

2 participants