Skip to content

Commit

Permalink
status: split off error messages into a new 'peer_status' type.
Browse files Browse the repository at this point in the history
Several daemons (onchaind, hsm) want to use the status messages, but
don't communicate with peers.  The coming changes made them drag in
more code they didn't need, so instead we have a different
non-overlapping type.

We combine the status_received_errmsg and status_sent_errmsg
into a single status_peer_error, with the presence or not of the
'error_for_them' field indicating direction. 

We also rename status_fatal_connection_lost() to
peer_failed_connection_lost() to fit in.

Signed-off-by: Rusty Russell <[email protected]>
  • Loading branch information
rustyrussell committed Feb 19, 2018
1 parent 65ff5f8 commit f76ff90
Show file tree
Hide file tree
Showing 17 changed files with 156 additions and 145 deletions.
1 change: 1 addition & 0 deletions channeld/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ CHANNELD_COMMON_OBJS := \
common/derive_basepoints.o \
common/dev_disconnect.o \
common/gen_status_wire.o \
common/gen_peer_status_wire.o \
common/htlc_state.o \
common/htlc_tx.o \
common/htlc_wire.o \
Expand Down
11 changes: 6 additions & 5 deletions channeld/channel.c
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ static void do_peer_write(struct peer *peer)
r = write(PEER_FD, peer->peer_outmsg + peer->peer_outoff,
len - peer->peer_outoff);
if (r < 0)
status_fatal_connection_lost();
peer_failed_connection_lost();

peer->peer_outoff += r;
if (peer->peer_outoff == len)
Expand Down Expand Up @@ -1640,7 +1640,7 @@ static void peer_conn_broken(struct peer *peer)

wire_sync_write(GOSSIP_FD, take(cupdate));
}
status_fatal_connection_lost();
peer_failed_connection_lost();
}

static void resend_revoke(struct peer *peer)
Expand Down Expand Up @@ -1738,7 +1738,7 @@ static void resend_commitment(struct peer *peer, const struct changed_htlc *last
}

/* Our local wrapper around read_peer_msg */
static void channeld_io_error(const char *what_i_was_doing, struct peer *peer)
static void channeld_io_error(struct peer *peer)
{
peer_conn_broken(peer);
}
Expand All @@ -1754,7 +1754,8 @@ static bool channeld_send_reply(struct crypto_state *cs,

static u8 *channeld_read_peer_msg(struct peer *peer)
{
return read_peer_msg(peer, &peer->cs, &peer->channel_id,
return read_peer_msg(peer, &peer->cs, peer->gossip_index,
&peer->channel_id,
channeld_send_reply,
channeld_io_error,
status_fail_errpkt,
Expand Down Expand Up @@ -1788,7 +1789,7 @@ static void peer_reconnect(struct peer *peer)
peer->next_index[LOCAL],
peer->revocations_received);
if (!sync_crypto_write(&peer->cs, PEER_FD, take(msg)))
status_fatal_connection_lost();
peer_failed_connection_lost();

/* Read until they say something interesting */
while ((msg = channeld_read_peer_msg(peer)) == NULL);
Expand Down
1 change: 1 addition & 0 deletions closingd/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ CLOSINGD_COMMON_OBJS := \
common/daemon_conn.o \
common/dev_disconnect.o \
common/derive_basepoints.o \
common/gen_peer_status_wire.o \
common/gen_status_wire.o \
common/htlc_wire.o \
common/memleak.o \
Expand Down
12 changes: 6 additions & 6 deletions closingd/closing.c
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,12 @@ static struct bitcoin_tx *close_tx(const tal_t *ctx,
/* Handle random messages we might get, returning the first non-handled one. */
static u8 *closing_read_peer_msg(const tal_t *ctx,
struct crypto_state *cs,
u64 gossip_index,
const struct channel_id *channel)
{
u8 *msg;

while ((msg = read_peer_msg(ctx, cs, channel,
while ((msg = read_peer_msg(ctx, cs, gossip_index, channel,
sync_crypto_write_arg,
status_fail_io,
status_fail_errpkt,
Expand Down Expand Up @@ -118,11 +119,10 @@ static void do_reconnect(struct crypto_state *cs,
next_index[LOCAL],
revocations_received);
if (!sync_crypto_write(cs, PEER_FD, take(msg)))
status_fatal_connection_lost();
;
peer_failed_connection_lost();

/* Wait for them to say something interesting */
msg = closing_read_peer_msg(tmpctx, cs, channel_id);
msg = closing_read_peer_msg(tmpctx, cs, gossip_index, channel_id);

if (!fromwire_channel_reestablish(msg, NULL, &their_channel_id,
&next_local_commitment_number,
Expand Down Expand Up @@ -202,7 +202,7 @@ static void send_offer(struct crypto_state *cs,

msg = towire_closing_signed(tmpctx, channel_id, fee_to_offer, &our_sig);
if (!sync_crypto_write(cs, PEER_FD, take(msg)))
status_fatal_connection_lost();
peer_failed_connection_lost();

tal_free(tmpctx);
}
Expand Down Expand Up @@ -248,7 +248,7 @@ static uint64_t receive_offer(struct crypto_state *cs,

/* Wait for them to say something interesting */
do {
msg = closing_read_peer_msg(tmpctx, cs, channel_id);
msg = closing_read_peer_msg(tmpctx, cs, gossip_index, channel_id);

/* BOLT #2:
*
Expand Down
10 changes: 8 additions & 2 deletions common/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ COMMON_SRC_NOGEN := \
common/wire_error.c \
common/withdraw_tx.c

COMMON_SRC_GEN := common/gen_status_wire.c
COMMON_SRC_GEN := common/gen_status_wire.c common/gen_peer_status_wire.c

COMMON_HEADERS_NOGEN := $(COMMON_SRC_NOGEN:.c=.h) common/overflows.h common/htlc.h common/status_levels.h
COMMON_HEADERS_GEN := common/gen_htlc_state_names.h common/gen_status_wire.h
COMMON_HEADERS_GEN := common/gen_htlc_state_names.h common/gen_status_wire.h common/gen_peer_status_wire.h

COMMON_HEADERS := $(COMMON_HEADERS_GEN) $(COMMON_HEADERS_NOGEN)
COMMON_SRC := $(COMMON_SRC_NOGEN) $(COMMON_SRC_GEN)
Expand All @@ -68,6 +68,12 @@ common/gen_status_wire.h: $(WIRE_GEN) common/status_wire.csv
common/gen_status_wire.c: $(WIRE_GEN) common/status_wire.csv
$(WIRE_GEN) ${@:.c=.h} status < common/status_wire.csv > $@

common/gen_peer_status_wire.h: $(WIRE_GEN) common/peer_status_wire.csv
$(WIRE_GEN) --header $@ peer_status < common/peer_status_wire.csv > $@

common/gen_peer_status_wire.c: $(WIRE_GEN) common/peer_status_wire.csv
$(WIRE_GEN) ${@:.c=.h} peer_status < common/peer_status_wire.csv > $@

check-makefile: check-common-makefile

check-common-makefile:
Expand Down
24 changes: 22 additions & 2 deletions common/peer_failed.c
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
#include <ccan/io/io.h>
#include <ccan/tal/str/str.h>
#include <common/crypto_sync.h>
#include <common/gen_peer_status_wire.h>
#include <common/gen_status_wire.h>
#include <common/peer_failed.h>
#include <common/status.h>
#include <common/wire_error.h>
#include <stdarg.h>
#include <unistd.h>
#include <wire/gen_peer_wire.h>

/* We only support one channel per peer anyway */
void peer_failed_(int peer_fd, int gossip_fd,
Expand All @@ -29,5 +30,24 @@ void peer_failed_(int peer_fd, int gossip_fd,
io_fd_block(peer_fd, false);
sync_crypto_write(cs, peer_fd, msg);

status_fatal_sent_errmsg(take(msg), desc, channel_id);
msg = towire_status_peer_error(NULL, channel_id,
desc, cs, gossip_index, msg);
tal_free(desc);
status_send_fatal(take(msg));
}

/* We're failing because peer sent us an error message */
void peer_failed_received_errmsg(int peer_fd, int gossip_fd,
struct crypto_state *cs, u64 gossip_index,
const char *desc,
const struct channel_id *channel_id)
{
u8 *msg = towire_status_peer_error(NULL, channel_id,
desc, cs, gossip_index, NULL);
status_send_fatal(take(msg));
}

void peer_failed_connection_lost(void)
{
status_send_fatal(take(towire_status_peer_connection_lost(NULL)));
}
10 changes: 10 additions & 0 deletions common/peer_failed.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,14 @@ void peer_failed_(int peer_fd, int gossip_fd,
const struct channel_id *channel_id,
const char *fmt, ...)
PRINTF_FMT(6,7) NORETURN;

/* We're failing because peer sent us an error message */
void peer_failed_received_errmsg(int peer_fd, int gossip_fd,
struct crypto_state *cs, u64 gossip_index,
const char *desc,
const struct channel_id *channel_id) NORETURN;

/* I/O error */
void peer_failed_connection_lost(void) NORETURN;

#endif /* LIGHTNING_COMMON_PEER_FAILED_H */
11 changes: 11 additions & 0 deletions common/peer_status_wire.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#include <common/crypto_state.h>

# An error occurred: if error_for_them, that to go to them.
status_peer_error,0xFFF4
# This is implied if error_for_them, but master tries not to parse packets.
status_peer_error,,channel,struct channel_id
status_peer_error,,desc,wirestring
status_peer_error,,crypto_state,struct crypto_state
status_peer_error,,gossip_index,u64
status_peer_error,,len,u16
status_peer_error,,error_for_them,len*u8
39 changes: 24 additions & 15 deletions common/read_peer_msg.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include <ccan/structeq/structeq.h>
#include <common/crypto_sync.h>
#include <common/peer_failed.h>
#include <common/ping.h>
#include <common/read_peer_msg.h>
#include <common/status.h>
Expand All @@ -16,7 +17,7 @@ static void handle_ping(const u8 *msg,
const struct channel_id *channel,
bool (*send_reply)(struct crypto_state *, int,
const u8 *, void *),
void (*io_error)(const char *, void *),
void (*io_error)(void *),
void *arg)
{
u8 *pong;
Expand All @@ -26,25 +27,28 @@ static void handle_ping(const u8 *msg,
take(towire_errorfmt(msg, channel,
"Bad ping %s",
tal_hex(msg, msg))), arg);
io_error("Bad ping received", arg);
io_error(arg);
}

status_debug("Got ping, sending %s", pong ?
wire_type_name(fromwire_peektype(pong))
: "nothing");

if (pong && !send_reply(cs, peer_fd, pong, arg))
io_error("Failed writing pong", arg);
io_error(arg);
}

u8 *read_peer_msg_(const tal_t *ctx,
int peer_fd, int gossip_fd,
struct crypto_state *cs,
struct crypto_state *cs, u64 gossip_index,
const struct channel_id *channel,
bool (*send_reply)(struct crypto_state *cs, int fd,
const u8 *TAKES, void *arg),
void (*io_error)(const char *what_i_was_doing, void *arg),
void (*err_pkt)(const char *desc, const struct channel_id *,
void (*io_error)(void *arg),
void (*err_pkt)(int peer_fd, int gossip_fd,
struct crypto_state *cs, u64 gossip_index,
const char *desc,
const struct channel_id *channel_id,
void *arg),
void *arg)
{
Expand All @@ -53,7 +57,7 @@ u8 *read_peer_msg_(const tal_t *ctx,

msg = sync_crypto_read(ctx, cs, peer_fd);
if (!msg)
io_error("reading from peer", arg);
io_error(arg);

if (is_gossip_msg(msg)) {
/* Forward to gossip daemon */
Expand Down Expand Up @@ -86,7 +90,8 @@ u8 *read_peer_msg_(const tal_t *ctx,
* - MUST ignore the message.
*/
if (structeq(&chanid, channel) || channel_id_is_all(&chanid))
err_pkt(err, &chanid, arg);
err_pkt(peer_fd, gossip_fd, cs, gossip_index,
err, &chanid, arg);

return tal_free(msg);
}
Expand All @@ -102,7 +107,7 @@ u8 *read_peer_msg_(const tal_t *ctx,
"Multiple channels"
" unsupported")),
arg))
io_error("Sending error for other channel ", arg);
io_error(arg);
return tal_free(msg);
}

Expand All @@ -116,15 +121,19 @@ bool sync_crypto_write_arg(struct crypto_state *cs, int fd, const u8 *msg,
return sync_crypto_write(cs, fd, msg);
}

/* Helper: calls status_fatal_connection_lost. */
void status_fail_io(const char *what_i_was_doing, void *unused)
/* Helper: calls peer_failed_connection_lost. */
void status_fail_io(void *unused)
{
status_fatal_connection_lost();
peer_failed_connection_lost();
}

/* Helper: calls status_fatal_received_errmsg() */
void status_fail_errpkt(const char *desc, const struct channel_id *c,
/* Helper: calls peer_failed_received_errmsg() */
void status_fail_errpkt(int peer_fd, int gossip_fd,
struct crypto_state *cs, u64 gossip_index,
const char *desc,
const struct channel_id *channel_id,
void *unused)
{
status_fatal_received_errmsg(desc, c);
peer_failed_received_errmsg(peer_fd, gossip_fd,
cs, gossip_index, desc, channel_id);
}
35 changes: 22 additions & 13 deletions common/read_peer_msg.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ struct channel_id;
* read_peer_msg - read & decode in a peer message, handling common ones.
* @ctx: context to allocate return packet from.
* @cs: the cryptostate (updated)
* @gossip_index: the gossip_index
* @chanid: the channel id (for identifying errors)
* @send_reply: the way to send a reply packet (eg. sync_crypto_write_arg)
* @io_error: what to do if there's an IO error (eg. status_fail_io)
Expand All @@ -22,38 +23,46 @@ struct channel_id;
* This returns NULL if it handled the message, so it's normally called in
* a loop.
*/
#define read_peer_msg(ctx, cs, chanid, send_reply, io_error, err_pkt, arg) \
read_peer_msg_((ctx), PEER_FD, GOSSIP_FD, (cs), (chanid), \
#define read_peer_msg(ctx, cs, gossip_index, chanid, send_reply, \
io_error, err_pkt, arg) \
read_peer_msg_((ctx), PEER_FD, GOSSIP_FD, (cs), (gossip_index), \
(chanid), \
typesafe_cb_preargs(bool, void *, (send_reply), (arg), \
struct crypto_state *, int, \
const u8 *), \
typesafe_cb_preargs(void, void *, (io_error), (arg), \
const char *), \
typesafe_cb(void, void *, (io_error), (arg)), \
typesafe_cb_preargs(void, void *, (err_pkt), (arg), \
const char *, \
int, int, \
struct crypto_state *, \
u64, const char *, \
const struct channel_id *), \
arg)

/* Helper: sync_crypto_write, with extra args it ignores */
bool sync_crypto_write_arg(struct crypto_state *cs, int fd, const u8 *TAKES,
void *unused);

/* Helper: calls status_fatal_connection_lost. */
/* FIXME: Remove what_i_was_doing arg */
void status_fail_io(const char *what_i_was_doing, void *unused);
/* Helper: calls peer_failed_connection_lost. */
void status_fail_io(void *unused);

/* Helper: calls status_fatal_received_errmsg() */
void status_fail_errpkt(const char *desc, const struct channel_id *c,
/* Helper: calls peer_failed_received_errmsg() */
void status_fail_errpkt(int peer_fd, int gossip_fd,
struct crypto_state *cs, u64 gossip_index,
const char *desc,
const struct channel_id *channel_id,
void *unused);

u8 *read_peer_msg_(const tal_t *ctx,
int peer_fd, int gossip_fd,
struct crypto_state *cs,
struct crypto_state *cs, u64 gossip_index,
const struct channel_id *channel,
bool (*send_reply)(struct crypto_state *cs, int fd,
const u8 *TAKES, void *arg),
void (*io_error)(const char *what_i_was_doing, void *arg),
void (*err_pkt)(const char *desc, const struct channel_id *,
void (*io_error)(void *arg),
void (*err_pkt)(int peer_fd, int gossip_fd,
struct crypto_state *cs, u64 gossip_index,
const char *desc,
const struct channel_id *channel_id,
void *arg),
void *arg);

Expand Down
Loading

0 comments on commit f76ff90

Please sign in to comment.