Nothing major here:
1. size_t for lengths.
2. pass engine to checksum_verify, as caller wants ->len (avoid repeating 13/15 magic numbers).
3. Use x.member instesad of (&x)->member.
4. Return memcmp result directly instead of if.
5. Spacing removal, `;;` removal.
6. codexl is a bool `true`/`false` not 0/1 (it's the same, but clearer)
7. Make sanity_check assign *fail directly.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Clean restart of daemon after a tx-abort is a nice way to work around
the 'persistent' disconnect that we t-bast noticed.
Changelog-Fixed: `dualopend`: Fix behavior for tx-aborts. No longer hangs, appropriately continues re-init of RBF requests without reconnction msg exchange.
Bug Report:
- initiate a channel open eclair -> cln
- wait for the transaction to be published
- eclair initiates rbf, and cancels it by sending tx_abort before exchanging commit_sig
- at that point everything looks good, cln echoes the tx_abort and stays connected
- eclair initiates another RBF attempt and sends tx_init_rbf: for some unknown reason,
cln answers with channel_reestablish (??) followed by an error saying
"Bad reestablish message: WIRE_TX_INIT_RBF"
Diagnosis:
CLN is doing a reconnect after a tx-abort is sent.
Extra Test:
Realized that if we abort, we won't correctly advanced to NORMAL if
blocks are mined while we're in hanging state. CLN should advance
after block containing channel open is mined.
Reported-By: @t-bast
This way unreserving the PSBT will work as intended, and we don't have
to keep track of how many times we've called reserved for any one input.
Technically we're supposed to not reserve inputs at *all* while doing
opens, this moves us slightly closer to that.
Also as update_own_node_announcement is called nearly continuously
under normal operation by maybe_send_own_node_announce, the timer should
not be freed continuously - better to only free before actually
refreshing.
When an outdated own node announcement is present, it fails the
nannounce_different test and also fails to kick off the forced regen
timer.
Changelog-Fixed: Node announcements are refreshed more reliably.
This will at least *help* the case where these were not populated, causing us
to send errors without channel_updated appended.
It's not perfect: we can still send such errors if the gossip store is
corrupted, and we still send them for private channels, but it should
help.
(The much better fix is far more invasive, so slips to next release!)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The alias may not be set for non-alias channels after they
confirm. The other branch is safe because we only consider active
channels.
Changelog-None
Fixes#6450
Make sure we've completely processed htlc, so we will definitely consider it an old spend. If we're too fast, l2 might consider it a legitimate unilateral close:
```
# Make sure both sides got revoke_and_ack for final.
l1.daemon.wait_for_log('peer_in WIRE_REVOKE_AND_ACK')
l2.daemon.wait_for_log('peer_in WIRE_REVOKE_AND_ACK')
# Now we really mess things up!
bitcoind.rpc.sendrawtransaction(tx)
bitcoind.generate_block(1)
l2.daemon.wait_for_log(' to ONCHAIN')
# FIXME: l1 should try to stumble along!
# l2 should spend all of the outputs (except to-us).
# Could happen in any order, depending on commitment tx.
needle = l2.daemon.logsearch_start
((_, txid1, blocks1), (_, txid2, blocks2)) = \
> l2.wait_for_onchaind_txs(('OUR_PENALTY_TX',
'THEIR_REVOKED_UNILATERAL/DELAYED_CHEAT_OUTPUT_TO_THEM'),
('OUR_PENALTY_TX',
'THEIR_REVOKED_UNILATERAL/OUR_HTLC'))
tests/test_closing.py:687:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
contrib/pyln-testing/pyln/testing/utils.py:1264: in wait_for_onchaind_txs
r = self.daemon.wait_for_log('Telling lightningd about {} to resolve {}'
contrib/pyln-testing/pyln/testing/utils.py:346: in wait_for_log
return self.wait_for_logs([regex], timeout)
```
You can see l2 here:
```
lightningd-2 2023-07-27T03:34:24.533Z DEBUG 0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518-onchaind-chan#1: Their unilateral tx, old commit point
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The fields were missing because they weren't annotated with a type and
a description. Adding those fixes them.
Changelog-Fixed: msggen: `listpays` now includes the missing `amount_msat` and `amount_sent_msat` fields
No-schema-diff-check: fields were always there, just undocumented!
Abstracts search and directory traversal. Adds support for installing
from a local git repository, a local directory, or a web hosted git repo
without relying on an api.
Changelog-Changed: Reckless can now install directly from local sources.
This cause of cascading failure was pointed out by @t-bast: if fees spike and
you don't timeout an outgoing onchain HTLC, you should nonetheless fail the incoming htlc
because otherwise the incoming peer will close on you.
Of course, there's a risk of losing funds, but this only happens if you weren't going to get the HTLC spend in time anyway. And it would also catch any other reason that the downstream onchain goes wrong, containing the damage.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Reported-by: @t-bast
Changelog-Fixed: Protocol: We will close incoming HTLCs early if the outgoing HTLC is stuck onchain long enough, to avoid cascating failure.
The test actually triggers this:
1. We don't get our commitment tx mined at all (we block it).
2. By the time the peer does, the HTLC is expired.
3. We have the preimage but we don't even try, since it's expired.
We should at least *try* to collect the HTLC in this case.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Caught by leak detection, we just re-assigned this when we retried: sure,
it's temporary, but it's technically a leak.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I shut down bitcoind during a test, and bcli leak reports flooded in.
They're all temporary, but this fixes them.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This update incorporates the proposed version of lnprototest from
the patch [1], which includes the following fixes:
- Corrects the `ExpectError` event and updates BOLT 7 to expect a
warning instead of an error.
- Implements a new test for when the runner sends a bad signature
within the announcement_signatures message.
[1] https://github.com/rustyrussell/lnprototest/pull/100
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
This commit addresses an issue to enhance the resilience of core
lightning when receiving node announcements.
According to BOLT 7 (The announcement_signatures Message),
if the node_signature OR the bitcoin_signature is NOT correct,
it is recommended to either send a warning and close the connection or send an error and fail the channel.
In this commit, we take a strict approach. If any error is detected, we
send an error and fail the open channel operation.
This is because the announcement_signatures operation is optional,
and we assume that it must be correct.
lnprototest at commit dea47c29b5541dbfe7fe53cc2598330e897fa4f4 report
the following error now.
```
2023-07-06T21:03:20.930Z DEBUG hsmd: Shutting down
ERROR root:helpers.py:170 Traceback (most recent call last):
File "/home/vincent/Github/lightning/external/lnprototest/tests/helpers.py", line 167, in run_runner
runner.run(test)
File "/home/vincent/Github/lightning/external/lnprototest/lnprototest/runner.py", line 99, in run
all_done = sequence.action(self)
^^^^^^^^^^^^^^^^^^^^^
File "/home/vincent/Github/lightning/external/lnprototest/lnprototest/structure.py", line 55, in action
all_done &= e.action(runner)
^^^^^^^^^^^^^^^^
File "/home/vincent/Github/lightning/external/lnprototest/lnprototest/event.py", line 365, in action
raise EventError(self, "{}: message was {}".format(err, msg.to_str()))
lnprototest.errors.EventError: `Expected msgtype-warning, got msgtype-error: message was error channel_id=a37362839b13f61cfe82d35bd397b1264c389b245847cfb6111b38892546dc77 data=4661696c656420746f20766572696679206e6f64655f7369676e61747572652e` on event [{"event": "ExpectMsg", "file": "test_bolt2-01-close_channel.py", "pos": "157"},]
============================================================================================================================================================== short test summary info ===============================================================================================================================================================
FAILED tests/test_bolt2-01-close_channel.py::test_close_channel_shutdown_msg_normal_case_receiver_side - AssertionError: `Expected msgtype-shutdown, got msgtype-error: message was error channel_id=a37362839b13f61cfe82d35bd397b1264c389b245847cfb6111b38892546dc77 data=4661696c656420746f20766572696679206e6f64655f7369676e61747572652e` on event [{"event": "ExpectMsg", "file": "test_bolt2-01-close_channel.py", "pos": "75"},]
FAILED tests/test_bolt2-01-close_channel.py::test_close_channel_shutdown_msg_wrong_script_pubkey_receiver_side - AssertionError: `Expected msgtype-warning, got msgtype-error: message was error channel_id=a37362839b13f61cfe82d35bd397b1264c389b245847cfb6111b38892546dc77 data=4661696c656420746f20766572696679206e6f64655f7369676e61747572652e` on event [{"event": "ExpectMsg", "file": "test_bolt2-01-close_channel.py", "pos": "157"},]
```
Changelog-Fixes: channeld: Verify the signature sent in announcement_signatures by the counterparty
Reported-by: lnprototest
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
This is the simplest solution, not the best, but there's significant risk in try to remove the "we have a path" assumption in the code pay code.
Includes removing a `tal_steal` which was incorrect: the buffer has the same lifetime as the plugin, so if we steal it then things get messy when we free the struct payment.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: `pay` will now pay your own invoices if you try.
Previously, the "payment" and "invoice" paths were completely separate, but this now calls both. It bypasses htlc_sets (and thus, cannot do MPP), and bypasses the hook too: the former is tied closely to HTLCs, and the hook is also very htlc-centric.
Includes finishing unfinished sentence in sendpay man page, as a bonus.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: `sendpay` now allows self-payment of invoices, by specifying an empty route.
Rusty Russell
adi2011
Shahana Farooqui
niftynei
Alex Myers
Adi Shankara
Christian Decker
Matt Morehouse
Vincenzo Palazzo