Preserve comments in config file when using Qt

Since ConfigParser does not sufficiently support comments
in config objects, in order to support dynamic update of
the config from within a run, this PR edits the config
file at the same time as updating the config object, so
as to prevent the earlier problem that all comments in the
config file were lost whenever Qt updated the file with
the new config object.

jmclient/jmclient/__init__.py

@@ -21,7 +21,7 @@ from .storage import (Argon2Hash, Storage, StorageError,
                       StoragePasswordError, VolatileStorage)
 from .cryptoengine import BTCEngine, BTC_P2PKH, BTC_P2SH_P2WPKH, EngineError
 from .configure import (
-    load_program_config, get_p2pk_vbyte, jm_single, get_network,
+    load_program_config, get_p2pk_vbyte, jm_single, get_network, update_persist_config,
     validate_address, get_irc_mchannels, get_blockchain_interface_instance,
     get_p2sh_vbyte, set_config, is_segwit_mode, is_native_segwit_mode)
 from .blockchaininterface import (BlockchainInterface,

jmclient/jmclient/configure.py

@@ -6,6 +6,7 @@ import io
 import logging
 import os
 import binascii
+import re
 
 from configparser import ConfigParser, NoOptionError
 
@@ -541,6 +542,59 @@ def get_blockchain_interface_instance(_config):
         raise ValueError("Invalid blockchain source")
     return bc_interface
 
+def update_persist_config(section, name, value):
+    """ Unfortunately we cannot persist an updated config
+    while preserving the full set of comments with ConfigParser's
+    model (the 'set no-value settings' doesn't cut it).
+    Hence if we want to update and persist, we must manually
+    edit the file at the same time as editing the in-memory
+    config object.
+
+    Arguments: section and name must be strings (and
+    section must already exist), while value can be any valid
+    type for a config value, but must be cast-able to string.
+
+    Returns: False if the config setting was not found,
+    or True if it was found and edited+saved as intended.
+    """
+
+    m_line  = re.compile(r"^\s*" + name + r"\s*" + "=", re.IGNORECASE)
+    m_section = re.compile(r"\[\s*" + section + r"\s*\]", re.IGNORECASE)
+
+    # Find the single line containing the specified value; only accept
+    # if it's the right section; create a new copy of all the config
+    # lines, with that one line edited.
+    # If one match is found and edited, rewrite the config and update
+    # the in-memory config, else return an error.
+    sectionname = None
+    newlines = []
+    match_found = False
+    with open(jm_single().config_location, "r") as f:
+        for line in f.readlines():
+            newline  = line
+            # ignore comment lines
+            if line.strip().startswith("#"):
+                newlines.append(line)
+                continue
+            regexp_match_section = m_section.search(line)
+            if regexp_match_section:
+                # get the section name from the match
+                sectionname = regexp_match_section.group().strip("[]").strip()
+            regexp_match = m_line.search(line)
+            if regexp_match and sectionname and sectionname.upper(
+                ) == section.upper():
+                # We have the right line; change it
+                newline = name + " = " + str(value)+"\n"
+                match_found = True
+            newlines.append(newline)
+    # If it wasn't found, do nothing but return an error
+    if not match_found:
+        return False
+    # success: update in-mem and re-persist
+    jm_single().config.set(section, name, value)
+    with open(jm_single().config_location, "wb") as f:
+        f.writelines([x.encode("utf-8") for x in newlines])
+    return True
 
 def is_segwit_mode():
     return jm_single().config.get('POLICY', 'segwit') != 'false'

scripts/joinmarket-qt.py

@@ -68,7 +68,7 @@ JM_CORE_VERSION = '0.5.5'
 JM_GUI_VERSION = '10'
 
 from jmbase import get_log
-from jmclient import load_program_config, get_network,\
+from jmclient import load_program_config, get_network, update_persist_config,\
     open_test_wallet_maybe, get_wallet_path, get_p2sh_vbyte, get_p2pk_vbyte,\
     jm_single, validate_address, weighted_order_choose, Taker,\
     JMClientProtocolFactory, start_reactor, get_schedule, schedule_to_text,\
@@ -92,8 +92,6 @@ def update_config_for_gui():
     '''The default joinmarket config does not contain these GUI settings
     (they are generally set by command line flags or not needed).
     If they are set in the file, use them, else set the defaults.
-    These *will* be persisted to joinmarket.cfg, but that will not affect
-    operation of the command line version.
     '''
     gui_config_names = ['gaplimit', 'history_file', 'check_high_fee',
                         'max_mix_depth', 'txfee_default', 'order_wait_time',
@@ -107,13 +105,6 @@ def update_config_for_gui():
         if gcn not in [_[0] for _ in gui_items]:
             jm_single().config.set("GUI", gcn, gcv)
 
-
-def persist_config():
-    '''This loses all comments in the config file.
-    TODO: possibly correct that.'''
-    with open('joinmarket.cfg', 'w') as f:
-        jm_single().config.write(f)
-
 def checkAddress(parent, addr):
     valid, errmsg = validate_address(str(addr))
     if not valid:
@@ -249,12 +240,17 @@ class SettingsTab(QDialog):
                 oval = 'true' if checked else 'false'
             log.debug('setting section: ' + section + ' and name: ' + oname +
                       ' to: ' + oval)
-            jm_single().config.set(section, oname, oval)
+            if not update_persist_config(section, oname, oval):
+                log.warn("Unable to persist config change to file: " + str(section) + str(oname) + str(oval))
 
         else:  #currently there is only QLineEdit
             log.debug('setting section: ' + section + ' and name: ' + str(t[
                 0].text()) + ' to: ' + str(t[1].text()))
-            jm_single().config.set(section, str(t[0].text()), str(t[1].text()))
+            if not update_persist_config(section, str(t[0].text()), str(t[1].text())):
+                # we don't include GUI as it's not required to be persisted:
+                if not section == "GUI":
+                    log.warn("Unable to persist config change to file: " + str(
+                        section) + str(t[0].text()) + str(t[1].text()))
             if str(t[0].text()) == 'blockchain_source':
                 jm_single().bc_interface = get_blockchain_interface_instance(
                     jm_single().config)
@@ -1270,7 +1266,6 @@ class JMMainWindow(QMainWindow):
         quit_msg = "Are you sure you want to quit?"
         reply = JMQtMessageBox(self, quit_msg, mbtype='question')
         if reply == QMessageBox.Yes:
-            persist_config()
             event.accept()
             if self.reactor.threadpool is not None:
                 self.reactor.threadpool.stop()

scripts/qtsupport.py

@@ -68,15 +68,13 @@ config_tips = {
     'rpc_port': 'port for connecting to bitcoind over rpc',
     'rpc_user': 'user for connecting to bitcoind over rpc',
     'rpc_password': 'password for connecting to bitcoind over rpc',
-    'host': 'hostname for IRC server (or comma separated list)',
-    'channel': 'channel name on IRC server (or comma separated list)',
-    'port': 'port for connecting to IRC server (or comma separated list)',
-    'usessl': "'true'/'false' to use SSL for each connection to IRC\n" +
-    "(or comma separated list)",
-    'socks5': "'true'/'false' to use a SOCKS5 proxy for each connection" +
-    "to IRC (or comma separated list)",
-    'socks5_host': 'host for SOCKS5 proxy (or comma separated list)',
-    'socks5_port': 'port for SOCKS5 proxy (or comma separated list)',
+    'host': 'hostname for IRC server',
+    'channel': 'channel name on IRC server',
+    'port': 'port for connecting to IRC server',
+    'usessl': "'true'/'false' to use SSL for each connection to IRC\n",
+    'socks5': "'true'/'false' to use a SOCKS5 proxy for each connection",
+    'socks5_host': 'host for SOCKS5 proxy',
+    'socks5_port': 'port for SOCKS5 proxy',
     'maker_timeout_sec': 'timeout for waiting for replies from makers',
     'merge_algorithm': 'for dust sweeping, try merge_algorithm = gradual, \n' +
     'for more rapid dust sweeping, try merge_algorithm = greedy \n' +
@@ -106,6 +104,7 @@ config_tips = {
     "documentation for details on how to set up certs if you use this.",
     "history_file": "Location of the file storing transaction history",
     "segwit": "Only used for migrating legacy wallets; see documentation.",
+    "native": "NOT currently supported, except for PayJoin (command line only)",
     "console_log_level": "one of INFO, DEBUG, WARN, ERROR; INFO is least noisy;\n" +
     "consider switching to DEBUG in case of problems.",
     "absurd_fee_per_kb": "maximum satoshis/kilobyte you are willing to pay,\n" +