[dlc-dev] Contract id breaks on any changes to offer serialization

[Thibaut Le Guilly]

Ah ok sorry I see what you mean now, it should indeed be specified to be 1,
I'll update the PR.

On Wed, Feb 9, 2022 at 8:42 AM Chris Stewart <chris at suredbits.com> wrote:

> Yes but was is the protocol version when we merge #163? Apologies if I am
> missing it. I definitely think it should _not_ be zero as that is what the
> contract flags are serialized as currently I believe.
>
> On Tue, Feb 8, 2022 at 5:28 PM Thibaut Le Guilly <
> thibaut at cryptogarage.co.jp> wrote:
>
>> > From a quick glance through #163, I don't see a protocol version
>> defined.
>> It's actually one of the main things I wanted to get as part of the
>> change :)! It's here:
>> https://github.com/discreetlogcontracts/dlcspecs/pull/163/files#diff-e0f5b925f91a1c09c6daf26f9a7d28816cb9ff9f08863faca719b7ee0a1cc065R69
>>
>> On Wed, Feb 9, 2022 at 5:18 AM Chris Stewart <chris at suredbits.com> wrote:
>>
>>> If we add the `tempContractId` to the offer message, this will break
>>> existing offer messages. Maybe this isn't such a problem as the protocol
>>> version can be read off the front of the offer message on #163. If the
>>> protocol version is `0`, hash the offer message. If the protocol version is
>>> >= 1, use the tempContractId embedded in the offer message. From a quick
>>> glance through #163, I don't see a protocol version defined.
>>>
>>> -Chris
>>>
>>>
>>> On Sun, Feb 6, 2022 at 8:34 PM Thibaut Le Guilly <
>>> thibaut at cryptogarage.co.jp> wrote:
>>>
>>>> Hi Chris,
>>>>
>>>> I've tried to consider other approaches than the random temporary
>>>> contract id, but it seems to me that it is the best approach. One thing
>>>> that I found appealing was simply having a counter between the parties
>>>> which they would increment for each contract, but this leads to quite some
>>>> issues if one party gets out of sync or misbehaves so I think the random id
>>>> is just simpler.
>>>>
>>>> One thing to note is that this change will also require a change in the
>>>> offer message since we will have to include the temporary contract id
>>>> inside it (at the moment we don't since it is computed as a hash of the
>>>> offer message). If we agree on this I think it would be good to get the
>>>> change to the offer message together with the serialization update in #163.
>>>>
>>>> Cheers,
>>>>
>>>> Thibaut
>>>>
>>>> On Wed, Jan 12, 2022 at 11:53 PM Chris Stewart <chris at suredbits.com>
>>>> wrote:
>>>>
>>>>> For reference of our implementation specific issue, please see
>>>>> https://github.com/bitcoin-s/bitcoin-s/issues/3969
>>>>>
>>>>> On Wed, Jan 12, 2022 at 8:51 AM Chris Stewart <chris at suredbits.com>
>>>>> wrote:
>>>>>
>>>>>> When implementing 163
>>>>>> <https://github.com/discreetlogcontracts/dlcspecs/pull/163> inside
>>>>>> of bitcoin-s, i discovered that any existing wallets deployed with previous
>>>>>> serialization versions will have their contract ids
>>>>>> <https://github.com/discreetlogcontracts/dlcspecs/blob/master/Protocol.md#definition-of-contract_id>
>>>>>> broken and require database level migrations if you are persisting contract
>>>>>> ids.
>>>>>>
>>>>>> The offer message
>>>>>> <https://github.com/discreetlogcontracts/dlcspecs/blob/master/Protocol.md#the-offer_dlc-message>
>>>>>> is what contract ids are derived off of. If any of the nested data
>>>>>> structures have serialization changes inside the offer, this will break
>>>>>> contract ids. Things included in the offer are contract descriptors,
>>>>>> announcements etc. Thus if we make any changes to the binary level
>>>>>> serialization of the DLC spec, contract ids will break most likely.
>>>>>>
>>>>>> It was talked about on our open source developer call last night what
>>>>>> we can do to prevent this from happening. When we first wrote the DLC spec
>>>>>> we tried to copy as much stuff as we could from Lightning with two hopes
>>>>>>
>>>>>> 1. Learning from their deployment of Lightning
>>>>>> 2. Reducing the amount of bikeshedding
>>>>>>
>>>>>> The equivalent to `temp_contract_id` in Lightning is the
>>>>>> temporary_channel_id
>>>>>> <https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#definition-of-channel_id>.
>>>>>> It appears that they just pick a random nonce now. Here is the relevant
>>>>>> section from BOLT2
>>>>>>
>>>>>> >Prior to channel establishment, a temporary_channel_id is used,
>>>>>> which is a random nonce.
>>>>>>
>>>>>> >Note that as duplicate temporary_channel_ids may exist from
>>>>>> different peers, APIs which reference channels by their channel id before
>>>>>> the funding transaction is created are inherently unsafe. The only
>>>>>> protocol-provided identifier for a channel before funding_created has been
>>>>>> exchanged is the (source_node_id, destination_node_id,
>>>>>> temporary_channel_id) tuple. Note that any such APIs which reference
>>>>>> channels by their channel id before the funding transaction is confirmed
>>>>>> are also not persistent - until you know the script pubkey corresponding to
>>>>>> the funding output nothing prevents duplicative channel ids.
>>>>>>
>>>>>> So it appears that they are just randomly generating a 32 byte nonce.
>>>>>> It seems we should just do the same. This means that whenever we introduce
>>>>>> a binary level serialization change, we don't have to run database
>>>>>> migrations to recalculate all contract ids.
>>>>>>
>>>>>> It does appear that our `contract_id` calculation will work as that
>>>>>> is calculated with
>>>>>>
>>>>>> funding_txid XOR temp_contract_id
>>>>>>
>>>>>> which seems to adhere to the wisdom on the Lightning RFC. Once we
>>>>>> know the funding_txid we have a way to reasonably guarantee unique entropy
>>>>>> that leads to a stable contract id identifier, just as lightning can
>>>>>> calculate a stable channel_id when they have the funding tx.
>>>>>>
>>>>>> This seems like an easy backward compatible deployment as well, as we
>>>>>> can just assume all existing `temp_contract_id` were randomly generated
>>>>>> (even though in reality they are hash(offer_bytes)).
>>>>>>
>>>>>> -Chris
>>>>>>
>>>>>>
>>>>>>
>>>>> dlc-dev mailing list
>>>>> dlc-dev at mailmanlists.org
>>>>> https://mailmanlists.org/mailman/listinfo/dlc-dev
>>>>>
>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mailmanlists.org/pipermail/dlc-dev/attachments/20220209/6b67f2b1/attachment-0001.htm>