To limit attack surface.
Context:
- both in daemon mode and in GUI mode, we start an RPC server
- the RPC server uses HTTP basic auth, with a random password that is saved in the config file
- read access to the config file implies access to the RPC server
- the traffic is unencrypted
- by default the server listens
- on Windows, on localhost TCP
- all other platform, via unix domain sockets
- if an attacker can listen to localhost TCP traffic, and there was traffic
- they could see the plaintext RPC password and issue their own commands
- e.g. if wireshark was already installed on the system, this might not require root access
- the "ping" and "gui" commands are used by everyday operations that affect most users:
- "ping" is used when trying to launch a second instance of electrum, to contact the first instance and enforce "singleton" behaviour
- "gui" is used for URI handling (`$ xdg-open bitcoin:asdasd`)
- many other sensitive commands, that operate on wallets, require *also* the wallet password
- but note that wallet.unlock can be used by the user to bypass this and store the wallet password in memory (exposed in GUI)
I propose locking down the RPC server when running in GUI mode:
- we still start it, as it is used for "ping" and "gui" RPCs, however we disable all other RPCs
- we could opt-in enable it, using a config var, except that ofc would not help against an attacker that has filesystem write access to the config file
- so I think it's even safer to just "hardcode" disable it: however the functionality is useful for development
- I propose we branch based on `constants.net.TESTNET`
- an alternative we could branch on that is hard to fake is `is_git_clone` in run_electrum
rm the `assert fee >= 0, f"{fee=!r} must be non-negative satoshis"`
from `Abstract_Wallet.get_tx_fee_warning()` to prevent an exception when
users load a psbt with negative tx fee.
It is much easier to reason about the rpcserver if we don't allow changing its basic settings while it is already running. What does it mean to change the TCP port it is listening on ("rpcport") if it's already running? It is even problematic to change the rpcpassword: care needs to be taken to already update it for the current server.
(ref https://github.com/spesmilo/electrum/issues/6762)
This commit disallows changing all of the "rpc*" config variables if the daemon is already running.
---
Simultaneously, it also ensures rpc_password is always set and auth cannot be disabled.
Previously if there was a daemon running, and the user ran
`$ electrum setconfig rpcpassword ""` that would leave the RPC unauthenticated
for the current session. However next time the daemon restarted, get_rpc_credentials would see
the unset password and generate one.
I think this was the worst of both worlds:
- we did not really allow removing the rpc password, except for the current session, and
- perhaps unexpectedly, we would generate a new password on daemon restart
Instead now we explicitly make sure the RPC server can never get into a state where it does not have a password set.
Based on a report by `Zuzana Kotásková <36777@mail.vsfs.cz>`
The 2fa secret is not selectable or copyable, this is very inconveniant
when setting up a new 2fa wallet as the user has to somehow manually
write the secret e.g. on a paper to then enter it again in their 2fa
app. This makes the secret string copyable by clicking on it.
Catch NetLegacySinglesigScriptType and convert it to a
UserFacingException if the user tries to import a private key for which
it is not possible to get a singlesig descriptor (e.g. p2wsh).
Fixes#10536
- could not find a single project that still actually cares about bip70 [0]
- well except maybe BitPay.
- but I cannot test with BitPay:
- they have a testnet3 staging environment on test.bitpay.com
- but the SSL cert they use for bip70 has expired in 2021
- the webUI probably also has not been updated since then...
- they claim to have added LN support in 2022 in a blog post,
but it's not there on test.bitpay.com
- on mainnet, they require KYC before payment
- < ... angry noises >
- their loss then, I don't care.
- this is code that no one wants to maintain
- this does not yet delete the signed bip70 payment data for historical txs
- but it is no longer possible to export it from the GUI
[0]: https://bitcoinops.org/en/topics/bip70-payment-protocol/
As it's failing due to relative imports, this might have been broken since py2->py3 migration.
```
$ python3 ./electrum/interface.py
Traceback (most recent call last):
File "/home/user/wspace/electrum/./electrum/interface.py", line 31, in <module>
import asyncio
File "/usr/lib/python3.13/asyncio/__init__.py", line 8, in <module>
from .base_events import *
File "/usr/lib/python3.13/asyncio/base_events.py", line 18, in <module>
import concurrent.futures
File "/usr/lib/python3.13/concurrent/futures/__init__.py", line 8, in <module>
from concurrent.futures._base import (FIRST_COMPLETED,
...<9 lines>...
as_completed)
File "/usr/lib/python3.13/concurrent/futures/_base.py", line 7, in <module>
import logging
File "/home/user/wspace/electrum/electrum/logging.py", line 6, in <module>
import logging.handlers
ModuleNotFoundError: No module named 'logging.handlers'; 'logging' is not a package
```
For multiple transactions, split summary in total sent/received and a balance change.
move duplicated code to wallet.get_user_notifications_for_new_txns()
If SwapManager.percentage was a 0.2 float, rounding differences would
cause an exception in the fee calculation inverse sanity check when entering 20
000 sats into the SwapDialog. By making self.percentage a decimal we can
prevent this kind of issue.
```
File "/home/user/code/vibecoding_vm/electrum/electrum/gui/qt/swap_dialog.py", line 294, in on_send_edited
recv_amount = self.swap_manager.get_recv_amount(send_amount, is_reverse=self.is_reverse)
File "/home/user/code/vibecoding_vm/electrum/electrum/submarine_swaps.py", line 1320, in get_recv_amount
if abs(send_amount - inverted_send_amount) > 1:
~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
TypeError: unsupported operand type(s) for -: 'int' and 'NoneType'
```
Adds a link to the plugins.electrum.org website so users who open the
plugins dialog out of curiosity get guided to the website and can
discover other plugins and learn more about the system.
skip pending swaps in the swapserver history/summary cli commands.
They are not relevant and don't contain all required informations yet.
Fixes https://github.com/spesmilo/electrum/issues/10521
Fixes https://github.com/spesmilo/electrum/issues/10525
```
File "/home/electrum/electrum-fork/electrum/daemon.py", line 268, in handle
response['result'] = await f(*params)
^^^^^^^^^^^^^^^^
File "/home/electrum/electrum-fork/electrum/daemon.py", line 381, in run_cmdline
result = await func(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/electrum/electrum-fork/electrum/commands.py", line 207, in func_wrapper
File "/home/electrum/electrum-fork/electrum/commands.py", line 2349, in func_wrapper
group = parser.add_argument_group('network options')
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/electrum/electrum-fork/electrum/plugins/swapserver/__init__.py", line 79, in get_summary
swap_history = await get_history(self)
^^^^^^^^^^^^^^^^^^^^^^^
File "/home/electrum/electrum-fork/electrum/commands.py", line 207, in func_wrapper
File "/home/electrum/electrum-fork/electrum/commands.py", line 2349, in func_wrapper
group = parser.add_argument_group('network options')
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/electrum/electrum-fork/electrum/plugins/swapserver/__init__.py", line 60, in get_history
'date': swap['date'].strftime("%Y-%m-%d"),
^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'strftime'
```
The `until` filter would limit the relay to only send us events created
up until this timestamp. If the user opens a swap transport by opening
the swap dialog, and keeps the dialog open the dialog will naturally age
above this limit and the relay will stop sending the client swapserver
events as they have (legitimately) been created after this timestamp.
As sanity check we still have the comparison against the current
timestamp in the event parsing loop to prevent pre/backdating.
Fixes https://github.com/spesmilo/electrum/issues/10520
Waste less space in the dialog by limiting the stretch to 10px and
resize the servers_list with the dialog by setting stretch=1 so it can
be made larger.
Fixes https://github.com/spesmilo/electrum/issues/10519
Non-Ascii characters should not be converted for
checksum calculation.
This will give consistent hash to BIP-128 and its
Javascript code example.
Timelock-Recovery Plans that contained only ascii
characters are not affected.
Also, 8 hex-chars is enough for a checksum.
The typical flow of an update is:
---UPDATE--->
--- SIG --->
<--REVACK----
<-- SIG ----
---REVACK--->
It makes sense to try to send a sig ("commitment_signed") right after we send an update.
It also makes sense right after we send revack.
Besides those times, we could call "maybe_send_commitment" at *any* time, that is safe, and depending on other call locations, it might be an optimisation, however it is not needed.
In particular it is unclear why we had those calls when we *receive* updates (and only for certain types of updates - not consistently).
I moved my node to standard port again because the non-standard one was apparently used by malware. I also updated the domain name (both work at the moment, but I prefer the shorter one).
self.oldfeeRate is initialized as int in `QETxRbfFeeBumper` and
`QETxCanceller`. However QML expects it to be a string. Initializing
it as string fixes the exception. Previously this didn't happen as
`Abstract_Wallet.add_info_from_wallet_and_network()` would never
return False, so the `get_tx()` method would immediately overwrite
the self.oldfeeRate variable with a string.
```
20.95 | D | gui.qml.qetxfinalizer | TxMonMixin.__init__
20.95 | E | gui.qml.qeapp.Exception_Hook | exception caught by crash reporter
TypeError: unable to convert a Python 'int' object to a C++ 'QString' instance
```
Early return the update() methods of QETxRbfFeeBumper and QETxCanceller
if it fails to fetch missing tx information from the Network to prevent
an exception in `Abstract_Wallet.bump_fee()` and
`Abstract_Wallet.dscancel()`. See
https://github.com/spesmilo/electrum/issues/5502#issuecomment-40213084270
`Transaction.add_info_from_network` would swallow a `NetworkException`
even when `ignore_network_issues=False` is passed.
This causes `Transaction.add_info_from_wallet_and_network` to
incorrectly return True even if the operation failed.
If `QETxRbfFeeBumper` then incorrectly proceeds assuming the call was
successful `Abstract_Wallet.bump_fee()` would raise an `Exception("tx
missing info from network")`.
Should fix the traceback in
https://github.com/spesmilo/electrum/issues/5502#issuecomment-4021308427.
Fixes exception in lookup_invoice:
```
62.69 | E | plugins.nwc.nwcserver.NWCServer | Error handling nwc request
Traceback (most recent call last):
File "/home/user/code/vibecoding_vm/electrum/electrum/plugins/nwc/nwcserver.py", line 381, in run_request_task
await task
File "/home/user/code/vibecoding_vm/electrum/electrum/util.py", line 1211, in wrapper
return await func(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/user/code/vibecoding_vm/electrum/electrum/plugins/nwc/nwcserver.py", line 526, in handle_lookup_invoice
info = self.wallet.lnworker.get_payment_info(invoice.payment_hash, direction=RECEIVED)
^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Invoice' object has no attribute 'payment_hash'
```
Always includes bolt11 invoice in response, even if the client already
sent it to us in the request. It doesn't seem useful and is marked
optional in the spec but https://sandbox.albylabs.com considers the
response invalid if the invoice is not included.
Also consider htlcs that are still inflight when calculating
the amount/fee of a payment to prevent calculating an incorrect
payment value/fee when calling get_payment_info directly after
a payment succeeds (one htlc settled, but others could still be
inflight).
This assumes that once a payments htlc got settled all other inflight
htlcs will also get settled.
Include the lightning routing fees into the payment budget accounting
to prevent untrusted nwc clients from exceeding the budget by crafting
a high fee route.
There is now only add_to_budget which will return None if the budget
doesn't allow for this spend. It will always add the spend to the
budget, even if the user has no budget so it can be used for display
purposes (and simpler code, only one path).
Also stop timer when dialog is finished, to avoid re-generating txs with the same input coin set, which
results in an exception as these coins have signatures when the swap has started.
f321x