raspiblitz / raspiblitz

Get your own Bitcoin & Lightning Node running - on a RaspberryPi with a nice LCD
MIT License
2.44k stars 520 forks source link

duplicate lines in raspiblitz.conf #969

Closed frennkie closed 4 years ago

frennkie commented 4 years ago

I think I have found at least 2 if not 3 cases where lines in the config file /mnt/hdd/raspiblitz.conf were (at least) duplicated due to this "check":

source /mnt/hdd/raspiblitz.conf
# [...]
# add default value to raspi config if needed
if [ ${#nodeJS} -eq 0 ]; then
  echo "nodeJS=off" >> /mnt/hdd/raspiblitz.conf
fi

I currently don't have the patience to hunt this down.. but I think this needs to be checked/replaced in multiple places.

frennkie commented 4 years ago

Well.. this is used intensively..

admin@host:~/config.scripts $ grep "} -eq 0 ]; then" *  2>/dev/null | wc -l
147
openoms commented 4 years ago

I think I have found at least 2 if not 3 cases where lines in the config file /mnt/hdd/raspiblitz.conf were (at least) duplicated due to this "check":

source /mnt/hdd/raspiblitz.conf
# [...]
# add default value to raspi config if needed
if [ ${#nodeJS} -eq 0 ]; then
echo "nodeJS=off" >> /mnt/hdd/raspiblitz.conf
fi

@frennkie this only adds an extra line if the raspiblitz.conf does not have the entry yet. It does not create duplicates as far as I see.

Could be the problem with parsing the conf that the entries are in different order depending on in what order the services have been installed (and restored)?

We could make the conf standard by listing all the possible on creation with =off or default value respectively.

For example my raspiblitz.conf on the 1.4RC2 currently (TUI shows black screen with cursor when installed):

# RASPIBLITZ CONFIG FILE
raspiBlitzVersion='1.4'
network=bitcoin
chain=main
hostname=TestRPi4
publicIP='X.X.X.X'
runBehindTor=on
autoPilot=off
autoNatDiscovery=off
rtlWebinterface=on
setnetworkname=0
lndPort='9735'
lndAddress=''
touchscreen=0
nodeJS=on
BTCRPCexplorer=on
frennkie commented 4 years ago

TUI shows black screen with cursor when installed

@openoms please check home/pi/.cache/lxsession/LXDE-pi/run.log

frennkie commented 4 years ago

Could be the problem with parsing the conf that the entries are in different order depending on in what order the services have been installed (and restored)?

Possible..

this only adds an extra line if the raspiblitz.conf does not have the entry yet.

Well... It would actually add an entry if there was a line like this present:

nodeJS=

I really don't like this "loose coupling"... At the beginning the conf is sourced and sometime later the length check happens.

With my alternative (see c330da2) there is a direct connection between the check whether any line is present and the add.

openoms commented 4 years ago

You are right, changing all the checks for the entry being present to use grep would 100% avoid duplicates.

The random order problem is difficult to control as old versions would carry over non-standard raspiblitz.conf-s.

openoms commented 4 years ago

Cleaned the possibility of duplicate entries in: https://github.com/rootzoll/raspiblitz/commit/3b2f867a842b7ebd80855a26ec1e106ec9bdb0e6

openoms commented 4 years ago

I got a line flashing up on my status screen:

While reading from '<string>' [line 17]: option 'tlsextraip' in section 'Application Options'  already exists

I wonder if this blitz.configcheck.py erroring on the double entry in my lnd.conf?

I have Zerotier set up so have both in line 16-17:

tlsextraip=0.0.0.0
tlsextraip=172.X  

https://github.com/lightningnetwork/lnd/blob/master/sample-lnd.conf#L31 specifically says that Setting multiple tlsextraip= entries is allowed.

Can this be made to not show an error?

openoms commented 4 years ago

Just referring here from the PR above to ask if any new entry to raspiblitz.conf would cause a parsing problem with the BlitzTUI?

Will need to update that as well.

rootzoll commented 4 years ago

Just referring here from the PR above to ask if any new entry to raspiblitz.conf would cause a parsing problem with the BlitzTUI?

New entry should not make any problem.

I was running tests and so far all look good on my machine. If there are any other reports on this issue I will close next week for release.

rootzoll commented 4 years ago

Closing issue for v.14 release.