Reported-by: @niftynei
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Plugins: we clean up properly if a plugin fails to start, and we don't kill all processes if it's from `plugin startdir`.
Don’t send the funding spend to onchaind if we detect it in inflights (aka. a splice). While we already prevented onchaind_funding_spent from being called directly, the call to wallet_channeltxs_add meant onchaind_funding_spent would be called *anyway* on restart. This is now fixed.
Additionally there was a potential for a race problem depending on the firing order of the channel depth and and funding spent events.
Instead of requiring these events fire in a specific order, we make a special “memory only” inflight object to prevent the race regardless of firing order.
Changelog-Fixed: Splice: bugfix for restart related race condition interacting with adversarial close detection.
In spec commit 498f104fd399488c77f449d05cb21c0b604636a2 (August 2021),
Bastien Teinturier removed the requirement that the mutual close fee be
less than or equal the final commitment tx.
We adopted that change in v0.10.2, but we made sure to never offer a fee
under the final commitment tx's fee, so we didn't break older nodes.
However, the closing tx can actually be larger than the final commitment tx!
The final commit tx has a 22-byte P2WKH output and a 34-byte P2WSH output;
the closing can have two 34-byte outputs, making it 4*8 = 32 Sipa heavier.
Previously this would only happen if both sides asked for P2WSH outputs,
but now it happens with P2TR, which we now do.
The result is that we create a tx which is below the finally commitment
tx fee, and may be below minrelayfee (as it was in regtest).
So it's time to remove that backwards-compatibility hack.
Changelog-Fixed: Protocol: We may propose mutual close transaction which has a slightly higher fee than the final commitment tx (depending on the outputs, e.g. two taproot outputs).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: #6545
The main function here is payment_reconsider:
* Each payment has a list of pay_flow.
* This is populated in try_paying(), calling add_payflows & sendpay_new_flows.
* When we get a notification, we resolve a pay_flow using one of the pay_flow_failedxxx
or pay_flow_succeeded functions.
* They call payment_reconsider() which cleans up finished flows decides what to do:
often calling try_paying again.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Treat it just like "PAY_TRY_OTHER_ROUTE", except it is from the final node:
this means we correctly process that it "succeeded".
Add a test: this crashes sometimes, but it's cleaned up soon...
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This was recommended by @t-bast: if the final spec commits to something
compatible, we can simply advertize and accept both features, but if it
does change in incompatible ways we won't cause problems for nodes
who implement the official spec.
(I split this, so first, we remove the OPT_SPLICE entirely, to make
sure we caught them all. --RR)
Suggested-by: @t-bast
Changelog-None
EXPERIMENTAL_SPLICING=1 turns it on for *all* tests, to make sure we don't
accidentally break those. But we can (and should!) run the splice test
under every possible CI scenario.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I noticed this while debugging an issue with ACINQ, that we got upset,
but didn't trigger a reconnect cycle.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Protocol: We now close connection with a peer if adding an HTLC times out (which may be a TCP connectivity issue).
In this case, the user's default was info, but they specifically asked for debug
from one plugin. Since there were no per-file filters, it set filtering to the
default level, info, and rejected it. Since it's been explicitly filtered in,
we need to pass it at this point.
Reported-by: @wtogami
Fixes: #6503
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This was strongly recommended by Russell O'Connor: the "ms" implies that
it's a BIP-32 master secret, and this is CLN specific.
If we changed the hrp to "cln" it would be better, but apparently that
means we no longer fit in a "standard billfold metal wallet" (and
our code assumes a 2-byte prefix anyway).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It really has to be 0, since it's the complete secret. And we didn't handle
it well, (`a` would be treated as 0, for example!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Thread the signed tx through so close's JSON return contains that,
rather than the unsigned channel->last_tx.
We have to split the "get cmd_id" from "resolve the close commands" though;
and of course, as before, we don't actually print the txids of multiple
transactions even though we may have multi in flight due to splice!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: JSON-RPC: `close` returns a `tx` field with witness data populated (i.e. signed).
Fixes: #6440
Update the lightningd <-> channeld interface with lots of new commands to needed to facilitate spicing.
Implement the channeld splicing protocol leveraging the interactivetx protocol.
Implement lightningd’s channel_control to support channeld in its splicing efforts.
Changelog-Added: Added the features to enable splicing & resizing of active channels.
If you actually ran your node with the botched "last_invoice_created_index" typo migration
(fortunately, not release, just master) you can get a db with both the real "last_invoices_created_index" and the bad "last_invoice_created_index" entries.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
- adopt "const <type> *"convention
- remove use_shadow option for some pyln tests
- show prob. information of flows into paynotes
- show prob. of success of entire payment flow in paynotes
- minflow: We were not releasing the memory of flow arrays when replacing
them with a new canditate.
- use memleak_scan_obj in memleak_check
- replace u64 with size_t
Signed-off-by: Lagrang3 <eduardo.quintana@pm.me>
- remove internal gheap checks
- add check for arc_t.chanidx overflow
- remove outdated comments
- check the delta flow bounds before augmenting along a path
- get_flow_paths uses a dynamic tal array instead of a list.
- fix a unit test that depended on the order of returned flows
- fix bug: lightnind doesn't like if I reuse the partid of a failed
flow, therefore use a higher partid than any of the previous attempts.
- plugin_err instead of LOG_BROKEN if sendpay fails and we cannot get a
an error code.
- fix wrong comments.
- remove the background timer.
- This is a bugfix. Previous to this the MCF network was built using the
knowledge of the min and max liquidity but it didn't take into account
pending HTLCs.
- Also remove the min_prob_success option but hardcode a 90% value.
Removing some options that are not relevant to the user, they're kept
for developer mode only:
- base_fee_penalty
- min_prob_success
- prob_cost_factor
- remove heap.h, not used
Signed-off-by: Lagrang3 <eduardo.quintana@pm.me>
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
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>
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.
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>
