This was put in late 2019, and @t-bast says Eclair doesn't ignore their
errors and has had no issues.
It also conflicts with https://github.com/lightning/bolts/pull/932
which suggests you *should* fail when you receive an error.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In the case where the peer sends an error (and hangs up) immediately
after init, connectd *doesn't actually read the error* (even after all the
previous fixes so it actually receives the error!).
This is because to tried to first write WIRE_CHANNEL_REESTABLISH, and
that fails, so it never tries to read. Generally, we should ignore
write failures; we'll find out if the socket is closed when we read
nothing.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
msg_queue was originally designed for inter-daemon comms, and so it has
a special mechanism to mark that we're trying to send an fd. Unfortunately,
a peer could also send such a message, confusing us!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
dev_blackhole_fd was a hack, and doesn't work well now we are async
(it worked for sync comms in per-peer daemons, but now we could sneak
through a read before we get to the next write).
So, make explicit flags and use them. This is much easier now we
have all peer comms in one place.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We now let gossipd do it.
This also means there's nothing left in 'struct per_peer_state' to
send across the wire (the fds are sent separately), so that gets
removed from wire messages too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We actually intercept the gossip_timestamp_filter, so the gossip_store
mechanism inside the per-peer daemon never kicks off for normal connections.
The gossipwith tool doesn't set OPT_GOSSIP_QUERIES, so it gets both, but
that only effects one place.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
channeld can't do it any more: it's using local sockets. Connectd
can do it, and simply does it by type.
Amazingly, on my machine the timing change *always* caused
test_channel_receivable() to fail, due to a latent race.
Includes feedback from @cdecker.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As connectd handles more packets itself, or diverts them to/from gossipd,
it's the only place we can implement the dev_disconnect logic.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Now connectd is doing the crypto, we can use normal wire io. We
create helper functions to clearly differentiate between "peer" comms
and intra-daemon comms though.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We temporarily hack to sync_crypto_write/sync_crypto_read functions to
not do any crypto, and do it all in connectd.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. tal_strndup(.., str, strlen(str)) == tal_strdup()
2. tal_strdup also takes(), so document that.
3. Avoid passing 'struct sha256' on the stack: use ptr.
4. Generally, structures shouldn't keep pointers to things they don't own.
In this case, mvt->node_id.
5. Make payment_hash a pointer, since NULL is more natural than an all-zero
hash.
And add NON_NULL_ARGS() to the functions; it's cumbersome, but make it
fairly clear what params are optional.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Once connectd is doing this, we can't close as soon as we send,
and in fact we can't do 'fail write' either.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The "read until closed" trick doesn't work if the other end doesn't
close (as found in the next patch, where we use DEV_DISCONNECT_DISABLE_AFTER).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
connectd is going to end up using this do demux; make it fast and complete.
Fixing this reveals a problem in openingd: it now extracts the channel_id
from funding_signed (which is where we transition off the temporary), and
gets upset. So fix that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We need to stash/save the amount of the lease fees on a leased channel,
we do this by re-using the 'push' amount field on channel (which is
technically correct, since we're essentially pushing the fee amount to
the peer).
Also updates a bit of how the pushes are accounted for (pushed to now
has an event; their channel will open at zero but then they'll
immediately register a push event).
Leases fees are treated exactly the same as pushes, except labeled
differently.
Required adding a 'lease_fee' field to the inflights so we keep track of
the fee for the lease until the open happens.
We record the amount of fees collected for a routed payment. For
simplicity's sake on the data agg side, we record the fee payment on
*BOTH* the incoming htlc and the outgoing htlc. Note that this results
in double counting if you add up the fees from both an in-routed and
out-routed payment.
Get rid of the 'movement_idx', since we replay events now.
Since we're removing a field from the 'coin_movement' event emission, we
bump the version type.
Changelog-Updated: `coin_movements` events have been revamped and are now on version 2.
The old model of coin movements attempted to compute fees etc and log
amounts, not utxos. This is not as robust, as multi-party opens and dual
funded channels make it hard to account for fees etc correctly.
Instead, we move towards a 'utxo' view of the onchain events. Every
event is either the creation or 'destruction' of a utxo. For cases where
the value of the utxo is not (fully) debited/credited to our account, we
also record the output_value. E.g. channel closings spend a utxo who's
entire value we may not own.
Since we're now tracking UTXOs onchain, we can now do more complex
assertions about the onchain footprint of them. The integration tests
have been updated to now use more 'chain aware' assertions about the
ending state.
we're pivoting from a txid based world to a outpoint based world. every
coin movement (onchain) will correspond with a outpoint; only the spend
of an outpoint will have a tx_txid
We're not going to do 'spend tracks' any more; instead we'll emit an
event whenever an output is included in a broadcast tx
(even if the broadcast fails!!)
Suggested-by: Rusty Russell
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Changelog-Changed: Support hsm specific error error code in lightning-cli
And turn "" includes into full-path (which makes it easier to put
config.h first, and finds some cases check-includes.sh missed
previously).
config.h sets _GNU_SOURCE which really needs to be done before any
'#includes': we mainly got away with it with glibc, but other platforms
like Alpine may have stricter requirements.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Various unit tests were creating temporary files unconditionally in /tmp
and were not cleaning up after themselves. Introduce a new variant of
mkstemp(3p) that respects the TMPDIR environment variable, and use it in
the offending unit tests. This allows each test run to use a dedicated
TMPDIR that can be cleaned up after the run.
Changelog-None
Signed-off-by: Matt Whitlock <c-lightning@mattwhitlock.name>
As of 2b923a0367c5f9154fcec706e3302cc4658dd889.
Recurrence quotes need to be marked separately, since they're no longer
in offers main bolt.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This builds on the enctlv vectors, but actually goes all the way
to creating a modern onionmessage.
Thanks to Thomas H for corrections!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is from 6e99c5feaf60cb797507d181fe583224309318e9
We renamed the enctlv field to encrypted_recipient_data in the spec, and the
new onion_message is message 513. We don't handle it until the next patch.
Two renames:
1. blinding_seed -> blinding_point.
2. enctlv -> encrypted_recipient_data.
We don't do a compat cycle for our JSON APIs for these experimental
features only used by our own plugins, we just rename.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Temporarily disable sendpay_blinding test which uses obsolete onionmsg;
there's still some debate on the PR about how blinded HTLCs will work.
Changelog-EXPERIMENTAL: onionmessage: removed support for v0.10.1 onion messages.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
No idea why TCSAFLUSH was used, could not find anything in PR comments.
Also cannot explain exactly what causes the problem, but the hang can be reproduced
*with* TCSAFLUSH and not with TCSANOW.
According to termios doc:
TCSANOW
the change occurs immediately.
TCSAFLUSH
the change occurs after all output written to the object referred by fd has been
transmitted, and all input that has been received but not read will be discarded
before the change is made.
This happened in my tal_dump(), and I couldn't see how we ended up
with object having more than one "backtrace". Adding asserts that we
never added a second backtrace didn't trigger.
Finally I wondered if we were tal_steal() backtraces, and sure enough
we do that blinding in one place: libwally wrapping. So fix that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
valgrind locally complains about the allocations in autodata leaking:
```
==138200== 16 bytes in 1 blocks are still reachable in loss record 1 of 2
==138200== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==138200== by 0x10D41A: autodata_register_ (autodata.c:20)
==138200== by 0x10E7B8: register_autotype_type_to_string (type_to_string.h:79)
==138200== by 0x10F5CA: register_one_type_to_string0 (block.c:259)
==138200== by 0x19734C: __libc_csu_init (in /home/rusty/devel/cvs/lightning/common/test/run-route-specific)
==138200== by 0x4A3D03F: (below main) (libc-start.c:264)
==138200==
==138200== 176 bytes in 1 blocks are still reachable in loss record 2 of 2
==138200== at 0x483DFAF: realloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==138200== by 0x10D472: autodata_register_ (autodata.c:26)
==138200== by 0x122D37: register_autotype_type_to_string (type_to_string.h:79)
==138200== by 0x122F1F: register_one_type_to_string0 (node_id.c:50)
==138200== by 0x19734C: __libc_csu_init (in /home/rusty/devel/cvs/lightning/common/test/run-route-specific)
==138200== by 0x4A3D03F: (below main) (libc-start.c:264)
==138200==
make: *** [Makefile:638: unittest/common/test/run-route-specific] Error 7
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
blob[] is really a string from the commandline; leave it as a char.
And parsing is much simpler than this code makes it seem!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This surprised me, since the CHANGELOG for [0.8.2] said:
We now announce multiple addresses of the same type, if given. ([3609](https://github.com/ElementsProject/lightning/pull/3609))
But it lied!
Changelog-Fixed: We really do allow providing multiple addresses of the same type.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
October was the date Torv2 is no longer supported by the Tor Project;
it will probably not work at all by next release, so we should remove
it now even though it's not quite the 6 months we prefer for
deprecation cycles.
I still see 110 nodes advertizing Torv2 (vs 10,292 Torv3); we still
parse and display it, we just don't advertize or connect to it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
for every new added htlc, check that adding it won't go over our 'dust
budget' (which assumes a slightly higher than current feerate, as this
prevents sudden feerate changes from overshooting our dust budget)
note that if the feerate changes surpass the limits we've set, we
immediately fail the channel.
If we're over the dust limit, we fail it immediatey *after* commiting
it, but we need a way to signal this throughout the lifecycle, so we add
it to htlc_in struct and persist it through to the database.
If it's supposed to be failed, we fail after the commit cycle is
completed.
To reduce the surface area of amount of a channel balance that can be
eaten up as htlc dust, we introduce a new config
'--max-dust-htlc-exposure-msat', which sets the max amount that any
channel's balance can be added as dust
Changelog-Added: config: new option --max-dust-htlc-exposure-msat, which limits the total amount of sats to be allowed as dust on a channel
This also inadvertently fixes a latent bug: before this patch, in the
`subd` function in `lightningd/subd.c`, we would close `execfail[1]`
*before* doing an `exec`.
We use an EOF on `execfail[1]` as a signal that `exec` succeeded (the
fd is marked CLOEXEC), and otherwise use it to pump `errno` to the
parent.
The intent is that this fd should be kept open until `exec`, at which
point CLOEXEC triggers and close that fd and sends the EOF, *or* if
`exec` fails we can send the `errno` to the parent process vua that
pipe-end.
However, in the previous version, we end up closing that fd *before*
reaching `exec`, either in the loop which `dup2`s passed-in fds (by
overwriting `execfail[1]` with a `dup2`) or in the "close everything"
loop, which does not guard against `execfail[1]`, only
`dev_disconnect_fd`.
It's probably not worth fixing for the other daemons.
Changelog-Changed: JSON-RPC: `ping` now only works if we have a channel with the peer.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We keep the now-removed chains field, and in deprecated mode, we set it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: bolt12: `chains` in invoice_request and invoice is deprecated, `chain` is used instead.
Main changes are:
1. Uses point32 instead of pubkey32.
2. Uses issuer instead of vendor.
3. Uses byte instead of u8.
4. blinded_path num_hops is now a byte, not u16 (we don't use that yet!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: bolt12: `vendor` is deprecated: the field is now called `issuer`.
The latest ones use lno, not lni (this unit tests loads from
../lightning-rfc, silently exiting if it doesn't have the test
vector).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
By popular merge-hell demand.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Build: Python is now required to build, as generated files are no longer checked into the repository.
After recent header files clean-up it was not possible to
build c-lightning 7401b2682. This patch fixes it both for
Alpine Linux and OpenBSD.
Proposed-by: nathanael <nathanael@dalliard.ch>
Changelog-None
Before:
Ten builds, laptop -j5, no ccache:
```
real 0m36.686000-38.956000(38.608+/-0.65)s
user 2m32.864000-42.253000(40.7545+/-2.7)s
sys 0m16.618000-18.316000(17.8531+/-0.48)s
```
Ten builds, laptop -j5, ccache (warm):
```
real 0m8.212000-8.577000(8.39989+/-0.13)s
user 0m12.731000-13.212000(12.9751+/-0.17)s
sys 0m3.697000-3.902000(3.83722+/-0.064)s
```
After:
Ten builds, laptop -j5, no ccache: 8% faster
```
real 0m33.802000-35.773000(35.468+/-0.54)s
user 2m19.073000-27.754000(26.2542+/-2.3)s
sys 0m15.784000-17.173000(16.7165+/-0.37)s
```
Ten builds, laptop -j5, ccache (warm): 1% faster
```
real 0m8.200000-8.485000(8.30138+/-0.097)s
user 0m12.485000-13.100000(12.7344+/-0.19)s
sys 0m3.702000-3.889000(3.78787+/-0.056)s
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is best-practice (to ensure prototypes match up), but there were a
few places we didn't (at least, directly). Make it a requirement,
either of form "foo.h" or <dir/foo.h>.
The noise is the change to our print templates.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We make it a first-class citizen internally, even though we won't use
it over the wire (at least, non-experimental builds). This scheme
follows the latest draft, in which features are flagged compulsory.
We also add several helper functions.
Since uses the *even* bits (as per latest spec), not the *odd* bits,
we have some other fixups.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We want to use this to handle the simple description for channel_type.
It also needs to handle variable-size types (just like subtypes).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This touches a lot of text, mainly to change "if `option_anchor_outputs`"
to "if `option_anchors`"
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This includes the new bolt11 test vectors, and also removes the
requirement that HTLCs be less than 2^32 msat. We keep that for now
because Electrum enforced it on receive: in two releases we will stop
that too.
So no longer warn about needing mpp in that case either.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: Protocol: No longer restrict HTLCs to
This check is going away anyway (only Electrum enforced it), but we
know that all wumbo peers expect large HTLCs to work today.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: Allow sending large HTLCs if peer offers option_support_large_channel (> 4294967295msat)
Otherwise libwally pushes the psbt-key for 'witness script' onto the
serialized version and we fail the 'is this identical' check.
Relevant line from libwally, where if bytes, we push a psbt_key.
```
static void push_typed_varbuff(unsigned char **cursor, size_t *max,
uint64_t type,
const unsigned char *bytes, size_t bytes_len)
{
if (bytes) {
push_psbt_key(cursor, max, type, NULL, 0);
push_varbuff(cursor, max, bytes, bytes_len);
}
}
```
Reported-By: @grubles
Changelog-Fixed: openchannel_signed would fail on PSBT comparison of materially identical PSBTs
After some discussion with @shesek, and my own usage, we agreed that
a more comprehensive interface, which explicitly supports grouping,
is desirable.
Thus keys are now arrays, with the semantic that a key is either a
parent or has a value, never both.
For convenience in the JSON schema, we always return them as arrays,
though we accept simple strings as arguments.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We add a generation counter, and allow update or del conditional
on a given generation.
Formalizes error codes, too, since we have more now.
Suggested-by: @shesek
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I spent an hour thinking this code had a bug (see test vector fix);
we *do* overallocate the tree, but that's deliberate: we fill with NULLs
and ignore on recursion.
The Merkle recurse comment had an out-by-one, and the NULL-pad
technique used was uncommented.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We were printing out the final merkle root before calculating it,
resulting in the final one being the same as the previous.
Reported-by: Aditya Sharma
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@shesek points out that we called this field created_at in bolt11 decode,
which makes more sense anyway.
Changelog-EXPERIMENTAL: bolt12 decode `timestamp` field deprecated in favor of new name `created_at`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When a request comes through, we forward it over to the funder who
uses the currently set policy to figure out how to handle it.
Includes small update to the policy engine which decides whether or not
to fund a request.
Changelog-Experimental: Plugins: `openchannel2` hook now includes optional fields for a channel lease request
