bitcoin-core / gui-qml

Bitcoin GUI (experimental QML-based fork)
MIT License
111 stars 40 forks source link

Only write options after onboarding #284

Closed johnny9 closed 4 months ago

johnny9 commented 1 year ago

Windows Intel macOS Apple Silicon macOS ARM64 Android

hebasto commented 1 year ago

Needs rebase.

johnny9 commented 1 year ago

update from 45e3262 to 08be85b:

thebagmaster commented 1 year ago

ACK 08be85b I tested these on Ubuntu 22.04.2 and it does what it says on the tin. Understood that we may need to buffer the settings writing to file in on-boarding; especially if the storage location is not set and may change. Maybe we should do our best to save what we can of what the user configured? Are there situations where we buffer settings and different settings that we save immediately?

hebasto commented 1 year ago

Please rebase.

https://discord.com/channels/903125802726596648/903126039084015638/1146922061449666602

johnny9 commented 1 year ago

Update from 08be85b to 51df97d

D33r-Gee commented 10 months ago

tACK on WSL Ubuntu 22.04 on signet

Works as expected, the settings file gets created after the onboarding has finished.

settings.json:

{ "listen": true, "natpmp": true, "prune": 2861 }

Do we want the “Listen” and “natpmp” options to be true by default or was it just for testing this PR?

D33r-Gee commented 10 months ago

tACK on Android armv7 everything worked as expected

The settings.json was created after the onboarding with the following values:

{ "prune": 2861 }

johnny9 commented 6 months ago

Update 51df97d to 61f8e6c

johnny9 commented 6 months ago

In branch johnny9:write_after_onboard:

  • If the user doesn't change anything setting.json file will look like:
{
    "dbcache": 16000,
    "listen": true,
    "natpmp": true,
    "prune": 1907,
    "server": true,
    "upnp": true
}
  • If the user disables any settings setting.json file will look like:
{
    "dbcache": 16000,
}

After the user makes any changes on the settings mentioned above next time the user starts bitcoin-qt up again it looks like the UI doesn't match the configuration file (settings.json), is this expected?

@pablomartin4btc I wasn't able to reproduce the output you were seeing during onboarding but I think I fixed the settings not applying at next startup with initializing the values in the constructor.

D33r-Gee commented 6 months ago

tACK https://github.com/bitcoin-core/gui-qml/commit/61f8e6c5fcb0c6c31a6283c3b7fdf6b9f82eba5e

On Ubuntu 22.04 and Android (armv7)

Works as expected, settings.json shows defaults:

{
    "listen": true,
    "prune": 1907
}
johnny9 commented 6 months ago

dbcache setting is not persisted anymore ("dbcache": 16000,), is this correct?

It should only persist if you set it to a value other than nDefaultDbCache (450)

pablomartin4btc commented 6 months ago

dbcache setting is not persisted anymore ("dbcache": 16000,), is this correct?

It should only persist if you set it to a value other than nDefaultDbCache (450)

Perfect, it works.

johnny9 commented 6 months ago

Update from 61f8e6c to 4f557b0

johnny9 commented 6 months ago

Update from 4f557b0 to 9e56907