Peer Protocol for Contract Negotiation Specification #59

nkohen · 2020-08-13T19:18:33Z

Re-wrote the Transactions specification to match the new adaptor signature based DLCs and to match the BOLT style.

It is not in its final state but it should be a good starting point to get merged in!

Related to #46

TODO:

Test Vectors

benthecarman · 2020-08-13T21:32:25Z

Messaging.md

+* `contract_info`: ???
+* `oracle_info`: ???


Can probably port over the way we do these in bitcoin-s?

I think it might depend on the discussion on oracle specifications. It could end up just being a URI.

Do we need to think about multi-oracle support here?

So I'm thinking that will be the version 0 of them but I actually want these to be true TLV types that can very between number of oracles, kinds of oracles (multi-R values for example) etc. Same with contract info and others

Tracked here if I'm not mistaken: #61

Protocol.md

benthecarman · 2020-08-13T22:03:11Z

Protocol.md

+    - MUST reject the contract.
+  - MUST NOT broadcast the funding transaction before receipt of a valid `sign_dlc`.
+  - on receipt of a valid `sign_dlc`:
+    - SHOULD broadcast the funding transaction.



Maybe add something about if the Initiator doesn't see the funding tx in a timely manner, they can exit the DLC by self-spending one of their funding inputs.

I agree that should be specified but I believe it belongs in its own doc similar to BOLT 5

Should also include fee-bumping and such in that doc

benthecarman · 2020-08-13T22:06:47Z

Protocol.md

-Ichiro Kuwahara
-Thibaut Le Guilly
-Nadav Kohen
+[ FIXME: Add Authors ]


still need the FIXME?

Yeah, all the BOLTs have this so I figured in this case I would put it here as unlike the tx doc others have worked on this pretty extensively previously

Tibo-lg

Looks nice! Put quite some comments but maybe many things are just me being confused so sorry in advance if that's the case.

Messaging.md

Tibo-lg · 2020-08-14T00:22:26Z

Messaging.md

+* `contract_info`: ???
+* `oracle_info`: ???


I think it might depend on the discussion on oracle specifications. It could end up just being a URI.

Messaging.md

Protocol.md

Tibo-lg · 2020-08-14T01:30:47Z

Protocol.md

+   * [`u16`:`num_funding_inputs`]
+   * [`num_funding_inputs*funding_input`:`funding_inputs`]
+   * [`spk`:`change_spk`]
+   * [`cet_signatures`:`cet_signatures`]


again I feel here the type should be nb_cets*signature.

But we need a label on each signature to point it to what outcome it signs for no? Or I guess we could use ordering explicitly (like copying the structure of the contract info) but this would need to be added to the specification if this were the case. I'll probably hold off judgement until the PR where cet_signatures as a TLV is defined.

Yes I was assuming that the order of signatures and outcomes was corresponding, but I feel this is an acceptable assumption, as I can't think of a reason not to do that.

It would be nice to decide something on this. One reason I can think of to follow your approach is that we might have to split them in several messages in which case having the index can be practical.

Protocol.md

matthewjablack · 2020-08-14T22:46:29Z

Protocol.md

+   * [`chain_hash`:`chain_hash`]
+   * [`contract_info`:`contract_info`]
+   * [`oracle_info`:`oracle_info`]
+   * [`point`:`funding_pubkey`]


I'm guessing offer_dlc assumes counterparty has already been established (whether off-chain or through an orderbook) since counterparty pubkey is required to derive funding_pubkey

I'm not sure what you mean by

counterparty pubkey is required to derive funding_pubkey

as I don't believe this is true.

Regardless though, yes this entire document assumes the existence of some kind of communication layer as is stated at the top of the Messaging doc

funding_pubkey is a 2-of-2 multisig Script right? So don't you need pubkey of both alice and bob to derive funding_pubkey?

funding_pubkey is just one party's public key (which is one of the two that goes into the funding script public key)

nkohen · 2020-08-14T23:09:46Z

Note to self: This was just updated: niftynei/lightning-rfc#1

Christewart · 2020-08-15T19:26:47Z

Messaging.md

+
+Various fundamental types are referred to in the message specifications:
+
+* `byte`: an 8-bit byte


signed or unsigned?

this question doesn't apply to byte, it isn't a number, just 8 bits.

Messaging.md

Christewart · 2020-08-15T19:35:24Z

Protocol.md

+2. data:
+   * [`byte`:`contract_flags`]
+   * [`chain_hash`:`chain_hash`]
+   * [`contract_info`:`contract_info`]


So how/where is "contract_info" specified? There is an assumption here that peers know how to parse/interpret contract_info? Do we need a similar spec for this just like we are talking about oracles in #55 ?

see #59 (comment)

Christewart · 2020-08-15T19:39:17Z

Protocol.md

+   * [`u32`:`contract_maturity_cltv`]
+   * [`u32`:`contract_timeout_cltv`]


Hmm, Matt does have a good point. It might be nice to invalidate refund transactions when contract terms are updated. Perhaps we can use the Lightning penalty mechanism?

Christewart · 2020-08-15T19:52:32Z

Protocol.md

+  - the `chain_hash` value is set to a hash of a chain that is unknown to the receiver.
+  - the `contract_info` refers to events unknown to the receiver.
+  - the `oracle_info` refers to an oracle unknown to the receiver.
+  - it considers `feerate_per_vb` too small for timely processing or unreasonably large.


Maybe this is implied here, but if the number of funding_inputs is something ridiculous you should reject as well.

The only time this would lead to a rejection is if the resulting funding transaction would not be valid so I'm not sure I need to say this? Will double check against Lisa's spec.

I should have been more specific about what I meant by "valid". In this case I meant valid in terms of relay policy on the bitcoin p2p network. If you want more background on this you look at the AcceptToMemoryPool (ATMP) logic in bitcoin core.

and you should definitely checkout this excellent discussion from @ariard about relay policies on the bitcoin network in the future

bitcoin/bitcoin#14895 (comment)

Protocol.md

Christewart · 2020-08-15T19:59:24Z

Protocol.md


-## Validation
+This message gives all of the initiator's the signatures, which allows the receiver


nit,: initiator -> offerer/maker etc

Christewart · 2020-08-15T20:02:05Z

Protocol.md

-* B fund amount: confirm that the fund amount matches what was agreed upon.
+  - if any `signature` is incorrect:
+    - MUST reject the contract.
+  - MUST NOT broadcast the funding transaction before receipt of a valid `sign_dlc`.


what happens if this is not followed?

A free option

benthecarman · 2020-09-04T05:49:24Z

Protocol.md


-#### CET CSV Delay
-The number to use as input to the CSV in the [CETs](Transactions.md#outputs-1).
+  - set undefined bits in `contract_flags` to 0.


Above you say they should be ignored, here you say they should be set to 0. Maybe change above to say that it should expect 0 and fail otherwise?

I think that in the current state it is easiest to implement this way but it should change before we have an official v0.1: #74

Protocol.md

Messaging.md

Protocol.md

nkohen · 2020-09-08T21:39:58Z

Relevant open issues:

#76
#74
#73
#72
#69
#62
#61
#60
#53

nkohen added the contract-negotiation label Aug 13, 2020

nkohen added this to the v0.1 milestone Aug 13, 2020

This was referenced Aug 13, 2020

Links between Specifications Documents #60

Open

Specify how an eventId should be calculated #20

Closed

benthecarman reviewed Aug 13, 2020

View reviewed changes

Tibo-lg reviewed Aug 14, 2020

View reviewed changes

matthewjablack reviewed Aug 14, 2020

View reviewed changes

Christewart reviewed Aug 15, 2020

View reviewed changes

Tibo-lg mentioned this pull request Aug 17, 2020

Adaptor DLC Transactions Specification #57

Merged

1 task

This was referenced Aug 17, 2020

Oracle Info TLV Specification #61

Closed

Contract Info TLV Specification #62

Closed

benthecarman reviewed Sep 4, 2020

View reviewed changes

Protocol.md Show resolved Hide resolved

benthecarman reviewed Sep 4, 2020

View reviewed changes

Messaging.md Outdated Show resolved Hide resolved

ariard reviewed Sep 8, 2020

View reviewed changes

Protocol.md Show resolved Hide resolved

ariard reviewed Sep 8, 2020

View reviewed changes

Protocol.md Outdated Show resolved Hide resolved

nkohen added 2 commits September 8, 2020 16:06

Added Peer Protocol specification

4e4cd5f

Responded to review

846c9f4

nkohen mentioned this pull request Sep 8, 2020

Add TLV streams to specification for extensibility #73

Open

Responded to more review

e25275b

nkohen force-pushed the adaptor-protocol branch from cf7e49e to e25275b Compare September 8, 2020 21:34

nkohen merged commit b0cbed2 into discreetlogcontracts:master Sep 8, 2020

Tibo-lg mentioned this pull request Sep 9, 2020

Feedback over protocol #70

Closed


		Various fundamental types are referred to in the message specifications:

		* `byte`: an 8-bit byte

		* [`u32`:`contract_maturity_cltv`]
		* [`u32`:`contract_timeout_cltv`]


		## Validation
		This message gives all of the initiator's the signatures, which allows the receiver

Peer Protocol for Contract Negotiation Specification #59

Peer Protocol for Contract Negotiation Specification #59

Conversation

nkohen commented Aug 13, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benthecarman Aug 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benthecarman Aug 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tibo-lg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewjablack Aug 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewjablack Aug 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nkohen commented Aug 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nkohen commented Sep 8, 2020

benthecarman Aug 13, 2020 •

edited

Loading

benthecarman Aug 13, 2020 •

edited

Loading

matthewjablack Aug 14, 2020 •

edited

Loading

matthewjablack Aug 18, 2020 •

edited

Loading