Added a channel version to Commitments object
#1059
Conversation
In a backward-compatible way, by using the fact that the first object of a legacy `Commitments` was a public key, starting from 0x02 or 0x03.
| @@ -50,6 +50,14 @@ object ChannelCodecs extends Logging { | |||
| ("path" | keyPathCodec) :: | |||
| ("parent" | int64)).as[ExtendedPrivateKey] | |||
|
|
|||
| val channelVersionCodec: Codec[ChannelVersion] = discriminatorWithDefault[ChannelVersion]( | |||
| discriminator = discriminated[ChannelVersion].by(byte) | |||
| .typecase(0x01, provide(ChannelVersion.STANDARD)) | |||
There was a problem hiding this comment.
How about encoding the type byte inside the ChannelVersion object?
There was a problem hiding this comment.
Why would we do that? The object and its encoding are two different things.
There was a problem hiding this comment.
Each subtype of ChannelVersion will be associated with a particular byte that is used in the codec, since all versions will have a version byte it seemed reasonable to have it embedded in ChannelVersion itself. The code would become something like:
.typecase(ChannelVersion.STANDARD.typeByte, provide(ChannelVersion.STANDARD))
There was a problem hiding this comment.
You are thinking in terms of code factorization but I'm more concerned about proper separation of layers. With your proposal the codec would be partly defined in the ChannelCodecs file and partly in the ChannelTypes file. Meaning that modifying ChannelTypes could create backward incompatibility.
There was a problem hiding this comment.
Yes i agree that layer separation is quite important here and we definitely don't want to define the codec in two different places, however it felt reasonable to declare the codec byte close to the definition of the scala type for a certain commitment version (after all they are always and uniquely associated), i did something similar here although i did not include it in the object.
for consistency with `ChannelFlags`
Codecov Report
@@ Coverage Diff @@
## master #1059 +/- ##
==========================================
- Coverage 82.5% 73.87% -8.64%
==========================================
Files 99 126 +27
Lines 7587 8489 +902
Branches 298 325 +27
==========================================
+ Hits 6260 6271 +11
- Misses 1327 2218 +891
Continue to review full report at Codecov.
|
In a backward-compatible way, by using the fact that the first object of
a legacy
Commitmentswas a public key, starting from 0x02 or 0x03.