The history tab would show an incorrect feerate for partial/unsigned (local) txs,
if they had any p2sh/p2wsh txins. We would just guess the script is p2wpkh, and
use that for the size calc. Now with calling add_info_from_wallet, the correct
size is used to calculate the feerate.
(The gui tx dialogs call add_info_from_wallet independently, so the size/feerate
shown there were already correct.)
Some functions have an argument named "seed_type" in which it was annoying to call the seed_type() fn.
(especially for functions inside the same module)
- risk_of_burning_coins_as_fees is turned into a private (helper) method, only called by check_sighash.
UIs should only care about check_sighash.
- check_sighash returns instance of new class "TxSighashDanger" instead of tuple
- made warning levels more fine-grained (FEE_WARNING_SKIPCONFIRM vs FEE_WARNING_NEEDCONFIRM)
- this became more complicated than I had hoped for but I think it is worth it to ~merge
check_sighash and risk_of_burning_coins_as_fees into one.
related https://github.com/spesmilo/electrum/pull/8699
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.
The wizard should technically disallows this at creation time,
but this second layer sanity check could not hurt.
Also, looks like the wizard check is not working properly atm
(regression from qt wizard refactor).
Somewhat a follow-up to 649ce979ab.
This adds some safety belts so we don't accidentally sign a tx that
contains a dummy address.
Specifically we check that tx does not contain output for dummy addr:
- in wallet.sign_transaction
- in network.broadcast_transaction
The second one is perhaps redundant, but I think it does not hurt.
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
```
When exporting a tx as qr code, the prev txs are omitted to save space.
This causes problems with offline signers: software electrum signers will
just warn and then proceed, but hw devices will typically error.
- 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
Inheritance was overkill here, and now we can use inheritance for new functionality X
without having to create classes for all combinations of {X, is_testnet}.
We accept either when decoding - this only changes what we use when encoding.
Single quotes are annoying to use in a shell, as they often need to be escaped.
- 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
BIP-0174 specifies using standard bip32-compliant serialization for extended keys.
> The 78 byte serialized extended public key as defined by BIP 32.
closes https://github.com/spesmilo/electrum/issues/8036
- separate AddressSynchronizer from Wallet and LNWatcher
- the AddressSynchronizer class is referred to as 'adb' (address database)
- Use callbacks to replace overloaded methods
asyncio.get_event_loop() became deprecated in python3.10. (see https://github.com/python/cpython/issues/83710)
```
.../electrum/electrum/daemon.py:470: DeprecationWarning: There is no current event loop
self.asyncio_loop = asyncio.get_event_loop()
.../electrum/electrum/network.py:276: DeprecationWarning: There is no current event loop
self.asyncio_loop = asyncio.get_event_loop()
```
Also, according to that thread, "set_event_loop() [... is] not deprecated by oversight".
So, we stop using get_event_loop() and set_event_loop() in our own code.
Note that libraries we use (such as the stdlib for python <3.10), might call get_event_loop,
which then relies on us having called set_event_loop e.g. for the GUI thread. To work around
this, a custom event loop policy providing a get_event_loop implementation is used.
Previously, we have been using a single asyncio event loop, created with
util.create_and_start_event_loop, and code in many places got a reference to this loop
using asyncio.get_event_loop().
Now, we still use a single asyncio event loop, but it is now stored as a global in
util._asyncio_event_loop (access with util.get_asyncio_loop()).
I believe these changes also fix https://github.com/spesmilo/electrum/issues/5376
See testcase:
- imported wallet with addr1 and addr2
- three txs: tx1 funds addr1, tx2 funds addr2, tx3 spends all
- if we rm addr1 from the wallet,
- previously both tx1 and tx3 was removed (as tx3 is a child of tx1)
- now only tx1 is removed (tx3 still relates to the wallet via addr2)
fixes https://github.com/spesmilo/electrum/issues/7587
Imported wallets used to send change back to the "from address".
We keep this behaviour as default.
There has already been an option "Use change addresses" (exposed in GUI),
ignored so far by imported wallets (was only used by HD wallets).
With this change, imported wallets no longer ignore that option, and if set,
they will send change to a random unused imported address, instead of back to "from address".
If all addresses are used, it falls back to sending change back to the "from address".
see: https://github.com/spesmilo/electrum/pull/7330
see: https://github.com/spesmilo/electrum/issues/5353
SomberNight
Ondřej Vejpustek
Sander van Grieken
ThomasV
Andrew Kozlik