In 6e4ff6a7d2 ("offers: add a blinded path
if we have no advertized address") we were overzealous, and set blinded
paths not just for offers and invoicerequests, but for invoices themselves.
This has revealed various interop issues (which is great, but not good
for our users!) so we should disable that. It also reduces the reliability
of payments in general.
Changelog-None: fixes previously overzealous addition
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Some enums, structs, functions in pyln-grpc-proto/cln-grpc/cln-rpc
have been slightly renamed so they follow grpc and rust's naming convention
Use larger CLTVs and we see the failure from l3, complaining the CLTV is outside
the amount in the payment_constraint field, like:
```
Failing HTLC because of an invalid payload (TLV 10 pos 104): cltv_expiry 609 > payment_constraint 376
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The old blindedpay fields would have an entry per hop, but that was changed
to a single per-path entry. The devtools hadn't caught up.
Similarly, it didn't like descriptionless offer fields in invreqs and invoices.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This corner case started triggering on my machine with latest Bitcoind.
This test sabotages the closing negotiation, and as a result l1
doesn't see l2's CLOSING_SIGNED. l2 is happy, however, and it is in
CLOSINGD_COMPLETE. When l1 reconnects, it gets an error, and this causes
it to drop the unilateral tx to chain.
This unilateral tx from l1 replaces or races the mutual close tx from
l2, causing a unilateral close, which breaks our test.
Though this is a corner case, it's much friendlier to allow the
closing negotiation again until we actually see the close onchain.
This fixes the tests here, too.
```
def test_closing_negotiation_reconnect(node_factory, bitcoind):
disconnects = ['-WIRE_CLOSING_SIGNED',
'+WIRE_CLOSING_SIGNED']
l1, l2 = node_factory.line_graph(2, opts=[{'disconnect': disconnects,
'may_reconnect': True},
{'may_reconnect': True}])
l1.pay(l2, 200000000)
assert bitcoind.rpc.getmempoolinfo()['size'] == 0
l1.rpc.close(l2.info['id'])
l1.daemon.wait_for_log(r'State changed from CHANNELD_NORMAL to CHANNELD_SHUTTING_DOWN')
l2.daemon.wait_for_log(r'State changed from CHANNELD_NORMAL to CHANNELD_SHUTTING_DOWN')
# Now verify that the closing tx is in the mempool.
bitcoind.generate_block(6, wait_for_mempool=1)
sync_blockheight(bitcoind, [l1, l2])
for n in [l1, l2]:
# Ensure we actually got a mutual close.
> n.daemon.wait_for_log(r'Resolved FUNDING_TRANSACTION/FUNDING_OUTPUT by MUTUAL_CLOSE')
tests/test_closing.py:275:
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Protocol: We now renegotiate an interrupted close, even if we don't need it, instead of sending an error.
This is a bit too much boilerplate for these, which mainly do the same
thing.
We add annotaitons to new_peer_fd so the compiler knows that it cannot
return NULL, otherwise with -O3 we get:
```
lightningd/peer_control.c: In function ‘peer_connected_hook_final’:
lightningd/peer_control.c:1388:28: error: ‘error’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
1388 | take(towire_connectd_peer_send_msg(NULL, &channel->peer->id,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
lightningd/peer_control.c:1313:19: note: ‘error’ was declared here
1313 | const u8 *error;
| ^~~~~
lightningd/peer_control.c: In function ‘peer_spoke’:
lightningd/peer_control.c:1999:28: error: ‘error’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
1999 | take(towire_connectd_peer_send_msg(NULL, &peer->id,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make: *** [Makefile:311: lightningd/peer_control.o] Error 1
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The vast majority of incoming channel updates seem to be cut due
to age, which results in noisy logs. Similarly, the chanbackup
logging verbosity might better match the equivalent actions in
channeld, which are at the debug level.
Fixes: #8058
Changelog-None: introduced in 25.02
If the nSequence in the tx it produces is not at least the value we
test in the script, the tx will always fail:
```
error code: -26\nerror message:\nmandatory-script-verify-flag-failed (Locktime requirement not satisfied)
```
If we have a lease, the nSequence is max(lease-time-remaining,
to-self-delay), so have onchaind tell lightningd the correct nSequence.
Fixes: https://github.com/ElementsProject/lightning/issues/7460
Reported-by: https://github.com/pabpas
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: Correctly collect our own (delayed) funds if we have a unilateral close when we are still offering a lease.
current height + to_self_delay[LOCAL] is correct normally, but if we
have an outstanding lease it's longer. Not a big issue, because
lightningd will retry until its spendable, but wrong.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
What we expect to happen:
1. l3, which unilaterally closed, waits 4032 blocks (minus those mined) before
trying to send get its "to_local" funds back.
2. This should then succeed.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There are other ways we can disable it, of course (e.g full path name)
but this is the most obvious.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is neater than appending to some random file: we only do that once
if there's no "include" line to include a ".setconfig" file.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `setconfig` now has a `transient` flag which means it won't rewrite your config file.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The chanbackup plugin should update emergency.recover every time we
receive a new commitment secret.
Additionally, chanbackup should be able to serialize both the new
SCB format and the legacy format.
Key Changes:
- Added a commitment_revocation plugin hook to update emergency.recover whenever a new revocation secret is received.
- Implemented support for the new emergency.recover format while maintaining compatibility with the legacy format.
This function enables direct persistence of shachain data, ensuring all relevant information is saved upon retrieval.
Key Changes:
- Adds 'wallet_stub_shachain_init' function to store a retrieved shachain in the database.
- Saves initial metadata ('min_index' and 'num_valid') to the 'shachains' table.
- Iterates through known shachain secrets, adding each entry to 'shachain_known' with position, index, and hash.
These fields enhance 'stub_chan' to make channels which are capable to
create penalty transaction in case the peer broadcasts an old revoked state.
Key Changes:
- Added new params to stub_chan() to accomodate shachain, basepoints, opener, and remote_to_self_delay
- Check if any field is NULL inside scb_chan->tlvs and assign them appropriately
We define a new subtype 'modern_scb_chan' and a new tlvtype 'scb_tlvs' which includes
all the relevant information to create a penalty transaction when the peer tries to cheat.
Key Changes:
- Rename the old format to 'legacy_scb_chan' and define a new type 'modern_scb_chan'
- Include TLVs to 'modern_scb_chan'
- Create a new msgtype 'static_chan_backup_with_tlvs'
- Modify 'struct channel' to include 'struct modern_scb_chan'
- Add these two types to 'varsize_types' in generate.py
This change allows adding a length prefix to a serialized TLV. It will
be particularly useful for serializing all 'scb_chan' entries in
the 'emergency.recover' file.
Key Changes:
- Removed the need to loop in towire_tlv and fromwire_tlv, so the if conditions have been modified accordingly.
- During serialization, the length of the TLV is calculated before appending it, and it is stored in a temporary variable.
- For fromwire_tlv, only a simple length adjustment is required, and no loop is needed here either.
This change will allow subtypes in wiregen files to have tlvstreams.
Shifting tlv structs above subtypes in header_template is done to prevent
forward declaration.
Since generate-wire prepends 'tlv_' in tlvname, we
have to modify fromwire_subtype_field and towire_subtype_field in
impl_template to accommodate this.
Changelog-Added: This PR would turn our peers into watchtower and enable SCB to create penalty txn.
The parameter max_htlc_value_in_flight_msat stablished by peers on
channel opening (BOLT02) can now be retrived from the
gossmods_from_listpeerchannels API.
Adapted the corresponding callback functions in renepay and askrene to
take into account that value as a constraint to the value we can send
through a channel.
Changelog-Add: fetch max_htlc_value_in_flight_msat from gossmods_listpeerchannels API
Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
Changelog-Added: JSON-RPC: `listpeerchannels` new output fields `their_max_total_htlc_out_msat` and `our_max_total_htlc_out_msat` as the value of `max_htlc_value_in_flight` (as of BOLT02) set by the local and remote nodes on channel creation.
Changelog-Deprecated: JSON-RPC: `listpeerchannels` value `max_total_htlc_in_msat`: use `our_max_total_htlc_out_msat` instead to follow spec naming convention.
Otherwise, deprecating a field causes SELECT * to fail:
```
> l1.rpc.sql(f"SELECT * FROM peerchannels;")
...
> raise RpcError(method, payload, resp['error'])
E pyln.client.lightning.RpcError: RPC call failed: method: sql, payload: ('SELECT * FROM peerchannels;',), error: {'code': -1, 'message': 'query failed with access to peerchannels.max_total_htlc_in_msat is prohibited (Deprecated column table peerchannels.max_total_htlc_in_msat)'}
```
So if they use a wildcard, allow access: though "SELECT *" is fraught,
"COUNT(*)" is perfectly legit.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The `enableoffer` JSON schema is present, but it is not included in the `GENERATE_MARKDOWN` list within the Makefile. This resulted into missing `.7` and `.7.md` files, leading to missing manpage and the documentation portal page.
Changelog-None.
Eduardo points out that payment_failed kind of over-promises: it may
actually not fail the payment now (with slow mode).
It's more an indiciation that we're not trying any more payment parts,
so rename it to payment_give_up.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We can create this scenario by having one path force close. We take
the opportunity to log this, even in non-slow-mode, since it's interesting
(not our bug, but someone just lost money!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This was requested by Michael of Boltz; it's mainly useful if you plan to
try failed payments on a *different* node. In that case, there's a
theoretical possibility that slow parts of this payment could combine with
that from a different node and overpay.
We don't allow this from the same node, already.
Changelog-Added: xpay: `xpay-slow-mode` makes xpay wait for all parts of a payment to complete before returning success or failure.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1. Don't rely on the current attempt, make caller calculate total.
2. Save preimage inside attempt, for slow mode.
3. Hoist it higher in the file.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
