Commit Graph

2536 Commits

Author SHA1 Message Date
Rusty Russell 553ebc936b json: add json_add_txid.
I prefer the typesafety of specific functions, rather than having the
caller know that txids are traditionally reversed in bitcoin.

And we already have a bitcoin_txid_to_hex() function for this.

Closes: #411
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-21 11:05:38 +00:00
Andrew Chow f792e236eb Encode reversed hex for txid in listfunds 2017-12-21 11:05:38 +00:00
Andrew Chow 19116b6561 Functions for encoding reversed hex 2017-12-21 11:05:38 +00:00
William Casarin bcd49b063b fixup! wallet: insert address into peers table 2017-12-21 09:56:20 +00:00
William Casarin ce1d709d44 wireaddr: fix ipv6 formatting with ports in fmt_wireaddr
Correctly format ipv6 address with ports. This will also make it more compatible
with the new parse_wireaddr, which has been updated to parse ports. They are
inverses now.

Also add some tests that check this.

Signed-off-by: William Casarin <jb55@jb55.com>
2017-12-21 09:56:20 +00:00
William Casarin d89eb32c85 wallet: insert address into peers table
It looks like we were missing the address on insert into the peers table. This
will insert the address formatted by fmt_wireaddr. This happens to include the
ip and port.

This is fine, since parse_wireaddr has been updated to parse ports in ip address
strings.

Signed-off-by: William Casarin <jb55@jb55.com>
2017-12-21 09:56:20 +00:00
William Casarin 3e7aabe256 test: add tests for parse_ip_port
This tests the basic functionality of parse_ip_port, which is used in
parse_wireaddr.

Signed-off-by: William Casarin <jb55@jb55.com>
2017-12-21 09:56:20 +00:00
William Casarin 7ecccd50b9 wireaddr: add ip[:port] parsing
* Add port parsing support to parse_wireaddr. This is in preparation for storing
addresses in the peers table. This also makes parse_wireaddr a proper inverse of
fmt_wireaddr.

* Move parse_wireaddr to common/wireaddr.c this seems like a better place for
it. I bring along parse_ip_port with it for convenience. This also fixes some
issues with the upcoming ip/port parsing tests.

Signed-off-by: William Casarin <jb55@jb55.com>
2017-12-21 09:56:20 +00:00
Rusty Russell 90a5ba043c htlc_out: initialize hout->key.id, in case we destroy it before setting.
We set hout->key.id when channeld tells us what it is, but if channeld
dies before that we free the hout, and our destructor logs it:

Valgrind error file: valgrind-errors.20312
==20312== Use of uninitialised value of size 8
==20312==    at 0x53ABC9B: _itoa_word (_itoa.c:179)
==20312==    by 0x53B041F: vfprintf (vfprintf.c:1642)
==20312==    by 0x53B17D5: buffered_vfprintf (vfprintf.c:2330)
==20312==    by 0x53AEAA5: vfprintf (vfprintf.c:1301)
==20312==    by 0x53B7D63: fprintf (fprintf.c:32)
==20312==    by 0x128BAC: hout_subd_died (peer_htlcs.c:316)
==20312==    by 0x16D8E0: notify (tal.c:240)
==20312==    by 0x16DD95: del_tree (tal.c:400)
==20312==    by 0x16DDE7: del_tree (tal.c:410)
==20312==    by 0x16DDE7: del_tree (tal.c:410)
==20312==    by 0x16E1B4: tal_free (tal.c:509)
==20312==    by 0x162B5C: io_close (io.c:443)
==20312==    by 0x12D563: sd_msg_read (subd.c:508)
==20312==    by 0x161EA5: next_plan (io.c:59)
==20312==    by 0x1629A2: do_plan (io.c:387)
==20312==    by 0x1629E0: io_ready (io.c:397)
==20312==    by 0x164319: io_loop (poll.c:305)
==20312==    by 0x118E21: main (lightningd.c:334)

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-21 09:13:28 +01:00
Rusty Russell f293ff0a6a broadcast: don't reorder channel_announce when we get the real one.
If channel_announce is rebroadcast, it should replace the existing one
in-place.  We currently only do this if we start from the unsigned one
and replace it with the signed one when we hit 6 confirms.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-21 09:13:28 +01:00
Rusty Russell 425143646c channeld: fix dev_disconnect.
I noted a spurious failure on test_reconnect_sender_add1: we
actually sent an update_commit, which should have been suppressed.

This was because we call dev_disconnect() when we *dequeue* the packet,
which might be too late to suppress the timer.  So instead, call it
when the packet in enqueued, and flush synchronously to make sure
we get the right packet.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-21 09:13:28 +01:00
Rusty Russell bb601a1eeb gossipd/test/run-bench-find_route: don't abort if we try to route to ourselves
This would fail, and we'd free an uninitialized pointer.

Also, add us to .gitignore and clear up a comment.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 13:57:28 +01:00
Rusty Russell 3543530172 build_utxos: fix weight calculation, and make more accurate.
Accuracy improvements:

1. We assumed the output was a p2wpkh, but it can be user-supplied now.
2. We assumed we always had change; remove this for wallet_select_all.

Calculation out-by-one fixes:

1. We need to add 1 byte (4 sipa) for the input count.
2. We need to add 1 byte (4 sipa) for the output count.
3. We need to add 1 byte (4 sipa) for the output script length for each output.
4. We need to add 1 byte (4 sipa) for the input script length for each input.
5. We need to add 1 byte (4 sipa) for the PUSH optcode for each P2SH input.

The results are now a slight overestimate (due to guessing 73 bytes
for signature, whereas they're 71 or 72 in practice).

Fixes: #458
Reported-by: Jonas Nick @jonasnick
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:56:15 +01:00
Rusty Russell bbac67f108 withdraw_tx: don't create empty output if no change.
We were using changekey as the flag to produce change, not changesat,
but the caller was using changesat as the flag.

Also, don't allocate changekey at all if we don't need it; this means
valgrind will complain if we use it at all, too.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:56:15 +01:00
Rusty Russell abafcae5a6 lightningd/build_utxos: don't tell them they can pass NULL for change.
They can't: wallet_select_coins() doesn't allow it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:56:15 +01:00
Rusty Russell 070929f92e bitcoin/script: define standard scriptpubkey lengths.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:56:15 +01:00
Rusty Russell 31153e9506 test_lightning.py: fix running test_withdraw twice.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:56:15 +01:00
Rusty Russell 41a5d19198 wallet: don't leak channel struct when peer freed.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell e695e5db09 lightningd: require explicit LIGHTNINGD_DEV_MEMLEAK env var to do memleak.
Otherwise every allocation and free is slowed down.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell 6b9c525f35 lightningd: use env var not cmdline to suppress backtrace.
We now set it up *before* parsing cmdline, so this is more convenient.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Christian Decker ed9e580358 pytest: test_shutdown requires DEVELOPER=1
Signed-off-by: Christian Decker <decker.christian@gmail.com>
2017-12-20 12:43:10 +01:00
Christian Decker ee25547576 Re-enable builds with DEVELOPER=0
Two changes:
 - Fixed the function signature of noleak_ to match in both
   configurations
 - Added memleak.o to linker for tests

Generating the stubs for the unit tests doesn't really work since the
stubs are checked in an differ between the two configurations, so
adding memleak to the linker fixes that, by not requiring stubs to be
generated in the first place.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
2017-12-20 12:43:10 +01:00
Rusty Russell ed7108df15 test_lightningd.py: fail if nodes leak.
killall() was pretty broken, so I had to rewrite it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell 3ed496e45b lightningd: free failed output htlcs.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell ba6904f833 pay: close leaks.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell 85f065a6e7 peer_control: close leak in sign_last_tx.
We can call this multiple times.  The best solution is to add and remove
the signature so it's always unsigned as we expect it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell ed8baf5f88 funding_spent: fix leaks.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell 8c4eecc089 sphinx: fix leaks.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell f19e02eeaa pay: close leak in payment_failed.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell 38a0ee8638 jsonrpc: current pending commands are not a leak.
The pay command in particular, attaches a reasonable number of
temporaries to cmd, knowing they'll be freed once cmd is done.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell 1356913143 invoice: remove strange code.
This is called when we load from database: clearly our tests aren't thorough
enough because we were allocating and initializing `r` in an unused structure.

invs is also the owner already; functions which steal are a bit surprising
to callers, so we either document them, or just don't do it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell adf8740b96 peer_control: fix opening fundee leaks.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell 4957921e02 log: mark ltmp as notleak.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell e64320c76c options: suppress "leak" from opt_table allocation.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell 795a03e8c4 fromwire_bitcoin_tx: quieten leak reporting.
We create a temporary tx which is a child of the real tx, for simplicity of
marshalling.  That's OK.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell ccb7047291 lightningd: add notleak annotations.
We have things which we don't keep a pointer to, but aren't leaks.
Some are simply eternal (eg. listening sockets), others cases are
io_conn tied to the lifetime of an fd, and timers which expire.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell 6c951bf7e0 invoice: move link to the top of the structure.
memleak doesn't detect pointers to within an object, only pointers to their
exact address (it's simpler this way).  Moving the linked list to the
top of the structure means it can follow the chain.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell cec87ba08b peer_control: move link to the top of the structure.
memleak doesn't detect pointers to within an object, only pointers to their
exact address (it's simpler this way).  Moving the linked list to the
top of the structure means it can follow the chain.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell 20fcb5d6ef memleak: allow marking of entire tal trees to be noleak.
In particular, the pay command attaches all kinds of stuff to the
struct command; they're not really a leak, since commands expire.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell 7a8532830f memleak: track reallocs and frees in notleak()
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell 70d01b22e1 memleak: add second exclude arg to exclude current commands' jcon.
This is not a child of cmd, since they have independent lifetimes, but
we don't want to noleak them all, since it's only the one currently in
progress (and its children) that we want to exclude.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell 19b1b35d31 memleak: explicitly exclude ccan/io allocation.
It's a single, full-lifetime allocation; make sure we only exclude that
one, though this is fragile: tests will break if it moves.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell dfc132b2fe memleak: add backtrace to allocations.
We use the tal notifiers to attach a `backtrace` object on every
allocation.

This also means moving backtrace_state from log.c into lightningd.c, so
we can hand it to memleak_init().

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell c956d9f5eb lightningd: tal memleak detection, dev-memleak command.
This is a primitive mark-and-sweep-style garbage detector.  The core is
in common/ for later use by subdaemons, but for now it's just lightningd.
We initialize it before most other allocations.

We walk the tal tree to get all the pointers, then search the `ld`
object for those pointers, recursing down.  Some specific helpers are
required for hashtables (which stash bits in the unused pointer bits,
so won't be found).

There's `notleak()` for annotating things that aren't leaks: things
like globals and timers, and other semi-transients.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell 95df553813 lightningd: add dev-memdump command.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell 8c665835fa jsonrpc: make explicit call to indicate cmd is still pending.
jsonrpc handlers usually directly call command_success or
command_fail; not doing that implies they're waiting for something
async.

Put an explicit call (currently a noop) there, and add debugging
checks to make sure it's used.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell 42b9df59c1 update-mocks: allow it to handle missing notleak_
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 12:43:10 +01:00
Rusty Russell 32cee8620b routing: re-expose half_add_connection.
I made it static in one PR (745c7b223a),
Christian used it in another.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2017-12-20 08:19:28 +00:00
Christian Decker 1ec916076f pytest: Change test_gossip_jsonrpc to cover local-only gossip
Relatively simple: until we reach funding-depth the channels should be
known locally, so we can already route through them, but they should
not be announced to peers to which the connection is non-local.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
2017-12-20 06:14:53 +00:00
Christian Decker da0792e69c gossip: Add local channels when channeld tells us to
This adds the channel from us to the remote node and activates it with
our local parameters.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
2017-12-20 06:14:53 +00:00