refactor(cli): improve handling of sync version from arguments

hathor/builder/cli_builder.py

@@ -17,6 +17,7 @@
 import os
 import platform
 import sys
+from enum import Enum, auto
 from typing import Any, Optional
 
 from structlog import get_logger
@@ -45,6 +46,12 @@
 logger = get_logger()
 
 
+class SyncChoice(Enum):
+    V1_ONLY = auto()
+    V2_ONLY = auto()
+    BRIDGE = auto()
+
+
 class CliBuilder:
     """CliBuilder builds the core objects from args.
 
@@ -159,16 +166,32 @@ def create_manager(self, reactor: Reactor) -> HathorManager:
         hostname = self.get_hostname()
         network = settings.NETWORK_NAME
 
-        arg_sync_v2_only = self._args.x_sync_v2_only or self._args.sync_v2_only
-        if self._args.x_sync_v2_only:
-            self.log.warn('--x-sync-v2-only is deprecated and will be removed, use --sync-v2-only instead')
-
-        arg_sync_bridge = self._args.x_sync_bridge or self._args.sync_bridge
-        if self._args.x_sync_bridge:
+        sync_choice: SyncChoice
+        if self._args.sync_bridge:
+            sync_choice = SyncChoice.BRIDGE
+        elif self._args.sync_v2_only:
+            sync_choice = SyncChoice.V2_ONLY
+        elif self._args.x_sync_bridge:
             self.log.warn('--x-sync-bridge is deprecated and will be removed, use --sync-bridge instead')
-
-        enable_sync_v1 = not arg_sync_v2_only
-        enable_sync_v2 = arg_sync_v2_only or arg_sync_bridge
+            sync_choice = SyncChoice.BRIDGE
+        elif self._args.x_sync_v2_only:
+            self.log.warn('--x-sync-v2-only is deprecated and will be removed, use --sync-v2-only instead')
+            sync_choice = SyncChoice.V2_ONLY
+        else:  # default
+            sync_choice = SyncChoice.V1_ONLY
+
+        enable_sync_v1: bool
+        enable_sync_v2: bool
+        match sync_choice:
+            case SyncChoice.V1_ONLY:
+                enable_sync_v1 = True
+                enable_sync_v2 = False
+            case SyncChoice.V2_ONLY:
+                enable_sync_v1 = False
+                enable_sync_v2 = True
+            case SyncChoice.BRIDGE:
+                enable_sync_v1 = True
+                enable_sync_v2 = True
 
         pubsub = PubSubManager(reactor)
 

hathor/cli/run_node.py

@@ -103,16 +103,12 @@ def create_parser(cls) -> ArgumentParser:
         parser.add_argument('--sentry-dsn', help='Sentry DSN')
         parser.add_argument('--enable-debug-api', action='store_true', help='Enable _debug/* endpoints')
         parser.add_argument('--enable-crash-api', action='store_true', help='Enable _crash/* endpoints')
-        v2args = parser.add_mutually_exclusive_group()
-        v2args.add_argument('--x-sync-bridge', action='store_true',
-                            help='Enable support for running both sync protocols. DO NOT ENABLE, IT WILL BREAK.')
-        v2args.add_argument('--x-sync-v2-only', action='store_true',
-                            help='Disable support for running sync-v1. DO NOT ENABLE, IT WILL BREAK.')
-        # XXX: new safe arguments along side the unsafe --x- arguments so transition is easier
-        v2args.add_argument('--sync-bridge', action='store_true',
-                            help='Enable support for running both sync protocols.')
-        v2args.add_argument('--sync-v2-only', action='store_true',
-                            help='Disable support for running sync-v1.')
+        sync_args = parser.add_mutually_exclusive_group()
+        sync_args.add_argument('--sync-bridge', action='store_true',
+                               help='Enable running both sync protocols.')
+        sync_args.add_argument('--sync-v2-only', action='store_true', help='Disable support for running sync-v1.')
+        sync_args.add_argument('--x-sync-v2-only', action='store_true', help=SUPPRESS)  # old argument
+        sync_args.add_argument('--x-sync-bridge', action='store_true', help=SUPPRESS)  # old argument
         parser.add_argument('--x-localhost-only', action='store_true', help='Only connect to peers on localhost')
         parser.add_argument('--x-rocksdb-indexes', action='store_true', help=SUPPRESS)
         parser.add_argument('--x-enable-event-queue', action='store_true', help='Enable event queue mechanism')

tests/others/test_cli_builder.py

@@ -101,16 +101,31 @@ def test_memory_storage(self):
     def test_memory_storage_with_rocksdb_indexes(self):
         self._build_with_error(['--memory-storage', '--x-rocksdb-indexes'], 'RocksDB indexes require RocksDB data')
 
+    def test_sync_default(self):
+        manager = self._build(['--memory-storage'])
+        self.assertTrue(manager.connections.is_sync_version_enabled(SyncVersion.V1_1))
+        self.assertFalse(manager.connections.is_sync_version_enabled(SyncVersion.V2))
+
     def test_sync_bridge(self):
         manager = self._build(['--memory-storage', '--x-sync-bridge'])
         self.assertTrue(manager.connections.is_sync_version_enabled(SyncVersion.V1_1))
         self.assertTrue(manager.connections.is_sync_version_enabled(SyncVersion.V2))
 
+    def test_sync_bridge2(self):
+        manager = self._build(['--memory-storage', '--sync-bridge'])
+        self.assertTrue(manager.connections.is_sync_version_enabled(SyncVersion.V1_1))
+        self.assertTrue(manager.connections.is_sync_version_enabled(SyncVersion.V2))
+
     def test_sync_v2_only(self):
         manager = self._build(['--memory-storage', '--x-sync-v2-only'])
         self.assertFalse(manager.connections.is_sync_version_enabled(SyncVersion.V1_1))
         self.assertTrue(manager.connections.is_sync_version_enabled(SyncVersion.V2))
 
+    def test_sync_v2_only2(self):
+        manager = self._build(['--memory-storage', '--sync-v2-only'])
+        self.assertFalse(manager.connections.is_sync_version_enabled(SyncVersion.V1_1))
+        self.assertTrue(manager.connections.is_sync_version_enabled(SyncVersion.V2))
+
     def test_keypair_wallet(self):
         manager = self._build(['--memory-storage', '--wallet', 'keypair'])
         self.assertIsInstance(manager.wallet, Wallet)