- separate AddressSynchronizer from Wallet and LNWatcher
- the AddressSynchronizer class is referred to as 'adb' (address database)
- Use callbacks to replace overloaded methods
aiorpcx 0.20 changed the behaviour/API of TaskGroups.
When used as a context manager, TaskGroups no longer propagate
exceptions raised by their tasks. Instead, the calling code has
to explicitly check the results of tasks and decide whether to re-raise
any exceptions.
This is a significant change, and so this commit introduces "OldTaskGroup",
which should behave as the TaskGroup class of old aiorpcx. All existing
usages of TaskGroup are replaced with OldTaskGroup.
closes https://github.com/spesmilo/electrum/issues/7446
follow-up to 4346d2fc76
It's not just about the Synchronizer, the Verifier should not starve other jobs either...
(previously I thought the Verifier is not too important as it only makes
requests if there are new txs; however with LNWatcher its progress is not persisted)
When receiving the history of an address, the client behaved unexpectedly
if either of two checks failed.
The client checked that the txids in the history are unique, and that the
history matches the previously announced status. If either failed, it
would just log a line and do nothing. Importantly, the synchronizer could
even consider itself is_up_to_date, i.e. the GUI could show the wallet is
synced.
This is now changed such that:
- if the txid uniqueness test fails, we simply disconnect
- if the history is not consistent with previously announced status,
we wait a bit, make sure is_up_to_date is False in the meantime,
and then potentially disconnect
See rationale for these in the comments.
related: https://github.com/spesmilo/electrum/issues/7058#issuecomment-783613084
Scenario (prior to change):
User opens wallet1 with 10k addresses, and then immediately opens wallet2
with 100 addresses.
wallet1 will synchronise first, fully, and only then will wallet2 start syncing.
Now, wallet1 and wallet2 will sync concurrently (and wallet2 will finish much
sooner as expected).
as discussed in issue #6697, users with large wallets or slow
connections may never see their initial request-missing-tx run complete,
if we always use the same sync order.
This commit shuffles the addresses being requested every time a new
request round happens, so that (if enough time passes and enough initial
state requests are attempted) we will always get the latest state for
each address, regardless of how quickly the connection times out on us
some basic sanity checks
Previously if the server sent back a malformed response, it could partially corrupt a wallet file.
(as sometimes the response would get persisted, and issues would only arise later when the values were used)
Before this, we were subscribing to our addresses in their bip32 order,
leaking this information to servers. While this leak seems mostly harmless,
it is trivial to fix.
Note that there is a slight distinction between
`not tx.is_complete()` and `isinstance(tx, PartialTransaction)`,
which is that technically you can have a PSBT that is already complete
but was not yet converted to a standard bitcoin tx.
Triggering needs two consecutive scripthash status changes
in very quick succession. Client gets notification from server,
but then response to "blockchain.scripthash.get_history" will already contain
the changed-again history that has a different status.
20190627T101547.902638Z | INFO | synchronizer.[default_wallet] | receiving history mwXtx49BCGAiy4tU1r7MBX5VVLWSdtasCL 1
20190627T101547.903262Z | INFO | synchronizer.[default_wallet] | error: status mismatch: mwXtx49BCGAiy4tU1r7MBX5VVLWSdtasCL
User has wallet file with history that includes some txid; corresponding
raw tx is not in the "transactions" dict in the file however.
When the synchronizer starts up, it requests this "missing" txn from
the server... but what if the server does not know about it?
Maybe it was reorged and is not in the new best chain,
and not even in mempool. This was not handled previously.
fix#5122
The synchronizer would sometimes not send 'wallet_updated' triggers
if it was fast enough to do all the work between two 0.1 sec ticks.
(is_up_to_date() would return True both before and after)
related: 41e088693d
If our guess of a txn getting confirmed at the same height in the new chain
as it was at in the old chain is incorrect, there is a race between the
verifier and the synchronizer. If the verifier wins, the exception will cause
us to disconnect.
ThomasV
SomberNight
ln2max