network.get_transaction: move some response validation logic from Synchronizer

electrum/interface.py

@@ -181,6 +181,8 @@ class RequestTimedOut(GracefulDisconnect):
         return _("Network request timed out.")
 
 
+class RequestCorrupted(GracefulDisconnect): pass
+
 class ErrorParsingSSLCert(Exception): pass
 class ErrorGettingSSLCertFromServer(Exception): pass
 class ConnectError(NetworkException): pass
@@ -258,6 +260,9 @@ class Interface(Logger):
     def diagnostic_name(self):
         return str(NetAddress(self.host, self.port))
 
+    def __str__(self):
+        return f"<Interface {self.diagnostic_name()}>"
+
     def _set_proxy(self, proxy: dict):
         if proxy:
             username, pw = proxy.get('user'), proxy.get('password')

electrum/network.py

@@ -53,10 +53,11 @@ from .bitcoin import COIN
 from . import constants
 from . import blockchain
 from . import bitcoin
+from .transaction import Transaction
 from .blockchain import Blockchain, HEADER_SIZE
 from .interface import (Interface, serialize_server, deserialize_server,
                         RequestTimedOut, NetworkTimeout, BUCKET_NAME_OF_ONION_SERVERS,
-                        NetworkException)
+                        NetworkException, RequestCorrupted)
 from .version import PROTOCOL_VERSION
 from .simple_config import SimpleConfig
 from .i18n import _
@@ -66,7 +67,6 @@ if TYPE_CHECKING:
     from .channel_db import ChannelDB
     from .lnworker import LNGossip
     from .lnwatcher import WatchTower
-    from .transaction import Transaction
     from .daemon import Daemon
 
 
@@ -871,7 +871,7 @@ class Network(Logger):
                     if success_fut.exception():
                         try:
                             raise success_fut.exception()
-                        except RequestTimedOut:
+                        except (RequestTimedOut, RequestCorrupted):
                             await iface.close()
                             await iface.got_disconnected
                             continue  # try again
@@ -1068,8 +1068,19 @@ class Network(Logger):
     async def get_transaction(self, tx_hash: str, *, timeout=None) -> str:
         if not is_hash256_str(tx_hash):
             raise Exception(f"{repr(tx_hash)} is not a txid")
-        return await self.interface.session.send_request('blockchain.transaction.get', [tx_hash],
+        iface = self.interface
-                                                         timeout=timeout)
+        raw = await iface.session.send_request('blockchain.transaction.get', [tx_hash], timeout=timeout)
+        # validate response
+        tx = Transaction(raw)
+        try:
+            tx.deserialize()  # see if raises
+        except Exception as e:
+            self.logger.warning(f"cannot deserialize received transaction (txid {tx_hash}). from {str(iface)}")
+            raise RequestCorrupted() from e  # TODO ban server?
+        if tx.txid() != tx_hash:
+            self.logger.warning(f"received tx does not match expected txid {tx_hash} (got {tx.txid()}). from {str(iface)}")
+            raise RequestCorrupted()  # TODO ban server?
+        return raw
 
     @best_effort_reliable
     @catch_server_exceptions

electrum/synchronizer.py

@@ -221,15 +221,6 @@ class Synchronizer(SynchronizerBase):
         finally:
             self._requests_answered += 1
         tx = Transaction(raw_tx)
-        try:
-            tx.deserialize()  # see if raises
-        except Exception as e:
-            # possible scenarios:
-            # 1: server is sending garbage
-            # 2: there is a bug in the deserialization code
-            # 3: there was a segwit-like upgrade that changed the tx structure
-            #    that we don't know about
-            raise SynchronizerFailure(f"cannot deserialize transaction {tx_hash}") from e
         if tx_hash != tx.txid():
             raise SynchronizerFailure(f"received tx does not match expected txid ({tx_hash} != {tx.txid()})")
         tx_height = self.requested_tx.pop(tx_hash)