(We don't increment the gossip_store version, since there are only a
few commits since the last time we did this).
This lets the reader simply filter messages; this is especially nice since
the channel_announcement timestamp is *derived*, not in the actual message.
This also creates a 'struct gossip_hdr' which makes the code a bit
clearer.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Keeping the uintmap ordering all the broadcastable messages is expensive:
130MB for the million-channels project. But now we delete obsolete entries
from the store, we can have the per-peer daemons simply read that sequentially
and stream the gossip itself.
This is the most primitive version, where all gossip is streamed;
successive patches will bring back proper handling of timestamp filtering
and initial_routing_sync.
We add a gossip_state field to track what's happening with our gossip
streaming: it's initialized in gossipd, and currently always set, but
once we handle timestamps the per-peer daemon may do it when the first
filter is sent.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
They already send *us* gossip messages, so they have to be distinct anyway.
Why make us both do extra work?
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Encapsulating the peer state was a win for lightningd; not surprisingly,
it's even more of a win for the other daemons, especially as we want
to add a little gossip information.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We ask gossipd for the channel_update for the outgoing channel; any other
messages it sends us get queued for later processing.
But this is overzealous: we can shunt those msgs to the peer while
we're waiting. This fixes a nasty case where we have to handle
WIRE_GOSSIPD_NEW_STORE_FD messages by queuing the fd for later.
This then means that WIRE_GOSSIPD_NEW_STORE_FD can be handled
internally inside handle_gossip_msg(), since it's always dealt with
the same, simplifying all callers.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We use the high bit of the length field: this way we can still check
that the checksums are valid on deleted fields.
Once this is done, serially reading the gossip_store file will result
in a complete, ordered, minimal gossip broadcast. Also, the horrible
corner case where we might try to delete things from the store during
load time is completely gone: we only load non-deleted things.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Since we might soon be changing the payload it is a good idea to not just
expose the v0 payload, but also the raw payload for the plugin to
interpret. This might also include payloads that `lightningd` itself cannot
understand, but the plugin might.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Suggested-by: Corné Plooy <@bitonic-cjp>
Instead of reading the store ourselves, we can just send them an
offset. This saves gossipd a lot of work, putting it where it belongs
(in the daemon responsible for the specific peer).
MCP bench results:
store_load_msec:28509-31001(29206.6+/-9.4e+02)
vsz_kb:580004-580016(580006+/-4.8)
store_rewrite_sec:11.640000-12.730000(11.908+/-0.41)
listnodes_sec:1.790000-1.880000(1.83+/-0.032)
listchannels_sec:21.180000-21.950000(21.476+/-0.27)
routing_sec:2.210000-11.160000(7.126+/-3.1)
peer_write_all_sec:36.270000-41.200000(38.168+/-1.9)
Signficant savings in streaming gossip:
-peer_write_all_sec:48.160000-51.480000(49.608+/-1.1)
+peer_write_all_sec:35.780000-37.980000(36.43+/-0.81)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We can be spammy, which is good for tests, but bad for our simple message queue.
This avoids breaking our tests but also avoid the worst case (1M entries and counting!)
for gossip status messages in the case where we're syncing a large peer.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes#2518
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Changelog-fixed: `minconf` no longer gets wrapped around for large values, which was causing funds with insufficient confirmations to be selected.
This fixes block parsing on testnet; specifically, non-standard tx versions.
We hit a type bug in libwally (wallt_get_secp_context()) which I had to
work around for the moment, and the updated libsecp adds an optional hash
function arg to the ECDH function.
Fixes: #2563
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I tried to just do gossipd, but it was uncontainable, so this ended up being
a complete sweep.
We didn't get much space saving in gossipd, even though we should save
24 bytes per node.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Node ids are pubkeys, but we only use them as pubkeys for routing and checking
gossip messages. So we're packing and unpacking them constantly, and wasting
some space and time.
This introduces a new type, explicitly the SEC1 compressed encoding
(33 bytes). We ensure its validity when we load from the db, or get it
from JSON. We still use 'struct pubkey' for peer messages, which checks
validity.
Results from 5 runs, min-max(mean +/- stddev):
store_load_msec,vsz_kb,store_rewrite_sec,listnodes_sec,listchannels_sec,routing_sec,peer_write_all_sec
39475-39572(39518+/-36),2880732,41.150000-41.390000(41.298+/-0.085),2.260000-2.550000(2.336+/-0.11),44.390000-65.150000(58.648+/-7.5),32.740000-33.020000(32.89+/-0.093),44.130000-45.090000(44.566+/-0.32)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Pubkeys are not not actually DER encoding, but Pieter Wuille corrected
me: it's SEC 1 documented encoding.
Results from 5 runs, min-max(mean +/- stddev):
store_load_msec,vsz_kb,store_rewrite_sec,listnodes_sec,listchannels_sec,routing_sec,peer_write_all_sec
38922-39297(39180.6+/-1.3e+02),2880728,41.040000-41.160000(41.106+/-0.05),2.270000-2.530000(2.338+/-0.097),44.570000-53.980000(49.696+/-3),32.840000-33.080000(32.95+/-0.095),43.060000-44.950000(43.696+/-0.72)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is what all of this has been working towards: ripping out the handwoven
transaction handling. By removing the custom parsing we can finally switch
over to using `wally_tx` as sole representation of transactions in
memory. The commit is a bit larger but it's mostly removing setters and old
references to the input and output fields.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This is the second to last time I'm touching this file, just need to remove
the `tx->input` and `tx->output` swapping once they are removed.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
The `wally_tx_input`s do not keep track of their input value, which means we
need to track them ourselves if we try to sign these transactions at a later
point in time.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
1. Rename channel_funding_locked to channel_funding_depth in
channeld/channel_wire.csv.
2. Add minimum_depth in struct channel in common/initial_channel.h and
change corresponding init function: new_initial_channel().
3. Add confirmation_needed in struct peer in channeld/channeld.c.
4. Rename channel_tell_funding_locked to channel_tell_depth.
5. Call channel_tell_depth even if depth < minimum, and still call
lockin_complete in channel_tell_depth, iff depth > minimum_depth.
6. channeld ignore the channel_funding_depth unless its >
minimum_depth(except to update billboard, and set
peer->confirmation_needed = minimum_depth - depth).
1. amount operations should force you to check validity, rather than
needing a separate call, so make amount_msat_to_u32 return bool,
and WARN_UNUSED_RESULT it.
2. Create a special parsing function for this; not only does this mean
we now only need that one amount call, but also 'check' will correctly
fail with invalid amounts (it only does the parsing step).
3. If we create a primitive which we immediately take(), we allocate it
off NULL to make it clear we expect its lifetime to end here.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
==12787== Uninitialised byte(s) found during client check request
==12787== at 0x450AAC: memcheck_ (mem.h:247)
==12787== by 0x450B17: towire (towire.c:19)
==12787== by 0x45103D: towire_u8_array (towire.c:159)
==12787== by 0x443235: towire_wireaddr_internal (wireaddr.c:79)
==12787== by 0x46E6F2: towire_connectctl_init (gen_connect_wire.c:229)
==12787== by 0x40D6C8: connectd_init (connect_control.c:369)
==12787== by 0x4186D3: main (lightningd.c:701)
==12787== Address 0x682d8a9 is 361 bytes inside a block of size 568 alloc'd
==12787== at 0x4C2FD5F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==12787== by 0x4867A5: tal_resize_ (tal.c:694)
==12787== by 0x41F3EE: opt_add_addr_withtype (options.c:143)
==12787== by 0x41F4D7: opt_add_bind_addr (options.c:155)
==12787== by 0x47E364: parse_one (parse.c:121)
==12787== by 0x47F9C8: opt_parse (opt.c:210)
==12787== by 0x4212F9: handle_opts (options.c:892)
==12787== by 0x41864C: main (lightningd.c:667)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I tried to fundchannel 0.01btc, and of course it wanted 8 decimals exactly.
If I can't get this right, it's probably a bad idea.
I still don't allow whole number of btc though, since that's probably a mistake
and you're not supposed to put that much in c-lightning yet :)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In order to avoid having to ask the HSM for public keys to
their_unilateral/to_us outputs we just store the `scriptPubkey` with the UTXO,
which can then be converted to the P2WPKH address.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This allows us to specify that an output must have been confirmed before the
given maximum height. This allows us to specify a minimum number of
confirmations for an output to be selected.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
We need to do it in various places, but we shouldn't do it lightly:
the primitives are there to help us get overflow handling correct.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As a side-effect of using amount_msat in gossipd/routing.c, we explicitly
handle overflows and don't need to pre-prune ridiculous-fee channels.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Using param_tok is generally deprecated, as it doesn't give any sanity checking
for the JSON 'check' command. So make param_wtx usable directly, and
also make it have a struct amount_sat.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The current param_sat accepts "any": rename and move that to invoice.c
where it's called. We rename it to param_msat_or_any and invoice.c
is our first (trivial) amount_msat user.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
They're generally used pass-by-copy (unusual for C structs, but
convenient they're basically u64) and all possibly problematic
operations return WARN_UNUSED_RESULT bool to make you handle the
over/underflow cases.
The new #include in json.h means we bolt11.c sees the amount.h definition
of MSAT_PER_BTC, so delete its local version.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We need to still accept it when parsing the database, but this flag
should allow upgrade testing for devs building on top
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have a seed, which is for (future!) unit testing consistency. This
makes it change every time, so our pay_direct_test is more useful.
I tried restarting the noed around the loop, but it tended to fail
rebinding to the same port for some reason?
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Similar to the previous "handle peer input before gossip input", this
fixes similar potential deadlock for closingd and openingd which use
peer_or_gossip_sync_read.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
- result fundchannel command now depends on successful or failed broadcast of the funding tx
- failure returns error code FUNDING_BROADCAST_FAIL
- don't fail the channel when broadcast failed, but keep in CHANNELD_AWAITING_LOCKIN
- after fixing the initial broadcast failure, the user could manually rebroadcast the tx and
keep the channel
openingd/opening_funder_finished:
- broadcast_tx callback function now handles both success and failure
jsonrpc: added error code FUNDING_BROADCAST_FAIL
manpage: added error code returned by fundchannel command
This makes the user more aware of broadcast failure, so it hopefully doesn't
try to broadcast new tx's that depend on its change_outputs. Some users have reported (see
issue #2171) a whole sequence of fundings failing, because each funding was using the change
output of the previous one, which would not confirm.
Internally libplugin turns ' into ", which causes these messages to produce
bad JSON.
The real fix is to remove the '->" convenience substitution and port the
JSON creation APIs into common/ from lightningd/
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Christian points out that we can iterate by ->size rather than calling
json_next() to find the end (which traverses the entire object!).
Now ->size is reliable (since previous patch), this is OK.
Reported-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
jsmn would accept invalid JSON objects. This is bad because it would
set ->size incorrectly: we expect to have at least size * 2 tokens (in
pairs). We want to rely on ->size, but this would create an exploitable
buffer overflow!
Fortunately, this is fixed upstream, so we add a test and upgrade to v1.0.0.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Wasn't using valid JSON, but worked anyway. This is actually OK
because we don't rely on tok->size, but we want to, so another fix
coming.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The external/jsmn/README.md only says:
int size; // Number of child (nested) tokens
But it only counts *direct* children, or *direct* members for an object.
This test verifies this (the bug proved to be elsewhere: see next patch!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Had a couple of tests randomly fail because a valgrind error file was
not empty. It contained:
lightning_channeld: Writing out status 65520: Broken pipe
This shouldn't happen, but the simplest workaround is not to print
that (useless) error.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Currently only used by gossipd for channel elimination.
Also print them in canonical form (/[01]), so tests need to be
changed.
Suggested-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Christian and I both unwittingly used it in form:
*tal_arr_expand(&x) = tal(x, ...)
Since '=' isn't a sequence point, the compiler can (and does!) cache
the value of x, handing it to tal *after* tal_arr_expand() moves it
due to tal_resize().
The new version is somewhat less convenient to use, but doesn't have
this problem, since the assignment is always evaluated after the
resize.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is mainly just copying over the copy-editing from the
lightning-rfc repository.
[ Split to just perform changes after the UNKNOWN_PAYMENT_HASH change --RR ]
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Reported-by: Rusty Russell <@rustyrussell>
This is mainly just copying over the copy-editing from the
lightning-rfc repository.
[ Split to just perform changes prior to the UNKNOWN_PAYMENT_HASH change --RR ]
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Reported-by: Rusty Russell <@rustyrussell>
This causes a compiler warning if we don't do something with the
result (hopefully return immediately!).
We use was_pending() to ignore the result in the case where we
complete a command in a callback (thus really do want to ignore
the result).
This actually fixes one bug: we didn't return after command_fail
in json_getroute with a bad seed value.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Handers of a specific form are both designed to be used as callbacks
for param(), and also dispose of the command if something goes wrong.
Make them return the 'struct command_result *' from command_failed(),
or NULL.
Renaming them just makes sense: json_tok_XXX is used for non-command-freeing
parsers too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
These routines free the 'struct command': a common coding error is not
to return immediately.
To catch this, we make them return a non-NULL 'struct command_result
*', and we're going to make the command handlers return the same (to
encourage 'return command_fail(...)'-style usage).
We also provide two sources for external use:
1. command_param_failed() when param() fails.
2. command_its_complicated() for some complex cases.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
These are only supposed to be used when you want the token contents including
surrounding "". We should use this when reporting errors, but usually
we just want to access the tok members directly.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This was removed (as unused) in 6269a4c55d592e8720b7f2a304c21f61f7931238;
now I've even added tests.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have an incompatibility with lnd it seems: I've lost channels on
reconnect with 'sync error'. Since I never got this code to be reliable,
disable it for next release since I suspect it's our fault :(
And reenable the check which didn't work, for others to untangle.
I couldn't get option_data_loss_protect to be reliable, and I disabled
the check. This was a mistake, I should have either spent even more
time trying to get to the bottom of this (especially, writing test
vectors for the spec and testing against other implementations).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
json_escaped.[ch], param.[ch] and jsonrpc_errors.h move from lightningd/
to common/. Tests moved too.
We add a new 'common/json_tok.[ch]' for the common parameter parsing
routines which a plugin might want, taking them out of
lightningd/json.c (which now only contains the lightningd-specific
ones).
The rest is mainly fixing up includes.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I want to use param functions in plugins, and they don't have struct
command.
I had to use a special arg to param() for check to flag it as allowing
extra parameters, rather than adding a one-use accessor.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
json_tok* is used with 'struct command', so rename this to match the other
low-level json tok helpers.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This currently just invokes GDB, but we could generalize it (though
pdb doesn't allow attaching to a running process, other python
debuggers seem to).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is prep work for when we sign htlc txs with
SIGHASH_SINGLE|SIGHASH_ANYONECANPAY.
We still deal with raw signatures for the htlc txs at the moment, since
we send them like that across the wire, and changing that was simply too
painful (for the moment?).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We currently generally assume the features we offer are fixed; this
makes the code clearer and handles where we offer features iff
EXPERIMENTAL_FEATURES=1
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Needed for check command. I left the print function in since it was so
convenient for debugging purposes.
Signed-off-by: Mark Beckwith <wythe@intrig.com>
This simplifies lifetime assumptions. Currently all callers keep the
original around, but everything broke when I changed that in the next
patch.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
They were not universally used, and most are trivial accessors anyway.
The exception is getting the channel reserve: we have to multiply by 1000
as well as flip direction, so keep that one.
The BOLT quotes move to `struct channel_config`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We only take the pubkey and ignore all other fields, so we might as well
save the cycles used computing the hash for something else.
Signed-off-by: Jon Griffiths <jon_p_griffiths@yahoo.com>
We probably also want to call secp_randomise/wally_secp_randomize here
too, and since these calls all call setup_tmpctx, it probably makes
sense to have a helper function to do all that. Until thats done, I
modified the tests so grepping will show the places where the sequence
of calls is repeated.
Signed-off-by: Jon Griffiths <jon_p_griffiths@yahoo.com>
This avoids some very ugly switch() statements which mixed the two,
but we also take the chance to rename 'towire_gossip_' to
'towire_gossipd_' for those inter-daemon messages; they're messages to
gossipd, not gossip messages.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We had at least one bug caused by it not returning true when it had
queued something. Instead, just re-check thq queue after it's called.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If you steal something onto its own child, you create a loop. These are
expensive to check for at runtime, but they can hide from memleak and are
usually a bad idea. So we add a tal_steal() notify which does this work
in DEVELOPER mdoe.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This way there's no need for a context pointer, and freeing a msg_queue
frees its contents, as expected.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It means an extra allocation at startup, but it means we can hide the definition,
and use standard patterns (new_daemon_conn and typesafe callbacks).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This was suggested by Pierre-Marie as the solution to the 'same HTLC,
different CLTV' signature mismatch.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's the only user of them, and it's going to get optimized.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
gossip.pydiff --git a/common/test/run-json.c b/common/test/run-json.c
index 956fdda35..db52d6b01 100644
This also highlights the danger of searching the logs: that error
appeared previously in the logs, so we didn't notice that the actual
withdraw call gave a different error.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Give a clear error at the beginning if it's not bolt11 payment,
rather than falling foul of other checks.
This will work at least until some altcoin adapts the 'ln' prefix :)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Totally forgot to add this test. It just shows how a writer can take
exclusive access of a socket over multiple `io_write` calls, and
queuing all others behind it.
We've done this a number of times already where we're getting
exclusive access to either the out direction of a connection, or we
try to lock out the read side while we are responding to a previous
request. They usually are really cumbersome because we reach around to
the other direction to stop it from proceeding, or we flag our
exclusive access somewhere, and we always need to know whom to notify.
PR ElementsProject/lightning#1970 adds two new instances of this:
- Streaming a JSON response requires that nothing else should write
while the stream is active.
- We also want to stop reading new requests while we are responding
to one.
To remove the complexity of having to know whom to stop and notify
when we're done, this adds a simple `io_lock` primitive that can be
used to get exclusive access to a connection. This inverts the
requirement for notifications, since everybody registers interest in
the lock and they get notified if the lock holder releases it.
Rusty Russell
Christian Decker
lisa neigut
Lawrence Nahum
Michael Schmoock
Simon Vrouwe
trueptolemy
arowser
Mark Beckwith
Jon Griffiths