Update messaging and serialization #163

Tibo-lg · 2021-04-23T02:47:53Z

This PR is a proposal to update the update the serialization to make it more efficient, concise and upgradable. The current message format specifies hard coded type prefixes for embedded type that don't require them, making it a maintenance burden and inefficient. It also lacks any extensibility or upgradability mechanism.

Changes include:

Add protocol version to offer_dlc
Add tlv streams to wire messages
Define three types of subtypes and specific serialization format for each of them (and update type definitions accordingly)

I hesitated to move the wire message definitions to Messaging.md as previously discussed but to keep the diff small I didn't.

benthecarman · 2021-04-24T06:31:32Z

Protocol.md

+[//]: # (TODO: `protocol_version` should be advertised by DLC nodes in the future)
+The `protocol_version` value denotes the version of the protocol that the offering node is proposing.
+If the receiving node does not support the requested version, it should ignore the message.


maybe we should define a advertisement message now and just use that as an initial message in current setups so we don't have to define a new offer_dlc_vx once we have this.

let me know if I misunderstand you, but I feel having an advertisement message would not remove the need to have a prrotocol version field (if that's what you're proposing?). Nodes could potentially support multiple protocol version so announcing it in the offer message still seem necessary to me.

benthecarman · 2021-04-24T06:32:40Z

Messaging.md


-1. type: 55342 (`contract_info_v0`)
+1. type: 1


I am confused why we are removing these types

Before we used type tags in two cases:

for types that are embedded in others. In this case I couldn't see any use of the tag since if you know exactly what you want to parse a tag doesn't bring anything (happy to be corrected if I missed something).

for cases where there are multiple version of a field (this case). I couldn't see any reason to use type numbers that are assigned by LN for wire messages, as this is both less efficient and a waste of them AFAICT (again happy to be corrected if I'm wrong).

Messaging.md

nkohen · 2021-04-27T20:53:37Z

Messaging.md

+
+Sibling sub-types are prefixed with a `bigsize` type identifier.
+The type identifiers are specific to a set of type variants, and can thus be reused across different sub-types.
+Type identifiers are defined within the type definitions, starting from `1` and increasing by `1`.


do we not want to support the "it's okay to be odd" design here?

My thinking was that the odd/even requirements in LN are used for extensibility, and to determine whether a TLV or message is part of the base protocol or part of a higher layer (application specific) protocol. The way sibling types are defined, there is no case where they should be ignored, so I didn't see a good reason to use the odd/even requirements. If I am missing something and you have in mind a use case for it I'm happy to change.

Instead of envisioning sibling type, we could define an even type for a field, servicing as reference and any future, alternative field as an odd one ?

@ariard sorry I completely missed your comments. I don't really get what you mean here though, the point of sibling type is not for extensibility or upgradeability but just for cases where within a data structures, a field can have different types.

nkohen · 2021-04-27T20:54:34Z

Messaging.md

+
+#### Sibling sub-type
+
+Sibling sub-types are prefixed with a `bigsize` type identifier.


no bigsize length requirement?

This follows somehow from the previous point. The length requirement is useful for parsing unknown types or things that should be ignored, but I couldn't think of how it would be useful here (again happy to change it if there is a good reason).

Aren't you assuming everything is a static size then? An example of something that has non deterministic size is ECDSA signatures. IIRC they can very anywhere in between 68 - 72 bytes in size. If you don't explicitly encode the size, you can't parse the message if i understand this correctly.

In general for fields that have variable length we have length prefixes in the type definition itself, like your example of ECDSA signature

Messaging.md

nkohen · 2021-04-27T21:02:01Z

Messaging.md

-1. type: 55332 (`oracle_announcement`)
-2. data:
+1. data:
+   * [`bigsize`: `version`]


I'm not sure I understand, are these simple sub-types then? If so I don't get the point of a simple sub-type with a version number, wouldn't it make more sense to have a sibling sub-type with just one version?

You're right updated

nkohen · 2021-04-27T21:02:43Z

Protocol.md

+   * [`protocol_version`: `bigsize`]
   * [`byte`:`contract_flags`]


seems a little redundant to have both of these

I personally think the offer should signal the features it supports and requires which are two separate things, and then have a field in the accept with the feature set to be used, so for example you could have an offer that had a new feature enabled but not required where the accepter can choose whether or not to use it.

I personally see these two fields as providing different information.
protocol_version indicates which language I speak, and enables the receiver to determine whether they will be able to understand the content of the messages that are exchanged. Ideally in the future nodes should advertise which version of the protocol they support so that other peers will know how to communicate with them.
contract_flags are used to signal features as you mention.
Does that make sense?

In a Lightning/DLC world, I'm not sure if the protocol_version is making sense as new protocol subset can be introduced as new messages types and the question of whom to talk to is already solved with the feature bits space ?
I agree with the necessity of a contract_flags, see my other point mentioning the LN discussion on channel_type.

For me contract_flags ad protocol_version are two separate things. The protocol version is for breaking changes in the protocol specifications and enabling nodes to see whether they are compatible with a version or not. Contract flags is for a given protocol version whether a node supports a feature or not.
I guess we could use contract version for both but it doesn't feel optimal to me.
Happy to discuss though, I don't have that strong of an opinion about how things should be, just that it's practical to have some sort of upgradeability mechanism.

nkohen · 2021-04-27T21:05:27Z

Protocol.md

@@ -66,6 +66,7 @@ the funding transaction and CETs.

 1. type: 42778 (`offer_dlc_v0`)
 2. data:
+   * [`protocol_version`: `bigsize`]


nit: I think this is backwards, should be [`bigsize`:`protocol_version`]

Thanks fixed

nkohen · 2021-04-27T21:06:13Z

Protocol.md

@@ -81,6 +82,11 @@ the funding transaction and CETs.
   * [`u64`:`feerate_per_vb`]
   * [`u32`:`cet_locktime`]
   * [`u32`:`refund_locktime`]
+   * [`offer_tlvs`: `tlvs`]


I take it none of offer_tlvs, accept_tlvs or sign_tlvs have any currently existing fields correct?

nkohen · 2021-04-27T21:06:47Z

Messaging.md

+#### Optional sub-type
+
+Optional sub-types are also prefixed with `bigsize` type identifier, but the identifiers are defined in the containing structures using the `(Optional: x)` notation where `x` is a number denoting the identifier, starting at `1` and increasing by `1`.
+Optional sub-types should be gathered as the last fields of a type.


I'm not sure I understand the necessity for this requirement?

Especially seeing as simple sub-types can presumably have optional fields?

You're right removed.

nkohen · 2021-04-28T06:21:44Z

Messaging.md

+#### Optional sub-type
+
+Optional sub-types are also prefixed with `bigsize` type identifier, but the identifiers are defined in the containing structures using the `Optional(type_name, x)` notation where `type_name` is the name of the type and `x` is a number denoting the identifier, starting at `1` and increasing by `1`.
+In the absence of any optional sub-type in a structure, a zero byte `0x00` should be written to enable parsing.


Is this now a 0x00 for every absent optional field or for every empty group of consecutive fields or?

As discussed replaced the type identifier with a single byte indicating absence/presence of an optional field.

ariard

After reading this proposal, I'm leaning towards the LN-way for upgradeability/nesting.

If the message is flow is different, introduce a new message type. If the message flow is the same and we're extending message semantic, add a TLV field.

If we want nested data struct, I think we can achieve it the way onions (BOLT4) are done, versioning and new namespaces at each nesting entry ?

ariard · 2021-07-06T23:14:57Z

Messaging.md

+
+### Type-Length-Value
+
+A TLV stream is defined for each wire message, and can be used to extend the protocol and for application specific features.


Browsing through the BOLTs again, tlv_stream are extensions to already defined messages, allowing to add optional fields in a backward compatible-way ?

Throughout the protocol, a TLV (Type-Length-Value) format is used to allow for the backwards-compatible addition of new fields to existing message types.

IIUC your proposal correctly it defines the content of any of our DLC messages as a TLV namespace itself ? Not sure if it's achieving it's purpose of bandwidth-saving as the type/length components of a tlv_record are redundant information for mandatory fields ?

My point was to have tlv_stream for the exact same purpose as LN, so maybe the way I've written things is confusing?

ariard · 2021-07-06T23:17:47Z

Messaging.md

+
+Sibling sub-types are prefixed with a `bigsize` type identifier.
+The type identifiers are specific to a set of type variants, and can thus be reused across different sub-types.
+Type identifiers are defined within the type definitions, starting from `1` and increasing by `1`.


Instead of envisioning sibling type, we could define an even type for a field, servicing as reference and any future, alternative field as an odd one ?

ariard · 2021-07-06T23:19:57Z

Messaging.md

+
+#### Plain sub-type
+
+Plain sub-types do not have any particular format, and their field can simply be replaced in place where they are used.


Can you give an example of "plain sub-type" ? I'm not sure I'm following on the replacement part...

Added an example. I think the naming is probably not great. But basically the point is that before we had type prefixes for fields that were neither optional or sibling.

ariard · 2021-07-06T23:22:52Z

Protocol.md

+
+[//]: # (TODO: `protocol_version` should be advertised by DLC nodes in the future)
+The `protocol_version` value denotes the version of the protocol that the offering node is proposing.
+If the receiving node does not support the requested version, it should ignore the message.


Maybe we should wait for the channel_type discussion to settle on the LN-side and piggy-back there, defining new DLC features as-yet-another-channel ?

See lightning/bolts#880 and this week LN meeting : https://gnusha.org/lightning-dev/2021-07-05.log

Can you clarify what you would like to do here?

Christewart · 2021-09-19T17:26:31Z

Protocol.md

@@ -81,6 +82,11 @@ the funding transaction and CETs.
   * [`u64`:`feerate_per_vb`]
   * [`u32`:`cet_locktime`]
   * [`u32`:`refund_locktime`]
+   * [`offer_tlvs`: `tlvs`]
+
+[//]: # (TODO: `protocol_version` should be advertised by DLC nodes in the future)


is there any precedent for this in other deployed cryptocurrency protocols? Could you reference some resources you used for this if so?

It seems like this isn't just a problem with our protocol, but every cryptocurrency protocol that has versioning.

Oh sorry, this is a TODO nevermind.

I think having a protocol version is really common in any kind of protocol. It's for example used in the bitcoin p2p protocol: https://en.bitcoin.it/wiki/Protocol_documentation#version

it would be nice to have a list of known protocol_versions. My understanding is the first one is introduced in this PRs test vectors. It would be nice to have it listed somewhere in this doc

benthecarman · 2021-10-05T21:48:42Z

Messaging.md

+
+Sibling sub-types are prefixed with a `bigsize` type identifier.
+The type identifiers are specific to a set of type variants, and can thus be reused across different sub-types.
+Type identifiers are defined within the type definitions, starting from `1` and increasing by `1`.


why not start at 0?

+1 Looks like #171 starts with 0

matthewjablack · 2021-10-06T02:56:55Z

Messaging.md

   * [`u16`:`nb_nonces`]
   * [`nb_nonces*x_point`:`oracle_nonces`]
   * [`u32`:`event_maturity_epoch`]
   * [`event_descriptor`:`event_descriptor`]
   * [`string`:`event_id`]

-### The `oracle_announcement` Type
+### The `oracle_announcement` TLV


I'm confused about the purpose of the TLV here

Should there be a separate TLV and message format?

Currently OracleAnnouncementV0 and OracleAttestmentV0 are both TLVs (at least they're both implemented as TLVs in bitcoin-s and node-dlc)

(Also looks like TLV is not specified in #171 https://github.com/discreetlogcontracts/dlcspecs/pull/171/files#diff-b8a9c2888fe05c198758fbe393798cd0802b578f7eb07dbf82cc2c90616f97c3R402 )

+1 if nothing else is changing about the announcements I don't see the reasoning of changing the type

Sorry I confused myself. I think they should be wire messages as they can be broadcast directly (please see updated version). I guess we could define two formats but for the sake of simplicity I would personally prefer keeping a single one.

+1 if nothing else is changing about the announcements I don't see the reasoning of changing the type

I was thinking since we are going to have breaking changes on these anyway we might as well make it so that the wire message types follow each others (I left an open space for the close dlc. Though now thinking about it I wonder if close dlc should actually have an odd type number 🤔

Sorry I confused myself. I think they should be wire messages as they can be broadcast directly (please see updated version). I guess we could define two formats but for the sake of simplicity I would personally prefer keeping a single one.

+1 for having one format for sake of simplicity. Keeping it as a wire message makes sense to me

Though now thinking about it I wonder if close dlc should actually have an odd type number 🤔

What would be the rationale for this? I'm guessing because it's optional?

What would be the rationale for this? I'm guessing because it's optional?

Yes exactly, that's at least the semantics specified in BOLT 1: https://github.com/lightningnetwork/lightning-rfc/blob/master/01-messaging.md#lightning-message-format

I noticed that in the latest version of #163 OracleAnnouncement, OracleEvent, EnumEventDescriptor and DigitDecompositionEventDescriptor are all wire messages

This makes sense for OracleAnnouncement, since it's likely announcements and attestments would be communicated over the transport layer. However, shouldn't the wire messages inside OracleAnnouncement actually be a TLVs since it's unlikely for them to be communicated over the transport layer.

The reason was that it was decided not to touch any oracle related message in this PR so that no update to oracle software is required for now, and then make all oracle related changes as part of #167 at once.

Tibo-lg · 2021-10-14T05:08:51Z

Fixed the sibling type numbers which where not consistent with the latest update.

Tibo-lg · 2021-10-14T05:42:28Z

Noticed that we were sometimes using BigSize and sometimes u16 for collection prefixes. Consolidated it to use BigSize everywhere as for the CetSignatures type it is probably safer than having a restriction on size.

Christewart · 2021-10-31T22:15:48Z

Protocol.md

@@ -66,6 +66,7 @@ the funding transaction and CETs.

 1. type: 42778 (`offer_dlc_v0`)
 2. data:
+   * [`bigsize`: `protocol_version`]


I believe u16:num_funding_inputs is a bigsize in the test vectors (specifically enum_single_oracle_test.json)

Thanks I had forgotten to update it, fixed.

Christewart · 2021-10-31T22:20:54Z

Messaging.md


-1. type: 42772 (`funding_input_v0`)
-2. data:
+1. data:
   * [`u64`:`input_serial_id`]
   * [`u16`:`prevtx_len`]


I think this is a bigsize now, at least according to the enum_single_oracle_test.json

Yes thanks fixed.

Christewart · 2021-10-31T23:19:41Z

Protocol.md

@@ -185,7 +191,8 @@ and closing transactions.
   * [`u64`:`change_serial_id`]
   * [`cet_adaptor_signatures`:`cet_adaptor_signatures`]
   * [`signature`:`refund_signature`]
-   * [`negotiation_fields`:`negotiation_fields`]
+   * [`negotiation_fields`:`negotiation_fields`] (Optional: 1)
+   * [`accept_tlvs`: `tlvs`]


bigsize here too i think https://github.com/discreetlogcontracts/dlcspecs/pull/163/files#diff-e0f5b925f91a1c09c6daf26f9a7d28816cb9ff9f08863faca719b7ee0a1cc065R188

Sorry not sure which field you're referring to in this comment?

num_funding_inputs

Tibo-lg · 2021-11-02T02:01:31Z

Switched protocol version from BigSize to u32 as I find it cleaner.
Fixed the missing protocol version in the test vectors.

Christewart · 2022-04-19T22:16:36Z

PayoutCurve.md

@@ -69,7 +69,7 @@ In this section we detail the TLV serialization for a general `payout_function`.

 1. type: 42790 (`payout_function_v0`)


I think this needs to be updated as I don't believe its a TLV in the #163 test vectors?

Thanks! Fixed everything in that file, as well as the RoundingIntervals that was also not updated.

PayoutCurve.md

Christewart · 2022-05-02T13:36:54Z

NumericOutcome.md

@@ -66,8 +66,7 @@ up in the case of a tie.

 #### Rounding Interval Serialization

-1. type: 42788 (`rounding_intervals_v0`)
-2. data:
+1. data:
   * [`u16`:`num_rounding_intervals`]


@Tibo-lg Is this consistent with your implementation?

Addressed in 70032e3

I think there is another bug here, I believe this is prefixed by a bigsize in your implementation @Tibo-lg rather than a u16.

Yes in that case I should have updated the spec to be consistent with other collection prefixes. Updated now.

Messaging.md

Christewart · 2022-05-29T20:43:12Z

Protocol.md

@@ -181,17 +194,18 @@ and closing transactions.
   * [`point`:`funding_pubkey`]
   * [`spk`:`payout_spk`]
   * [`u64`:`payout_serial_id`]
-   * [`u16`:`num_funding_inputs`]
+   * [`bigsize`:`num_funding_inputs`]


to make this change we need to either

Add a version to the top of the accept_dlc_tlv_v0

revert this back to u16

The reason this is needed is to preserve backwards compatability with the previous version of the spec. This is how i've implemented the backwards compatible parse currently

//parser for `offer_tlv` val fundingInputs = { if (protocolVersionOpt.isEmpty) { //means we are working with the old version of the spec iter.takeU16PrefixedList(() => iter.take(FundingInputV0TLV)) } else { //means we are using version 1 of the spec iter.takeBigSizePrefixedList(() => iter.take(FundingInputV0TLV)) } }

parser for the accept_tlv that is an attempt at backwards compatibility

//accept_tlv // //we don't have a version # here so we don't know what version we are trying to parse? //I suspect this code in the try block will not always cause an exception //so this is truely a guess. try { iter.takeU16PrefixedList(() => iter.take(FundingInputV0TLV)) } catch { case scala.util.control.NonFatal(_) => iter.takeBigSizePrefixedList(() => iter.take(FundingInputV0TLV)) }

The crux of the issue is that if funding inputs are prefixed with a u16 as required in the old version of the spec, a bigsize can be mistake as a u16. bigsize is required by the new version of the spec, which gives us ambiguity in the parser.

I don't mind adding a version at the top of the message, but can't you just keep the version from the offer message?

That doesn't work for a few reasons

Its a layer violation to require a database read just to parse a tlv correctly.

You cannot guarantee you will parse the accept_tlv since you don't know which serialization format it uses to obtain information to make the database read. I guess you might be able to parse the first 32 bytes (tempContractId) to parse just the 32 bytes, make the database read, and then determine which format the offer had, but this is very cumbersome.

Lets keep the messages -- at the binary serialization level -- context free so that you can parse the offer, accept, and sign tlvs independent of one another.

Added a protocol_version field to the accept message and updated the test vectors. I wonder if it wouldn't be better to also add one to the sign message, what do you think?

I wonder if it wouldn't be better to also add one to the sign message, what do you think?

I think its probably a good idea. AFAIK, the sign_tlv does not have the same sort of ambiguity (funding_inputs) that offer,accept have so it isn't strictly necessary now, but it seems prudent to future proof a bit.

it seems prudent to future proof a bit.

Yeah I feel like that too. Updated docs and test vectors accordingly.

Christewart · 2022-06-08T23:32:49Z

So this PR on bitcoin-s passes all test vectors in this PR:

I believe we have achieved the 2 impl requirement, which means we may want to merge this? 👀 👀

bitcoin-s/bitcoin-s#3859

Christewart · 2022-06-12T13:35:59Z

As discussed at the June meeting, we have 2 implementations now so merging this.

Tibo-lg force-pushed the serialization-update-proposal branch from 7820462 to 690a335 Compare April 23, 2021 02:48

benthecarman reviewed Apr 24, 2021

View reviewed changes

nkohen reviewed Apr 27, 2021

View reviewed changes

Tibo-lg force-pushed the serialization-update-proposal branch from 690a335 to 34bb0e9 Compare April 28, 2021 01:24

nkohen reviewed Apr 28, 2021

View reviewed changes

Tibo-lg force-pushed the serialization-update-proposal branch from 34bb0e9 to 5bfa850 Compare April 28, 2021 06:41

ariard reviewed Jul 6, 2021

View reviewed changes

Tibo-lg added the breaking The proposed change breaks backward compatibility label Sep 16, 2021

Christewart reviewed Sep 19, 2021

View reviewed changes

Tibo-lg force-pushed the serialization-update-proposal branch from 5bfa850 to 0103cc5 Compare September 21, 2021 01:01

benthecarman reviewed Oct 5, 2021

View reviewed changes

Christewart mentioned this pull request Oct 5, 2021

Update messaging and serialization #171

Closed

Tibo-lg force-pushed the serialization-update-proposal branch 2 times, most recently from d544073 to 927ce76 Compare October 6, 2021 01:43

matthewjablack reviewed Oct 6, 2021

View reviewed changes

Tibo-lg force-pushed the serialization-update-proposal branch 3 times, most recently from 271be3f to d6a25a6 Compare October 6, 2021 05:29

Tibo-lg force-pushed the serialization-update-proposal branch from d6a25a6 to f413e79 Compare October 14, 2021 05:08

Tibo-lg force-pushed the serialization-update-proposal branch from 588190c to a291f99 Compare October 15, 2021 12:53

Christewart reviewed Oct 31, 2021

View reviewed changes

Tibo-lg force-pushed the serialization-update-proposal branch 4 times, most recently from ea650db to 2147ee2 Compare November 2, 2021 02:00

Tibo-lg added 2 commits April 18, 2022 13:40

Add temporary contract id to offer message

7fdd6a9

Rename leftEndPoint to endPoint

4ee603b

Tibo-lg force-pushed the serialization-update-proposal branch from 319dd78 to 4ee603b Compare April 18, 2022 04:41

Update test vectors

25fe7b2

Christewart reviewed Apr 19, 2022

View reviewed changes

Christewart reviewed Apr 26, 2022

View reviewed changes

PayoutCurve.md Show resolved Hide resolved

Fix forgotten types

8517c72

Tibo-lg force-pushed the serialization-update-proposal branch from 5221dc7 to 8517c72 Compare April 26, 2022 23:38

Christewart reviewed May 2, 2022

View reviewed changes

Tibo-lg added 2 commits May 3, 2022 20:27

Replace bigsize with u64 in payout curve and numeric outcome

70032e3

Update test vectors

463dc59

Tibo-lg force-pushed the serialization-update-proposal branch from 9518a5a to 463dc59 Compare May 3, 2022 11:28

Christewart reviewed May 3, 2022

View reviewed changes

Messaging.md Show resolved Hide resolved

Tibo-lg added 4 commits May 4, 2022 21:03

use bigsize for vector prefix in rounding intervals

dcec86b

Update test vectors setting base as u16

8a78d9c

Update test vector with fix to precision field

5240cc5

Update test vectors to include temporary contract id in offer message

0fc681f

Christewart reviewed May 29, 2022

View reviewed changes

Tibo-lg added 3 commits May 30, 2022 10:32

Add protocol version to accept message

d738783

Add protocol version to sign message

990fec1

Fix test vectors

b4264fc

Tibo-lg force-pushed the serialization-update-proposal branch from 85680d5 to b4264fc Compare May 31, 2022 00:08

Tibo-lg mentioned this pull request May 31, 2022

v0 Oracle Changes #167

Open

Christewart mentioned this pull request Jun 8, 2022

Tuesday June 8 2022 Meeting #197

Merged

Christewart merged commit 8ecdf2a into discreetlogcontracts:master Jun 12, 2022

Christewart mentioned this pull request Sep 19, 2022

2022 08 07 DLC spec version 0 bitcoin-s/bitcoin-s#4790

Open

Christewart mentioned this pull request Oct 6, 2022

2022 10 05 Delete legacy DLCSerializationVersion.Alpha DLCs for a cleaner upgrade to v0 spec bitcoin-s/bitcoin-s#4817

Merged

matthewjablack mentioned this pull request Jun 20, 2023

Add cso info to offers AtomicFinance/node-dlc#189

Merged


		#### Sibling sub-type

		Sibling sub-types are prefixed with a `bigsize` type identifier.

		* [`protocol_version`: `bigsize`]
		* [`byte`:`contract_flags`]


		### Type-Length-Value

		A TLV stream is defined for each wire message, and can be used to extend the protocol and for application specific features.


		#### Plain sub-type

		Plain sub-types do not have any particular format, and their field can simply be replaced in place where they are used.

		@@ -69,7 +69,7 @@ In this section we detail the TLV serialization for a general `payout_function`.

		1. type: 42790 (`payout_function_v0`)

Update messaging and serialization #163

Update messaging and serialization #163

Conversation

Tibo-lg commented Apr 23, 2021 • edited Loading

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

Christewart Sep 19, 2021 • edited Loading

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

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

ariard 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

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 Oct 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewjablack Oct 6, 2021 • edited Loading

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

Tibo-lg commented Oct 14, 2021

Tibo-lg commented Oct 14, 2021

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

Tibo-lg commented Nov 2, 2021

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

Tibo-lg commented Apr 23, 2021 •

edited

Loading

Christewart Sep 19, 2021 •

edited

Loading

matthewjablack Oct 6, 2021 •

edited

Loading

matthewjablack Oct 6, 2021 •

edited

Loading