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

Offers preparation #3685

Merged
merged 9 commits into from
May 6, 2020
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ endif

ifeq ($(COMPAT),1)
# We support compatibility with pre-0.6.
COMPAT_CFLAGS=-DCOMPAT_V052=1 -DCOMPAT_V060=1 -DCOMPAT_V061=1 -DCOMPAT_V062=1 -DCOMPAT_V070=1 -DCOMPAT_V072=1 -DCOMPAT_V073=1 -DCOMPAT_V080=1 -DCOMPAT_V081=1
COMPAT_CFLAGS=-DCOMPAT_V052=1 -DCOMPAT_V060=1 -DCOMPAT_V061=1 -DCOMPAT_V062=1 -DCOMPAT_V070=1 -DCOMPAT_V072=1 -DCOMPAT_V073=1 -DCOMPAT_V080=1 -DCOMPAT_V081=1 -DCOMPAT_V082=1
endif

# Timeout shortly before the 600 second travis silence timeout
Expand Down
6 changes: 5 additions & 1 deletion channeld/channeld.c
Original file line number Diff line number Diff line change
Expand Up @@ -349,12 +349,16 @@ static const u8 *get_local_channel_update(const tal_t *ctx, struct peer *peer)
static void make_channel_local_active(struct peer *peer)
{
u8 *msg;
const u8 *annfeatures = get_agreed_channelfeatures(tmpctx,
peer->our_features,
peer->their_features);

/* Tell gossipd about local channel. */
msg = towire_gossipd_local_add_channel(NULL,
&peer->short_channel_ids[LOCAL],
&peer->node_ids[REMOTE],
peer->channel->funding);
peer->channel->funding,
annfeatures);
wire_sync_write(peer->pps->gossip_fd, take(msg));

/* Tell gossipd and the other side what parameters we expect should
Expand Down
2 changes: 1 addition & 1 deletion common/gossip_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ struct per_peer_state;
/**
* gossip_store -- On-disk storage related information
*/
#define GOSSIP_STORE_VERSION 7
#define GOSSIP_STORE_VERSION 8

/**
* Bit of length we use to mark a deleted record.
Expand Down
2 changes: 2 additions & 0 deletions gossipd/gossip_peerd_wire.csv
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ msgtype,gossipd_local_add_channel,3503
msgdata,gossipd_local_add_channel,short_channel_id,short_channel_id,
msgdata,gossipd_local_add_channel,remote_node_id,node_id,
msgdata,gossipd_local_add_channel,satoshis,amount_sat,
msgdata,gossipd_local_add_channel,flen,u16,
msgdata,gossipd_local_add_channel,features,u8,flen

# Send this channel_update.
msgtype,gossipd_local_channel_update,3504
Expand Down
49 changes: 46 additions & 3 deletions gossipd/gossip_store.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,44 @@ static bool append_msg(int fd, const u8 *msg, u32 timestamp,
return true;
}

#ifdef COMPAT_V082
/* The upgrade from version 7 is trivial */
static bool can_upgrade(u8 oldversion)
{
return oldversion == 7;
}

static bool upgrade_field(u8 oldversion, u8 **msg)
{
assert(can_upgrade(oldversion));

/* We only need to upgrade this */
if (fromwire_peektype(*msg) == WIRE_GOSSIPD_LOCAL_ADD_CHANNEL) {
/* Append two 0 bytes, for (empty) feature bits */
tal_resizez(msg, tal_bytelen(*msg) + 2);
}
return true;
}
Comment on lines +126 to +136
Copy link
Member

Choose a reason for hiding this comment

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

Patching these on the fly seems a bit strange to me: couldn't we have optional fields at the end, maybe even make the switch to TLV encode the local_add_channel instead? That'd allow us to add/remove optional fields without bumping the gossip_store version later. It'd be better future proofing imho.

Copy link
Contributor Author

@rustyrussell rustyrussell May 5, 2020

Choose a reason for hiding this comment

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

Well, these (internal) messages are unusual: usually we don't need to worry about compatibility at all. I only did this because it was easy: I don't want all the callers to have to worry about missing fields.

I'm not committing to preserving the gossip_store in future revs!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, BTW, these are patched during our "offline compaction" run which always runs at startup and rewrites the files.

So they're not converted every time!

#else
static bool can_upgrade(u8 oldversion)
{
return false;
}

static bool upgrade_field(u8 oldversion, u8 **msg)
{
abort();
}
#endif /* !COMPAT_V082 */

/* Read gossip store entries, copy non-deleted ones. This code is written
* as simply and robustly as possible! */
static u32 gossip_store_compact_offline(void)
{
size_t count = 0, deleted = 0;
int old_fd, new_fd;
struct gossip_hdr hdr;
u8 version;
u8 oldversion, version = GOSSIP_STORE_VERSION;
struct stat st;

old_fd = open(GOSSIP_STORE_FILENAME, O_RDONLY);
Expand All @@ -143,8 +173,8 @@ static u32 gossip_store_compact_offline(void)
goto close_old;
}

if (!read_all(old_fd, &version, sizeof(version))
|| version != GOSSIP_STORE_VERSION) {
if (!read_all(old_fd, &oldversion, sizeof(oldversion))
|| (oldversion != version && !can_upgrade(oldversion))) {
status_broken("gossip_store_compact: bad version");
goto close_and_delete;
}
Expand Down Expand Up @@ -175,6 +205,19 @@ static u32 gossip_store_compact_offline(void)
continue;
}

if (oldversion != version) {
if (!upgrade_field(oldversion, &msg)) {
tal_free(msg);
goto close_and_delete;
}

/* Recalc msglen and header */
msglen = tal_bytelen(msg);
hdr.len = cpu_to_be32(msglen);
hdr.crc = cpu_to_be32(crc32c(be32_to_cpu(hdr.timestamp),
msg, msglen));
}

if (!write_all(new_fd, &hdr, sizeof(hdr))
|| !write_all(new_fd, msg, msglen)) {
status_broken("gossip_store_compact_offline: writing msg len %zu to new store: %s",
Expand Down
36 changes: 36 additions & 0 deletions gossipd/gossipd.c
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,41 @@ static struct gossip_halfchannel_entry *hc_entry(const tal_t *ctx,
return e;
}

/*~ We don't keep channel features in memory; they're rarely used. So we
* remember if it exists, and load it off disk when needed. */
static u8 *get_channel_features(const tal_t *ctx,
struct gossip_store *gs,
const struct chan *chan)
{
secp256k1_ecdsa_signature sig;
u8 *features;
struct bitcoin_blkid chain_hash;
struct short_channel_id short_channel_id;
struct node_id node_id;
struct pubkey bitcoin_key;
struct amount_sat sats;
const u8 *ann;

/* This is where we stash a flag to indicate it exists. */
if (!chan->half[0].any_features)
return NULL;

/* Could be a channel_announcement, could be a local_add_channel */
ann = gossip_store_get(tmpctx, gs, chan->bcast.index);
if (!fromwire_channel_announcement(ctx, ann, &sig, &sig, &sig, &sig,
&features, &chain_hash,
&short_channel_id,
&node_id, &node_id,
&bitcoin_key, &bitcoin_key)
&& !fromwire_gossipd_local_add_channel(ctx, ann, &short_channel_id,
&node_id, &sats, &features))
status_failed(STATUS_FAIL_INTERNAL_ERROR,
"bad channel_announcement / local_add_channel at %u: %s",
chan->bcast.index, tal_hex(tmpctx, ann));

return features;
}

/*~ Marshal (possibly) both channel directions into entries. */
static void append_channel(struct routing_state *rstate,
const struct gossip_getchannels_entry ***entries,
Expand All @@ -980,6 +1015,7 @@ static void append_channel(struct routing_state *rstate,
e->local_disabled = is_chan_local_disabled(rstate, chan);
e->public = is_chan_public(chan);
e->short_channel_id = chan->scid;
e->features = get_channel_features(e, rstate->gs, chan);
if (!srcfilter || node_id_eq(&e->node[0], srcfilter))
e->e[0] = hc_entry(*entries, chan, 0);
else
Expand Down
18 changes: 13 additions & 5 deletions gossipd/routing.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ HTABLE_DEFINE_TYPE(struct pending_node_announce, pending_node_announce_keyof,
struct unupdated_channel {
/* The channel_announcement message */
const u8 *channel_announce;
/* The feature bitmap within it */
const u8 *features;
/* The short_channel_id */
struct short_channel_id scid;
/* The ids of the nodes */
Expand Down Expand Up @@ -571,7 +573,8 @@ struct chan *new_chan(struct routing_state *rstate,
const struct short_channel_id *scid,
const struct node_id *id1,
const struct node_id *id2,
struct amount_sat satoshis)
struct amount_sat satoshis,
const u8 *features)
{
struct chan *chan = tal(rstate, struct chan);
int n1idx = node_id_idx(id1, id2);
Expand Down Expand Up @@ -606,6 +609,8 @@ struct chan *new_chan(struct routing_state *rstate,
init_half_chan(rstate, chan, n1idx);
init_half_chan(rstate, chan, !n1idx);

/* Stash hint here about whether we have features */
chan->half[0].any_features = tal_bytelen(features) != 0;
uintmap_add(&rstate->chanmap, scid->u64, chan);

/* Initialize shadow structure if it's local */
Expand Down Expand Up @@ -1651,6 +1656,7 @@ bool routing_add_channel_announcement(struct routing_state *rstate,

uc = tal(rstate, struct unupdated_channel);
uc->channel_announce = tal_dup_talarr(uc, u8, msg);
uc->features = tal_steal(uc, features);
uc->added = gossip_time_now(rstate);
uc->index = index;
uc->sat = sat;
Expand Down Expand Up @@ -2098,7 +2104,7 @@ bool routing_add_channel_update(struct routing_state *rstate,
if (uc) {
assert(!chan);
chan = new_chan(rstate, &short_channel_id,
&uc->id[0], &uc->id[1], sat);
&uc->id[0], &uc->id[1], sat, uc->features);
}

/* Discard older updates */
Expand Down Expand Up @@ -2904,9 +2910,10 @@ bool handle_local_add_channel(struct routing_state *rstate,
struct node_id remote_node_id;
struct amount_sat sat;
struct chan *chan;
u8 *features;

if (!fromwire_gossipd_local_add_channel(msg, &scid, &remote_node_id,
&sat)) {
if (!fromwire_gossipd_local_add_channel(msg, msg, &scid, &remote_node_id,
&sat, &features)) {
status_peer_broken(peer ? &peer->id : NULL,
"Unable to parse local_add_channel message: %s",
tal_hex(msg, msg));
Expand All @@ -2925,7 +2932,8 @@ bool handle_local_add_channel(struct routing_state *rstate,
type_to_string(tmpctx, struct short_channel_id, &scid));

/* Create new (unannounced) channel */
chan = new_chan(rstate, &scid, &rstate->local_id, &remote_node_id, sat);
chan = new_chan(rstate, &scid, &rstate->local_id, &remote_node_id, sat,
features);
if (!index)
index = gossip_store_add(rstate->gs, msg, 0, false, NULL);
chan->bcast.index = index;
Expand Down
7 changes: 6 additions & 1 deletion gossipd/routing.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ struct half_chan {
/* Token bucket */
u8 tokens;

/* Feature cache for parent chan: squeezed in here where it would
* otherwise simply be padding. */
u8 any_features;

/* Minimum and maximum number of msatoshi in an HTLC */
struct amount_msat htlc_minimum, htlc_maximum;
};
Expand Down Expand Up @@ -361,7 +365,8 @@ struct chan *new_chan(struct routing_state *rstate,
const struct short_channel_id *scid,
const struct node_id *id1,
const struct node_id *id2,
struct amount_sat sat);
struct amount_sat sat,
const u8 *features);

/* Handlers for incoming messages */

Expand Down
4 changes: 2 additions & 2 deletions gossipd/test/run-bench-find_route.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ bool fromwire_gossip_store_channel_amount(const void *p UNNEEDED, struct amount_
bool fromwire_gossip_store_private_update(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u8 **update UNNEEDED)
{ fprintf(stderr, "fromwire_gossip_store_private_update called!\n"); abort(); }
/* Generated stub for fromwire_gossipd_local_add_channel */
bool fromwire_gossipd_local_add_channel(const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct node_id *remote_node_id UNNEEDED, struct amount_sat *satoshis UNNEEDED)
bool fromwire_gossipd_local_add_channel(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct node_id *remote_node_id UNNEEDED, struct amount_sat *satoshis UNNEEDED, u8 **features UNNEEDED)
{ fprintf(stderr, "fromwire_gossipd_local_add_channel called!\n"); abort(); }
/* Generated stub for fromwire_wireaddr */
bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct wireaddr *addr UNNEEDED)
Expand Down Expand Up @@ -149,7 +149,7 @@ static void add_connection(struct routing_state *rstate,
chan = get_channel(rstate, &scid);
if (!chan) {
chan = new_chan(rstate, &scid, &nodes[from], &nodes[to],
AMOUNT_SAT(1000000));
AMOUNT_SAT(1000000), NULL);
}

c = &chan->half[idx];
Expand Down
4 changes: 2 additions & 2 deletions gossipd/test/run-find_route-specific.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ bool fromwire_gossip_store_channel_amount(const void *p UNNEEDED, struct amount_
bool fromwire_gossip_store_private_update(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u8 **update UNNEEDED)
{ fprintf(stderr, "fromwire_gossip_store_private_update called!\n"); abort(); }
/* Generated stub for fromwire_gossipd_local_add_channel */
bool fromwire_gossipd_local_add_channel(const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct node_id *remote_node_id UNNEEDED, struct amount_sat *satoshis UNNEEDED)
bool fromwire_gossipd_local_add_channel(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct node_id *remote_node_id UNNEEDED, struct amount_sat *satoshis UNNEEDED, u8 **features UNNEEDED)
{ fprintf(stderr, "fromwire_gossipd_local_add_channel called!\n"); abort(); }
/* Generated stub for fromwire_wireaddr */
bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct wireaddr *addr UNNEEDED)
Expand Down Expand Up @@ -134,7 +134,7 @@ get_or_make_connection(struct routing_state *rstate,
abort();
chan = get_channel(rstate, &scid);
if (!chan)
chan = new_chan(rstate, &scid, from_id, to_id, satoshis);
chan = new_chan(rstate, &scid, from_id, to_id, satoshis, NULL);

/* Make sure it's seen as initialized (index non-zero). */
chan->half[idx].bcast.index = 1;
Expand Down
4 changes: 2 additions & 2 deletions gossipd/test/run-find_route.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ bool fromwire_gossip_store_channel_amount(const void *p UNNEEDED, struct amount_
bool fromwire_gossip_store_private_update(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u8 **update UNNEEDED)
{ fprintf(stderr, "fromwire_gossip_store_private_update called!\n"); abort(); }
/* Generated stub for fromwire_gossipd_local_add_channel */
bool fromwire_gossipd_local_add_channel(const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct node_id *remote_node_id UNNEEDED, struct amount_sat *satoshis UNNEEDED)
bool fromwire_gossipd_local_add_channel(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct node_id *remote_node_id UNNEEDED, struct amount_sat *satoshis UNNEEDED, u8 **features UNNEEDED)
{ fprintf(stderr, "fromwire_gossipd_local_add_channel called!\n"); abort(); }
/* Generated stub for fromwire_wireaddr */
bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct wireaddr *addr UNNEEDED)
Expand Down Expand Up @@ -142,7 +142,7 @@ static void add_connection(struct routing_state *rstate,

chan = get_channel(rstate, &scid);
if (!chan)
chan = new_chan(rstate, &scid, from, to, satoshis);
chan = new_chan(rstate, &scid, from, to, satoshis, NULL);

c = &chan->half[node_id_idx(from, to)];
/* Make sure it's seen as initialized (index non-zero). */
Expand Down
6 changes: 3 additions & 3 deletions gossipd/test/run-overlong.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ bool fromwire_gossip_store_channel_amount(const void *p UNNEEDED, struct amount_
bool fromwire_gossip_store_private_update(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u8 **update UNNEEDED)
{ fprintf(stderr, "fromwire_gossip_store_private_update called!\n"); abort(); }
/* Generated stub for fromwire_gossipd_local_add_channel */
bool fromwire_gossipd_local_add_channel(const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct node_id *remote_node_id UNNEEDED, struct amount_sat *satoshis UNNEEDED)
bool fromwire_gossipd_local_add_channel(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct node_id *remote_node_id UNNEEDED, struct amount_sat *satoshis UNNEEDED, u8 **features UNNEEDED)
{ fprintf(stderr, "fromwire_gossipd_local_add_channel called!\n"); abort(); }
/* Generated stub for fromwire_wireaddr */
bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct wireaddr *addr UNNEEDED)
Expand Down Expand Up @@ -163,7 +163,7 @@ int main(void)
if (!mk_short_channel_id(&scid, i, i-1, 0))
abort();
chan = new_chan(rstate, &scid, &ids[i], &ids[i-1],
AMOUNT_SAT(1000000));
AMOUNT_SAT(1000000), NULL);

hc = &chan->half[node_id_idx(&ids[i-1], &ids[i])];
hc->bcast.index = 1;
Expand All @@ -183,7 +183,7 @@ int main(void)
if (!mk_short_channel_id(&scid, i, 1, 0))
abort();
chan = new_chan(rstate, &scid, &ids[i], &ids[1],
AMOUNT_SAT(1000000));
AMOUNT_SAT(1000000), NULL);
hc = &chan->half[node_id_idx(&ids[1], &ids[i])];
hc->bcast.index = 1;
hc->base_fee = 1 << i;
Expand Down
2 changes: 1 addition & 1 deletion gossipd/test/run-txout_failure.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ bool cupdate_different(struct gossip_store *gs UNNEEDED,
char *fmt_wireaddr_without_port(const tal_t *ctx UNNEEDED, const struct wireaddr *a UNNEEDED)
{ fprintf(stderr, "fmt_wireaddr_without_port called!\n"); abort(); }
/* Generated stub for fromwire_gossipd_local_add_channel */
bool fromwire_gossipd_local_add_channel(const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct node_id *remote_node_id UNNEEDED, struct amount_sat *satoshis UNNEEDED)
bool fromwire_gossipd_local_add_channel(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, struct node_id *remote_node_id UNNEEDED, struct amount_sat *satoshis UNNEEDED, u8 **features UNNEEDED)
{ fprintf(stderr, "fromwire_gossipd_local_add_channel called!\n"); abort(); }
/* Generated stub for fromwire_wireaddr */
bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct wireaddr *addr UNNEEDED)
Expand Down
1 change: 1 addition & 0 deletions lightningd/gossip_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ static void json_add_halfchan(struct json_stream *response,
json_add_num(response, "delay", he->delay);
json_add_amount_msat_only(response, "htlc_minimum_msat", he->min);
json_add_amount_msat_only(response, "htlc_maximum_msat", he->max);
json_add_hex_talarr(response, "features", e->features);
json_object_end(response);
}

Expand Down
4 changes: 4 additions & 0 deletions lightningd/gossip_msg.c
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ fromwire_gossip_getchannels_entry(const tal_t *ctx,
fromwire_short_channel_id(pptr, max, &entry->short_channel_id);
entry->public = fromwire_bool(pptr, max);
entry->local_disabled = fromwire_bool(pptr, max);
entry->features = fromwire_tal_arrn(entry,
pptr, max, fromwire_u16(pptr, max));

if (fromwire_bool(pptr, max)) {
entry->e[0] = tal(entry, struct gossip_halfchannel_entry);
Expand Down Expand Up @@ -180,6 +182,8 @@ void towire_gossip_getchannels_entry(u8 **pptr,
towire_short_channel_id(pptr, &entry->short_channel_id);
towire_bool(pptr, entry->public);
towire_bool(pptr, entry->local_disabled);
towire_u16(pptr, tal_bytelen(entry->features));
towire_u8_array(pptr, entry->features, tal_bytelen(entry->features));
if (entry->e[0]) {
towire_bool(pptr, true);
towire_gossip_halfchannel_entry(pptr, entry->e[0]);
Expand Down
1 change: 1 addition & 0 deletions lightningd/gossip_msg.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ struct gossip_getchannels_entry {
bool local_disabled;
/* NULL if we haven't received an update */
struct gossip_halfchannel_entry *e[2];
u8 *features;
};

struct gossip_getnodes_entry *
Expand Down
Loading