Values for exponential search are based on available fee budget:
we try with budget/64, budget/32, ..., budget/1 (spread uniformly among the selected Trampoline Forwarders).
Hence, if we make the fee budget configurable, that will usefully affect the trampoline fees as well.
related https://github.com/spesmilo/electrum/issues/9033
Instead of some functions operating with hex strings,
and others using bytes, this consolidates most things to use bytes.
This mainly focuses on bitcoin.py and transaction.py,
and then adapts the API usages in other files.
Notably,
- scripts,
- pubkeys,
- signatures
should be bytes in almost all places now.
This gives more time for the client to come back online.
see https://github.com/spesmilo/electrum/issues/8940
- re note on submarine_swaps.py#L53:
lnpeer.Peer.maybe_fulfill_htlc only checks against MIN_FINAL_CLTV_DELTA_ACCEPTED(=144),
so this increased cltv_delta is not enforced when receiving the htlc on ln.
It is put in the invoice, so the sender is supposed to honour it ofc.
It would be nice to enforce it (make the check in maybe_fulfill_htlc dependent on
what was in the invoice).
- scenario:
- reuse same seed between two devices, LN enabled on both
- open channel using device1, import chan backup on device2
- local-force-close channel using device1
- tx1 (ctx) gets into mempool (or even mined), tx2 (sweep tx for to_local) is a local (future) tx
- history tab on device1 shows tx1 and tx2
- history tab on device2 was showing only tx2, and no info about tx1
- device2 knows about tx1, it was just not showing it previously.
With this commit, tx1 is now shown in the history.
- note: tx1 might linger in the mempool for an indeterminate amount of time, or even become local.
During that time, it is confusing on device2 not to show any feedback. Also,
if tx1 becomes local, it is useful to be able to rebroadcast it.
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
lnworker.suggest_splits for non-trampoline case tries to split amts over 5000 sat
but mpp_split.suggest_splits does not return splits where any part is smaller than 10000 sat.
So in practice, without trampoline, invoices between 5k and ~20k sats could not be paid.
This suggests that the logic should not be scattered in multiple places but merged into mpp_split.py...
This commit just does a quick fix though, to try again without splitting if there was no solution.
- 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")
The convention is that edges (start_node -> edge_node) store
the policy/fees for the *start_node*.
This is what the non-trampoline edges were already using (for a long time),
but the trampoline ones were off-by-one (policy was for end_node),
which was then worked around in multiple places, to correct for...
i.e. I think because of all the workarounds, there was no actual bug,
but it was just very confusing.
Also note that the prior usage of trampoline edges would not work if
we (sender) were not directly connected to a TF (trampoline-forwarder)
but had extra edges in the route to even get to the first TF.
Having the policy corresponding to the start_node of the edge would work
even in that case.
Note: this issue is currently not detected in python unittests,
it shows up only in regtest, and is not currently tested.
One would need to use a proper LNWallet instance in unit tests.
ThomasV
SomberNight
jinjiadu
Sander van Grieken
MrNaif2018