Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow even unknown messages, by registration, and allow sending them #6689

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions connectd/connectd.c
Original file line number Diff line number Diff line change
Expand Up @@ -2100,6 +2100,10 @@ static struct io_plan *recv_req(struct io_conn *conn,
start_shutdown(daemon, msg);
goto out;

case WIRE_CONNECTD_SET_CUSTOMMSGS:
set_custommsgs(daemon, msg);
goto out;

case WIRE_CONNECTD_DEV_MEMLEAK:
if (daemon->developer) {
dev_connect_memleak(daemon, msg);
Expand Down Expand Up @@ -2209,6 +2213,7 @@ int main(int argc, char *argv[])
daemon->gossip_store_fd = -1;
daemon->shutting_down = false;
daemon->dev_suppress_gossip = false;
daemon->custom_msgs = NULL;

/* stdin == control */
daemon->master = daemon_conn_new(daemon, STDIN_FILENO, recv_req, NULL,
Expand Down
3 changes: 3 additions & 0 deletions connectd/connectd.h
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,9 @@ struct daemon {
/* Shutting down, don't send new stuff */
bool shutting_down;

/* What (even) custom messages we accept */
u16 *custom_msgs;

/* Hack to speed up gossip timer */
bool dev_fast_gossip;
/* Hack to avoid ping timeouts */
Expand Down
5 changes: 5 additions & 0 deletions connectd/connectd_wire.csv
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ msgtype,connectd_activate,2025
# Do we listen?
msgdata,connectd_activate,listen,bool,

# Set the allowed (i.e. don't hang up on!) unknown messages.
msgtype,connectd_set_custommsgs,2007
msgdata,connectd_set_custommsgs,len,u32,
msgdata,connectd_set_custommsgs,msgnums,u16,len

# Connectd->master, I am ready.
msgtype,connectd_activate_reply,2125
msgdata,connectd_activate_reply,failmsg,?wirestring,
Expand Down
49 changes: 38 additions & 11 deletions connectd/multiplex.c
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,13 @@ static void send_ping(struct peer *peer)
set_ping_timer(peer);
}

void set_custommsgs(struct daemon *daemon, const u8 *msg)
{
tal_free(daemon->custom_msgs);
if (!fromwire_connectd_set_custommsgs(daemon, msg, &daemon->custom_msgs))
master_badmsg(WIRE_CONNECTD_SET_CUSTOMMSGS, msg);
}

void send_custommsg(struct daemon *daemon, const u8 *msg)
{
struct node_id id;
Expand Down Expand Up @@ -701,24 +708,44 @@ static void handle_gossip_timestamp_filter_in(struct peer *peer, const u8 *msg)
wake_gossip(peer);
}

static bool find_custom_msg(const u16 *custom_msgs, u16 type)
{
for (size_t i = 0; i < tal_count(custom_msgs); i++) {
if (custom_msgs[i] == type)
return true;
}
return false;
}

static bool handle_custommsg(struct daemon *daemon,
struct peer *peer,
const u8 *msg)
{
enum peer_wire type = fromwire_peektype(msg);
if (type % 2 == 1 && !peer_wire_is_internal(type)) {
/* The message is not part of the messages we know how to
* handle. Assuming this is a custommsg, we just forward it to the
* master. */
status_peer_io(LOG_IO_IN, &peer->id, msg);
daemon_conn_send(daemon->master,
take(towire_connectd_custommsg_in(NULL,
&peer->id,
msg)));
return true;
} else {

/* Messages we expect to handle ourselves. */
if (peer_wire_is_internal(type))
return false;

/* We log it, since it's not going to a subdaemon */
status_peer_io(LOG_IO_IN, &peer->id, msg);

/* Even unknown messages must be explicitly allowed */
if (type % 2 == 0 && !find_custom_msg(daemon->custom_msgs, type)) {
send_warning(peer, "Invalid unknown even msg %s",
tal_hex(msg, msg));
/* We "handled" it... */
return true;
}

/* The message is not part of the messages we know how to
* handle. Assuming this is a custommsg, we just forward it to the
* master. */
daemon_conn_send(daemon->master,
take(towire_connectd_custommsg_in(NULL,
&peer->id,
msg)));
return true;
}

/* We handle pings and gossip messages. */
Expand Down
3 changes: 3 additions & 0 deletions connectd/multiplex.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ void send_manual_ping(struct daemon *daemon, const u8 *msg);
/* When lightningd says to send a custom message (from a plugin) */
void send_custommsg(struct daemon *daemon, const u8 *msg);

/* When lightningd says what custom messages we can recv */
void set_custommsgs(struct daemon *daemon, const u8 *msg);

/* Lightningd wants to talk to you. */
void peer_connect_subd(struct daemon *daemon, const u8 *msg, int fd);

Expand Down
7 changes: 6 additions & 1 deletion contrib/pyln-client/pyln/client/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,13 +217,15 @@ def __init__(self, stdout: Optional[io.TextIOBase] = None,
dynamic: bool = True,
init_features: Optional[Union[int, str, bytes]] = None,
node_features: Optional[Union[int, str, bytes]] = None,
invoice_features: Optional[Union[int, str, bytes]] = None):
invoice_features: Optional[Union[int, str, bytes]] = None,
custom_msgs: Optional[List[int]] = None):
self.methods = {
'init': Method('init', self._init, MethodType.RPCMETHOD)
}

self.options: Dict[str, Dict[str, Any]] = {}
self.notification_topics: List[str] = []
self.custom_msgs = custom_msgs

def convert_featurebits(
bits: Optional[Union[int, str, bytes]]) -> Optional[str]:
Expand Down Expand Up @@ -931,6 +933,9 @@ def _getmanifest(self, **kwargs) -> JSONType:
if features is not None:
manifest['featurebits'] = features

if self.custom_msgs is not None:
manifest['custommessages'] = self.custom_msgs

return manifest

def _init(self, options: Dict[str, JSONType],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ The `getmanifest` method is required for all plugins and will be called on start
"method": "mycustomnotification"
}
],
"custommessages": [
11008, 11010
],
"nonnumericids": true,
"dynamic": true
}
Expand All @@ -103,6 +106,8 @@ The `featurebits` object allows the plugin to register featurebits that should b

The `notifications` array allows plugins to announce which custom notifications they intend to send to `lightningd`. These custom notifications can then be subscribed to by other plugins, allowing them to communicate with each other via the existing publish-subscribe mechanism and react to events that happen in other plugins, or collect information based on the notification topics.

The `custommessages` array allows the plugin to tell `lightningd` to explicity allow these (unknown) custom messages: we normally disconnect with an error if we receive these. This only makes sense if you also subscribe to the `custommsg` hook.

Plugins are free to register any `name` for their `rpcmethod` as long as the name was not previously registered. This includes both built-in methods, such as `help` and `getinfo`, as well as methods registered by other plugins. If there is a conflict then `lightningd` will report an error and kill the plugin, this aborts startup if the plugin is _important_.

#### Types of Options
Expand Down Expand Up @@ -232,4 +237,4 @@ The `startup` field allows a plugin to detect if it was started at `lightningd`
### Timeouts

During startup ("startup" is true), the plugin has 60 seconds to return `getmanifest` and another 60 seconds to return `init`, or gets killed.
When started dynamically via the [lightning-plugin](ref:lightning-plugin) JSON-RPC command, both `getmanifest` and `init` should be completed within 60 seconds.
When started dynamically via the [lightning-plugin](ref:lightning-plugin) JSON-RPC command, both `getmanifest` and `init` should be completed within 60 seconds.
8 changes: 3 additions & 5 deletions doc/lightning-sendcustommsg.7.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,9 @@ top, not for direct use by end-users.

The message must be a hex encoded well-formed message, including the 2-byte
type prefix, but excluding the length prefix which will be added by the RPC
method. The messages must not use even-numbered types, since these may require
synchronous handling on the receiving side, and can cause the connection to be
dropped. The message types may also not use one of the internally handled
method. The message types may not be one of the internally handled
types, since that may cause issues with the internal state tracking of
Core Lightning.
Core Lightning. We do (as of *v23.11*) allow sending of even types, but note that peers (as per the spec) will disconnect on receiving unknown even types.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inconsistent line breaks


The node specified by `node_id` must be a peer, i.e., it must have a direct
connection with the node receiving the RPC call, and the connection must be
Expand All @@ -33,7 +31,7 @@ not have a connection, or are synchronous daemons that do not handle
spontaneous messages.

On the reveiving end a plugin may implement the `custommsg` plugin hook and
get notified about incoming messages.
get notified about incoming messages, and allow additional unknown even types in their getmanifest response.

RETURN VALUE
------------
Expand Down
14 changes: 7 additions & 7 deletions lightningd/connect_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,7 @@ static unsigned connectd_msg(struct subd *connectd, const u8 *msg, const int *fd
case WIRE_CONNECTD_SEND_ONIONMSG:
case WIRE_CONNECTD_CUSTOMMSG_OUT:
case WIRE_CONNECTD_START_SHUTDOWN:
case WIRE_CONNECTD_SET_CUSTOMMSGS:
/* This is a reply, so never gets through to here. */
case WIRE_CONNECTD_INIT_REPLY:
case WIRE_CONNECTD_ACTIVATE_REPLY:
Expand Down Expand Up @@ -810,13 +811,12 @@ static struct command_result *json_sendcustommsg(struct command *cmd,
}

if (type % 2 == 0) {
return command_fail(
cmd, JSONRPC2_INVALID_REQUEST,
"Cannot send even-typed %d custom message. Currently "
"custom messages are limited to odd-numbered message "
"types, as even-numbered types might result in "
"disconnections.",
type);
/* INFO the first time, then DEBUG */
static enum log_level level = LOG_INFORM;
log_(cmd->ld->log, level, dest, false,
"sendcustommsg id=%s sending a custom even message (%u)",
cmd->id, type);
level = LOG_DBG;
}

peer = peer_by_id(cmd->ld, dest);
Expand Down
56 changes: 55 additions & 1 deletion lightningd/plugin.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@
#include <common/memleak.h>
#include <common/timeout.h>
#include <common/version.h>
#include <connectd/connectd_wiregen.h>
#include <dirent.h>
#include <errno.h>
#include <lightningd/io_loop_with_timers.h>
#include <lightningd/notification.h>
#include <lightningd/plugin.h>
#include <lightningd/plugin_control.h>
#include <lightningd/plugin_hook.h>
#include <lightningd/subd.h>
#include <sys/stat.h>

/* Only this file can include this generated header! */
Expand Down Expand Up @@ -192,6 +194,32 @@ struct command_result *plugin_register_all_complete(struct lightningd *ld,
return NULL;
}

static void tell_connectd_custommsgs(struct plugins *plugins)
{
struct plugin *p;
size_t n = 0;
u16 *all_msgs = tal_arr(tmpctx, u16, n);

/* Not when shutting down */
if (!plugins->ld->connectd)
return;

/* Gather from all plugins. */
list_for_each(&plugins->plugins, p, list) {
size_t num = tal_count(p->custom_msgs);
/* Blah blah blah memcpy NULL blah blah */
if (num == 0)
continue;
tal_resize(&all_msgs, n + num);
memcpy(all_msgs + n, p->custom_msgs, num * sizeof(*p->custom_msgs));
n += num;
}

/* Don't bother sorting or uniquifying. If plugins are dumb, they deserve it. */
subd_send_msg(plugins->ld->connectd,
take(towire_connectd_set_custommsgs(NULL, all_msgs)));
}

static void destroy_plugin(struct plugin *p)
{
struct plugin_rpccall *call;
Expand Down Expand Up @@ -232,6 +260,9 @@ static void destroy_plugin(struct plugin *p)
"shutting down lightningd!");
lightningd_exit(p->plugins->ld, 1);
}

if (tal_count(p->custom_msgs))
tell_connectd_custommsgs(p->plugins);
}

static u32 file_checksum(struct lightningd *ld, const char *path)
Expand Down Expand Up @@ -305,6 +336,8 @@ struct plugin *plugin_register(struct plugins *plugins, const char* path TAKES,
p->important = important;
p->parambuf = tal_steal(p, parambuf);
p->params = tal_steal(p, params);
p->custom_msgs = NULL;

return p;
}

Expand Down Expand Up @@ -1545,7 +1578,7 @@ static const char *plugin_parse_getmanifest_response(const char *buffer,
struct plugin *plugin,
const char **disabled)
{
const jsmntok_t *resulttok, *dynamictok, *featurestok, *tok;
const jsmntok_t *resulttok, *dynamictok, *featurestok, *custommsgtok, *tok;
const char *err;

*disabled = NULL;
Expand Down Expand Up @@ -1619,6 +1652,25 @@ static const char *plugin_parse_getmanifest_response(const char *buffer,
}
}

custommsgtok = json_get_member(buffer, resulttok, "custommessages");
if (custommsgtok) {
size_t i;
const jsmntok_t *t;

if (custommsgtok->type != JSMN_ARRAY)
return tal_fmt(plugin, "custommessages must be array, not '%.*s'",
json_tok_full_len(custommsgtok),
json_tok_full(buffer, custommsgtok));
plugin->custom_msgs = tal_arr(plugin, u16, custommsgtok->size);
json_for_each_arr(i, t, custommsgtok) {
if (!json_to_u16(buffer, t, &plugin->custom_msgs[i]))
return tal_fmt(plugin, "custommessages %zu not a u16: '%.*s'",
i,
json_tok_full_len(t),
json_tok_full(buffer, t));
}
}

tok = json_get_member(buffer, resulttok, "nonnumericids");
if (tok) {
if (!json_to_bool(buffer, tok, &plugin->non_numeric_ids))
Expand Down Expand Up @@ -1929,6 +1981,8 @@ static void plugin_config_cb(const char *buffer,
plugin_cmd_succeeded(plugin->start_cmd, plugin);
plugin->start_cmd = NULL;
}
if (tal_count(plugin->custom_msgs))
tell_connectd_custommsgs(plugin->plugins);
check_plugins_initted(plugin->plugins);
}

Expand Down
3 changes: 3 additions & 0 deletions lightningd/plugin.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ struct plugin {
/* Notification topics that this plugin has registered with us
* and that other plugins may subscribe to. */
const char **notification_topics;

/* Custom message types we want to allow incoming */
u16 *custom_msgs;
};

/**
Expand Down
13 changes: 13 additions & 0 deletions tests/plugins/allow_even_msgs.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/usr/bin/env python3
from pyln.client import Plugin

plugin = Plugin(custom_msgs=[43690])


@plugin.hook('custommsg')
def on_custommsg(peer_id, payload, plugin, message=None, **kwargs):
plugin.log("Got message {}".format(int(payload[:4], 16)))
return {'result': 'continue'}


plugin.run()
Loading
Loading