Use `jm_single().config.getboolean()` instead of string comparisons

[kristapsk]

Makes parsing of all boolean configs consistent.

From configparser docs:

accepted values for the option are '1', 'yes', 'true', and 'on', which cause this method to return True, and '0', 'no', 'false', and 'off', which cause it to return False.

[kristapsk]

Not sure what causes this error, cannot reproduce locally.

jmbase/test/test_commands.py .                                           [ 88%]
jmdaemon/test/test_daemon_protocol.py .

!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! KeyboardInterrupt !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
/home/runner/work/joinmarket-clientserver/joinmarket-clientserver/jmvenv/lib/python3.8/site-packages/twisted/trial/_asynctest.py:402: KeyboardInterrupt
(to show a full traceback on KeyboardInterrupt use --full-trace)

[AdamISZ]

Not sure what causes this error, cannot reproduce locally.

jmbase/test/test_commands.py .                                           [ 88%]
jmdaemon/test/test_daemon_protocol.py .

!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! KeyboardInterrupt !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
/home/runner/work/joinmarket-clientserver/joinmarket-clientserver/jmvenv/lib/python3.8/site-packages/twisted/trial/_asynctest.py:402: KeyboardInterrupt
(to show a full traceback on KeyboardInterrupt use --full-trace)

Thanks for report, I'll take a look at it shortly.

[AdamISZ]

My gut reaction is that this change is just not necessary. (e.g. I had never seen anyone use 'on' to switch on a flag in a config file). It's probably best to just make sure each comment states unambiguously what is required (and i like just sticking with 'true' and only, exactly 'true' for a boolean, while using '1' vs '0' is also fine, but we should probably endeavour to be consistent over the whole config file).

While sticking with a standard from configparser is certainly not terrible, I'd rather we keep it as simple as possible.

[kristapsk]

While sticking with a standard from configparser is certainly not terrible, I'd rather we keep it as simple as possible.

Using default getboolean() seems simpler to me than writing custom converter.

[AdamISZ]

While sticking with a standard from configparser is certainly not terrible, I'd rather we keep it as simple as possible.

Using default getboolean() seems simpler to me than writing custom converter.

It's a very reasonable perspective. But note this will take some review work. For example, the data passed over AMP between client and server includes booleans that are currently being passed as strings. That has to be checked carefully.

Indeed, checking it now I see an error there:

https://github.com/JoinMarket-Org/joinmarket-clientserver/blob/625bbae50fa8cdfd42971eb211dc3d7951ddeb7c/jmdaemon/jmdaemon/irc.py#L94-L98

This checks for that string being 'true', but it will be True instead so it won't pick it up.

We also have to make sure to update comments in config, we have to make sure that no config variable reads have been accidentally missed (I mean, sure that's obvious, but it's easy to get a small error).

[AdamISZ]

This checks for that string being 'true', but it will be True instead so it won't pick it up.

Uh, what am I saying, it will first do .lower() on a bool which will crash, which could most likely be why the test suite failed. double checking shortly.

[AdamISZ]

OK confirmed locally. If you change the above line from if self.socks5.lower() =='true' to if self.socks5: then the Keyboard Interrupt!! goes away. Sorry the error message is so bad.

[AdamISZ]

By the same token, the checks in this code section for self.usessl and self.socks5 have to be changed: (I think the testsuite doesn't hit it because ssl is not enabled)

https://github.com/JoinMarket-Org/joinmarket-clientserver/blob/625bbae50fa8cdfd42971eb211dc3d7951ddeb7c/jmdaemon/jmdaemon/irc.py#L150-L172