Skip to content

TLV improvements and full spec compatibility#1069

Merged
t-bast merged 5 commits intomasterfrom
tlv-truncated-integers
Jul 11, 2019
Merged

TLV improvements and full spec compatibility#1069
t-bast merged 5 commits intomasterfrom
tlv-truncated-integers

Conversation

@t-bast
Copy link
Member

@t-bast t-bast commented Jul 11, 2019

This PR adds support for truncated integers as defined in the spec.
The test vectors are updated to include all test vectors from rusty's spec PR.
It also provides many changes to the tlv and tlv stream classes:

  • The tlv trait doesn't need a type field, the codec should handle that
  • A TLV stream should be scoped to a specific subtrait of tlv
  • Stream validation is done inside the codec instead of the tlv stream: it makes it more convenient for application layers to create tlv streams and manipulate them

I'm starting a thread on scodec's gitter to see if we can do something better than the tag function that encodes the tlv to extract the discriminator, if that yields something I'll update the PR.

@t-bast t-bast requested review from pm47 and sstone July 11, 2019 11:36
t-bast added 2 commits July 11, 2019 13:37
TLV doesn't have an abstract `type` field anymore.
This is scoped to the codec itself.
TLV streams are now scoped to a specific namespace (subtrait of TLV).
@t-bast t-bast force-pushed the tlv-truncated-integers branch from 9291da2 to bfb58dd Compare July 11, 2019 11:37
@codecov-io
Copy link

codecov-io commented Jul 11, 2019

Codecov Report

Merging #1069 into master will increase coverage by 8.75%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1069      +/-   ##
==========================================
+ Coverage    73.7%   82.46%   +8.75%     
==========================================
  Files         126       99      -27     
  Lines        8458     7589     -869     
  Branches      321      298      -23     
==========================================
+ Hits         6234     6258      +24     
+ Misses       2224     1331     -893
Impacted Files Coverage Δ
...src/main/scala/fr/acinq/eclair/wire/TlvTypes.scala 100% <100%> (ø) ⬆️
...main/scala/fr/acinq/eclair/wire/CommonCodecs.scala 97.61% <100%> (ø) ⬆️
...rc/main/scala/fr/acinq/eclair/wire/TlvCodecs.scala 100% <100%> (ø) ⬆️
...src/main/scala/fr/acinq/eclair/router/Router.scala 85.81% <0%> (-0.95%) ⬇️
...c/main/scala/fr/acinq/eclair/channel/Channel.scala 83.6% <0%> (-0.38%) ⬇️
.../fr/acinq/eclair/gui/stages/OpenChannelStage.scala
...r/gui/controllers/NotificationPaneController.scala
...gui/src/main/scala/fr/acinq/eclair/gui/FxApp.scala
...cala/fr/acinq/eclair/gui/utils/GUIValidators.scala
...ui/src/main/scala/fr/acinq/eclair/JavafxBoot.scala
... and 24 more

@t-bast t-bast merged commit 1621e39 into master Jul 11, 2019
@t-bast t-bast deleted the tlv-truncated-integers branch July 11, 2019 15:25
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.

4 participants