Over time, it has cost us more developer cycles than it has gained.
It has hidden intermittant bugs, and allowed cruft to accumulate:
when we eventually tried to figure out what was going wrong, the
actual change which caused it was now stale and forgotten.
This was a particular bane during the connectd rewrite, and I
worked through some issues which had occurred before, but were not
more likely.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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 should be using amount_msat always. Many tests were not. Plus,
deprecating it simplifies the code.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: JSONRPC: `sendpay` `route` elements `msatoshi` (use `amount_msat`)
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`)
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.
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).
Trips on our RPC checking introduced at the same time: msat should be
an integer or an "xxxmsat"/sat/btc string.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Suggested-by: @t-bast
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `listforwards` has new entry `style`, currently "legacy" or "tlv".
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.
Using a canned sendonion call from an old version (v0.10.2), where I
forced it to use legacy onion for l2.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is the part which works: we have another old PR (or simply
uncomment the FIXME section) for the routehint case.
Closes: #4781
See-also: #4808
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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.
Suggested by @m-schmook, I realized that if we append it later I'll
never get it right: I expect parameters min and max, not max and min!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: you can now alter the `htlc_minimum_msat` and `htlc_maximum_msat` your node advertizes.
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>
We need to add some, since our internal representations of
htlc_maximum_msat round up, and we need to disable mpp which succeeds
in getting a payment through by splitting.
We also allow dev_routes to suppress invoice routehints altogether.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This restores the behaviour prior to `lightningd: use our cached
channel_update for errors instead of asking gossipd.`, where gossipd
would refuse to give us channel_updates for unannounced channels.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If the HTLCs are completely negotiated, we can get a channel break when
we mine a pile of blocks. This is mainly seen with Postgres, due to the db
speed.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If we fund a channel between two nodes, then mine all the blocks to
announce it, any other nodes may see the announcement before the
blocks, causing CI to complain about "bad gossip":
```
lightningd-4: 2022-01-25T22:33:25.468Z DEBUG 032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e-gossipd: Ignoring future channel_announcment for 113x1x1 (current block 112)
lightningd-4: 2022-01-25T22:33:25.468Z DEBUG 032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e-gossipd: Bad gossip order: WIRE_CHANNEL_UPDATE before announcement 113x1x1/0
lightningd-4: 2022-01-25T22:33:25.468Z DEBUG 032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e-gossipd: Bad gossip order: WIRE_CHANNEL_UPDATE before announcement 113x1x1/1
lightningd-4: 2022-01-25T22:33:25.468Z DEBUG 032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e-gossipd: Bad gossip order: WIRE_NODE_ANNOUNCEMENT before announcement 032cf15d1ad9c4a08d26eab1918f732d8ef8fdc6abb9640bf3db174372c491304e
```
Add a new helper for this case, and use it where there are more than 2 nodes.
Cleans up test_routing_gossip and a few other places which did this manually.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We also no longer strip the type off: everyone handles both forms, and
Eclair doesn't strip (and it's easier!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We seem to hit a race between manual reconnect (with address hint) and an automatic
reconnection attempt which fails:
```
> l4.rpc.connect(l3.info['id'], 'localhost', l3.port)
...
E pyln.client.lightning.RpcError: RPC call failed: method: connect, payload: {'id': '035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d', 'host': 'localhost', 'port': 41285}, error: {'code': 401, 'message': 'All addresses failed: 127.0.0.1:36678: Connection establishment: Connection refused. '}
```
See how it didn't even try the given address?
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>
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>
CI seems to be OOM killing us; 5 may be too many under valgrind.
VALGRIND=1 pytest tests/test_pay.py::test_fetchinvoice
Before:
1 passed in 199.33s (0:03:19)
After:
1 passed in 177.91s (0:02:57)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fails liquid-regtest otherwise; liquid tends to hit the dust limit
earlier than non-liquid tx, and MPP exacerbates this by divvying up
payments into dusty bits then attempting to shove them through the same
channel, hitting the dust max. The MPP then fails as not all the parts
were able to arrive at their destination.
Let's make this a softer launch by just warning on the channel til the
feerates go back down.
You can also 'fix' this by upping your dust limit with
the `max-dust-htlc-exposure-msat` config.
