Skip to content

Extended Queries: use TLV format for optional data#972

Closed
sstone wants to merge 27 commits into
extended-queries-optionalfrom
extended-queries-optional-tlv
Closed

Extended Queries: use TLV format for optional data#972
sstone wants to merge 27 commits into
extended-queries-optionalfrom
extended-queries-optional-tlv

Conversation

@sstone
Copy link
Copy Markdown
Member

@sstone sstone commented Apr 26, 2019

The RFC PR has already been updated, and this adds more flexibility as node that don't understand some fields can just skip over them. Timestamps and checksums have also been separated.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Jun 11, 2019

Codecov Report

Merging #972 into extended-queries-optional will increase coverage by 8.8%.
The diff coverage is 82.52%.

@@                     Coverage Diff                     @@
##           extended-queries-optional    #972     +/-   ##
===========================================================
+ Coverage                      73.89%   82.7%   +8.8%     
===========================================================
  Files                            125      98     -27     
  Lines                           8474    7660    -814     
  Branches                         314     289     -25     
===========================================================
+ Hits                            6262    6335     +73     
+ Misses                          2212    1325    -887
Impacted Files Coverage Δ
...src/main/scala/fr/acinq/eclair/db/PaymentsDb.scala 100% <ø> (ø) ⬆️
.../src/main/scala/fr/acinq/eclair/db/NetworkDb.scala 100% <ø> (ø) ⬆️
...ain/scala/fr/acinq/eclair/wire/CommandCodecs.scala 100% <ø> (ø) ⬆️
...cala/fr/acinq/eclair/blockchain/WatcherTypes.scala 100% <ø> (ø) ⬆️
.../scala/fr/acinq/eclair/payment/CommandBuffer.scala 75% <0%> (ø) ⬆️
...lockchain/bitcoind/rpc/ExtendedBitcoinClient.scala 92.3% <0%> (ø) ⬆️
...cala/fr/acinq/eclair/api/FormParamExtractors.scala 81.81% <100%> (ø) ⬆️
...c/main/scala/fr/acinq/eclair/payment/Auditor.scala 88.37% <100%> (ø) ⬆️
.../main/scala/fr/acinq/eclair/io/Authenticator.scala 92% <100%> (ø) ⬆️
...la/fr/acinq/eclair/db/sqlite/SqliteNetworkDb.scala 98.61% <100%> (ø) ⬆️
... and 104 more

@pm47
Copy link
Copy Markdown
Member

pm47 commented Jun 24, 2019

This probably needs to be rebased from #1045.

@t-bast
Copy link
Copy Markdown
Member

t-bast commented Jun 24, 2019

Yes, I discussed it with @sstone and he said he would rebase this on top of #1045 once #1045 is stabilized (hopefully today).

sstone and others added 14 commits July 2, 2019 14:08
* Reject payments for expired invoices
It is functionnaly the same but it's cleaner and removes the need for
tuples in the success case.
* Add eclair-cli to eclair's docker image

* Add instructions to use eclair-cli inside the docker container
Hide some internals (tlvFallback, generictlv codecs).
Add a length-prefixed tlv stream codec.
This tests was checking that we could read extended `query_short_channel_ids`, with an optional list
of query flags. This optional type is now a TLV, and its type is 1 (and not 0 as previously defined here)
All the data contained in `node_announcement`, `channel_announcement`
and `channel_update` is to be included in the signature, including
unknown trailing fields. We were ignoring them, causing signature
verification to fail when there was unknown fields.

In the case of `channel_update` there is a backward compatibility issue
to handle, because when persisting channel data in state `NORMAL`, we
used to store the `channel_update` followed by other data, and without
prefixing it with size information.

To work around that we use the same trick as before, based on an
additional discriminator codec.
Checkpoints generated on July 9th 2019
* Wrap all routes in toStrictEntity
Comment thread eclair-core/src/main/scala/fr/acinq/eclair/wire/LightningMessageTypes.scala Outdated
sstone and others added 3 commits July 10, 2019 13:21
We now define TLV classes `type` property in their companion object.
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
@sstone sstone marked this pull request as ready for review August 22, 2019 12:15
@sstone sstone closed this Aug 22, 2019
@sstone sstone deleted the extended-queries-optional-tlv branch January 27, 2020 13:26
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.

5 participants