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)
after a reorg, in a many fork/orphan chains scenario,
we would sometimes not undo SPV for enough blocks
functions in blockchain.py somewhat based on kyuupichan/bitcoinX@5126bd15ef0c9ba36e17a455513452ebed7b2328
The --skipmerklecheck optional flag makes Electrum tolerate invalid
merkle proofs from the server. This is useful for building Electrum
servers that need a minimum amount of storage, though of course users
should only enable it if they completely trust the connected server.
If auto_connect is enabled, allow jumping between forks too.
(Previously auto_connect was only switching servers on a given fork,
not across forks)
If there is a preferred fork set, jump to that (and stay);
if there isn't, always jump to the longest fork.
blockchain.read_header is expensive. do cheap tests first
on a wallet with 11k txns, that is synced except for spv proofs,
finishing sync now takes 80 sec instead of 180 sec
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.
fixes race between verifier and block header download.
scenario: client starts, connects to server. while client was offline,
there was a reorg. txn A was not mined in the old chain, but is mined
after reorg. client subscribes to addresses and starts downloading headers,
concurrently. server tells client txn A is mined at height H >= reorg height.
client sees it has block header at height H, asks for SPV proof for txn A.
but the header the client has is still the old one, the verifier was faster
than the block header download (race...). client receives proof. proof is
incorrect for old header. client disconnects.
This makes the method easier to read.
Skip negative tx heights too. A transaction height can be negative too, see the
wallet modules TX_HEIGHT_LOCAL and TX_HEIGHT_UNCONF_PARENT constants.
* Rename synchronous_get to synchronous_send
This makes it more inline with the method 'send' of which
synchronous_send is the, well, synchronous version.
* Move protocol strings from scripts to network
This is again a small step in the right direction. The network module is
going to accumulate more and more of these simple methods. Once
everything is moved into that module, that module is going to be split.
Note that I've left the scripts which use scripts/util.py alone. I
suspect the same functionality can be reached when using just
lib/network.py and that scripts/util.py is obsolete.
* Remove protocol string from verifier and websocket
Websocket still has some references, that'll take more work to remove. Once the
network module has been split this should be easy.
I took the liberty to rename a variable to better show what it is.
* Remove protocol strings from remainder
The naming scheme I'm following for the newly introduced methods in the network
module is: 'blockchain.<subject>.<action>' -> def <action>_(for|to)_<subject>
* Move explicit protocol calls closer to each other
This makes it easier to keep track of the methods which are due to be
extracted.
* Remove `send` when using `get_transaction`
This is the final step to formalize (the informal) interface of the network
module.
A chance of note is changed interface for async/sync calls. It is no longer
required to use the `synchronous_send` call. Merely NOT passing a callback
makes the call synchronous. I feel this makes the API more intuitive to work
with and easier to replace with a different network module.
* Remove send from get_merkle_for_transaction
The pattern which emerged for calling the lambda yielded an slight refactor.
I'm not happy with the name for the `__invoke` method.
* Remove explict send from websockets
* Remove explicit send from scripts
* Remove explicit send from wallet
* Remove explicit sync_send from commands, scripts
* Remove optional timeout parameter
This parameter doesn't seem to be used a lot and removing it makes the
remaining calls easier. Potentionally a contentious choice!
* Rename `broadcast` to `broadcast_transaction`
Doing so makes the method name consistent with the other ElectrumX protocol
method names.
* Remove synchronous_send
Now every method is intuitive in what it does, no special handling required.
The `broadcast_transaction` method is weird. I've opted not to change the
return type b/c I found it hard to know what the exact consequences are. But
ideally this method should just works as all the other ElectrumX related
messages. On the other hand this shows nicely how you _can_ do something
differnt quite easy.
* Rename the awkwardly name `__invoke` method
The new name reflects what it does.
* Process the result of linter feedback
I've used flake8-diff (and ignored a couple of line length warnings).
* Rename tx_response to on_tx_response
This fell through the cracks when this branch was rebased.
* subscript_to_scripthash should be get_balance
An oversight while refactoring.
* Add missing return statement
Without this statement the transaction would have been broadcasted twice.
* Pass list of tuples to send not single tuple
* Add @staticmethod decorator
* Fix argument to be an array
SomberNight
zebra-lucky
ThomasV
chris-belcher
Janus
Harm Aarts
Francisco J. Hernandez Heras