There are hardly any lightningd-specific JSON functions: all that's left
are the feerate ones, and there's already a comment that we should have
a lightningd/feerate.h.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have them split over common/param.c, common/json.c,
common/json_helpers.c, common/json_tok.c and common/json_stream.c.
Change that to:
* common/json_parse (all the json_to_xxx routines)
* common/json_parse_simple (simplest the json parsing routines, for cli too)
* common/json_stream (all the json_add_xxx routines)
* common/json_param (all the param and param_xxx routines)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Since it's a deprecation, we simply ignore one, rather than properly
checking they match etc.
Fixes: #5386
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The gossip_store version was bumbed to 10 in PR #5239 and #5375 has
apparently not been rebased on top of that when it was merged, causing
the `run-gossmap_canned` test to fail when parsing the v9 gossip
store.
Changelog-None
This will be used to decouple internal use of gossip from what is
passed to gossip peers. Updates GOSSIP_STORE_VERION to 10.
Changelog-Changed: gossip_store updated to version 10.
Usually we won't see this, since private is deleted. But we could
have already read the private channel before that. Handle it properly.
(Tested by removing the gossip_store deletion code and making sure
this worked).
We have to fix up the test, which announces a channel twice!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Thanks to @zerofeerouting for another report.
"desc" here is the sanitized message, eg:
"ERROR error channel 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef: internal error"
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Prior to 0.11.0 we had cases where we would treat errors
as warnings: regretfully, this is still needed. This message
in particular has been widely reported, and it now causes
channel force closes.
Downgrade and log. I did insert some snarky log message earlier,
but hey, I'm sure CLN has done worse things to our peers!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Protocol: treat LND "internal error" as warnings, not force close events (as we did in v0.10).
@whitslack complained of large CPU usage by connectd at startup;
I ran perf record on connectd on my machine (which sees a little spike, only)
and I see the cost of reading and discarding the entries:
```
- 95.52% 5.24% lightning_conne lightning_connectd [.] gossip_store_next
- 90.28% gossip_store_next
+ 40.27% tal_alloc_arr_
+ 22.78% tal_free
+ 11.74% crc32c
+ 9.32% fromwire_peektype
+ 4.10% __libc_pread64 (inlined)
1.70% be32_to_cpu
```
Much of this is caused by the search for our own gossip: keeping this separately
would be even better, but this fix is minimal.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: connectd: reduce initial CPU load when connecting to peers.
This changes many fields: in non-deprecated mode, they're now raw integers.
This was always the intention, but the transition was never completed.
Suggested-By: @ShahanaFarooqui
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: "_msat" fields can be raw numbers, not "123msat" strings (please handle both!)
Changelog-Deprecated: JSON-RPC: "_msat" fields as "123msat" strings (will be only numbers)
We had json_add_amount_msat_only(), which was designed to be used to
print out msat fields, if we had sats.
However, we misused it, so split it into the three different cases:
1. json_add_amount_sat_msat: We are using it correctly, with a field called
xxx_msat.
2. json_add_amount_sats_deprecated: We were using it wrong, so deprecate
the old field and create a new one which does end in _msat.
3. json_add_sats: we were using it to hand sats as a JSON parameter to an
interface, where "XXXsat".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: Plugins: `rbf_channel` and `openchannel2` hooks `their_funding` (use `their_funding_msat`)
Changelog-Deprecated: Plugins: `openchannel2` hook `dust_limit_satoshis` (use `dust_limit_msat`)
Changelog-Deprecated: Plugins: `openchannel` hook `funding_satoshis` (use `funding_msat`)
Changelog-Deprecated: Plugins: `openchannel` hook `dust_limit_satoshis` (use `dust_limit_msat`)
Changelog-Deprecated: Plugins: `openchannel` hook `channel_reserve_satoshis` (use `channel_reserve_msat`)
Changelog-Deprecated: Plugins: `channel_opened` notification `amount` (use `funding_msat`)
Changelog-Deprecated: JSON-RPC: `listtransactions` `msat` (use `amount_msat`)
Changelog-Deprecated: Plugins: `htlc_accepted` `forward_amount` (use `forward_msat`)
Per BIP-0171, the signature map is of pubkey to "The signature as would
be pushed to the stack from a scriptSig or witness".
Fixes 5298
Changelog-Fixed: PSBT: Fix signature encoding to comply with BIP-0171.
Signed-off-by: Jon Griffiths <jon_p_griffiths@yahoo.com>
This likely lead to a number of false errors when attempting to
route. We deemed a channel to be unusable as soon as either direction
isn't usable. This is bad since it excludes not only zeroconf
channels (which have different scids for the two directions), but it
also excludes any channel that we haven't seen an update from
yet. This was likely introduced when attemting to exclude nodes that
haven't sent a disable, but their peer has, but this is not necessary
as the unresponsive node would be marked as isolated by all its peers,
so we don't need to artificially mark a channel direction as disabled
when really we can't even enter the node to traverse the channel in
that direction.
Changelog-Fixed: routing: Fixed an issue where we would exclude the entire channel if either direction was disabled, or we hadn't seen an update yet.
After this, we can exactly reproduce the vectors (in DEVELOPER mode).
1. Move payment_metadata position to match test vector.
2. Create flag to suppress `c` field production.
3. Some vectors put secret before payment_hash, hack that in.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We do this (send warnings) in almost all cases anyway, so mainly this
is a textual update, but there are some changes:
1. Send ERROR not WARNING if they send a malformed commitment secret.
2. Send WARNING not ERROR if they get the shutdown_scriptpubkey wrong (vs upfront)
3. Send WARNING not ERROR if they send a bad shutdown_scriptpubkey (e.g. p2pkh in future)
4. Rename some vars 'err' to 'warn' to make it clear we send a warning.
This means test_option_upfront_shutdown_script can be made reliable, too,
and it now warns and doesn't automatically close channel.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This doesn't have an effect now (except in experimental mode), but it
will when we support anchors. So we deprecate the use of those in the
close command too.
For experimental mode we have to avoid using p2pkh; adapt that test.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: JSON-RPC: `shutdown` no longer allows p2pkh or p2sh addresses.
The signatures on the new examples are sometimes different from what we produce though?
They're valid, however.
And one example has an unneeded feature 5-bit; it's not *wrong*, but
it's not optimal.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Partial revert of 43a833e405
"lightningd: remove support for legacy onion format."; we restore the
ability to decode legacy onions for forwarding, but not to generate them.
(We don't accept them properly since making payment_secret compulsory
anyway, so no real change there!)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: Protocol: ... but we still forward legacy HTLC onions for now.
I removed these prematurely: we *haven't* had a release since
introducing them!
This consists of reverting d15d629b8b
"plugins/fetchinvoice: remove obsolete string-based API." and
plugins/fetchinvoice: remove obsolete string-based
API. "onion_messages: remove obs2 support."
Some minor changes due to updated fromwire_tlv API since they
were removed, but not much.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: REVERT: Removed backwards compat with onion messages from v0.10.1.
There's a race under CI, where a channel is deleted then we see the
channel_update in the gossip store. We assumed this wouldn't happen,
but it can!
```
[gw1] [ 95%] FAILED tests/test_connection.py::test_multichan
[gw1] [ 95%] ERROR tests/test_connection.py::test_multichan
...
> raise ValueError(str(errors))
E ValueError:
E Node errors:
E - lightningd-3: had BROKEN messages
E - lightningd-3: Node exited with return code 1
E Global errors:
...
lightningd-3: 2022-03-28T00:11:42.160Z DEBUG wallet: Owning output 0 100000sat (SEGWIT) txid 30616903feba1839a3834e2b3b6123759ce1fe0d76414ca77e2dbc17414772e0 CONFIRMED
lightningd-3: 2022-03-28T00:11:42.392Z DEBUG hsmd: Client: Received message 5 from client
lightningd-3: 2022-03-28T00:11:42.393Z DEBUG hsmd: new_client: 2
lightningd-3: 2022-03-28T00:11:42.398Z INFO plugin-topology: Killing plugin: exited during normal operation
lightningd-3: 2022-03-28T00:11:42.400Z **BROKEN** plugin-topology: Plugin marked as important, shutting down lightningd!
...
----------------------------- Captured stderr call -----------------------------
topology: update for channel 105x1x1 not found!
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Means that field is now optional in JSON output.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `delinvoice` has a new parameter `desconly` to remove description.
LNURL wants this so they can include images etc in descriptions.
Replaces: #4892
Changelog-Added: JSON-RPC: `invoice` has a new parameter `deschashonly` to put hash of description in bolt11.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It was tlv_fields_valid that wanted a non-const: now that's gone, we
can make this correctly const.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Callers were supposed to call "tlv_fields_valid" after fromwire_tlv,
but few did. Make this the default, and call the underlying function
directly where we want to be more flexible (one place).
This loses the ability to allow misordered fields, or to pass through
*any* even fields. We restore that for special cases in the next
patch.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Requiring the caller to allocate them is ugly, and differs from
other types.
This means we need a context arg if we don't have one already.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
No more "towire_offer", but "towire_tlv_offer".
This means we double-up on the unfortunately-named `tlv_payload` inside
the onion, but we should rename that in the spec when we remove
old payloads.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In particular, this changes the name of a field in invoice_request:
`payer_signature` becomes simply `signature`. So we allow both for
now, and send the old one unless deprecated_apis is disabled.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When we did fill them in, we filled them in wrong: the offset should be
the offset in the message, not the field number!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Also has to fix up tests.
Changelog-Fixed: cli doesn't required anymore to confirm the password if the `hsm_secret` is already encrypted.
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Things allocated by libwally all get the tal_name "wally_tal",
which cost me a few hours trying to find a leak.
In the case where we're making one of the allocations the parent
of the others (e.g. a wally_psbt), we can do better: supply a name
for the tal_wally_end().
So I add a new tal_wally_end_onto() which does the standard
tal_steal() trick, and also changes the (typechecked!) name.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As per proposal in https://github.com/lightning/bolts/pull/962
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: protocol: support for legacy onion format removed, since everyone supports the new one.
We often hand an exclude pointer (usually the current command) to
memleak. But when we encountered this we would stop iterating, rather
than just ignore it: this means we would often ignore significant siblings.
In particular, fixing this (which has always been there) reveals many
previously-undetected leaks.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We allocate the default, then callback allocates over the top. Mark
params with a default, so we can free that when it's called.
(We can't do this generally, since not all param args are actually
pointers to pointers, though opt_param_def has to be).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It looks like decode_c doesn't set have_c unlike the other decode_
methods. At the start of the function, decode_c checks have_c to see if
it's set, but it is never set. It seems like this could allow for
duplicate c tags, which is probably not intended.
Signed-off-by: William Casarin <jb55@jb55.com>
And in particular, fix onchaind grinding code which used the
actual number of inputs and outputs (which already includes the
fee output); that breaks with the next patch which fixes other
calculations.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The blockheight is zero though, since these aren't included in a block
yet.
We also don't issue an 'external' deposit event if we can tell that the
address you're sending to actually belongs to our wallet (we'll issue a
deposit event when it gets included in a block)
If a coin move concerns an external account, it's really useful to know
which 'internal' account initiated the transfer.
We're about to add a notification for withdrawals, so we can use this to
track wallet pushes to outside addresses
Changelog-Added: JSONRPC: `coin_movement` to 'external' accounts now include an 'originating_account' field
connectd does this internally now using ccan/io, with appropriate
credit for ZmnSCPxj who wrote this code in the first place.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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
If there's a rate-card for liquidity, we don't know about it until
after startup (the plugin *should* call us at init to tell us what their
current rates are)
This is in preparation for removing support (next release?).
Changelog-Changed: Protocol: We now assume nodes support TLV onions (non-legacy) unless we have a node_announcement which says they don't.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And fix up the mess we'd made:
1. We didn't order merkles by lesser-first.
2. We didn't correctly construct tree with last nodes on shortest path.
Now we have tests!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: protocol: offer signature format changed.
We usually assume we're fetching an invoice we are going to pay, so we
look up the previous payment for the payer key, and other sanity
checks.
This adds a developer option to fetchinvoice, which allows it to force
its own payer key, which it uses to sign directly and bypasses these
checks.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As per latest spec revision.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: BOLT12 offers can now be unsigned, for really short QR codes.
100+ is for experimentation, modern spec practice is to assign feature bits
sequentially as PRs get added, to avoid later renumbering.
Still respect the old bit for now.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Let the callers do that (only channeld needs to do this).
We temporarily send an error on unknown reestablish in openingd, as
this mimic previous behavior and avoids breaking tests (it does leave
a BROKEN message in the logs though, so
test_funding_external_wallet_corners needs to ignore that for now.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It handles all the cases of retransmission, and in the normal case
retransmits shutdown and immediately returns for us to run closingd.
This is actually far simpler and reduces code duplication.
[ Includes fixup to stop warn_unused_result from Christian ]
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Protocol: We could get stuck on signature exchange if we needed to retransmit the final revoke_and_ack.
This allows us to ensure a packet is read by the other end, but we
don't read anything else from them or write anything to them.
Using '+' is similar, but because it closes the connection, the peer
might notice before receiving the packet (such as if it does a write).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We were accidentally using the port that the tor service was
connecting to, not the /torport the user said to use.
Fixes: #4597
Reported-by: @openoms
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Config: `addr` autotor and statictor /torport arguments now advertized correctly.
This suppresses some "may-be-uninitialized" warnings later. It makes
gcc pickier about how we ignore the result though :(
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Only v0 has specific length restrictions: taproot is v1 32 bytes long,
but explicitly other lengths remain undefined.
I noticed that I added option_shutdown_anysegwit as EXPERIMENTAL in
the last release, but didn't CHANGELOG it. Then I changed it to
non-experimental as a spec update, but didn't CHANGELOG it then
either, so let's do that now!
Changelog-Added: Protocol: We now send and accept `option_shutdown_anysegwit` so you can close channels to v1+ segwit addresses.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `withdraw`, `close` (and others) now accept taproot (and other future) segwit addresses.
I did this by copying the updated bech32 code, and then re-patching in
our minor changes:
1. Headers modded (we need size_t)
2. Explicit length for bech32_encode/decode (not 90).
3. Exposing and bech32_ prefix for convert_bits, charset, charset_rev.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We don't actually set desired_type yet, but this handles it.
Changelog-EXPERIMENTAL: Protocol: we can now upgrade old channels to `option_static_remotekey` from https://github.com/lightningnetwork/lightning-rfc/pull/868
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
For quiescence, we can't have sent any updates at all.
But for upgrades on reconnection, we may have already added
uncommitted HTLCs for retransmission, but they don't count towards
"are we quiesced" since they're not sent yet.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This includes anysegwit and the updated HTLC tiebreak test vector. It
also adds explicit wording for invalid per_commitment_secret (which
nicely matches our code already!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Tor v2 hidden services have been deprecated for a while:
https://blog.torproject.org/v2-deprecation-timeline .
This prevents user from being able to set them in the configuration
and to connect to them while still letting us be able to parse them
for gossip.
Changelog-Deprecated: lightningd: v2 Tor addresses. Use v3. See https://blog.torproject.org/v2-deprecation-timeline.
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
This takes an extra 8 bytes per channel, but means we can go back and
get more information about them; this is implemented in
gossmap_chan_get_update_details() which is what listchannels will need.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Equivalent to gossipd/test/run-find_route.c and gossipd/test/run-find_route-specific.c
except they use gossmap.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There are several reports of desynchronization with LND here; a simple
approach is to only have one feerate change in flight at any time.
Even if this turns out to be our fault, it's been a historic area of
confusion, so this restriction seems reasonable.
Changelog-Fixed: Protocol: Don't create more than one feerate change at a time, as this seems to desync with LND.
Fixes: #4152
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The main change which affects us is that 2016 blocks to forget a channel
is a fixed number in the spec; we make this clear by renaming the
(developer-only) max_funding_unconfirmed to dev_max_funding_unconfirmed
and making it compile DEVELOPER only.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
They're not defined to be, though we've not seen this on Linux (testing
showed that it is page-level atomic, which means it can still happen across
page boundaries though!). This was pointed out by whitslack in
https://github.com/ElementsProject/lightning/issues/4288
In practice, this just means not complaining when it happens, and also
not trying to get tricky to use it on MacOS (we can safely seek & write,
since we're single-threaded).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: Removed bogus UNUSUAL log about gossip_store 'short test'.
You can now activate dual-funded channels using the
`--experimental-dual-fund` flag
Changelog-Changed: Config: `--experimental-dual-fund` runtime flag will enable dual-funded protocol on this node
clang 10.0.0 (erroneously?) claims an enum_side cannot be >= NUM_SIDES.
Make it clear that we're testing the raw u8 for validity.
Fixes: #4409
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's not unheard of for people to give the wrong funding tx to us,
getting their funds stuck. Interestingly, we can allow mutual close
using a different txid and output number as long as they (solely)
funded the channel, and the channel hasn't been used.
This defines a "play area" feature to do just that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> If the peer's revocation basepoint is unknown (e.g. `open_channel2`),
> a temporary `channel_id` should be found by using a zeroed out basepoint
> for the unknown peer.
We consolidate to the latest/singular RFC patch for dual-funding, so
there's just a single patchfile for the change. Plus we move back to the
opener setting the desired feerate, the accepter merely declines to
participate if they disagree with the set rate.
Caused by missing common/iso4217.c from common/Makefile:
```
In file included from ./common/iso4217.h:4,
from common/iso4217.c:3:
./wire/wire.h:7:10: fatal error: secp256k1_recovery.h: No such file or directory
7 | #include <secp256k1_recovery.h>
| ^~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make: *** [Makefile:265: common/iso4217.o] Error 1
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This was likely missed because we don't run the tests under valgrind anymore
due to time constraints. I do run them on a semi-regular basis, which is why
I found this.
No more sending "all-channel" errors; in particular, gossipd now only
sends warnings (which make us hang up), not errors, and peer_connected
rejections are warnings (and disconnect), not errors.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Plugins: `peer_connected` rejections now send a warning, not an error, to the peer.
And make all the callers choose which one. In general, I prefer warn,
which lets them reconnect and try again, however some places are either
stated that they must be errors in the spec itself, or in openingd
where we abandon the channel when we close the connection anyway.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Protocol: we now send warning messages and close the connection, except on unrecoverable errors.
This is in line with the warnings draft, where all-zeroes in a
channel_id is no longer special (i.e. it will be ignored).
But gossipd would send these if it got upset with us, so it's best
practice to ignore them for now anyway.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: we treat error messages from peer which refer to "all channels" as warnings, not errors.
This takes from the draft spec at https://github.com/lightningnetwork/lightning-rfc/pull/834
Note that if this draft does not get included, the peer will simply
ignore the warning message (we always close the connection afterwards
anyway).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: we now report the new (draft) warning message.
Now we create a separate set of local mods, and apply and unapply it.
This is more efficient than the previous approach, since we can do
some work up-front. It's also more graceful (and well-defined) when a
local modification overlaps an existing one.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We should actually be including this (as it may define _GNU_SOURCE
etc) before any system headers. But where we include <assert.h> we
often didn't, because check-includes would complain that the headers
included it too.
Weaken that check, and include config.h in C files before assert.h.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The fetchinvoice and offers plugins disable themselves if the option
isn't enabled (it's enabled by default on EXPERIMENTAL_FEATURES).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: `experimental-offers` enables fetch, payment and creation of (early draft) offers.
Don't include exp directly, use an ifdef in common/bolt12
(like we do for peer and onion wiregen files).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Note that this also changes so the feature is not represented in channels,
reflecting the recent drafts.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: `experimental-onion-messages` enables send, receive and relay of onion messages.
Allow a user to switch on dual-funding without needing to compile
as EXPERIMENTAL_FEATURES.
Doesn't work yet, since everything is still behind
'EXPERIMENTAL_FEATURES' compile time flags... but useful for testing
This is experimental for now, but can eventually deprecated
'decodepay' and even decode other kinds of messages.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Our new "decode" command will also handle bolt11. We make a few cleanups:
1. Avoid type_to_string() in JSON, instead use format functions directly.
2. Don't need to escape description now that JSON core does that for us.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This makes for more useful errors. It prints where it was up to in
the guide, but doesn't print the entire JSON it's scanning.
Suggested-by: Christian Decker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In several places we want to access the first element of an array.
This uses a '[indexnum:xxx]' form which is a bit weird, but works similarly
to the way we specify member matches.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This takes a JSON-style format string, and does intelligent parsing,
removing a lot of boilerplate from code which needs to deal with JSON.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rusty Russell
adi2011
Michael Schmoock
Christian Decker
Alex Myers
Greg Sanders
Vincenzo Palazzo
Jon Griffiths
Michael Dance
Ken Sedgwick
niftynei
William Casarin
lightning-developer
ZmnSCPxj jxPCSnmZ
Simon Vrouwe
Matt Whitlock
Andrew Toth
Dustin Dettmer
W. J. van der Laan
Jan Sarenik
Antoine Poinsot
openoms
openoms
Nalin Bhardwaj