RFC: Is it OK to reset unused prune and proxy settings to default values after restarting? #596

1. 
   ryanofsky commented at 2:59 pm on April 29, 2022: contributor

   I’m opening this issue to get feedback and figure out how to resolve a disagreement that came up in https://github.com/bitcoin/bitcoin/pull/15936#discussion_r850568749.

   Background

   PR https://github.com/bitcoin/bitcoin/pull/15936 unifies Bitcoin-Qt and Bitcoind settings so they are stored in <datadir>/settings.json instead of externally (in windows registry or external config directories via QSettings). It fixes current behavior where Bitcoind and Bitcoin-Qt can be started from the same datadir but may have inconsistent settings applied. It should make it easier to see what settings are in effect, because they will live in the datadir instead of being merged from outside sources. And it can enable new features like a bitcoin-cli config command that can update the unified settings dynamically.

   The problem is the <datadir>/settings.json file does not currently store unused prune and proxy values when prune and proxy settings are disabled. So if you check the “Prune block storage” box and change the default “2GB” value to something else, then uncheck the box, then restart, then recheck the box, the previous value you typed will be lost, and the default “2GB” will be shown again. The same thing happens with the “Connect through SOCKS5 proxy” box. If you change the default tor proxy address “localhost:9050” to something else, and then disable the proxy, and restart, then recheck the box, the proxy address shown will be the default proxy address, not whatever proxy address you typed previously.

   It is possible to change this behavior by storing unused proxy and prune settings somewhere. I don’t personally think storing unused proxy or pruning settings values is a good thing or that it justifies extra complexity. But it would be possible to implement as a followup to https://github.com/bitcoin/bitcoin/pull/15936. One approach I implemented in https://github.com/ryanofsky/bitcoin/commit/4e86ab5dfc9bf40502aca0f9e3a4690f6f5dfc19 does this by adding prune-prev and proxy-prev settings that hold unused values when corresponding prune and proxy settings are disabled. Other approaches like storing unused prune and proxy settings in QSettings, or completely changing the representation of prune and proxy settings so they have “enabled” fields separate from the value fields would also be possible.

   Questions

   1. What is ideal behavior when checking pruning/proxy boxes, typing pruning/proxy values, unchecking the boxes, restarting Bitcoin-Qt, checking the boxes again? Is it ideal to show default values (2GB and localhost:9050) when these boxes are checked? Or is ideal to show previous values? Or is some other behavior better?

   2. Is this issue important and should it block #15936? Would it be ok for #15936 to implement simplest possible behavior of not saving unused prune and proxy values to disk, and displaying default pruning and proxy values when the boxes are checked after a restart?

   Screenshots

   These are the settings in question for reference

2. ryanofsky added the label Feature on Apr 29, 2022
3. ryanofsky cross-referenced this on Apr 29, 2022 from issue interfaces: Expose settings.json methods to GUI by ryanofsky
4. 
   Sjors commented at 3:35 pm on April 29, 2022: member

   It would be ideal to show the previously entered value. Especially for a (Tor) proxy it could be quite annoying if the IP and port are forgotten every time you turn them off.

   However I don’t know if turning proxies on and off is a common use case. It’s not something I’ve encountered. So at this point I don’t see it as a high priority or blocking issue.

   Another point brought up in the thread:

   I think it would be less surprising and more obvious what is going on if we clear the values after the user disables the option. Like, immediately after the click on the checkbox. Then the behavior will be consistent between “close & re-open the config dialog box” and “close & re-open the app”

   IUC this will require modifying OptionsDialog as well as OptionsModel, and will make interactions between the classes more involved.

   If we end up adding complexity, I’d rather just store the value.

5. 
   ryanofsky commented at 3:44 pm on April 29, 2022: contributor

   I think it would be less surprising and more obvious what is going on if we clear the values after the user disables the option. Like, immediately after the click on the checkbox.

   If we end up adding complexity, I’d rather just store the value.

   Thanks, I didn’t mention this alternative in the description, because I’m pretty sure nobody actually wants it. The question isn’t really “should option values be immediately reset to default as soon as they are disabled?” but “is it worth preserving disabled option values across restarts, and if so, how should those disabled option values be stored on disk?”

6. 
   Rspigler commented at 6:39 pm on May 2, 2022: contributor

   I think the prune-prev and proxy-prev approach is best, especially because there is a reset button on that screen (so users will be able to return to default settings).

   I do not believe this is blocking for #15936

7. 
   vasild commented at 12:39 pm on May 3, 2022: contributor

   Thanks for the input! Just a note - currently in master the values are preserved through restart when disabled.
8. ryanofsky cross-referenced this on May 20, 2022 from issue Add settings.json prune-prev, proxy-prev, onion-prev settings by ryanofsky
9. ryanofsky cross-referenced this on May 20, 2022 from issue Unify bitcoin-qt and bitcoind persistent settings by ryanofsky
10. 
    jarolrod commented at 8:36 pm on October 24, 2022: member

    I’m for the prev values approach.

    If a user chose a prune target in the gui, ran a pruned node, then for whatever reason disabled it; they still decided what prune target they would like, and we should display this value in the box for easy configuration if they would ever run in prune mode again. In terms of thinking about user-flows this situation should be rare, yet, it would be what the user expects. “Hey bitcoin gui, why are you making me choose my prune target every time, I told you I want it to prune to 10gb if I’m running in pruned mode.”

    Following the same logic, the proxy settings provide a stronger case for restoring these values. While prune targets would rarely change, I’d assume it’s an even rarer occurrence to change your proxy settings. And there are situations where you would cycle it off and on, especially if you’re running the gui on a laptop, traveling and using different networks.

11. 
    ryanofsky commented at 10:38 pm on October 24, 2022: contributor

    I’m for the prev values approach.

    All right, it sounds like most people prefer storing disabled values over resetting them, and PR #603 implements the change that’s been asked for, so it can be reviewed and hopefully merged.

    I’ll close this issue, since the question it raises “RFC: Is it OK to reset unused prune and proxy settings to default values after restarting?” seems to have a clear answer now: “No”. It can be reopened if there is need for more discussion.

12. ryanofsky closed this on Oct 24, 2022

    ---

13. 
    hebasto commented at 3:32 pm on October 26, 2022: member

    While reviewing #603, I’ve made an opinion that

    Other approaches like storing unused prune and proxy settings in QSettings

    looks nicer, as all those settings are GUI-specific.

14. hebasto referenced this in commit e43ff4eab2 on Feb 15, 2023
15. bitcoin-core locked this on Oct 26, 2023

Contributors
ryanofsky Sjors Rspigler vasild jarolrod hebasto

Labels
Feature