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.
Note in particular that check_password_for_directory was not safe to use while the daemon had wallets loaded,
as the same file would have two corresponding Wallet() instances in memory. This was specifically handled in
the kivy GUI, on the caller side, by stopping-before and reloading-after the wallets; but it was dirty to
have the caller handle this.
get_unused_addresses() has been broken since #7730, because
addresses are considered as permanently used if they are in
the list of keys of receive_requests. This is true even if
an address is used as fallback for a lightning payment. This
means that the number of lightning payments we can receive
is constrained by the gap limit.
If a payment succeeds off-chain, we want to be able to reuse
its fallback address in other requests (this does not reduce
privacy, because invoices already share the same public key).
This implies that we should not use the onchain address as key
for lightning-enabled requests in wallet.receive_requests. If
we did, paid invoices would be overwritten when the address is
reused. That is the reason for the wallet_db upgrade.
Related: a3faf85e3c
- add new index: requests_rhash_to_key (fixes#7845)
- when creating a request, do not save its description in labels.
Instead, return it as default value in wallet.get_label_by_rhash
lnworker:
- rename 'payments' to 'payment_info'
- add note to delete_payment_info
commands: rename 'rmrequest' to 'delete_request'
- revert the logic of do_breach_remedy to what it was
before 0ca3d66d15,
but now calling self.maybe_redeem unconditionally.
- replace mempool transactions only if the fee increases
- do not notify the GUI if a local tx is replaced
- delete labels when replacing
The wallet needs its own up_to_date logic:
- the adb being up_to_date means all its addresses are synced
- but an HD wallet might decide to roll the gap limit and generate new addresses
- the adb does not know about this...
- the wallet should be considered *not* up_to_date
- relatedly, it is now the wallet that decides to reset the network request counters
- note that wallet.main() was never cleaned up previously.
- now wallet gets its own taskgroup, which is cleaned up in wallet.stop.
Follow-up to adb refactor: 121d8732f1
- separate AddressSynchronizer from Wallet and LNWatcher
- the AddressSynchronizer class is referred to as 'adb' (address database)
- Use callbacks to replace overloaded methods
These methods return a list of channels that can be rebalanced,
in order to receive or send a given amount.
Also add 'channels' parameter to submarine swaps.
Previously, swaps were not considering which channel to use.
When we do not have liquidity to pay an invoice:
- add 'rebalance' option in order to pay an invoice
- use the suggested channel in the 'swap' option
When we do not have the liquidity to receive an invoice:
- add 'Rebalance' and 'Swap' buttons to the receive tab
I believe lightning requests created before https://github.com/spesmilo/electrum/pull/7730
can have an amount of None - ones created after have amount 0 instead.
We could do a wallet db upgrade potentially.
Regardless, the type hint is `get_amount_sat(self) -> Union[int, str, None]`,
so None should be handled. (well, arguably "!" should be handled too...)
```
E | gui.qt.exception_window.Exception_Hook | exception caught by crash reporter
Traceback (most recent call last):
File "...\electrum\electrum\gui\qt\request_list.py", line 101, in item_changed
self.parent.show_receive_request(req)
File "...\electrum\electrum\gui\qt\main_window.py", line 1279, in show_receive_request
URI = req.get_bip21_URI(lightning=bip21_lightning)
File "...\electrum\electrum\invoices.py", line 164, in get_bip21_URI
amount = int(self.get_amount_sat())
TypeError: int() argument must be a string, a bytes-like object or a real number, not 'NoneType'
```
```
E | gui.qt.exception_window.Exception_Hook | exception caught by crash reporter
Traceback (most recent call last):
File "...\electrum\electrum\gui\qt\request_list.py", line 101, in item_changed
self.parent.show_receive_request(req)
File "...\electrum\electrum\gui\qt\main_window.py", line 1281, in show_receive_request
can_receive_lightning = self.wallet.lnworker and req.get_amount_sat() <= self.wallet.lnworker.num_sats_can_receive()
TypeError: '<=' not supported between instances of 'NoneType' and 'decimal.Decimal'
```
It does not make sense to count change outputs in our unconfirmed balance,
because our balance will not be negatively affected if the transaction does
not get confirmed.
It is also incorrect to add signed values of get_addr_balance in order to compute
the balance over a domain. For example, this leads to incoming and outgoing
transactions cancelling out in our total unconfirmed balance.
This commit looks at the coins that are spent by a transaction. If those
coins belong to us and are confirmed, we do not count the transaction outputs
in our unconfirmed balance.
As a result, get_balance always returns positive values for unconfirmed balance.
Scenario:
- 2of2 multisig wallet with device1 and device2
- disconnect all devices
- open wallet file
- fail all pairings at wallet-open
- connect device2
- try to sign a tx
At this point Electrum will try to find the device for keystore1 first, and there is only a single unpaired device: device2.
Automatic pairing of keystore1 and device2 will fail, due to device id mismatching compared to what is persisted on disk for keystore1, so the user is prompted for manual selection. The selection dialog is somewhat confusing as it is not clear that the app is asking to select a device for keystore1. Pairing would fail, so the user is expected to cancel the dialog. If they cancel, keystore1 is skipped, and we try to pair for keystore2 now, and device2 will pair with it automatically.
fixes https://github.com/spesmilo/electrum/issues/4199#issuecomment-1112552416
