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
5 changes: 3 additions & 2 deletions gossipd/routing.c
Original file line number Diff line number Diff line change
Expand Up @@ -2904,9 +2904,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 Down
2 changes: 1 addition & 1 deletion 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
2 changes: 1 addition & 1 deletion 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
2 changes: 1 addition & 1 deletion 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
2 changes: 1 addition & 1 deletion 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
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
85 changes: 77 additions & 8 deletions tests/test_gossip.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
from ephemeral_port_reserve import reserve
from fixtures import * # noqa: F401,F403
from fixtures import TEST_NETWORK
from pyln.client import RpcError
from pyln.client import RpcError, Millisatoshi
from utils import (
wait_for, TIMEOUT, only_one, sync_blockheight, expected_node_features
wait_for, TIMEOUT, only_one, sync_blockheight, expected_node_features, COMPAT
)

import json
Expand Down Expand Up @@ -920,7 +920,7 @@ def test_gossip_store_load(node_factory):
"""Make sure we can read canned gossip store"""
l1 = node_factory.get_node(start=False)
with open(os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, 'gossip_store'), 'wb') as f:
f.write(bytearray.fromhex("07" # GOSSIP_STORE_VERSION
f.write(bytearray.fromhex("08" # GOSSIP_STORE_VERSION
"000001b0" # len
"fea676e8" # csum
"5b8d9b44" # timestamp
Expand Down Expand Up @@ -952,7 +952,7 @@ def test_gossip_store_load_announce_before_update(node_factory):
"""Make sure we can read canned gossip store with node_announce before update. This happens when a channel_update gets replaced, leaving node_announce before it"""
l1 = node_factory.get_node(start=False)
with open(os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, 'gossip_store'), 'wb') as f:
f.write(bytearray.fromhex("07" # GOSSIP_STORE_VERSION
f.write(bytearray.fromhex("08" # GOSSIP_STORE_VERSION
"000001b0" # len
"fea676e8" # csum
"5b8d9b44" # timestamp
Expand Down Expand Up @@ -995,7 +995,7 @@ def test_gossip_store_load_amount_truncated(node_factory):
"""Make sure we can read canned gossip store with truncated amount"""
l1 = node_factory.get_node(start=False, allow_broken_log=True)
with open(os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, 'gossip_store'), 'wb') as f:
f.write(bytearray.fromhex("07" # GOSSIP_STORE_VERSION
f.write(bytearray.fromhex("08" # GOSSIP_STORE_VERSION
"000001b0" # len
"fea676e8" # csum
"5b8d9b44" # timestamp
Expand Down Expand Up @@ -1419,7 +1419,7 @@ def test_gossip_store_load_no_channel_update(node_factory):

# A channel announcement with no channel_update.
with open(os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, 'gossip_store'), 'wb') as f:
f.write(bytearray.fromhex("07" # GOSSIP_STORE_VERSION
f.write(bytearray.fromhex("08" # GOSSIP_STORE_VERSION
"000001b0" # len
"fea676e8" # csum
"5b8d9b44" # timestamp
Expand All @@ -1446,7 +1446,7 @@ def test_gossip_store_load_no_channel_update(node_factory):
l1.rpc.call('dev-compact-gossip-store')

with open(os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, 'gossip_store'), "rb") as f:
assert bytearray(f.read()) == bytearray.fromhex("07")
assert bytearray(f.read()) == bytearray.fromhex("08")


@unittest.skipIf(not DEVELOPER, "gossip without DEVELOPER=1 is slow")
Expand All @@ -1456,7 +1456,8 @@ def test_gossip_store_compact_on_load(node_factory, bitcoind):
l2.restart()

wait_for(lambda: l2.daemon.is_in_log(r'gossip_store_compact_offline: [5-8] deleted, 9 copied'))
wait_for(lambda: l2.daemon.is_in_log(r'gossip_store: Read 1/4/2/0 cannounce/cupdate/nannounce/cdelete from store \(0 deleted\) in 1460 bytes'))

wait_for(lambda: l2.daemon.is_in_log(r'gossip_store: Read 1/4/2/0 cannounce/cupdate/nannounce/cdelete from store \(0 deleted\) in [0-9]* bytes'))


def test_gossip_announce_invalid_block(node_factory, bitcoind):
Expand Down Expand Up @@ -1656,3 +1657,71 @@ def test_torport_onions(node_factory):

assert l1.daemon.is_in_log('45321,127.0.0.1:{}'.format(l1.port))
assert l2.daemon.is_in_log('x2y4zvh4fn5q3eouuh7nxnc7zeawrqoutljrup2xjtiyxgx3emgkemad.onion:45321,127.0.0.1:{}'.format(l2.port))


@unittest.skipIf(not COMPAT, "needs COMPAT to convert obsolete gossip_store")
def test_gossip_store_upgrade_v7_v8(node_factory):
"""Version 8 added feature bits to local channel announcements"""
l1 = node_factory.get_node(start=False)

# A channel announcement with no channel_update.
with open(os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, 'gossip_store'), 'wb') as f:
f.write(bytearray.fromhex("07000000428ce4d2d8000000000daf00"
"00670000010001022d223620a359a47f"
"f7f7ac447c85c46c923da53389221a00"
"54c11c1e3ca31d5900000000000f4240"
"000d8000000000000000000000000000"
"00008e3af3badf000000001006008a01"
"02005a9911d425effd461f803a380f05"
"e72d3332eb6e9a7c6c58405ae61eacde"
"4e2da18240ffb3d5c595f85e4f78b594"
"c59e4d01c0470edd4f5afe645026515e"
"fe06226e46111a0b59caaf126043eb5b"
"bf28c34f3a5e332a1fc7b2b73cf18891"
"0f00006700000100015eaa5eb0010100"
"06000000000000000000000001000000"
"0a000000003b0233800000008e074a6e"
"0f000000001006008a0102463de636b2"
"f46ccd6c23259787fc39dc4fdb983510"
"1651879325b18cf1bb26330127e51ce8"
"7a111b05ef92fe00a9a089979dc49178"
"200f49139a541e7078cdc506226e4611"
"1a0b59caaf126043eb5bbf28c34f3a5e"
"332a1fc7b2b73cf188910f0000670000"
"0100015eaa5eb0010000060000000000"
"000000000000010000000a000000003b"
"023380"))

l1.start()

assert l1.rpc.listchannels()['channels'] == [
{'source': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59',
'destination': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518',
'short_channel_id': '103x1x1',
'public': False,
'satoshis': 1000000,
'amount_msat': Millisatoshi(1000000000),
'message_flags': 1,
'channel_flags': 0,
'active': False,
'last_update': 1588223664,
'base_fee_millisatoshi': 1,
'fee_per_millionth': 10,
'delay': 6,
'htlc_minimum_msat': Millisatoshi(0),
'htlc_maximum_msat': Millisatoshi(990000000)},
{'source': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518',
'destination': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59',
'short_channel_id': '103x1x1',
'public': False,
'satoshis': 1000000,
'amount_msat': Millisatoshi(1000000000),
'message_flags': 1,
'channel_flags': 1,
'active': False,
'last_update': 1588223664,
'base_fee_millisatoshi': 1,
'fee_per_millionth': 10,
'delay': 6,
'htlc_minimum_msat': Millisatoshi(0),
'htlc_maximum_msat': Millisatoshi(990000000)}]
3 changes: 1 addition & 2 deletions tools/gen/header_template
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,7 @@ extern const struct tlv_record_type tlvs_${tlv.name}[];
% endfor
void towire_${subtype.name}(u8 **p, const ${subtype.type_name()} *${subtype.name});
% if subtype.is_varsize():
${subtype.type_name()} *
fromwire_${subtype.name}(const tal_t *ctx, const u8 **cursor, size_t *plen);
${subtype.type_name()} *fromwire_${subtype.name}(const tal_t *ctx, const u8 **cursor, size_t *plen);
% else:
void fromwire_${subtype.name}(const u8 **cursor, size_t *plen, ${subtype.type_name()} *${subtype.name});
% endif
Expand Down
13 changes: 9 additions & 4 deletions tools/generate-wire.py
Original file line number Diff line number Diff line change
Expand Up @@ -500,10 +500,11 @@ def post_process(self):
def write(self, options, output):
template = self.find_template(options)
enum_sets = []
enum_sets.append({
'name': options.enum_name,
'set': self.messages.values(),
})
if len(self.messages.values()) != 0:
enum_sets.append({
'name': options.enum_name,
'set': self.messages.values(),
})
stuff = {}
stuff['top_comments'] = self.top_comments
stuff['options'] = options
Expand Down Expand Up @@ -534,6 +535,9 @@ def main(options, args=None, output=sys.stdout, lines=None):

# Create a new 'master' that serves as the coordinator for the file generation
master = Master()
for i in options.include:
master.add_include('#include <{}>'.format(i))

try:
while True:
ln, line = next(genline)
Expand Down Expand Up @@ -676,6 +680,7 @@ def main(options, args=None, output=sys.stdout, lines=None):
action="store_true", default=False)
parser.add_argument("--page", choices=['header', 'impl'], help="page to print")
parser.add_argument('--expose-tlv-type', action='append', default=[])
parser.add_argument('--include', action='append', default=[])
parser.add_argument('header_filename', help='The filename of the header')
parser.add_argument('enum_name', help='The name of the enum to produce')
parser.add_argument("files", help='Files to read in (or stdin)', nargs=REMAINDER)
Expand Down