- the unlock command was replaced by an option to load_wallet,
because some applications (the swapserver plugin) need to be
executed with an unlocked password. Now the swapserver plugin
waits until the wallet is unlocked.
- wallet.unlock now checks password unconditionally, see #8799
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.
Previously, generally, in case of any error, commands would raise a generic "Exception()" and the CLI/RPC would convert that and return it as `str(e)`.
With this change, we now distinguish "user-facing exceptions" (e.g. "Password required" or "wallet not loaded") and "internal errors" (e.g. bugs).
- for "user-facing exceptions", the behaviour is unchanged
- for "internal errors", we now pass around the traceback (e.g. from daemon server to rpc client) and show it to the user (previously, assuming there was a daemon running, the user could only retrieve the exception from the log of that daemon). These errors use a new jsonrpc error code int (code 2).
As the logic only changes for "internal errors", I deem this change not to be compatibility-breaking.
----------
Examples follow.
Consider the following two commands:
```
@command('')
async def errorgood(self):
from electrum.util import UserFacingException
raise UserFacingException("heyheyhey")
@command('')
async def errorbad(self):
raise Exception("heyheyhey")
```
----------
(before change)
CLI with daemon:
```
$ ./run_electrum --testnet daemon -d
starting daemon (PID 9221)
$ ./run_electrum --testnet errorgood
heyheyhey
$ ./run_electrum --testnet errorbad
heyheyhey
$ ./run_electrum --testnet stop
Daemon stopped
```
CLI without daemon:
```
$ ./run_electrum --testnet -o errorgood
heyheyhey
$ ./run_electrum --testnet -o errorbad
heyheyhey
```
RPC:
```
$ curl --data-binary '{"id":"curltext","jsonrpc":"2.0","method":"errorgood","params":[]}' http://user:pass@127.0.0.1:7777
{"id": "curltext", "jsonrpc": "2.0", "error": {"code": 1, "message": "heyheyhey"}}
$ curl --data-binary '{"id":"curltext","jsonrpc":"2.0","method":"errorbad","params":[]}' http://user:pass@127.0.0.1:7777
{"id": "curltext", "jsonrpc": "2.0", "error": {"code": 1, "message": "heyheyhey"}}
```
----------
(after change)
CLI with daemon:
```
$ ./run_electrum --testnet daemon -d
starting daemon (PID 9254)
$ ./run_electrum --testnet errorgood
heyheyhey
$ ./run_electrum --testnet errorbad
(inside daemon): Traceback (most recent call last):
File "/home/user/wspace/electrum/electrum/daemon.py", line 254, in handle
response['result'] = await f(*params)
File "/home/user/wspace/electrum/electrum/daemon.py", line 361, in run_cmdline
result = await func(*args, **kwargs)
File "/home/user/wspace/electrum/electrum/commands.py", line 163, in func_wrapper
return await func(*args, **kwargs)
File "/home/user/wspace/electrum/electrum/commands.py", line 217, in errorbad
raise Exception("heyheyhey")
Exception: heyheyhey
internal error while executing RPC
$ ./run_electrum --testnet stop
Daemon stopped
```
CLI without daemon:
```
$ ./run_electrum --testnet -o errorgood
heyheyhey
$ ./run_electrum --testnet -o errorbad
0.78 | E | __main__ | error running command (without daemon)
Traceback (most recent call last):
File "/home/user/wspace/electrum/./run_electrum", line 534, in handle_cmd
result = fut.result()
File "/usr/lib/python3.10/concurrent/futures/_base.py", line 458, in result
return self.__get_result()
File "/usr/lib/python3.10/concurrent/futures/_base.py", line 403, in __get_result
raise self._exception
File "/home/user/wspace/electrum/./run_electrum", line 255, in run_offline_command
result = await func(*args, **kwargs)
File "/home/user/wspace/electrum/electrum/commands.py", line 163, in func_wrapper
return await func(*args, **kwargs)
File "/home/user/wspace/electrum/electrum/commands.py", line 217, in errorbad
raise Exception("heyheyhey")
Exception: heyheyhey
```
RPC:
```
$ curl --data-binary '{"id":"curltext","jsonrpc":"2.0","method":"errorgood","params":[]}' http://user:pass@127.0.0.1:7777
{"id": "curltext", "jsonrpc": "2.0", "error": {"code": 1, "message": "heyheyhey"}}
$ curl --data-binary '{"id":"curltext","jsonrpc":"2.0","method":"errorbad","params":[]}' http://user:pass@127.0.0.1:7777
{"id": "curltext", "jsonrpc": "2.0", "error": {"code": 2, "message": "internal error while executing RPC", "data": {"exception": "Exception('heyheyhey')", "traceback": "Traceback (most recent call last):\n File \"/home/user/wspace/electrum/electrum/daemon.py\", line 254, in handle\n response['result'] = await f(*params)\n File \"/home/user/wspace/electrum/electrum/commands.py\", line 163, in func_wrapper\n return await func(*args, **kwargs)\n File \"/home/user/wspace/electrum/electrum/commands.py\", line 217, in errorbad\n raise Exception(\"heyheyhey\")\nException: heyheyhey\n"}}}
```
- rename globals
- also rm hardcoded strings from qml
- use consistent unit names in qml
(previously mixed sat/vB and sat/byte (latter coming from core lib))
qml: use backend sighash check and add user confirmation path
qt: use backend sighash check and add user confirmation path
qml: include get_warning_for_risk_of_burning_coins_as_fees test in txdetails
qt: add warning icon to sighash warning
add sighash and fee checks to wallet.sign_transaction, making all warnings fatal unless ignore_warnings is set to True
tests: test sign_transaction on both code paths with ignore_warnings True and False,
raise specific exceptions TransactionPotentiallyDangerousException and TransactionDangerousException
gui/qt/rbf_dialog.py (old) lines 57-64 were implementing logic that should not be part of GUI code.
Case in point, gui/qml/qetxfinalizer.py (old) lines 511-513 duplicated half of that logic but not the other half.
That logic is now moved to wallet.get_bumpfee_strategies_for_tx().
More context: a user on irc got confused when using the qml gui. They were sending "max" and wanted to bump_fee.
The qml gui selected the "preserve_payment" strategy by default, using which there was no solution, and the user
did not notice that the strategy can be altered (via the "method" dropdown). The qt gui had logic to select
"decrease_payment" by default in such a case (which does find a solution to bump) but this logic was not
duplicated in the qml gui.
Instead of duplicating the logic, this commit moves it to shared lib code.
Those commands were only used for testing.
inject_fees was particularly misleading, because an electrum
instance with an active network connection regularly updates
its fees, thus voiding the effecct of that command.
A new config API is introduced, and ~all of the codebase is adapted to it.
The old API is kept but mainly only for dynamic usage where its extra flexibility is needed.
Using examples, the old config API looked this:
```
>>> config.get("request_expiry", 86400)
604800
>>> config.set_key("request_expiry", 86400)
>>>
```
The new config API instead:
```
>>> config.WALLET_PAYREQ_EXPIRY_SECONDS
604800
>>> config.WALLET_PAYREQ_EXPIRY_SECONDS = 86400
>>>
```
The old API operated on arbitrary string keys, the new one uses
a static ~enum-like list of variables.
With the new API:
- there is a single centralised list of config variables, as opposed to
these being scattered all over
- no more duplication of default values (in the getters)
- there is now some (minimal for now) type-validation/conversion for
the config values
closes https://github.com/spesmilo/electrum/pull/5640
closes https://github.com/spesmilo/electrum/pull/5649
Note: there is yet a third API added here, for certain niche/abstract use-cases,
where we need a reference to the config variable itself.
It should only be used when needed:
```
>>> var = config.cv.WALLET_PAYREQ_EXPIRY_SECONDS
>>> var
<ConfigVarWithConfig key='request_expiry'>
>>> var.get()
604800
>>> var.set(3600)
>>> var.get_default_value()
86400
>>> var.is_set()
True
>>> var.is_modifiable()
True
```
This fixes an inconsistency where the 'expiration' field was
relative for invoices, and absolute timestamp for requests.
This in turn fixes QML the timer refreshing the request list.
In order to prevent any API using that field from being silently
broken, the 'expiration' field is renamed as 'expiry'.
When called via jsonrpc (but not via cli) with non-string amounts,
there could be a rounding error resulting in sending 1 sat less.
example:
```
$ ./run_electrum --testnet -w ~/.electrum/testnet/wallets/test_segwit_2 paytomany '[["tb1q6k5h4cz6ra8nzhg90xm9wldvadgh0fpttfthcg", 0.00033389]]' --fee 0
02000000000101b9e6018acb16952e3c9618b069df404dc85544eda8120e5f6e7cd7e94ce5ae8d0100000000fdffffff02fd8100000000000016001410c5b97085ec1637a9f702852f5a81f650fae1566d82000000000000160014d5a97ae05a1f4f315d0579b6577daceb5177a42b024730440220251d2ce83f6e69273de8e9be8602fbcf72b9157e1c0116161fa52f7e04db6e4302202d84045cc6b7056a215d1db3f59884e28dadd5257e1a3960068f90df90b452d1012102b0eff3bf364a2ab5effe952cba33521ebede81dac88c71951a5ed598cb48347b3a022500
$ curl --data-binary '{"id":"curltext","method":"paytomany","params":{"outputs":[["tb1q6k5h4cz6ra8nzhg90xm9wldvadgh0fpttfthcg", 0.00033389]], "fee": 0, "wallet": "/home/user/.electrum/testnet/wallets/test_segwit_2"}}' http://user:pass@127.0.0.1:7777
{"id": "curltext", "jsonrpc": "2.0", "result": "02000000000101b9e6018acb16952e3c9618b069df404dc85544eda8120e5f6e7cd7e94ce5ae8d0100000000fdffffff02fe8100000000000016001410c5b97085ec1637a9f702852f5a81f650fae1566c82000000000000160014d5a97ae05a1f4f315d0579b6577daceb5177a42b0247304402206ef66b845ca298c14dc6e8049cba9ed19db1671132194518ce5d521de6f5df8802205ca4b1aee703e3b98331fb9b88210917b385560020c8b2a8a88da38996b101c4012102b0eff3bf364a2ab5effe952cba33521ebede81dac88c71951a5ed598cb48347b39022500"}
```
^ note that first tx has output for 0.00033389, second tx has output for 0.00033388
fixes https://github.com/spesmilo/electrum/issues/8274
- wallet.add_input_info() previously had a fallback to download parent
prev txs from the network (after a lookup in wallet.db failed).
wallet.add_input_info() is not async, so the network request cannot
be done cleanly there and was really just a hack.
- tx.add_info_from_wallet() calls wallet.add_input_info() on each txin,
in which case these network requests were done sequentially, not concurrently
- the network part of wallet.add_input_info() is now split out into new method:
txin.add_info_from_network()
- in addition to tx.add_info_from_wallet(), there is now also tx.add_info_from_network()
- callers of old tx.add_info_from_wallet() should now called either
- tx.add_info_from_wallet(), then tx.add_info_from_network(), preferably in that order
- tx.add_info_from_wallet() alone is sufficient if the tx is complete,
or typically when not in a signing context
- callers of wallet.bump_fee and wallet.dscancel are now expected to have already
called tx.add_info_from_network(), as it cannot be done in a non-async context
(but for the common case of all-inputs-are-ismine, bump_fee/dscancel should work regardless)
- PartialTxInput.utxo was moved to the baseclass, TxInput.utxo
Always use "." as decimal point, and " " as thousands separator.
Previously,
- for decimal point, we were using
- "." in some places (e.g. AmountEdit, most fiat amounts), and
- `locale.localeconv()['decimal_point']` in others.
- for thousands separator, we were using
- "," in some places (most fiat amounts), and
- " " in others (format_satoshis)
I think it is better to be consistent even if whatever we pick differs from the locale.
Using whitespace for thousands separator (vs comma) is probably less confusing for people
whose locale would user "." for ts and "," for dp (as in e.g. German).
The alternative option would be to always use the locale. Even if we decide to do that later,
this refactoring should be useful.
closes https://github.com/spesmilo/electrum/issues/2629
- 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
