Explicit channel type in channel open #1867
Conversation
t-bast
force-pushed
the
channel-types
branch
3 times, most recently
from
42f1bce to
28618e7
Compare
eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelFeatures.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/ChannelTlv.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelFeatures.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/wire/protocol/LightningMessageTypes.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
Outdated
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
Outdated
Show resolved
Hide resolved
| stay() using d.copy(channels = d.channels + (TemporaryChannelId(temporaryChannelId) -> channel)) | ||
| val defaultChannelType = ChannelTypes.pickChannelType(d.localFeatures, d.remoteFeatures) | ||
| val chosenChannelType: Either[InvalidChannelType, SupportedChannelType] = msg.channelType_opt match { | ||
| case None => Right(defaultChannelType) |
There was a problem hiding this comment.
Does this mean that we'll add a channel_type to our accept message even there was none in our peer's open message ? If so it looks like a spec violation ?
There was a problem hiding this comment.
Yes, we always send back a channel_type. It's not a spec violation, the only thing the spec requires is:
- if it sets `channel_type`:
- MUST set it to the `channel_type` from `open_channel`
If it wasn't set in open_channel, we're free to do what we want, and it's an odd message type so it's always ok to include.
There was a problem hiding this comment.
But then it will be ignored, so what is the point ?
There was a problem hiding this comment.
It doesn't hurt, is future proof, and we don't have to manage an option?
There was a problem hiding this comment.
Also, it can be viewed as "FYI I'll be using that channel type"?
There was a problem hiding this comment.
Exactly, it's simpler in the flow to always use channel_types internally, because otherwise you can't as easily distinguish in this piece of code the case where no channel type was sent and the case where an invalid one was sent (it would need a bit more code).
There was a problem hiding this comment.
I guess that what bothered me is that it's not "symmetric" (we set it but they did not) but it does work.
There was a problem hiding this comment.
I've put some test improvements on branch https://github.com/ACINQ/eclair/tree/channel-types-pm.
eclair-core/src/test/scala/fr/acinq/eclair/channel/states/a/WaitForAcceptChannelStateSpec.scala
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #1867 +/- ##
==========================================
+ Coverage 87.52% 87.62% +0.10%
==========================================
Files 159 159
Lines 12104 12157 +53
Branches 502 483 -19
==========================================
+ Hits 10594 10653 +59
+ Misses 1510 1504 -6
|
Add support for lightning/bolts#880 This lets node operators open a channel with different features than what the implicit choice based on activated features would use.
* ChannelType contains a set of features * TLV doesn't expose the feature bits directly * An unsupported channel type class is explicitly added
Add support for lightning/bolts#880
This lets node operators open a channel with different features than what the implicit choice based on activated features would use.
This has been tested successfully against lightningnetwork/lnd#5373