For memory-usage reasons, struct chan doesn't use a tal destructor, in
favor of us calling free_chan in the right places.
In DEVELOPER mode, we should check that is the case.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We've been slack, but it's going to be important for testing
ratelimiting. And it currently has a minor memory leak.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rather than reaching into data structures, let them register their own
callbacks. This avoids us having to expose "memleak_remove_xxx"
functions, and call them manually.
Under the hood, this is done by having a specially-named tal child of
the thing we want to assist, containing the callback.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
`make update-mocks` is usually run in DEVELOPER mode, but then it includes
definitions for functions which aren't declared in non-DEVELOPER mode.
We hacked this in a few places, but it's fragile, and worst, now we
have EXPERIMENTAL_FEATURES as well, it's complex.
Instead, declare developer-only functions (but don't define them).
This is a bit more awkward if you accidentally use one in
non-DEVELOPER code (link error rather than compile error), but makes
autogenerating test mocks much easier.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fortunately, again, only happens with EXPERIMENTAL_FEATURES.
If the query causes us not to actually send anything, we won't
get called again. This can validly happen if they only asked for
the node_announcements, for example.
(Found by protocol tests).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Our "are we finished?" logic was wrong: it tested if there are no more
node_announcements, but it's possible that there were no node_announcements
for either end of the channel whose information we sent.
This is actually quite unusual on the real network: looking at mainnet
statis from last May, 4301 of 4337 nodes have node_announcements.
However, with query flags it's much more likely, since they might not
ask for node announcements at all.
(Found by gossip protocol tests)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
These both allow us to reproduce the test vectors in the next patch. But
using Z_DEFAULT_COMPRESSION is a reasonable idea anyway.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Make the TLV element a simple array. This is a bit neater, in fact, and
makes the test vectors in that 557 PR work.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In fact, we always generate them, we only send them if asked. And we set
the flags to 0 if not --enable-experimental-features, so we never send in
that case.
Generating checksums involves pulling the channel_update from the
gossip_store, which is suboptimal: there's a FIXME to store the
checksum in memory.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We're about to use the for gossip extended info too, which *don't* put
the encoding byte at the beginning of the data stream. So this removes
some "scids" from function names and separates out the "prepend a byte"
case from the "external encoding_type" case.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
These indicate what fields we are to return. If there's now TLV, or we
haven't got --enable-experimental-features, it's set to all 1s so behaviour
is unchanged.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We ignored this before, which meant that the DEVELOPER-mode check that we
delete the correct record didn't check that it wasn't already deleted.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We always know the length, so we don't need it. It causes much extra work
when we want to delete a record, which I suspect may cause issues amongst
some users who've been seeing gossip_store corruption.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We added a random channel to the list, but we can just free it immediately
(since traversal of a uintmap isn't altered by deletion).
This was introduced in d1f43d993a where we explicitly call free_chan
rather than relying on destructors.
Fixes: #2837
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
==1503== Use of uninitialised value of size 8
==1503== at 0x566786B: _itoa_word (_itoa.c:179)
==1503== by 0x566AF0D: vfprintf (vfprintf.c:1642)
==1503== by 0x569790F: vsnprintf (vsnprintf.c:114)
==1503== by 0x156CCB: do_vfmt (str.c:66)
==1503== by 0x156DB1: tal_vfmt_ (str.c:92)
==1503== by 0x1289CD: status_vfmt (status.c:141)
==1503== by 0x128AAC: status_fmt (status.c:151)
==1503== by 0x118E05: route_prune (routing.c:2495)
==1503== by 0x11DE2D: gossip_refresh_network (gossipd.c:1997)
==1503== by 0x1292B8: timer_expired (timeout.c:39)
==1503== by 0x12088C: main (gossipd.c:3075)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
updates the bolt version to 6639cef095a2ecc7b8f0c48c6e7f2f906fbfbc58.
this requires us to use the new bolt parser at generate-bolt.py
and updates to all of the type specifications (ie. from u8 -> byte)
Rewriting the gossip_store is much more trivial when we don't have
any pointers into it, so add some simple offline compaction code
and disable the automatic compaction code.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The crashes in #2750 are mostly caused by us trying to partially truncate
the store. The simplest fix for release is to discard the whole thing if
we detect a problem.
This is a workaround: it'd be far nicer to try to recover.
Fixes: #2750
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We hit the timestamp assert on #2750; it shouldn't happen, but crashing
doesn't leave much information.
Reported-by: @m-schmook
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If something went wrong and there was an old one, we were
appending to it!
Reported-by: @SimonVrouwe
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We might have channel_announcements which have no channel_update: normally
these don't get written into the store until there is one, but if the
store was truncated it can happen. We then get upset on compaction, since
we don't have an in-memory representation of the channel_announcement.
Similarly, we leave the node_announcement pending until after that
channel_announcement, leading to a similar case.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We can't continue, since we've moved the indexes. We'll just crash
anyway, as seen from bugs #2742 and #2743.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We catch node_announcements for nodes where we haven't finished
analyzing the channel_announcement yet (either because we're still
checking UTXO, or in this case, because we're waiting for a channel_update).
But we reference count the pending_node_announce, so if we have
multiple channels pending, we might try to insert it twice. Clear it
so this doesn't happen.
There's a second bug where we continue to catch node_announcements
until *all* the channel_announcements are no longer pending; this is fixed
by removing it from the map.
Fixes: #2735
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
trueptolemy
Rusty Russell
lisa neigut
darosior
ZmnSCPxj