Add a `sendcustommsg` RPC call and a `custommsg` plugin hook for experimental protocol extensions by cdecker · Pull Request #3315 · ElementsProject/lightning

cdecker · 2019-12-04T23:21:23Z

This pull request implements the custommsg facility that allows users to
implement custom protocol extensions on top of what c-lightning offers. The
implementation aims to maximize the flexibility given to the users while at
the same time minimizing the dangers of sending custom messages (see the
limitations section).

Limitations

Custom messages are not allowed to have the same type as an internally
handled message type. This is necessary to ensure that the custom messages
injected at one end do not throw off the internal state handling code at
the other end. This means that it's ok to implement things that are in the
spec (e.g., experimental extensions that are not yet fully specified), but
once c-lightning implements them internally they may no longer be used.
Custom message types are limited to odd-numbered types. This is necessary
since even-typed messages are required to drop the connection if they are
unknown. If we were to defer that decision to a plugin we would need to
lock the owning daemon, and lightningd, until the plugin has returned the
verdict on whether this is known or unknown. Odd-typed messages on the
other hand can be dropped silently meaning we can easily continue
processing while the plugin deliberates on whether or not it knows how to
handle the message.
Custom messages can only be sent if a peer is owned by openingd, i.e.,
before opening a channel and currently just gossiping, or by channeld,
i.e., we currently have a channel open and are not closing it. The only
other daemon that talks to a peer directly is closingd which doesn't
handle unexpected messages well (hence it also doesn't handle gossip),
however in most cases that state is very transitory, so it is unlikely to
be necessary that we need to send custommsgs while owned by closingd.

Examples

The limitations mentioned above aside, this PR opens a wide variety of
possible extensions and opportunity for experimental implementations of
protocols. This is just a minimal list of things that came to my mind while
working on this:

@renepickhardt's idea of sharing a bit more information about capacities in
the friend-of-a-friend network in order to collaborate on finding
rebalancings.
Add a new broadcast for on-chain data, e.g., also transport block headers
on top of the encryptedoverlay network to provide hints to peers that there
is a new block or that they might be lagging behind.
Add a new broadcast for liquidity announcements or exchange rate offers for
cross-chain atomic swaps.
More advanced gossip sync mechanisms such as set-reconciliation. These
might negotiate the set differences in custommsgs and then send
non-custommsgs for the actual exchange of information.
Many more that others will come up with...

Testing

The test_custommsg test aims to cover all the limitations and also test the
happy path, in either of the two supported subdaemons. Currently it does not
test when a peer is owned by closingd, however that is identical to not
being owned at all.

Open Questions

We currently don't do anything with the result from the hook, we could
as well make it a notification. However we might later implement the
hook-chaining and we'd likely want to exit the chain as soon as a
plugin has handled the message instead of continuing to send it to the
other plugins. There the result could tell us whether it is handled or
not.
Should we make this experimental only for now?
Should we make this generally available (non-developer)?

Changelog-None

darosior

Looks good, but answering the open question I'd argue for sendcustommsg to be non-dev as it does not seem to be a shootgun for a regular user as could be other dev-only commands (such as dev-forget-channel). It also allows a plugin using this command to not require a compilation with enable-developer. But removing some restrictions could be developer-only 😁 ..

Custom message types are limited to odd-numbered types.

darosior · 2019-12-09T21:19:31Z

lightningd/peer_control.c

+    "utility",
+    json_sendcustommsg,
+    "Send a custom message to the peer with the given {node_id}",
+    .verbose = "sendcustommsg node_id hexcustommsg",


nit: dev-sendcustommsg node_id hexcustommsg

tests/test_misc.py

cdecker · 2019-12-09T22:57:23Z

Looks good, but answering the open question I'd argue for sendcustommsg to be non-dev as it does not seem to be a shootgun for a regular user as could be other dev-only commands (such as dev-forget-channel). It also allows a plugin using this command to not require a compilation with enable-developer. But removing some restrictions could be developer-only ..

Custom message types are limited to odd-numbered types.

Good point, however I get the feeling we might end up confusing people, and they might think this is to send some sort of chat message to an arbitrary node. It's more of a "don't show a normal mortal what they aren't supposed to use" kind of deal :-)

ZmnSCPxj · 2019-12-17T02:44:08Z

Good point, however I get the feeling we might end up confusing people, and they might think this is to send some sort of chat message to an arbitrary node. It's more of a "don't show a normal mortal what they aren't supposed to use" kind of deal :-)

Maybe rename to sendcustomprotocolmsg and customprotocolmsg to deter such?

cdecker · 2020-01-06T12:02:02Z

Good point, however I get the feeling we might end up confusing people, and they might think this is to send some sort of chat message to an arbitrary node. It's more of a "don't show a normal mortal what they aren't supposed to use" kind of deal :-)

Maybe rename to sendcustomprotocolmsg and customprotocolmsg to deter such?

Let's keep it a developer-only option for now, and see if my fears of causing confusion is justified. If it is we can always rename while making it non-dev-only :-)

rustyrussell

Ack b73414b

Trivial requests for enhancements which could go in separate PR.

Am in two minds about dev-. Should we drop it?

rustyrussell · 2020-01-28T01:18:45Z

wire/Makefile

+	$(BOLT_GEN) -s --page header $@ common_wire_type < $< > $@
+
+wire/gen_common_wire.c: wire/common_wire_csv $(WIRE_BOLT_DEPS) wire/Makefile
+	$(BOLT_GEN) -s --page impl ${@:.c=.h} common_wire_type < $< > $@


TODO: enhance devtools/decodemsg to decode internal messages. That is not only useful, if it's constructed as a giant switch() statement, it also ensures we don't have any duplicate message numbers anywhere (otherwise it won't compile).

Meanwhile, we should probably have such a switch statement somewhere in the source (even if it's just manually maintained) to force this check?

rustyrussell · 2020-01-28T01:21:34Z

tools/gen/impl_template

+	}
+	return false;
+}
+


can we use thse functions instead of the hacky "name contains INVALID" checks now?

cdecker · 2020-01-28T13:12:39Z

Am in two minds about dev-. Should we drop it?

Let's keep it for now and see how crazy people go with custommsg, and we can promote it in a later release.

These messages may be exchanged between the master and any daemon. For now these are just the daemons that a peer may be attached to at any time since the first example of this is the custommsg infrastructure.

This is currently in opening_control since that's the only part that has access to the uncommitted_channel internals. Otherwise it's independent from the specific daemon.

This command injects a custom message into the encrypted transport stream to the peer, allowing users to build custom protocols on top of c-lightning without requiring any changes to c-lightning itself.

We cannot let users use `sendcustommsg` to inject messages that are handled internally since it could result in our internal state tracking being borked.

Most of the work is done in `lightningd`, here we just need to queue the message itself.

This solves a couple of issues with the need to synchronously drop the connection in case we were required to understand what the peer was talking about while still allowing users to experiment, just not kill connections.

This is mainly meant as a marker so that we can later remove the code if we decide to make the handling of custommsgs a non-developer option. It marks the place that we would otherwise handle what in dev-mode is a custommsg.

This completes the custommsg epic, finally we are back where we began all that time ago (about 4 hours really...): in a plugin that implements some custom logic.

It's a dev-* command for now, but better document it so people can use it rather than having them guess how it's supposed to work.

cdecker · 2020-01-28T22:50:08Z

Had to rebase on top of master and I'll address the nits in a cleanup PR. Merging to keep the conflict surface low :-)

ACK 09c72dd

cdecker requested review from niftynei and rustyrussell December 4, 2019 23:21

cdecker force-pushed the custommsg branch 6 times, most recently from 5640447 to 30c089f Compare December 9, 2019 15:11

cdecker marked this pull request as ready for review December 9, 2019 15:53

cdecker force-pushed the custommsg branch from 3641287 to 1193d26 Compare December 9, 2019 16:01

cdecker mentioned this pull request Dec 9, 2019

BOLT 1: Define custom message type range lightning/bolts#705

Merged

niftynei added the Optech Make Me Famous! label Dec 9, 2019

darosior reviewed Dec 9, 2019

View reviewed changes

cdecker added this to the next milestone Dec 11, 2019

cdecker force-pushed the custommsg branch from cd00e82 to 51978d4 Compare December 12, 2019 21:21

cdecker force-pushed the custommsg branch 2 times, most recently from 7beec1a to 0fc7895 Compare January 3, 2020 19:51

cdecker force-pushed the custommsg branch from 0fc7895 to f6c86d5 Compare January 6, 2020 12:02

dr-orlovsky mentioned this pull request Jan 15, 2020

Analyze sendcustommsg feature by c-lightning for RGB applicability LNP-BP/LNPBPs#17

Closed

cdecker force-pushed the custommsg branch from f6c86d5 to b73414b Compare January 17, 2020 16:21

rustyrussell approved these changes Jan 28, 2020

View reviewed changes

cdecker added 5 commits January 28, 2020 14:13

wire: Add common messages that are independent of daemons

b581028

These messages may be exchanged between the master and any daemon. For now these are just the daemons that a peer may be attached to at any time since the first example of this is the custommsg infrastructure.

subd: Add a helper to get the current owning subd for a given peer

0ba41a4

This is currently in opening_control since that's the only part that has access to the uncommitted_channel internals. Otherwise it's independent from the specific daemon.

subd: Allow sending common messages to subdaemons

2e5328d

json-rpc: Add sendcustommsg command

32a241d

This command injects a custom message into the encrypted transport stream to the peer, allowing users to build custom protocols on top of c-lightning without requiring any changes to c-lightning itself.

json-rpc: Don't let users send messages that are handled internally

d74bb32

We cannot let users use `sendcustommsg` to inject messages that are handled internally since it could result in our internal state tracking being borked.

cdecker added 10 commits January 28, 2020 14:13

openingd: Implement sendcustommsg handling in openingd

2670c89

Most of the work is done in `lightningd`, here we just need to queue the message itself.

channeld: Send messages if instructed to by lightningd

7224ca7

channeld: Forward custommsgs to lightningd

e8f47bf

openingd: Forward custommsgs to lightningd

032ff80

json-rpc: Restrict custommsgs to be odd-typed

2228953

This solves a couple of issues with the need to synchronously drop the connection in case we were required to understand what the peer was talking about while still allowing users to experiment, just not kill connections.

common: Add an assertion for custommsgs in gossip handler

31757a8

This is mainly meant as a marker so that we can later remove the code if we decide to make the handling of custommsgs a non-developer option. It marks the place that we would otherwise handle what in dev-mode is a custommsg.

peer: Add custommsg hook and wire it into channeld and openingd

5cb4285

pytest: Add a plugin for custommsgs and check that they get the msgs

0e9b7fd

This completes the custommsg epic, finally we are back where we began all that time ago (about 4 hours really...): in a plugin that implements some custom logic.

docs: Add manpage for dev-sendcustommsg

8fabee0

It's a dev-* command for now, but better document it so people can use it rather than having them guess how it's supposed to work.

doc: Add a section about the custommsg hook in the plugin doc

09c72dd

cdecker force-pushed the custommsg branch from b73414b to 09c72dd Compare January 28, 2020 13:13

cdecker merged commit 602b81f into ElementsProject:master Jan 28, 2020

This was referenced Jan 31, 2020

plugin: Allow plugins to register featurebits #3477

Merged

Allow hook chaining for the htlc_accepted hook #3489

Merged

ariard mentioned this pull request May 16, 2020

RFC: Introducing AltNet, a pluggable framework for alternative transports bitcoin/bitcoin#18988

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a `sendcustommsg` RPC call and a `custommsg` plugin hook for experimental protocol extensions#3315

Add a `sendcustommsg` RPC call and a `custommsg` plugin hook for experimental protocol extensions#3315
cdecker merged 15 commits intoElementsProject:masterfrom
cdecker:custommsg

cdecker commented Dec 4, 2019 •

edited

Loading

Uh oh!

darosior left a comment

Uh oh!

darosior Dec 9, 2019

Uh oh!

Uh oh!

cdecker commented Dec 9, 2019

Uh oh!

ZmnSCPxj commented Dec 17, 2019

Uh oh!

cdecker commented Jan 6, 2020

Uh oh!

rustyrussell left a comment

Uh oh!

rustyrussell Jan 28, 2020

Uh oh!

rustyrussell Jan 28, 2020

Uh oh!

cdecker commented Jan 28, 2020

Uh oh!

cdecker commented Jan 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

cdecker commented Dec 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Limitations

Examples

Testing

Open Questions

Uh oh!

darosior left a comment

Choose a reason for hiding this comment

Uh oh!

darosior Dec 9, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cdecker commented Dec 9, 2019

Uh oh!

ZmnSCPxj commented Dec 17, 2019

Uh oh!

cdecker commented Jan 6, 2020

Uh oh!

rustyrussell left a comment

Choose a reason for hiding this comment

Uh oh!

rustyrussell Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

rustyrussell Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

cdecker commented Jan 28, 2020

Uh oh!

cdecker commented Jan 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cdecker commented Dec 4, 2019 •

edited

Loading