From aee2d8e12060e0568559627130ca8a9dc9b12bc9 Mon Sep 17 00:00:00 2001 From: SomberNight Date: Mon, 17 Sep 2018 03:35:25 +0200 Subject: [PATCH] verifier: fix a race during reorgs related: 41e088693de72d9111ce1ce21e42b5aa017627dd 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. --- electrum/address_synchronizer.py | 6 ++++++ electrum/synchronizer.py | 3 --- electrum/verifier.py | 16 +++++++++++++--- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/electrum/address_synchronizer.py b/electrum/address_synchronizer.py index 4930140aa..aa46335db 100644 --- a/electrum/address_synchronizer.py +++ b/electrum/address_synchronizer.py @@ -575,6 +575,12 @@ class AddressSynchronizer(PrintError): if self.verifier: self.verifier.remove_spv_proof_for_tx(tx_hash) + def remove_unverified_tx(self, tx_hash, tx_height): + with self.lock: + new_height = self.unverified_tx.get(tx_hash) + if new_height == tx_height: + self.unverified_tx.pop(tx_hash, None) + def add_verified_tx(self, tx_hash: str, info: VerifiedTxInfo): # Remove from the unverified map and add to the verified map with self.lock: diff --git a/electrum/synchronizer.py b/electrum/synchronizer.py index eb40f3bfd..5a7518bcd 100644 --- a/electrum/synchronizer.py +++ b/electrum/synchronizer.py @@ -75,9 +75,6 @@ class Synchronizer(PrintError): history = self.wallet.history.get(addr, []) if history_status(history) == status: return - # note that at this point 'result' can be None; - # if we had a history for addr but now the server is telling us - # there is no history if addr in self.requested_histories: return # request address history diff --git a/electrum/verifier.py b/electrum/verifier.py index 06ab9e8e6..37ca83394 100644 --- a/electrum/verifier.py +++ b/electrum/verifier.py @@ -24,6 +24,7 @@ import asyncio from typing import Sequence, Optional +import aiorpcx from aiorpcx import TaskGroup from .util import PrintError, bh2u, VerifiedTxInfo @@ -84,9 +85,19 @@ class SPV(PrintError): await group.spawn(self._request_and_verify_single_proof, tx_hash, tx_height) async def _request_and_verify_single_proof(self, tx_hash, tx_height): - merkle = await self.network.get_merkle_for_transaction(tx_hash, tx_height) + try: + merkle = await self.network.get_merkle_for_transaction(tx_hash, tx_height) + except aiorpcx.jsonrpc.RPCError as e: + self.print_error('tx {} not at height {}'.format(tx_hash, tx_height)) + self.wallet.remove_unverified_tx(tx_hash, tx_height) + try: self.requested_merkle.remove(tx_hash) + except KeyError: pass + return # Verify the hash of the server-provided merkle branch to a # transaction matches the merkle root of its block + if tx_height != merkle.get('block_height'): + self.print_error('requested tx_height {} differs from received tx_height {} for txid {}' + .format(tx_height, merkle.get('block_height'), tx_hash)) tx_height = merkle.get('block_height') pos = merkle.get('pos') merkle_branch = merkle.get('merkle') @@ -100,8 +111,7 @@ class SPV(PrintError): raise GracefulDisconnect(e) # we passed all the tests self.merkle_roots[tx_hash] = header.get('merkle_root') - try: - self.requested_merkle.remove(tx_hash) + try: self.requested_merkle.remove(tx_hash) except KeyError: pass self.print_error("verified %s" % tx_hash) header_hash = hash_header(header)