tverifier: fix logic bug. after reorg, some verifs were not undone - electrum - Electrum Bitcoin wallet
HTML git clone https://git.parazyd.org/electrum
DIR Log
DIR Files
DIR Refs
DIR Submodules
---
DIR commit bca6ad52410ca5ee8c5f4af3e589c0bcb24d8c5f
DIR parent 9a7112009057bf40c40ed7644ec5a6e3619b6a8a
HTML Author: SomberNight <somber.night@protonmail.com>
Date: Tue, 26 Mar 2019 21:01:43 +0100
verifier: fix logic bug. after reorg, some verifs were not undone
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
Diffstat:
M electrum/address_synchronizer.py | 4 ++--
M electrum/blockchain.py | 21 +++++++++++++++++++++
M electrum/tests/test_blockchain.py | 56 +++++++++++++++++++++++++++++++
M electrum/verifier.py | 15 +++++++--------
4 files changed, 86 insertions(+), 10 deletions(-)
---
DIR diff --git a/electrum/address_synchronizer.py b/electrum/address_synchronizer.py
t@@ -525,14 +525,14 @@ class AddressSynchronizer(PrintError):
with self.lock:
return dict(self.unverified_tx) # copy
- def undo_verifications(self, blockchain, height):
+ def undo_verifications(self, blockchain, above_height):
'''Used by the verifier when a reorg has happened'''
txs = set()
with self.lock:
for tx_hash in self.db.list_verified_tx():
info = self.db.get_verified_tx(tx_hash)
tx_height = info.height
- if tx_height >= height:
+ if tx_height > above_height:
header = blockchain.read_header(tx_height)
if not header or hash_header(header) != info.header_hash:
self.db.remove_verified_tx(tx_hash)
DIR diff --git a/electrum/blockchain.py b/electrum/blockchain.py
t@@ -201,6 +201,27 @@ class Blockchain(util.PrintError):
with blockchains_lock:
return list(filter(lambda y: y.parent==self, blockchains.values()))
+ def get_parent_heights(self) -> Mapping['Blockchain', int]:
+ """Returns map: (parent chain -> height of last common block)"""
+ with blockchains_lock:
+ result = {self: self.height()}
+ chain = self
+ while True:
+ parent = chain.parent
+ if parent is None: break
+ result[parent] = chain.forkpoint - 1
+ chain = parent
+ return result
+
+ def get_height_of_last_common_block_with_chain(self, other_chain: 'Blockchain') -> int:
+ last_common_block_height = 0
+ our_parents = self.get_parent_heights()
+ their_parents = other_chain.get_parent_heights()
+ for chain in our_parents:
+ if chain in their_parents:
+ h = min(our_parents[chain], their_parents[chain])
+ last_common_block_height = max(last_common_block_height, h)
+ return last_common_block_height
@with_lock
def get_branch_size(self) -> int:
DIR diff --git a/electrum/tests/test_blockchain.py b/electrum/tests/test_blockchain.py
t@@ -70,6 +70,62 @@ class TestBlockchain(SequentialTestCase):
self.assertTrue(chain.can_connect(header))
chain.save_header(header)
+ def test_get_height_of_last_common_block_with_chain(self):
+ blockchain.blockchains[constants.net.GENESIS] = chain_u = Blockchain(
+ config=self.config, forkpoint=0, parent=None,
+ forkpoint_hash=constants.net.GENESIS, prev_hash=None)
+ open(chain_u.path(), 'w+').close()
+ self._append_header(chain_u, self.HEADERS['A'])
+ self._append_header(chain_u, self.HEADERS['B'])
+ self._append_header(chain_u, self.HEADERS['C'])
+ self._append_header(chain_u, self.HEADERS['D'])
+ self._append_header(chain_u, self.HEADERS['E'])
+ self._append_header(chain_u, self.HEADERS['F'])
+ self._append_header(chain_u, self.HEADERS['O'])
+ self._append_header(chain_u, self.HEADERS['P'])
+ self._append_header(chain_u, self.HEADERS['Q'])
+
+ chain_l = chain_u.fork(self.HEADERS['G'])
+ self._append_header(chain_l, self.HEADERS['H'])
+ self._append_header(chain_l, self.HEADERS['I'])
+ self._append_header(chain_l, self.HEADERS['J'])
+ self._append_header(chain_l, self.HEADERS['K'])
+ self._append_header(chain_l, self.HEADERS['L'])
+
+ self.assertEqual({chain_u: 8, chain_l: 5}, chain_u.get_parent_heights())
+ self.assertEqual({chain_l: 11}, chain_l.get_parent_heights())
+
+ chain_z = chain_l.fork(self.HEADERS['M'])
+ self._append_header(chain_z, self.HEADERS['N'])
+ self._append_header(chain_z, self.HEADERS['X'])
+ self._append_header(chain_z, self.HEADERS['Y'])
+ self._append_header(chain_z, self.HEADERS['Z'])
+
+ self.assertEqual({chain_u: 8, chain_z: 5}, chain_u.get_parent_heights())
+ self.assertEqual({chain_l: 11, chain_z: 8}, chain_l.get_parent_heights())
+ self.assertEqual({chain_z: 13}, chain_z.get_parent_heights())
+ self.assertEqual(5, chain_u.get_height_of_last_common_block_with_chain(chain_l))
+ self.assertEqual(5, chain_l.get_height_of_last_common_block_with_chain(chain_u))
+ self.assertEqual(5, chain_u.get_height_of_last_common_block_with_chain(chain_z))
+ self.assertEqual(5, chain_z.get_height_of_last_common_block_with_chain(chain_u))
+ self.assertEqual(8, chain_l.get_height_of_last_common_block_with_chain(chain_z))
+ self.assertEqual(8, chain_z.get_height_of_last_common_block_with_chain(chain_l))
+
+ self._append_header(chain_u, self.HEADERS['R'])
+ self._append_header(chain_u, self.HEADERS['S'])
+ self._append_header(chain_u, self.HEADERS['T'])
+ self._append_header(chain_u, self.HEADERS['U'])
+
+ self.assertEqual({chain_u: 12, chain_z: 5}, chain_u.get_parent_heights())
+ self.assertEqual({chain_l: 11, chain_z: 8}, chain_l.get_parent_heights())
+ self.assertEqual({chain_z: 13}, chain_z.get_parent_heights())
+ self.assertEqual(5, chain_u.get_height_of_last_common_block_with_chain(chain_l))
+ self.assertEqual(5, chain_l.get_height_of_last_common_block_with_chain(chain_u))
+ self.assertEqual(5, chain_u.get_height_of_last_common_block_with_chain(chain_z))
+ self.assertEqual(5, chain_z.get_height_of_last_common_block_with_chain(chain_u))
+ self.assertEqual(8, chain_l.get_height_of_last_common_block_with_chain(chain_z))
+ self.assertEqual(8, chain_z.get_height_of_last_common_block_with_chain(chain_l))
+
def test_parents_after_forking(self):
blockchain.blockchains[constants.net.GENESIS] = chain_u = Blockchain(
config=self.config, forkpoint=0, parent=None,
DIR diff --git a/electrum/verifier.py b/electrum/verifier.py
t@@ -168,18 +168,17 @@ class SPV(NetworkJobOnDefaultServer):
raise InnerNodeOfSpvProofIsValidTx()
async def _maybe_undo_verifications(self):
- def undo_verifications():
- height = self.blockchain.get_max_forkpoint()
- self.print_error("undoing verifications back to height {}".format(height))
- tx_hashes = self.wallet.undo_verifications(self.blockchain, height)
+ old_chain = self.blockchain
+ cur_chain = self.network.blockchain()
+ if cur_chain != old_chain:
+ self.blockchain = cur_chain
+ above_height = cur_chain.get_height_of_last_common_block_with_chain(old_chain)
+ self.print_error(f"undoing verifications above height {above_height}")
+ tx_hashes = self.wallet.undo_verifications(self.blockchain, above_height)
for tx_hash in tx_hashes:
self.print_error("redoing", tx_hash)
self.remove_spv_proof_for_tx(tx_hash)
- if self.network.blockchain() != self.blockchain:
- self.blockchain = self.network.blockchain()
- undo_verifications()
-
def remove_spv_proof_for_tx(self, tx_hash):
self.merkle_roots.pop(tx_hash, None)
try: