These are now logically arrays of pointers. This is much more natural,
and gets rid of the horrible utxo array converters.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Still writing the channel since some of the channel setup parameters
depends on `chan->id` to be set. If we later set the `chan->id`
signatures fail. This prevents OPENINGD channels showing up after
restarting.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
We were sideloading it, which is awkward, now it's a field that we can
actually use in the code.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
If needed, they should be directly #included; otherwise the following
patches really mess things up.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* Add optional addresstype param to newaddr, which can be one of:
- bech32
- p2sh-segwit
- nothing (defaults to p2sh-segwit)
The naming here mirrors bitcoind
* txfilter already looks for p2wpkh outputs, so we're covered there
Signed-off-by: William Casarin <jb55@jb55.com>
This was removed because withdraw-to-self results in more than the
change being sent to us.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Maintaining it was always fraught, since the command could go away
if the JSON RPC died. Most recently, it was broken again on shutdown
(see below).
In future we may allow pay commands to block on previous payments, so
it won't even be a 1:1 mapping. Generalize it: keep commands in a
simple list and do a lookup when a payment fails/succeeds.
Valgrind error file: valgrind-errors.5732
==5732== Invalid read of size 8
==5732== at 0x4149FD: remove_cmd_from_hout (pay.c:292)
==5732== by 0x468BAB: notify (tal.c:237)
==5732== by 0x469077: del_tree (tal.c:400)
==5732== by 0x4690C7: del_tree (tal.c:410)
==5732== by 0x46948A: tal_free (tal.c:509)
==5732== by 0x40F1EA: main (lightningd.c:362)
==5732== Address 0x69df148 is 1,512 bytes inside a block of size 1,544 free'd
==5732== at 0x4C2EDEB: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5732== by 0x469150: del_tree (tal.c:421)
==5732== by 0x46948A: tal_free (tal.c:509)
==5732== by 0x4198F2: free_htlcs (peer_control.c:1281)
==5732== by 0x40EBA9: shutdown_subdaemons (lightningd.c:209)
==5732== by 0x40F1DE: main (lightningd.c:360)
==5732== Block was alloc'd at
==5732== at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5732== by 0x468C30: allocate (tal.c:250)
==5732== by 0x4691F7: tal_alloc_ (tal.c:448)
==5732== by 0x40A279: new_htlc_out (htlc_end.c:143)
==5732== by 0x41FD64: send_htlc_out (peer_htlcs.c:397)
==5732== by 0x41511C: send_payment (pay.c:388)
==5732== by 0x41589E: json_sendpay (pay.c:513)
==5732== by 0x40D9B1: parse_request (jsonrpc.c:600)
==5732== by 0x40DCAC: read_json (jsonrpc.c:667)
==5732== by 0x45C706: next_plan (io.c:59)
==5732== by 0x45D1DD: do_plan (io.c:387)
==5732== by 0x45D21B: io_ready (io.c:397)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We move it into jsonrpc where it belongs, and make it fail the command.
This means it can tell us exactly what was wrong.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
So many channels stuck in channeld_awaiting_lockin, and I don't like
suggesting manually editing the DB, so this adds a very simple way to
sync with bitcoind's UTXO view. `dev` since it is dangerous, then
again if bitcoind says those funds aren't available there's little we
can do anyway.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
The previous tests didn't make sense anyway, but I think they were trying
to exclude onchain channels.
We delete completely forgotten channels anyway now, so we don't need
such testing.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Error code is inverted (which makes sense: who returns 'true' on
error?), and anyway there's a leak if we do error.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
`pay_index` has no valid value if not PAID anyway, so
we should correctly leave it uninitialized.
Analysis via valgrind will catch incorrect use of
uninitialized fields.
If we load it with a dummy 0 value, then an
incorrect use of `pay_index` whan invoice is not
PAID will not get caught by valgrind.
From test_reconnect_sender_add1:
lightningd(13643):BROKEN: backtrace: wallet/wallet.c:1537 (wallet_payment_set_status) 0x561c91b03080
lightningd(13643):BROKEN: backtrace: lightningd/pay.c:67 (payment_failed) 0x561c91ac4f99
lightningd(13643):BROKEN: backtrace: lightningd/peer_htlcs.c:132 (fail_out_htlc) 0x561c91acf627
lightningd(13643):BROKEN: backtrace: lightningd/peer_htlcs.c:321 (hout_subd_died) 0x561c91acfb62
When payment fails, we call wallet_payment_set_status; this is perfectly
possible before it's been committed.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
For performance, we delay entering the 'wallet_payment' into the db
until we actually commit to the HTLC (when we have to touch the DB
anyway).
This opens a race where we can try to pay twice, and since it's not in
the database yet, we don't notice the duplicate.
So remove the temporary payment field from htlc_out, which was always
an uncomfortable hack, and make the wallet code abstract over the
deferred entry a little by maintaining a 'unstored_payments' list
and incorporating that in results.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We need these to decode any returned errors.
We remove it from struct pay_command too, and load directly from db
when we need it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We should be saving this, as it's our proof of payment. Also, we return
it if they try to pay again.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Paid invoices need to know how much was actually paid: both for the case
where no 'msatoshi' amount was specified, and for the normal case, where
clients are permitted to overpay in order to help them disguise their
payments.
While we migrate the db, we leave this field as 0 for old paid
invoices. This is unhelpful for accounting, but at least clearly
indicates what happened if we find this in the wild.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This reuses the same code internally, and also now means that we deal
correctly with "any" msatoshi invoices: the old code would a return
'msatoshi' of 0 in that case.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We were using int32 for msatoshi values for outputs, which would
overflow for values larger than 2^32.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
So far we have been generating the tx both in the HSM and in the
caller, and had to rely on them generating exactly the same
transaction. This makes it a lot simpler by fully signing and
serializing the TX on the HSM side and the caller just needs to unpack
and broadcast it.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This is necessary to grad the their_unilateral/to-us outputs since
they aren't being harvested by `onchaind`
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This gives us a lower bound on where funding tx could be.
In theory, it could be lower than this if we get a reorganization, but
in practice this is already a 1-block buffer (since we can't get into
current block, only the next one).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's only used for tests, but it's better to use the wallet_channels_load_active like
the real code.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's just a sha256_double, but importantly when we convert it to a
string (in type_to_string, which is used in logging) we use
bitcoin_txid_to_hex() so it's reversed as people expect.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
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>
* 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>
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>
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>
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>
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>
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>
