util.callback_mgr.callbacks was not getting properly cleared between tests.
Every time an Abstract_Wallet or an LNWorker (or many other subclasses of EventListener) is instantiated,
self.register_callbacks() is called in __init__, which puts callbacks into util.callback_mgr.callbacks.
These are only cleaned up if we explicitly call Abstract_Wallet.stop() or LNWorker.stop() later, which we usually do not do in the tests.
As a result, when running multiple unit tests in a row, lots of objects created in a given testcase are never GC-ed and leak into subsequent tests. This is not only a memory leak, but wastes compute too: when events are triggered and cbs get called, these old objects also have their cbs called.
After running all (~1061) unit tests, I observe util.callback_mgr.callbacks had 30 events with a total of 3156 callbacks stored.
On my laptop, running all unit tests previously took ~115 sec, and now it takes ~73 sec.
I don't understand what the "coins not used" comment meant here.
It was added in the change away from the old config.WALLET_BATCH_RBF option
(ab14c3e138).
The `coins` param *is used* in wallet.get_candidates_for_batching.
Without setting that, the returned set of candidates was restricted to
only base txs that had a large enough change output to cover *all* the newly added outputs.
Instead, it is desirable to allow adding new inputs.
- fix: qml gui errors when trying to open a wallets with only keystore-encryption
- fixes https://github.com/spesmilo/electrum/issues/10171
- qml gui to prompt for password on wallet open even if wallet is not storage-encrypted
'prepayment', which corresponds to the trusted part of the
lightning payment.
We use 2*sm.mining_fee, where 'mining_fee' is the flat part of
the server fee. However, future protocol should probably allow
to set a value that does not depend on 'mining_fee'.
(note that LND uses a hardcoded amount).
1. Do not remove local transaction in find_base_tx.
This logic was intended to cleanup claim transactions that are
never broadcast (for example, if the counterparty gets a refund)
(see 1bf1de36cb)
However, this code is too unspecific and may result in fund loss,
because the transaction being removed may contain outgoing payments.
For example, if the electrum server is not responsive, the tx will
be seen as local and deleted. In that case, another payment will
be attempted, thus paying twice.
2. Do not remove tx after try_broadcasting returns False.
The server might be lying to us. We can only remove the local tx
if there is a base_tx, because the next tx we create will try to
spend the same output.
I want to see where initial_data arg is actually set from.
Looks like hardly anywhere(?)
It is really hard to see because the wizard is crazy-dynamic, there's polymorphism and mixins everywhere.
closes https://github.com/spesmilo/electrum/issues/10119
also:
- wallet.get_onchain_history was broken with from_height/to_height args
- "show_fees" param is and was non-existent. fees are always added to output
- MyEncoder(json.JSONEncoder) changed a bit:
- I am pretty sure cutting the last 3 chars was intended to cut off the seconds
- however that was making incorrect assumptions about what datetime.isoformat() returns
- which depends on whether microsecond precision is available or whether an explicit timezone is set
- this now makes it clear that we want minutes-resolution, but still leaves the timezone-ambiguity
Often when the wallet creates a tx, the flow is:
- create unsigned tx
- sign tx
- broadcast tx, but don't save it in history
- server sends notification that status of a subscribed address changed
- client calls scripthash.get_history
- client sees txid in scripthash.get_history response
- client calls blockchain.transaction.get to request missing tx
Instead, now when we broadcast a tx on an interface, we cache that tx *for that interface*,
and just before calling blockchain.transaction.get, we lookup in the cache.
Hence this will often save a network request.
