We must not process incoming updates for a given channel until we ~finished reestablishing it.
Consider both parties have some unacked updates they want to replay during reestablish.
If Bob reacts to Alice's replayed stuff before he himself replays his stuff, madness ensues.
I think this should fix the remaining part of https://github.com/spesmilo/electrum/pull/8778
(timing issues when running the unit tests with py3.12)
This allows making any message handler async in lnpeer.
Note: `process_message` is only called from `_message_loop`.
There are(would be) basically three types of message handlers:
1. "traditional blocking msg handlers". non-async ones. When these handlers are called, `process_message` naturally blocks until the handler returns, which means `_message_loop` also blocks until the message is fully processed before starting the next iteration.
2. "async blocking msg handlers". async ones where we want the previous property, i.e. we want the `_message_loop` to wait until the handler finishes. We await the handler inside `process_message`, and `_message_loop` awaits `process_message`.
3. "async non-blocking msg handlers". async message handlers that can be spawned e.g. onto `Peer.taskgroup` and the loop is free to start processing subsequent messages. e.g. msg handlers that start a negotiation, such as `on_shutdown` and `on_open_channel`.
Any non-async message handler (`def on_...`) automatically goes into category 1.
An async message handler, by default, goes into category 2, "blocking";
to go into category 3 ("non-blocking"), we use the `runs_in_taskgroup` function decorator.
The existing logic of only updating the fee if it is not within 2x of
the current 2-block-eta does not work well for the current mempool.
The current mempool looks a bit weird: you need ~20 sat/vbyte to even get into it,
but there is only around 5 MB of txs paying >25 sat/vbyte.
The estimates look like this:
```
>>> config.fee_estimates
{144: 25764, 25: 27075, 10: 34538, 5: 34538, 2: 34538}
```
This commit changes the logic so that we send update_fee if the old rate is
- below 75% of the current 2-block-eta (instead of 50%), or
- below the 25-block-eta
In particular, lnd sends both chan_reest and then an error ("sync error").
It is critical we process the chan_reest and transition to WE_ARE_TOXIC before
processing the error (which would trigger us to force-close).
see spec 8a64c6a1ce/02-peer-protocol.md (L1504-L1506) :
> - upon reconnection:
> [...]
> - MUST transmit channel_reestablish for each channel.
> - MUST wait to receive the other node's channel_reestablish message before sending any other messages for that channel.
Assume Alice and Bob have a channel, and Alice is on an old state,
but neither of them knows the latter yet.
Timing scenarios:
1. Alice sends reest first, and Bob receives it before sending reest himself
- old code: Bob realises Alice is behind, Bob will force-close,
Bob won't send reest to Alice, so Alice does not learn she is behind
- new code: Bob realises Alice is behind, Bob will force-close,
Bob will still send reest to Alice, so Alice learns she is behind.
2. Bob sends reest first, and Alice receives it before sending reest herself
- old code: Alice learns she is behind. Alice won't send reest to Bob,
so Bob does not learn he is ahead, so Bob won't force-close.
- new code: Alice learns she is behind. Alice will still send reest to Bob
though with ctn=0 instead of actual. Bob learns he is ahead, so
Bob will force close.
3. Alice and Bob both send reest, and then they both receive what the other sent
- no change: Alice and Bob both learn Alice is behind. Bob will force-close.
- all forwarding types use the same flow
- forwarding callback returns a htlc_key or None
- forwarding info is persisted in lnworker:
- ongoing_forwardings
- downstream to upstream htlc_key
- htlc_key -> error_bytes
- a node scid alias is derived from the node ID
- the channel opening fee is sent in a TLV field of open_channel
- the server requires htlc settlement before broadcasting
(server does not trust client)
- introduce PaymentFeeBudget, which contains limits for fee budget and cltv budget
- when splitting a payment,
- the fee budget is linearly distributed between the parts
- this resolves a FIXME in lnrouter ("FIXME in case of MPP")
- the cltv budget is simply copied
- we could also add other kinds of budgets later, e.g. for the num in-flight htlcs
- resolves TODO in lnworker ("todo: compare to the fee of the actual route we found")
- is_trampoline was True iff we are the final recipient of a trampoline payment
- in that case, we were only comparing htlc.cltv_expiry against the outer onion cltv
- we should and do now also compare against the inner onion cltv
- the checks are changed from "!=" to "<", to adapt to bolts PR 1032
- see b38156b951
- note that the leniency is needed for the cltv off-by-one
added in eca10eb04d to work
propageate back to the sender.
lnworker: in htlc_fulfilled and htlc_failed, return early if the
htlc was forwarded, so that we do not trigger invoice callbacks
lnworker.pay_to_node(min_cltv_expiry=) expects a relative cltv, and
we were passing an absolute one.
It is not so clear what precisely should be passed here, and that is
because pay_to_node's API was not written with forwarding in mind.
The value chosen here is just some guess that typically should work.
- construct_channel_announcement: return also whether
node ids are in reverse order
- maybe_send_channel_announcement:
return early if signatures have not been received
- partial writes are append only.
- StoredDict objects will append partial writes to the wallet
file when items are added, replaced, removed.
- Lists in the wallet file that have not been registered
as StoredObject are converted to StoredList, which
overloads append() and remove(). Those methods too will
append partial writes to the wallet file.
- Unlike the old jsonpatch branch, this branch does not support
file encryption. Encrypted files always fully rewritten, even
if the change before encryption is a partial write.
Somewhat a follow-up to 649ce979ab.
This adds some safety belts so we don't accidentally sign a tx that
contains a dummy address.
Specifically we check that tx does not contain output for dummy addr:
- in wallet.sign_transaction
- in network.broadcast_transaction
The second one is perhaps redundant, but I think it does not hurt.
Due to an indendation error, fw_info was returned only for one
HTLC of the MPP. Thus, maybe_fulfill_htlc was called again with
the remaining HTLCs, which could possibly be failed due to MPP
timeout, resulting in fund loss for the forwarder.
- rename trampoline_forwardings -> final_onion_forwardings,
because this dict is used for both trampoline and hold invoices
- remove timeout from hold_invoice_callbacks (redundant with invoice)
- add test_failure boolean parameter to TestPeer._test_simple_payment,
in order to test correct propagation of OnionRoutingFailures.
- maybe_fulfill_htlc: raise an OnionRoutingFailure if we do not have
the preimage for a payment that does not have a hold invoice callback.
Without this, the above unit tests stall when we use test_failure=True
- introduce SentHtlcInfo named tuple
- some previously unnamed tuples are now much shorter:
create_routes_for_payment no longer returns an 8-tuple!
- sent_htlcs_q (renamed from sent_htlcs), is now keyed on payment_hash+payment_secret
(needed for proper trampoline forwarding)
- add RecvMPPResolution enum for possible states of a pending incoming MPP,
and use it in check_mpp_status
- new state: "FAILED", to allow nicely failing back the whole MPP set
- key more things with payment_hash+payment_secret, for consistency
(just payment_hash is insufficient for trampoline forwarding)
triggered a payment forwarding.
Final onions may trigger a payment forwarding, through the callback
returned by maybe_fulfill_htlc. In that case, we should not fail the
HTLC later; doing so might result in fund loss.
Remove test_simple_payment_with_hold_invoice_timing_out: once we
have accepted to forward a payment HTLC with a hold invoice, we
do not want to time it out, for the same reason.
