Simple protocol negotiation and setting of handlers

It turns out clients pass 0.10 instead of 1.0 as the protocol version.
Distinguish some handlers for 1.0 and 1.1 protocols.
Log protocol version request
Add tests of new library function

Author: Neil Booth

lib/jsonrpc.py

@@ -63,6 +63,7 @@ class JSONRPC(object):
     INVALID_RESPONSE = -100
     ERROR_CODE_UNAVAILABLE = -101
     REQUEST_TIMEOUT = -102
+    FATAL_ERROR = -103
 
     ID_TYPES = (type(None), str, numbers.Number)
     HAS_BATCHES = False
@@ -405,7 +406,8 @@ def error_bytes(self, message, code, id_=None):
         self.error_count += 1
         if not self.close_after_send:
             fatal_log = None
-            if code in (version.PARSE_ERROR, version.INVALID_REQUEST):
+            if code in (version.PARSE_ERROR, version.INVALID_REQUEST,
+                        version.FATAL_ERROR):
                 fatal_log = message
             elif self.error_count >= 10:
                 fatal_log = 'too many errors, last: {}'.format(message)

lib/util.py

@@ -269,3 +269,12 @@ def is_valid_hostname(hostname):
     if hostname and hostname[-1] == ".":
         hostname = hostname[:-1]
     return all(SEGMENT_REGEX.match(x) for x in hostname.split("."))
+
+def protocol_tuple(s):
+    '''Converts a protocol version number, such as "1.0" to a tuple (1, 0).
+
+    If the version number is bad, (0, ) indicating version 0 is returned.'''
+    try:
+        return tuple(int(part) for part in s.split('.'))
+    except Exception:
+        return (0, )

server/controller.py

@@ -100,7 +100,7 @@ def __init__(self, env):
              'address.get_balance address.get_history address.get_mempool '
              'address.get_proof address.listunspent '
              'block.get_header estimatefee relayfee '
-             'transaction.get transaction.get_merkle utxo.get_address'),
+             'transaction.get_merkle utxo.get_address'),
             ('server', 'donation_address'),
         ]
         self.electrumx_handlers = {'.'.join([prefix, suffix]):
@@ -672,14 +672,14 @@ def address_to_hashX(self, address):
             pass
         raise RPCError('{} is not a valid address'.format(address))
 
-    def script_hash_to_hashX(self, script_hash):
+    def scripthash_to_hashX(self, scripthash):
         try:
-            bin_hash = hex_str_to_hash(script_hash)
+            bin_hash = hex_str_to_hash(scripthash)
             if len(bin_hash) == 32:
                 return bin_hash[:self.coin.HASHX_LEN]
         except Exception:
             pass
-        raise RPCError('{} is not a valid script hash'.format(script_hash))
+        raise RPCError('{} is not a valid script hash'.format(scripthash))
 
     def assert_tx_hash(self, value):
         '''Raise an RPCError if the value is not a valid transaction
@@ -844,17 +844,23 @@ async def relayfee(self):
         to the daemon's memory pool.'''
         return await self.daemon_request('relayfee')
 
-    async def transaction_get(self, tx_hash, height=None):
+    async def transaction_get(self, tx_hash):
         '''Return the serialized raw transaction given its hash
 
         tx_hash: the transaction hash as a hexadecimal string
-        height: ignored, do not use
         '''
-        # For some reason Electrum passes a height.  We don't require
-        # it in anticipation it might be dropped in the future.
         self.assert_tx_hash(tx_hash)
         return await self.daemon_request('getrawtransaction', tx_hash)
 
+    async def transaction_get_1_0(self, tx_hash, height=None):
+        '''Return the serialized raw transaction given its hash
+
+        tx_hash: the transaction hash as a hexadecimal string
+        height: ignored, do not use
+        '''
+        # For some reason Electrum protocol 1.0 passes a height.
+        return await self.transaction_get(tx_hash)
+
     async def transaction_get_merkle(self, tx_hash, height):
         '''Return the markle tree to a confirmed transaction given its hash
         and height.

server/session.py

@@ -13,6 +13,7 @@
 
 from lib.hash import sha256, hash_to_str
 from lib.jsonrpc import JSONSession, RPCError, JSONRPCv2, JSONRPC
+import lib.util as util
 from server.daemon import DaemonError
 import server.version as version
 
@@ -35,7 +36,6 @@ def __init__(self, controller, kind):
         self.daemon = self.bp.daemon
         self.client = 'unknown'
         self.client_version = (1, )
-        self.protocol_version = '1.0'
         self.anon_logs = self.env.anon_logs
         self.last_delay = 0
         self.txs_sent = 0
@@ -113,19 +113,7 @@ def __init__(self, *args, **kwargs):
         self.hashX_subs = {}
         self.mempool_statuses = {}
         self.chunk_indices = []
-        self.electrumx_handlers = {
-            'blockchain.address.subscribe': self.address_subscribe,
-            'blockchain.block.get_chunk': self.block_get_chunk,
-            'blockchain.headers.subscribe': self.headers_subscribe,
-            'blockchain.numblocks.subscribe': self.numblocks_subscribe,
-            'blockchain.script_hash.subscribe': self.script_hash_subscribe,
-            'blockchain.transaction.broadcast': self.transaction_broadcast,
-            'server.add_peer': self.add_peer,
-            'server.banner': self.banner,
-            'server.features': self.server_features,
-            'server.peers.subscribe': self.peers_subscribe,
-            'server.version': self.server_version,
-        }
+        self.set_protocol_handlers(None)
 
     def sub_count(self):
         return len(self.hashX_subs)
@@ -164,7 +152,7 @@ async def notify(self, height, touched):
 
         for alias_status in changed:
             if len(alias_status[0]) == 64:
-                method = 'blockchain.script_hash.subscribe'
+                method = 'blockchain.scripthash.subscribe'
             else:
                 method = 'blockchain.address.subscribe'
             pairs.append((method, alias_status))
@@ -247,12 +235,12 @@ async def address_subscribe(self, address):
         hashX = self.controller.address_to_hashX(address)
         return await self.hashX_subscribe(hashX, address)
 
-    async def script_hash_subscribe(self, script_hash):
+    async def scripthash_subscribe(self, scripthash):
         '''Subscribe to a script hash.
 
-        script_hash: the SHA256 hash of the script to subscribe to'''
-        hashX = self.controller.script_hash_to_hashX(script_hash)
-        return await self.hashX_subscribe(hashX, script_hash)
+        scripthash: the SHA256 hash of the script to subscribe to'''
+        hashX = self.controller.scripthash_to_hashX(scripthash)
+        return await self.hashX_subscribe(hashX, scripthash)
 
     def server_features(self):
         '''Returns a dictionary of server features.'''
@@ -333,47 +321,98 @@ def server_version(self, client_name=None, protocol_version=None):
                                             in self.client.split('.'))
             except Exception:
                 pass
-        if protocol_version is not None:
-            self.protocol_version = protocol_version
+
+        self.log_info('protocol version {} requested'.format(protocol_version))
+        self.set_protocol_handlers(protocol_version)
+
         return version.VERSION
 
     async def transaction_broadcast(self, raw_tx):
         '''Broadcast a raw transaction to the network.
 
         raw_tx: the raw transaction as a hexadecimal string'''
-        # An ugly API: current Electrum clients only pass the raw
-        # transaction in hex and expect error messages to be returned in
-        # the result field.  And the server shouldn't be doing the client's
-        # user interface job here.
+        # This returns errors as JSON RPC errors, as is natural
         try:
             tx_hash = await self.daemon.sendrawtransaction([raw_tx])
             self.txs_sent += 1
             self.log_info('sent tx: {}'.format(tx_hash))
             self.controller.sent_tx(tx_hash)
             return tx_hash
         except DaemonError as e:
-            error = e.args[0]
+            error, = e.args
             message = error['message']
             self.log_info('sendrawtransaction: {}'.format(message),
                           throttle=True)
+            raise RPCError('the transaction was rejected by network rules.'
+                           '\n\n{}\n[{}]'.format(message, raw_tx))
+
+    async def transaction_broadcast_1_0(self, raw_tx):
+        '''Broadcast a raw transaction to the network.
+
+        raw_tx: the raw transaction as a hexadecimal string'''
+        # An ugly API: current Electrum clients only pass the raw
+        # transaction in hex and expect error messages to be returned in
+        # the result field.  And the server shouldn't be doing the client's
+        # user interface job here.
+        try:
+            return await self.transaction_broadcast(raw_tx)
+        except RPCError as e:
+            message, = e.args
             if 'non-mandatory-script-verify-flag' in message:
-                return (
+                message = (
                     'Your client produced a transaction that is not accepted '
                     'by the network any more.  Please upgrade to Electrum '
                     '2.5.1 or newer.'
                 )
 
-            return (
-                'The transaction was rejected by network rules.  ({})\n[{}]'
-                .format(message, raw_tx)
-            )
+            return message
+
+    def set_protocol_handlers(self, version_str):
+        controller = self.controller
+        if version_str is None:
+            version_str = version.PROTOCOL_MIN
+        ptuple = util.protocol_tuple(version_str)
+        # Disconnect if requested protocol version in unsupported
+        if (ptuple < util.protocol_tuple(version.PROTOCOL_MIN)
+                or ptuple > util.protocol_tuple(version.PROTOCOL_MAX)):
+            self.log_info('unsupported protocol version {}'
+                          .format(version_str))
+            raise RPCError('unsupported protocol version: {}'
+                           .format(version_str), JSONRPC.FATAL_ERROR)
+
+        handlers = {
+            'blockchain.address.subscribe': self.address_subscribe,
+            'blockchain.block.get_chunk': self.block_get_chunk,
+            'blockchain.headers.subscribe': self.headers_subscribe,
+            'blockchain.numblocks.subscribe': self.numblocks_subscribe,
+            'blockchain.transaction.broadcast': self.transaction_broadcast_1_0,
+            'blockchain.transaction.get': controller.transaction_get_1_0,
+            'server.add_peer': self.add_peer,
+            'server.banner': self.banner,
+            'server.features': self.server_features,
+            'server.peers.subscribe': self.peers_subscribe,
+            'server.version': self.server_version,
+        }
+
+        handlers.update(controller.electrumx_handlers)
+
+        if ptuple >= (1, 1):
+            # Remove deprecated methods
+            del handlers['blockchain.address.get_proof']
+            del handlers['blockchain.numblocks.subscribe']
+            del handlers['blockchain.utxo.get_address']
+            # Add new handlers
+            handlers.update({
+                'blockchain.scripthash.subscribe': self.scripthash_subscribe,
+                'blockchain.transaction.broadcast': self.transaction_broadcast,
+                'blockchain.transaction.get': controller.transaction_get,
+            })
+
+        self.electrumx_handlers = handlers
 
     def request_handler(self, method):
         '''Return the async handler for the given request method.'''
-        handler = self.electrumx_handlers.get(method)
-        if not handler:
-            handler = self.controller.electrumx_handlers.get(method)
-        return handler
+        return self.electrumx_handlers.get(method)
 
 
 class LocalRPC(SessionBase):

server/version.py

@@ -1,5 +1,5 @@
 # Server name and protocol versions
 
 VERSION = 'ElectrumX 1.0.17'
-PROTOCOL_MIN = '1.0'
-PROTOCOL_MAX = '1.0'
+PROTOCOL_MIN = '0.10'
+PROTOCOL_MAX = '1.1'

tests/lib/test_util.py

@@ -83,3 +83,13 @@ def test_is_valid_hostname():
     len255 = ('a' * 62 + '.') * 4 + 'abc'
     assert is_valid_hostname(len255)
     assert not is_valid_hostname(len255 + 'd')
+
+
+def test_protocol_tuple():
+    assert util.protocol_tuple(None) == (0, )
+    assert util.protocol_tuple("foo") == (0, )
+    assert util.protocol_tuple(1) == (0, )
+    assert util.protocol_tuple("1") == (1, )
+    assert util.protocol_tuple("0.1") == (0, 1)
+    assert util.protocol_tuple("0.10") == (0, 10)
+    assert util.protocol_tuple("2.5.3") == (2, 5, 3)