One really rough thing about how we did onchain fees is that the records update
every time a new event comes in.
The better way to do this is to create new entries for every adjustment,
so that reconciliation between printouts isn't a misery.
We add a timestamp and `update_count` to these records, so you can
roughly order them now (and have a good idea of the last time an event
that updated an onchain_fee occurred).
When the node starts up, it records missing/updated account balances
to the 'channel' events... which is kinda fucked for wallet + external
events now that i think about it but these are all treated the same
anyway so it's fine.
This is the magic piece that lets your bookkeeping data startup ok on an
already running/established node.
clightning doesn't give us any info about onchain fees (how could it?
it only knows about utxo object levels, and doesn't keep track of
how/when those are all related)
Instead, we keep running totals of the onchain fees for utxos. This
implements the master method for accounting for them, plus includes
tests to account for channel opens (across two accounts) as well as a
htlc-tx channel close.
Missing: we don't currently emit an event from cln for `withdraw`
initiated removal of funds, so the accounting for wallet -> external
funds is a bit janky. We don't account for the fees on these
transactions since we don't have the resulting 'external' event to
register them against!
Originally I (incorrectly?) assumed that since TX_COMMITMENT_SIGNED
always came before TX_SIGNATURES, we would always receive a response
from openchannel_update (w/ commitment_secured = true) before getting
notification of receipt of the peer's signatures.
But it's observable in the logs of hung tests that this in fact is a
wrong assumption -- the notification for the tx_sigs arrives at our
spender plugin before the callback from our openchannel_update RPC.
This mis-ordering causes a hang.
Luckily we're pretty much setup to handle this race already w/ states
etc, minus actually calling the method advance the plot in case we're
ready.
2022-07-26T05:37:59.4529095Z lightningd-1 2022-07-26T05:10:07.395Z DEBUG 035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d-dualopend-chan#2: peer_in WIRE_COMMITMENT_SIGNED
2022-07-26T05:37:59.4530452Z lightningd-1 2022-07-26T05:10:07.396Z DEBUG 035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d-hsmd: Got WIRE_HSMD_VALIDATE_COMMITMENT_TX
2022-07-26T05:37:59.4530719Z lightningd-1 2022-07-26T05:10:07.396Z DEBUG hsmd: Client: Received message 35 from client
2022-07-26T05:37:59.4531386Z lightningd-1 2022-07-26T05:10:07.396Z DEBUG 035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d-dualopend-chan#2: billboard: channel open: commitment received, sending to lightningd to save
2022-07-26T05:37:59.4531856Z lightningd-1 2022-07-26T05:10:07.398Z DEBUG 035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d-dualopend-chan#2: peer_in WIRE_TX_SIGNATURES
>>> 2022-07-26T05:37:59.4532553Z lightningd-1 2022-07-26T05:10:07.400Z DEBUG plugin-spenderp: mfc 60:`openchannel_peer_sigs` notice received for channel 9d145e763f08ee6f715ba7677f869cbb9580c7406f4d0b0ff3a0987efe501e13 <<<< THIS ONE WAS ASSUMED TO COME AFTER openchannel_update (next line)
2022-07-26T05:37:59.4533048Z lightningd-1 2022-07-26T05:10:07.400Z DEBUG plugin-spenderp: mfc 60, dest 0: openchannel_update 035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d returned.
2022-07-26T05:37:59.4554292Z lightningd-1 2022-07-26T05:10:07.400Z DEBUG plugin-spenderp: mfc 60: parallel `openchannel_update`.
2022-07-26T05:37:59.4555485Z lightningd-1 2022-07-26T05:10:07.400Z DEBUG plugin-spenderp: mfc 60: funding tx 50425e20dbf0ca6fe112a8811b8048edb5bfa8d2922079668c5f353b859b45cb
2022-07-26T05:37:59.4557934Z lightningd-1 2022-07-26T05:10:07.508Z DEBUG 0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518-hsmd: Got WIRE_HSMD_CUPDATE_SIG_REQ
2022-07-26T05:37:59.4558244Z lightningd-1 2022-07-26T05:10:07.508Z DEBUG hsmd: Client: Received message 3 from client
2022-07-26T05:37:59.4558738Z lightningd-3 2022-07-26T05:11:03.234Z DEBUG 0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518-gossipd: seeker: startup peer finished
2022-07-26T05:37:59.4559209Z lightningd-3 2022-07-26T05:11:03.234Z DEBUG 0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518-gossipd: seeker: state = PROBING_SCIDS Seeking scids 1 - 105
(The last 2 log messages (from a different node) are >1min after the last
log line from lightning-1, because lightning-1 hung)
Hacked lightningd up to test this (such that notification always sent
before the RPC response, works as intended w/ patch)
We were cmd was getting free'd but holding on to reference of the
thing was causing problems.
==523280== Invalid read of size 8
==523280== at 0x1B3E14: del_notifier_property (tal.c:326)
==523280== by 0x1B3E14: tal_del_notifier_ (tal.c:569)
==523280== by 0x1123E7: handle_rpc_reply (libplugin.c:671)
==523280== by 0x1123E7: rpc_read_response_one (libplugin.c:866)
==523280== by 0x1123E7: rpc_conn_read_response (libplugin.c:886)
==523280== by 0x1A7B53: next_plan (io.c:59)
==523280== by 0x1A7B53: do_plan (io.c:407)
==523280== by 0x1A7B53: io_ready (io.c:417)
==523280== by 0x1A9BDB: io_loop (poll.c:453)
==523280== by 0x1141D0: plugin_main (libplugin.c:1708)
==523280== by 0x10D7E4: main (commando.c:937)
==523280== Address 0x52de928 is 8 bytes inside a block of size 40 free'd
==523280== at 0x483F0C3: free (vg_replace_malloc.c:872)
==523280== by 0x1B2CDD: del_tree (tal.c:419)
==523280== by 0x1B37BB: tal_free (tal.c:486)
==523280== by 0x1B37BB: tal_free (tal.c:474)
==523280== by 0x110CB2: command_complete (libplugin.c:255)
==523280== by 0x110CB2: command_done_err (libplugin.c:390)
==523280== by 0x10F511: handle_reply (commando.c:560)
==523280== by 0x10F511: handle_custommsg (commando.c:609)
==523280== by 0x113877: ld_command_handle (libplugin.c:1441)
==523280== by 0x113877: ld_read_json_one (libplugin.c:1491)
==523280== by 0x113877: ld_read_json (libplugin.c:1511)
==523280== by 0x1A7B53: next_plan (io.c:59)
==523280== by 0x1A7B53: do_plan (io.c:407)
==523280== by 0x1A7B53: io_ready (io.c:417)
==523280== by 0x1A9BDB: io_loop (poll.c:453)
==523280== by 0x1141D0: plugin_main (libplugin.c:1708)
==523280== by 0x10D7E4: main (commando.c:937)
==523280== Block was alloc'd at
==523280== at 0x483C855: malloc (vg_replace_malloc.c:381)
==523280== by 0x1B3BBD: allocate (tal.c:250)
==523280== by 0x1B3BBD: add_notifier_property (tal.c:303)
==523280== by 0x1B3BBD: tal_add_destructor2_ (tal.c:529)
==523280== by 0x110725: jsonrpc_request_start_ (libplugin.c:181)
==523280== by 0x10E0EA: send_more_cmd (commando.c:643)
==523280== by 0x11243C: handle_rpc_reply (libplugin.c:696)
==523280== by 0x11243C: rpc_read_response_one (libplugin.c:866)
==523280== by 0x11243C: rpc_conn_read_response (libplugin.c:886)
==523280== by 0x1A7B53: next_plan (io.c:59)
==523280== by 0x1A7B53: do_plan (io.c:407)
==523280== by 0x1A7B53: io_ready (io.c:417)
==523280== by 0x1A9BDB: io_loop (poll.c:453)
==523280== by 0x1141D0: plugin_main (libplugin.c:1708)
==523280== by 0x10D7E4: main (commando.c:937)
==523280==
{
<insert_a_suppression_name_here>
If rune contains invalid UTF-8, offers (which implements decode) would
produce JSON with invalid UTF-8, which causes lightningd to complain
and kill it, and then die because it's an important plugin.
So don't decode invalid UTF-8!
Reported-by: @jb55
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The error routine returns a string literal in this case, which we can't take().
Reported-by: @jb55
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is usually a signal that lightningd is shutting down, so notify
any instance that is waiting on `plugin.join()`.
Changelog-Fixed: cln-plugin: Fixed an issue where plugins would hang indefinitely despite `lightningd` closing the connection
Connectd already does this when we *receive* an error or warning, but
now do it on send. This causes some slight behavior change: we don't
disconnect when we close a channel, for example (our behaviour here
has been inconsistent across versions, depending on the code).
When connectd is told to disconnect, it now does so immediately, and
doesn't wait for subds to drain etc. That simplifies the manual
disconnect case, which now cleans up as it would from any other
disconnection when connectd says it's disconnected.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Disconnecting a peer after openingd fails is not instantaneous:
we abort the open, so openingd sends out a WIRE_ERROR which makes
connectd close the connection.
As a result this test fails often. The simplest fix is to wait for a
second in multifundchannel before retrying, which is also robust
against behaviour changes if we decide *not* to disconnect in future.
Also make sure that addrhint ownership is correct, since this can
lead to a use-after-free if we filter dests.
```
tests/test_connection.py::test_multifunding_best_effort FAILED [100%]
======================================================= FAILURES ========================================================
_____________________________________________ test_multifunding_best_effort _____________________________________________
node_factory = <pyln.testing.utils.NodeFactory object at 0x7f6c0c95c1c0>
bitcoind = <pyln.testing.utils.BitcoinD object at 0x7f6c0c92a880>
@pytest.mark.openchannel('v1')
@pytest.mark.developer("disconnect=... needs DEVELOPER=1")
def test_multifunding_best_effort(node_factory, bitcoind):
'''
Check that best_effort flag works.
'''
disconnects = ["-WIRE_INIT",
"-WIRE_ACCEPT_CHANNEL",
"-WIRE_FUNDING_SIGNED"]
l1 = node_factory.get_node()
l2 = node_factory.get_node()
l3 = node_factory.get_node(disconnect=disconnects)
l4 = node_factory.get_node()
l1.fundwallet(2000000)
destinations = [{"id": '{}@localhost:{}'.format(l2.info['id'], l2.port),
"amount": 50000},
{"id": '{}@localhost:{}'.format(l3.info['id'], l3.port),
"amount": 50000},
{"id": '{}@localhost:{}'.format(l4.info['id'], l4.port),
"amount": 50000}]
for i, d in enumerate(disconnects):
# Should succeed due to best-effort flag.
> l1.rpc.multifundchannel(destinations, minchannels=2)
tests/test_connection.py:2070:
...
> raise RpcError(method, payload, resp['error'])
E pyln.client.lightning.RpcError: RPC call failed: method: multifundchannel, payload: {'destinations': [{'id': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59@localhost:41023', 'amount': 50000}, {'id': '035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d@localhost:41977', 'amount': 50000}, {'id': '0382ce59ebf18be7d84677c2e35f23294b9992ceca95491fcf8a56c6cb2d9de199@localhost:34943', 'amount': 50000}], 'minchannels': 2}, error: {'code': 305, 'message': 'Peer not connected at start', 'data': {'id': '0382ce59ebf18be7d84677c2e35f23294b9992ceca95491fcf8a56c6cb2d9de199', 'method': 'fundchannel_start'}}
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is a bit weird since it lives in the offers plugin, but it works
well. This should make runes much more approachable for people!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I'm assuming that nobody wants a rate slower than 1 per minute; we can
introduce 'drate' if we want a per-day kind of limit.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We support the old commando.py plugin, which stores a random secret,
as well as a more modern approach which uses makesecret.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is needed for invoice, which can be asked to commit to giant descriptions
(though that's antisocial!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au
Changelog-Added: Plugins: `commando` a new builtin plugin to send/recv peer commands over the lightning network, using runes.
Plugins are supposed to store their data in the datastore, and commando does so:
let's make it easier for them by providing convenience APIs.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rather than a generic "add member", provide two routines: one which
doesn't quote, and one which does.
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>
Fixes#5271
In-Collaboration-With: Base58 'n Coding Seminar Participants
Changelog-Changed: `fundchannel` now errors if you try to buy a liquidity ad but dont' have `experimental-dual-fund` enabled
The direction only depends on the ordering between node_ids, not the
short_channel_id, so we can include it and it won't change. This was
causing some trouble loading the `channel_hints` in the `pay` plugin.
This will eventually enable us to specify 0 for zeroconf channels.
Changelog-Added: JSON-RPC: Added `mindepth` argument to specify the number of confirmations we require for `fundchannel` and `multifundchannel`
Represents the "configuration" part of the "init" message during
plugin initialization.
Changelog-Added: cln_plugin: persist cln configuration from init msg
This code was buggy: handing "1000" as a parameter to
min_their_funding_msat, don't turn that into "1000sat"!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The name in the spec is `msat`, but I don't want to make an API exception.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `fetchinvoice` `changes` `amount_msat`
Changelog-Deprecated: JSON-RPC: `fetchinvoice` `changes` `msat` (use `amount_msat`)
This is consistent with our output changes, and increases consistency.
It also keeps future sanity checks happy, that we only use JSON msat
helpers with '_msat' fields.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `invoice`, `sendonion`, `sendpay`, `pay`, `keysend`, `fetchinvoice`, `sendinvoice`: `msatoshi` argument is now called `amount_msat` to match other fields.
Changelog-Deprecated: JSON-RPC: `invoice`, `sendonion`, `sendpay`, `pay`, `keysend`, `fetchinvoice`, `sendinvoice` `msatoshi` (use `amount_msat`)
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>
We now have ternary outcomes for `Builder.configure()` and
`Builder.start()`:
- Ok(Some(p)) means we were configured correctly, and can continue
with our work normally
- Ok(None) means that `lightningd` was invoked with `--help`, we
weren't configured (which is not an error since the `lightningd` just
implicitly told us to shut down) and user code should clean up and
exit as well
- Err(e) something went wrong, user code may report an error and exit.
Mostly comments and docs: some places are actually paths, which
I have avoided changing. We may migrate them slowly, particularly
when they're user-visible.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We were setting it on the root, but that doesn't get handed to
sendpay. Our schema doesn't *require* bolt11, either, so this was
missed (there could be a *bolt12* instead).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: JSON-RPC: `listpays` always includes `bolt11` or `bolt12` field.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `pay` has `description` parameter, will be required if bolt11 only has a hash.
Changelog-Deprecated: JSON-RPC: `pay` for a bolt11 which uses a `description_hash`, without setting `description`.
This is what LND does, and it's better for upper layers than trying to
twist our maxfeepercent / exemptfee heuristics to suit.
(I don't remember who complained about this, sorry!)
I'm doing this now because I want to add YA parameter next!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `pay` has new parameter `maxfee` for setting absolute fee (instead of using `maxfeepercent` and/or `exemptfee`)
I think the new pay command has proven itself in the last 18 months!
Also various pay tests took "compat" then didn't use it, so clean them
up.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: JSON-RPC: `legacypay` (`pay` replaced it in 0.9.0).
Make it always a number; this makes the JSON request specification
simpler. We allowed a number since v0.10.1.
(reserve=True is the default anyway, so usually it can be omitted:
reserve=False becomes reserve=0).
Changelog-Deprecated: JSON-RPC: `fundpsbt`/`utxopsbt` `reserve` must be a number, not bool (for `true` use 72/don't specify, for `false` use 0). Numbers have been allowed since v0.10.1.
See: https://github.com/ElementsProject/lightning/issues/4991
We seem to correctly set end_time everywhere, so this looks like
a use-after-free somehow? But this will fix the crash right here :(
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Suggested-by: Rusty Russell <@rustyrussell>
Changelog-Added: plugins: `cln-grpc` first class GRPC interface for remotely controlling nodes over mTLS authentication
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.
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>
We still use the channel hint here (as it's the only option), we just
warn about lack of capacity.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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.
For now hooks are treated identically to rpcmethods, with the
exception of not being returned in the `getmanifest` call. Later on we
can add typed handlers as well.
We wrap emitted messages into a JSON-RPC notification envelope and
write them to stdout. We use an indirection over an mpsc channel in
order to avoid deadlocks if we emit logs while holding the writer lock
on stdout.
Changelog-Experimental: option `--lease-fee-base-msat` renamed to `--lease-fee-base-sat`
Changelog-Experimental: option `--lease-fee-base-msat` deprecated and will be removed next release
1. Tell memleak about our linked-list of current payments.
2. Don't remove them from list until we actually free them (in destructor, naturally).
3. Decode invoices into tmpctx (we steal / copy what we want anyway).
4. Free params after we've used them.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. The dijkstra can be temporary, doesn't need to last as long as pay cmd.
2. We fail multiple times in several places, so don't leak old failreason.
3. Make payments findable by our memleak detector.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. We don't keep a pointer to payments (unlike pay, which keeps a
linked list), so mark it notleak.
2. plugin->our_features is overloaded for "features we want to set" (used by keysend)
and then "features we have". Create a new field, which is cleaner.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. p is a child of cmd, so it's freed by command_failed.
2. cltv_budget is set a few lines up to the same thing already.
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)
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>
Since we have the exact state of the channels from the `listpeers`
response we can filter out the ones that are not yet normal or not
anymore. This is more of a safety net, given that the `gossip_store`
should contain local disables, but better not be racy :-)
Changelog-None
incoming_capacity_msat field showed the value as microsat.
Changelog-Fixed: JSON-RPC: listincoming showed incoming_capacity_msat field 1000 times actual value.
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>
It's very similar to the previous, but there are a few changes:
1. The enctlv fields are numbered differently.
2. The message itself is a different number.
The onionmsg_path type is the same, however, so we keep that constant
at least.
The result is a lot of cut & paste, but we will delete the old one
next release.
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>
Not all plugins depended on their headers. Keep it simple: all
plugins depend on all plugin headers.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: pay: The route selection will now use the log-propability-based channel selection to increase success rate and reduce time to completion
We bias by channel linearly by capacity, scaled by median fee.
This means that we effectively double the fee if we would use the
entire capacity, and only increase it by 50% if we would only use
1/2 the capacity.
This should drive us towards larger channels.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Plugins: `pay` now biases towards larger channels, improving success probability.
sendonionmessage can fail when sending a reply, either because
the reply had a bad first peer, or because it went offline. The
latter happens in CI, which is how I found this.
Also fixed typo "onio" -> "onion".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes#4482Fixes#4481
Changelog-Added: pay: Payment attempts are now grouped by the pay command that initiated them
Changelog-Fixed: pay: `listpays` returns payments orderd by their creation date
Changelog-Fixed: pay: `listpays` no longer groups attempts from multiple attempts to pay an invoice
So far we've always been deferring the deletion, retry and early abort
logic to `sendonion` and `sendpay` which do not have the context to
decide if a call is legitimate or not (they were mostly based on
heuristics). By calling `listsendpays` for the invoice's
`payment_hash` we can identify what our `groupid` should be, but more
importantly we can also abort if another payment is pending or a prior
attempt has already succeeded.
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`.
We always allocate a new `struct command` when we get a full JSON
object from stdin:
b2df01dc73/plugins/libplugin.c (L1229-L1233)
If it happens to be a notification, we pass the `struct command` to
the handler, and not free it ourselves:
b2df01dc73/plugins/libplugin.c (L1270-L1275)
There are only nine points in `plugins/libplugin.c` where we `tal_free`
anything, and only one of them frees a `struct command`:
b2df01dc73/plugins/libplugin.c (L224-L234)
The above function `command_complete` is not appropriate for
notification handlers; the above function sends out a response
to our stdout, which a notification handler should not do.
However, as-is, it does mean that notification handling leaks
`struct command` objects, which can be problematic if we ever
have future built-in plugins which are significantly more
dependent on notifications.
This commit changes notification handlers to return
`struct command_result *`, because possibly in the future
notification handlers may want to perform `send_outreq`, so we
might as well use our standard convention for callbacks, and
to encourage future developers to check how to properly
terminate notification handlers (and free up the
`struct command`).
We also now provide a `notification_handled` function which a
notification handler must eventually call, as well as a
`notification_handler_pending` which is just a snowclone of
`command_still_pending`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Experimental: Protocol: Updated onion_message support to match updated draft specification (with backwards compat for old version)
We carefully copied the buffer, but the tok is inside an array. We get away
with it for now, but with coming changes it gets freed. We need to copy
the token and all the tokens within it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This comes in via the onion_message_ourpath hook, and we identify the
path by checking the node alias it came to (vs the obsolete version
which used the blinding).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It was really different from the way we decide the overall state of a
`pay` command's output. Now we use a more similar state decision,
based on collecting all states and checking them at the end to
determine the outcome.
We were checking against the wrong enum value, resulting in an
erroneous `pending` state as a final result.
Changelog-Fixed: pay: `pay` would sometimes misreport a final state of `pending` instead of `failed`
We had sent->path be a list of node_ids, but it makes more sense as
pubkeys so we can avoid conversion. Also, I find it easier to think
about (especially creating backwards paths) if we include *ourselves*
as the first element in the path.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
sendonionmessage is going to be the new one, and do much *less*.
As this is an internal experimental-only API, no deprecation cycle
required.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
offers contain an x-only pubkey: to route to them to need to know the
02 vs 03 prefix. If they're in the gossmap it's easy, but if they're
a directly-connected peer it's harder. We used to have
sendonionmessage tweak the key if it found a peer with the matching
key, but this was always a hack.
It turns out that we try to connect to the node anyway, which is
a noop if it's already connected. So try connecting to the other
parity if the first one fails.
Also, this registers when we fail to connect, and returns an error
rather than waiting for timeout.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
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>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Plugins: C plugins would could leak memory on every command (esp. seen when hammering topology's listchannels).
This lets memleak track them, but makes sure they don't leak; using
notleak could cover up a leak here.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
They get grafted into clone, so have them parented there. Otherwise
we get a small leak every time we RBF.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We weren't actually getting the last log out; this does that.
We have to fix test_bitcoin_failure which now notices the BROKEN
log message.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: libplugin: Fatal error messages from plugin_exit() now logged in lightningd.
It subsumes `decodepay`, and it's nicer if people can just assume it's
available at all times.
Changelog-Added: JSON-RPC: `decode` now available without `experimental-offers`
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As requested by @shesek: it's weird to fail if they ask for the exact
same thing (which is quite possible, since offers don't expire by
default).
And add a new "created" field so they can tell if they have an old
one.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We don't automatically *reject* an invoice which asks for a different
msat than we specified (caller may!), but we don't bother noting it
unless it is different.
Reported-by: @shesek
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
You could have quantity_min of 0, which makes no sense; spec has been
updated, so quote and enforce that.
Reported-by: @shesek
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>
Documentation for the funderupdate command on the funder plugin.
Realized we were missing the "leases_only" in the output; also adds
'_msat' to msat denominated outputs so they're parsed correctly by our
python bindings (also matches our naming conventions)
Changelog-EXPERIMENTAL: funder: `funderupdate` command to view and update params for contributing our wallet funds to v2 channel openings. Provides params for enabling `option_will_fund`.
If there's no plugin currently in place, we simply won't return any
funding at all, in which case we'd expect them to handle however
they want. (our implementation would fail the open, as we only accept
opens that have at least as much as we've requested provided)
We need to parse the feerate string, so we can figure out what our
weight fee will be for a leased channel, so we go get the feerate
and use that to calculate what our expected lease fee will be for
the requested amount.
We need to know what the lease we're expecting is. To do this
we pass around the hex encoded portion of the wire format.
We can use this passed in expected lease rates to confirm that the peer
is, in fact, using the same rates as what we have currently.
Changelog-Added: JSON-RPC: fundchannel, multifundchannel, and openchannel_init now accept a 'compact_lease' for any requested funds
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
Implement support for liquidity ads in `funder` plugin. We set the
command line options for the leases, as well as sending the updated ads
to lightningd (who then passes them through to gossipd)
We were getting off-by-one for the total amount that the change is for,
since it rounds the fee *down*, independent of the total weight of the
entire tx.
We fix this by using the diff btw the fee of the total weight (w/ and
w/o the change output)
So far we could not reach non-publicly reachable nodes with the
keysend command since we couldn't compute a route to them. With this
change we can add some routehints that we may have gotten from an
address book or a previous invoice, and make it more likely that we
can reach the destination.
Changelog-Added: keysend: `keysend` can now reach non-public nodes by providing the `routehints` argument if they are known.
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>
We don't do it for sendinvoice (yet?).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: `fetchinvoice` can take a payer note, and `listinvoice` will show the payer_notes received.
We don't support it (yet), but update the spec to include it.
We include the previous field (recurrence_signature) as a shim for the
moment, for compat with existing nodes. It's ugly, but next release
we'll stop *sending* it, then finally we'll stop accepting it!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
This also adds a `fetchinvoice-noconnect` option to suppress it too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: `fetchinvoice` and `sendinvoice` will connect directly if they can't find an onionmessage route.
Fixes: #4624
We didn't set this to false on non-regtest!
```
==198363== Conditional jump or move depends on uninitialised value(s)
==198363== at 0x10EF88: estimatefees_parse_feerate (bcli.c:443)
==198363== by 0x10F3BF: estimatefees_second_step (bcli.c:550)
==198363== by 0x10E720: bcli_finished (bcli.c:258)
==198363== by 0x1438A7: destroy_conn (poll.c:244)
==198363== by 0x1438CB: destroy_conn_close_fd (poll.c:250)
==198363== by 0x151A7A: notify (tal.c:240)
==198363== by 0x151F91: del_tree (tal.c:402)
==198363== by 0x15232D: tal_free (tal.c:486)
==198363== by 0x141EB8: io_close (io.c:450)
==198363== by 0x14400B: io_loop (poll.c:449)
==198363== by 0x114BB0: plugin_main (libplugin.c:1414)
==198363== by 0x1105C4: main (bcli.c:973)
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This mainly helps our CI under valgrind, which starts a fresh instance
and immediately calls the invoice command. This can cause the topology
plugin to try to access the gossmap file before it's created.
We can also move the gossmap reading in topology to init time.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Invoices need to know what the peers' update contains, to create
routehints. It also wants to know if a peer has no other public
channels (so-called "dead end" peers) to eliminate them from routehint
consideration.
This was previously done by a special function to ask gossipd.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Temporarily rename old getroute to getrouteold (we will remove this).
Changelog-Changed: JSON-RPC: `getroute` is now implemented in a plugin.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This makes the min_cltv_expiry_delta equal to Rust-Lightning's, which is
the highest minimum we know of.
Changelog-Changed: keysend now uses 22 for the final CTLV, making it rust-lightning compatible.
There are a couple of ways this can happen, but we shouldn't crash.
Fixed#4488Fixes#4533
Changelog-Fixed: pay: Fixed an issue when filtering routehints when we can't find ourselves in the local network view.
We were counting the attempts including the root payment, which
resulted in an off-by-one error with the `test_pay_low_max_htlcs`
test. Counting the children of the root payment after the presplitter
had a go is the correct way to do it, since at that time we only have
one level in the tree, no need to recurse and potentially count
ourselves.
This was triggered by having some part being started after the overall
command already gave up, cleaning up the `cmd` context from which the
routehints were allocated. The early exit of the command, as a result
from a terminal state does not guarantee that no later attempt will
try to find a route, especially if the attempt was started before we
knew that it is doomed.
Otherwise our schema is pretty meaningless, since invalid decodes
can have missing "required" fields.
Also fix a typo "blinded_payindo".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Experimental: JSON-RPC: `decode` now gives a `valid` boolean (it does partial decodes of some invalid data).
As mentioned in previous commits: "result" must be an object,
and anything else is an antipattern.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is hard to parse, and not extensible in future, and disagrees with
the man page (and caught by schema).
Technically this is an API break, but it can't be done neatly anyway
and it's unlikely someone is relying on this today :(
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSONRPC: `autocleaninvoice` now returns an object, not a raw string.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Plugins: `fundchannel` and `multifundchannel` will now reserve funding they use for 2 weeks instead of 12 hours.
I don't know why it thinks that blockheight is INT_MAX, but
we shouldn't wait forever anyway.
```
lightningd-1: 2021-05-25T01:22:19.472Z DEBUG plugin-pay: cmd 67 partid 0: Blockheight disagreement, not aborting.
lightningd-1: 2021-05-25T01:22:19.483Z INFO plugin-pay: cmd 67 partid 0: failed: WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS (reply from remote)
lightningd-1: 2021-05-25T01:22:19.483Z INFO plugin-pay: cmd 67 partid 0: Remote node appears to be on a longer chain, which causes CLTV timeouts to be incorrect. Waiting up to 49 seconds to catch up to block 2147483647 before retrying.
lightningd-1: 2021-05-25T01:23:08.489Z INFO plugin-pay: cmd 67 partid 0: Timed out while attempting to sync to blockheight returned by destination. Please finish syncing with the blockchain and try again.
lightningd-1: 2021-05-25T01:23:08.489Z INFO plugin-pay: cmd 67 partid 0: Remote node appears to be on a longer chain, which causes CLTV timeouts to be incorrect. Waiting up to 18446744073709551615 seconds to catch up to block 2147483647 before retrying.
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. We don't need to check for NULL before tal_count(NULL).
2. Use of json_for_each_arr iterator is probably better.
3. Weird indent fixed.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Users are more upset recently with the cost of unilateral closes
than they are the risk of being cheated. While we complete our
anchor implementation so we can use low fees there, let's
get less aggressive (we already have 34 or 18 blocks to close
in the worst case).
The changes are:
- Commit transactions were "2 CONSERVATIVE" now "6 ECONOMICAL".
- HTLC resolution txs were "3 CONSERVATIVE" now "6 ECONOMICAL".
- Penalty txs were "3 CONSERVATIVE" now "12 ECONOMICAL".
- Normal txs were "4 ECONOMICAL" now "12 ECONOMICAL".
There can be no perfect levels, but we have had understandable
complaints recently about how high our default fee levels are.
Changelog-Changed: Protocol: channel feerates reduced to bitcoind's "6 block ECONOMICAL" rate.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: `funder` plugin now has new command `funderupdate` which will show current funding configuration and allow you to modify them
