We force use of tal_wally_start/tal_wally_end around every wally
allocation, and with "end" make the caller choose where to reparent
everything.
This is particularly powerful where we allocate a tx or a psbt: we
want that tx or psbt to be the parent of the other allocations, so
this way we can reparent the tx or psbt, then reparent everything
else onto it.
Implementing psbt_finalize (which uses a behavior flag antipattern)
was tricky, so I ended up splitting that into 'psbt_finalize' and
'psbt_final_tx', which I think also makes the callers clearer.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. Rename memleak_enter_allocations to memleak_find_allocations.
2. Unify scanning for pointers into memleak_remove_region / memleak_remove_pointer.
3. Document the functions.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The next patch perturbed things enough that we suddenly started
getting (with --track-origins=yes):
Valgrind error file: valgrind-errors.120470
==120470== Use of uninitialised value of size 8
==120470== at 0x14EBD5: htable_val (htable.c:150)
==120470== by 0x14EC3C: htable_firstval_ (htable.c:165)
==120470== by 0x14F583: htable_del_ (htable.c:349)
==120470== by 0x11825D: pointer_referenced (memleak.c:65)
==120470== by 0x118485: scan_for_pointers (memleak.c:121)
==120470== by 0x118500: memleak_remove_region (memleak.c:130)
==120470== by 0x118A30: call_memleak_helpers (memleak.c:257)
==120470== by 0x118A8B: call_memleak_helpers (memleak.c:262)
==120470== by 0x118A8B: call_memleak_helpers (memleak.c:262)
==120470== by 0x118B25: memleak_find_allocations (memleak.c:278)
==120470== by 0x10EB12: closing_dev_memleak (closingd.c:584)
==120470== by 0x10F3E2: main (closingd.c:783)
==120470== Uninitialised value was created by a heap allocation
==120470== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==120470== by 0x1604E8: allocate (tal.c:250)
==120470== by 0x160AA9: tal_alloc_ (tal.c:428)
==120470== by 0x119BE0: new_per_peer_state (per_peer_state.c:24)
==120470== by 0x11A101: fromwire_per_peer_state (per_peer_state.c:95)
==120470== by 0x10FB7C: fromwire_closingd_init (closingd_wiregen.c:103)
==120470== by 0x10ED15: main (closingd.c:626)
==120470==
This is because there is uninitialized padding at the end of struct
peer_state.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
They previously prevented any child from being detected as leaks, now
they just mark the tal allocation itself as not being a leak.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
```
cc common/amount.c
common/amount.c:306:15: error: implicit conversion from 'unsigned long long' to
'double' changes value from 18446744073709551615 to 18446744073709551616
[-Werror,-Wimplicit-int-float-conversion]
if (scaled > UINT64_MAX)
~ ^~~~~~~~~~
/usr/include/sys/stdint.h:123:21: note: expanded from macro 'UINT64_MAX'
^~~~~~~~~~~~~~~~~~~~~
1 error generated.
gmake: *** [Makefile:254: common/amount.o] Error 1
bsd$
```
Fixes: #4044
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: delpay a new method to delete the payment completed or failed.
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
We can use a fixed value and close the channel if they don't cover their
amount; this wasn't really helping with anything other than setting a
floor for an expected feerate
Greatly simplify the changeset API. Instead of 'diff' we simply generate
the changes.
Also pulls up the 'next message' method, as at some point the
interactive tx protocol will be used for other things as well
(splices/closes etc)
Suggested-By: @rustyrussell
v2 of channel open uses the channel revocation basepoints to calculate
the channel_id, instead of the funding_txid + outnum
Moving away from the funding_txid opens the way for splicing + rbf
v2 channel open uses a different method to derive the channel_id, so now
we save it to the database so that we dont have to remember how to
derive it for each.
includes a migration for existing channels
There's a lot of it, and it means we can't `make check-source` on
these files.
Also bring bolt quotes up-to-date.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's now only needed by devtools/mkfunding, so include a reduced one
there, and this also means we remove tx_spending_utxos().
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This removes the reservation cleanup at startup, too, now they're all
using 'reserved_til'.
This changes test_withdraw, since it asserted that outputs were marked
spent as soon as we broadcast a transaction: now they're reserved until
it's mined. Similarly, test_addfunds_from_block assumed we'd see funds
as soon as we broadcast the tx.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `withdraw` now randomizes input and output order, not BIP69.
This avoids overwriting the ones in git, and generally makes things neater.
We have convenience headers wire/peer_wire.h and wire/onion_wire.h to
avoid most #ifdefs: simply include those.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We're going to make experimental versions of these completely separate files.
Also remove the dependency on the Makefile itself: it simply causes
unnecessary churn. We can always force-rebuild when we change a rule.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We create ALL_PROGRAMS, ALL_TEST_PROGRAMS, ALL_C_SOURCES and
ALL_C_HEADERS. Then the toplevel Makefile knows which are
autogenerated (by wildcard), so it can have all the rules to clean
them or check the source as necessary.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The user supplies callbacks to do channel selection and comparison.
Note that this continues to map the entire network; not just to the
source, for use with random routing.
Benchmarks: (using current mainnet gossip store)
/devtools/route gossip-store-2020-07-27 all 03c981ed4ad15837f29a212dc8cf4b31f274105b7c95274a41449bf496ebd2fe10 | grep 'Time to find path'
With nothing (i.e. DEVELOPER build)
Averages 17ms
With -Og (i.e. standard non-DEVELOPER build)
Averages 14ms
With -O3 -flto:
Averages 4ms
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I went overboard on optimization. I am so sorry:
1. Squeezed channel min/max into 16 bits.
2. Uses mmap and leaves node_ids in the file.
3. Uses offsets instead of pointers where possible.
4. Uses custom free-list to allocate inside arrays.
5. Ignores our autogenerated marshalling code in favor of direct derefs.
6. Carefully aligns everything so we use minimal ram.
The result is that the current gossip_store:
- load time (-O3 -flto laptop): 40msec
- load time (-g laptop i.e. DEVELOPER=0): 60msec
- load time (-O0 laptop i.e. DEVELOPER=1): 110msec
- Total memory: 2.6MB:
- 1.5MB for the array of channels
- 512k for the channel htable to map scid -> channel.
- 320k for the node htable to map nodeid -> node.
- 192k for the array of channels inside each node
- 94k for the array of nodes
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Note that other directories were explicitly depending on the generated
file, instead of relying on their (already existing) dependency on
$(LIGHTNINGD_HSM_CLIENT_OBJS), so we remove that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
See https://github.com/lightningnetwork/lightning-rfc/pull/767
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Protocol: channels now pruned after two weeks unless both peers refresh it (see lightning-rfc#767)
The jsmn parser is a beautiful piece of code. In particular, you can parse
part of a string, then continue where you left off.
We don't take advantage of this, however, meaning for large JSON objects
we parse them multiple times before finally having enough to complete.
Expose the parser state and tokens through the API, so the caller can pass
them in repeatedly. For the moment, every caller is allocates each time
(except the unit tests).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We're going to change the API on the more complete JSON parser, so
make and use a simple API for the easy cases.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Our psbt input/output comparison functions use serialization to compare
the things, but if there's a map with things in it and the map isn't
sorted exactly the same, it's highly likely you'll mark an identical inputs
as different.
To fix this, we sort all the input/output maps before linearizing them.
There's no stable ordering on unknown serialization, so linearizing
identical but mis-ordered unknown data will lead to 'wrong' results.
Instead, we just ignore any data that's in the psbt unknown struct.
There's probably also problems here with other PSBT maps. Really, this
needs a finer grained comparison function .... fuck
includes facilities for
- sorting psbt inputs by serial_id
- sorting psbt outputs by serial_id
- adding a serial_id
- getting a serial_id
- finding the diffset between two psbts
- adding a max_len to a psbt input
- getting a max_len from a psbt input
We need to remember this in the db (it's a P2WSH for option_anchor_outputs),
and we need to set nSequence to 1 to spend it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This also means we subtract 660 satoshis more everywhere we subtract
the base fee (except for mutual close, where the base fee is still
used).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
HTLC fees increase (larger weight), and the fee paid by the opener
has to include the anchor outputs (i.e. 660 sats).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This simplifies our test matrix, as we never have to handle talking
to peers that specify one but not the other.
This is particularly important for option_anchor_outputs which
assumes option_static_remotekey.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
```
E Global errors:
E - Node /tmp/ltests-o5mr9txw/test_htlc_out_timeout_1/lightning-1/ has memory leaks: [
E {
E "backtrace": [
E "ccan/ccan/tal/tal.c:442 (tal_alloc_)",
E "ccan/ccan/tal/tal.c:471 (tal_alloc_arr_)",
E "common/json_helpers.c:182 (json_add_address)",
E "common/json_helpers.c:242 (json_add_address_internal)",
E "lightningd/peer_control.c:1659 (json_getinfo)",
E "lightningd/jsonrpc.c:598 (command_exec)",
E "lightningd/jsonrpc.c:708 (rpc_command_hook_callback)",
E "lightningd/plugin_hook.c:278 (plugin_hook_call_)",
E "lightningd/jsonrpc.c:785 (plugin_hook_call_rpc_command)",
E "lightningd/jsonrpc.c:864 (parse_request)",
E "lightningd/jsonrpc.c:954 (read_json)",
E "ccan/ccan/io/io.c:59 (next_plan)",
E "ccan/ccan/io/io.c:435 (io_do_always)",
E "ccan/ccan/io/poll.c:300 (handle_always)",
E "ccan/ccan/io/poll.c:377 (io_loop)",
E "lightningd/io_loop_with_timers.c:24 (io_loop_with_timers)",
E "lightningd/lightningd.c:1013 (main)"
E ],
E "label": "common/json_helpers.c:182:char[]",
E "parents": [
E "common/json_stream.c:29:struct json_stream",
E "ccan/ccan/io/io.c:91:struct io_conn",
E "lightningd/lightningd.c:116:struct lightningd"
E ],
E "value": "0x555e17b303e8"
E }
E ]
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This prevents recompiling everything when you are changing just a doc, or
touching only one file among hundreds of sources, just because the
`gen_version.h` is changed, especially since only one source actually
depends on that header.
It's not all that rare to do these operations, and requiring annotations
for it is a little painful.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `delinvoice` will now report specific error codes: 905 for failing to find the invoice, 906 for the invoice status not matching the parameter.
Technically, they could do this themselves, but it's much nicer to have one
place to do it (and it makes sure we get the required information into the
PSBT, which is actually not entirely accessible through listfunds, as that
doesn't want to consult with the HSM for close outputs).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON RPC: new low-level coin selection `fundpsbt` routine.
We don't preserve detailed asset information at the moment, so provide a
way to convert from a sat to an amount_asset struct.
We also need a way to convert from an 'amount_asset' to a 'value' for
elements, which for explicit (i.e. non-blinded) asssets is a 0x01 prefix
plus the big-endian encoded value.
If you used feerate=750, instead of feerate="750" it didn't work, since the
token is not a string.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: JSON RPC: `withdraw` and `txprepare` `feerate` can be a JSON number.
These are pulled from wallet/wallet.c, with the fix now that we grind sigs.
This reduces the fees we pay slightly, as you can see in the coinmoves changes.
I now print out all the coin moves in suitable format before we match:
you only see this if the test fails, but it's really helpful.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
libwally's API requires us to pass in NULL pointers if the array size is
zero, so we update our array from wire-er to comply with this
requirement
[ Added fix to avoid tal_resize() of NULL -- RR ]
Jan Sarenik
Rusty Russell
Antoine Poinsot
Matt Whitlock
Vincenzo Palazzo
niftynei
ZmnSCPxj jxPCSnmZ
Christian Decker