Plugin messaging

[akumaigorodski]

This is a draft of how plugin messaging between two peers could work.

It makes sense to enable plugins to communicate with each other via an already existing and authenticated socket connection rather than for example getting them to open another HTTP endpoint just for that.

The idea is as follows:

1. Once connected, a peer who wants to know about other peer's messaging-enabled plugins sends QuerySupportedPlugins message.
2. The other peer replies with ReplySupportedPlugins so interested peer knows what to expect.
3. Remote peer can then send PluginMessage with pluginId attached, a Peer.scala would issue an event on seeing such a message which in turn would be intercepted by plugin and further processed according to plugin's specific rules.

[Christewart]

Hi @btcontract

We've begun hacking away at eclair to introduce some of the PTLC work (and future DLC work) we've been writing about at Suredbits for awhile now. Would these two use cases apply here?

https://github.com/bitcoin-s/eclair/pulls?q=is%3Apr+label%3Aptlc+

[akumaigorodski]

@Christewart I don't yet understand how PTLC/DLC works but if this involves communication between two peers then yes, a related plugin could define a completely independant system of messages which is convenient for your use case, no need for tlv data even.

[t-bast]

1. Once connected, a peer who wants to know about other peer's messaging-enabled plugins sends QuerySupportedPlugins message.
2. The other peer replies with ReplySupportedPlugins so interested peer knows what to expect.

I think this part is unnecessary, that is exactly why we have feature bits in place so I wouldn't want to add this to the base eclair layer.
I think it would be much cleaner to reserve some feature bits far in the future to each feature that you're adding.
If you use feature bits that are higher than 128, you're pretty much guaranteed that the spec will not override your choice before at least 10 years given the rate at which we add spec features 😆

And from an abstraction's point of view, it's also better. A node connecting to you doesn't care that a feature works via a plugin or via the base code; it shouldn't even care what implementation you're running, as long as you support the feature they want. And ideally in the long run, custom features that users like will end up being a standard implemented by many, which is better for the ecosystem.

There is a risk that another app uses the same feature bits for its experiments, but I don't think it's an issue in practice because if you think they support your feature because they've activate the same feature bit, when you'll exchange custom messages they will fail to decode and either side will close the connection.

My suggestion is the following flow instead:

1. Plugins inject feature bits for the features they support in eclair's NodeParams, so they get broadcast through the existing system.
2. At start-up, plugins inject the list of message types (the 2-byte number at the beginning of every lightning message) they're interested in
3. When eclair receives unknown messages, if the message type is in the list of messages supported by plugins, it broadcasts it to the event stream

With that system, we could even allow unknown even messages, which ensures that if you connect to a peer that turned on the same feature bits but doesn't really support the feature, the connection will be closed as soon as a message is sent/received because it's unknown to one side.

[akumaigorodski]

Here's what's done so far:

• Each plugin may declare an optional feature bit which must be higher than 128 and must not overlap with core used features (checks on this are done, I guess it makes sense to have plugin features always optional since plugins themselves are optional).
• Plugin features are injected into Init message and added to Peer.scala.
• PluginMessage now looks like PluginMessage(featureBit: Int, data: ByteVector) instead of PluginMessage(pluginId: Int, data: ByteVector), Peer can still quickly check if it's supported.
• While doing this sealed trait Feature became just trait Feature to reference it in Plugin.scala, is this acceptable?

At start-up, plugins inject the list of message types (the 2-byte number at the beginning of every lightning message) they're interested in

Do we really need to do it this way? Having actual plugin-specific message as data: ByteVector inside of PluginMessage gives plugin devs more freedom, as an extreme example some plugin may decide to exchange data in, say, Json format (converted to bytes).

[t-bast]

Great, it looks like this won't be too much added code and will add a lot of usefulness to plugins!

While doing this sealed trait Feature became just trait Feature to reference it in Plugin.scala, is this acceptable?

Yes I think it's totally acceptable, the Feature trait is not something we want to exhaustively pattern-match on so I'd go with it.

At start-up, plugins inject the list of message types (the 2-byte number at the beginning of every lightning message) they're interested in

Do we really need to do it this way? Having actual plugin-specific message as data: ByteVector inside of PluginMessage gives plugin devs more freedom, as an extreme example some plugin may decide to exchange data in, say, Json format (converted to bytes).

Yes I think my suggestion is more in-line with how lightning messages are used and will evolve (and fits what we're doing on the Acinq node for Phoenix features).

When the peer receives an unknown message, it checks in its nodeParams if something subscribed to that message type; if yes, it simply sends it to the eventStrean. That doesn't prevent you from creating a message RequestSomethingFromPlugin for which the inner data is JSON or protobuf. WDYT?

[akumaigorodski]

When the peer receives an unknown message

To make sure I understand this right:

• there will be something like UnknownMessage(tag: Int, data: ByteVector).
• lightningMessageCodec can be wrapped with discriminatorWithDefault which will fall back to UnknownMessage(tag, data).
• Peer will send an event if some plugin has subscribed to tag?

[t-bast]

Yes that's exactly what I had in mind (if there's not blocking point when implementing that).

[akumaigorodski]

I've started on this, but right at the outset this feels a bit futile since:

• discriminatorWithDefault returns Codec[A] so discriminatorWithDefault(lightningMessageCodec ...) will have to return a Codec[LightningMessage] so we will need to have UnknownMessage(tag: Int, data: ByteVector) extends LightningMessage anyway (identical to PluginMessage).
• since there are no checks on data: ByteVector at core level (and should not be) a plugin may sort of fool core and build its entire messaging system through one UnknownMessage with the same tag which is the same as PluginMessage.

Am I missing something?

[t-bast]

UnknownMessage(tag: Int, data: ByteVector) extends LightningMessage anyway (identical to PluginMessage).

Yes that's totally ok, it is a LightningMessage so it makes sense.

since there are no checks on data: ByteVector at core level (and should not be) a plugin may sort of fool core and build its entire messaging system through one UnknownMessage with the same tag which is the same as PluginMessage.

That's ok too, it's up to the plugin to decide how it wants to use it.
But what's important is that at the core layer, we make all usages possible (to either use multiple messages or use a single one).

[akumaigorodski]

Okay, this might be overly complex, maybe there are easier ways but anyway:

• there are two new LN message types:

  • KnownUnknownMessage (has a standard tag == STANDARD_UNKNOWN_MESSAGE_TAG so can be handled by lightningMessageCodec has no tag because it's known to be a standard one).
  • UnknownUnknownMessage (has tag != STANDARD_UNKNOWN_MESSAGE_TAG) so lightningMessageCodec fails on it and then lightningMessageCodecWithFallback handles it.

• Peer only handles UnknownUnknownMessages as the ones which are clearly "unknown unknown" (that is, sent by plugin with plugin-specific non-standard tag).

[t-bast]

This is shaping up nicely, let us know when you've tested this E2E with a sample plugin!

[akumaigorodski]

Besides mentioned fixes this one makes an UnknownMessage without subscribers to be logged in next case instead of failing silently.

[akumaigorodski]

I've made a sample here: https://github.com/btcontract/eclair-plugin-messaging-sample.

It pushes a json-encoded local feerate to remote peer on connect if remote peer supports this plugin (makes use of remoteInit and I've chosen json just to show anything is possible).

It compiles but I could not figure out how to build my Eclair snapshot jar such that it could run. Looks like something changed in build process and jars do not contain all dependencies inside now? Some instructions on building it would be appreciated.

[t-bast]

It compiles but I could not figure out how to build my Eclair snapshot jar such that it could run. Looks like something changed in build process and jars do not contain all dependencies inside now? Some instructions on building it would be appreciated.

We now use standard maven builds (to get reproducible builds) instead of capsule.
This means there is no single-jar deployment anymore, but mvn package will instead create a folder with all the necessary jar.
Isn't that working for you? I agree instructions for building plugins could be better than the small section we have on the README.md, if you have suggestions feel free to edit it!

[akumaigorodski]

My bad, a zip file with everything needed was being created all the time, but I did not pay attention to it.

OK, I've just tested this with two Eclair instances and it works, I can see Remote peer sent feerates=50602 via plugin in both logs, as it should be according to https://github.com/btcontract/eclair-plugin-messaging-sample/blob/master/src/main/scala/fr/acinq/messagingsample/app/Notifier.scala#L25

[t-bast]

Great, the overall architecture sounds good, let me know when this is ready for review.

[akumaigorodski]

There is a slight complication when using ConnectionInfo class: in onTransition only nextStateData is visible which is Peer.Data, not ConnectedData.

To solve it I've defined a method connectionInfo: Option[ConnectionInfo] = None on Peer.Data and overrided it in ConnectedData. As a result PeerConnected and UnknownMessageReceived now have Option[ConnectionInfo] instead of ConnectionInfo which is still easily matchable in plugin.

[t-bast]

There is a slight complication when using ConnectionInfo class: in onTransition only nextStateData is visible which is Peer.Data, not ConnectedData.

While I agree that nextStateData isn't properly typed in transitions, we know that for transitions that go to the CONNECTED state (which is where the PeerConnected event is sent) it will always be a ConnectedData, so that shouldn't be an Option.

You'll need to case nextStateData.asInstanceOf[ConnectedData] in the transition.
Once we migrate this actor to Akka typed we probably won't need this explicit cast.

[t-bast]

LGTM, thanks!