Especially during initial history sync, there are a lot of False->False up_to_date transitions
(e.g. adb.add_address generates one), and the GUI does some work for each, which adds up to a lot
of CPU usage for the full sync.
- replace complex strategies with a simpler choice,
between preserving or decreasing the payment.
- Always expose that choice to the user.
- Show the resulting fees to the user before they click OK
preference from the GUI, because the mempoolfullrbf option in
Bitcoin 0.24 makes RBF signaling pretty meaningless. Fixes#8088.
Note: RBF remains disabled for channel funding transactions.
In that case, the flag is actually only used as a semaphore
between different instances of the same wallet.
I find this easier to reason about than occasionally overwriting the items.
get_request_by_addr still only returns a single invoice for simplicity,
but now all logic regarding how to handle collisions is inside that method.
Replace get_key_for_outgoing_invoice, get_key_for_incoming_request
with Invoice.get_id()
When a new request is created, reuse addresses of expired requests (fixes#7927)
The API is changed for the following commands:
get_request, get_invoice,
list_requests, list_invoices,
delete_request, delete_invoice
`wallet.make_unsigned_transaction` can raise NotEnoughFunds or NoDynamicFeeEstimates.
These are "expected" exceptions that need to be handled or worked around. Added a note
of this in the docstring now.
We now handle NoDynamicFeeEstimates by allowing a static fallback fee in
`wallet.can_pay_onchain` and `lnworker.suggest_funding_amount`. It should be
fine to have a static fallback in these cases, as the user still has a chance
to set their own fee later in the flow.
(though ofc the static fallback might be too high in some mempool conditions,
in which case e.g. can_pay_onchain might return a false-negative, but this
is still an improvement over raising I believe)
fixes https://github.com/spesmilo/electrum/issues/5784
So that the two methods are consistent with each other.
As concrete motivation, see e.g.
- how the `getrequest(key)` command calls `wallet.get_request(key)`, and
- the `delete_request(address)` command calls `wallet.delete_request(address)`
The _receive_requests dict is keyed by either 'address' or 'rhash' (see get_key_for_receive_request):
- 'rhash' is used if `req.lightning_invoice is not None`
- address is used otherwise
The `get_request_by_address` method was quite error-prone: it only worked for requests that had an LN part...
fixes https://github.com/spesmilo/electrum/issues/7919
In the past, when creating payment requests, we keyed them by on-chain address,
and set/saved the msg of the request as label for the address.
Many places in the code were calling wallet.get_label(addr) with the expectation that
relevant payment requests are found and their message/description (if any) is considered.
wallet.get_label(key) is now made private, and instead the explicit non-polymorphic
wallet.get_label_for_{address,rhash,txid} alternatives should be used.
As the GUI allows saving such invoices, CLI should not break for these wallets.
Also note the pre-existing assert on the next line.
follow-up bf4455ef30
```
>>> list_invoices()
Traceback (most recent call last):
File "...\electrum\gui\qt\main_window.py", line 1506, in <lambda>
return lambda *args, **kwargs: f(method,
File "...\electrum\commands.py", line 191, in _run
result = fut.result()
File "...\Python310\lib\concurrent\futures\_base.py", line 446, in result
return self.__get_result()
File "...\Python310\lib\concurrent\futures\_base.py", line 391, in __get_result
raise self._exception
File "...\electrum\commands.py", line 154, in func_wrapper
return await func(*args, **kwargs)
File "...\electrum\commands.py", line 1188, in list_invoices
return [wallet.export_invoice(x) for x in l]
File "...\electrum\commands.py", line 1188, in <listcomp>
return [wallet.export_invoice(x) for x in l]
File "...\electrum\wallet.py", line 2405, in export_invoice
amount_sat = int(x.get_amount_sat())
ValueError: invalid literal for int() with base 10: '!'
```
There is some duplication between `wallet.get_onchain_request_status` and `wallet.is_onchain_invoice_paid`
(both in terms of code, and conceptually). `get_onchain_request_status` is used for incoming invoices (receive requests),
and `is_onchain_invoice_paid` is used for outgoing invoices. I think `get_onchain_request_status` existed first,
but as it uses txi/txo, it only works for ismine addresses, so `is_onchain_invoice_paid` was added later
(along with a `get_prevouts_by_scripthash` and corresponding new persisted data structure) to handle the non-ismine
addresses corresponding to outgoing invoices.
I think we could just merge the two functions together... (?)
and this PR does that.
