Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Unify bitcoin-qt and bitcoind persistent settings #15936

Draft
wants to merge 7 commits into
base: master
from

Conversation

@ryanofsky
Copy link
Contributor

ryanofsky commented May 1, 2019

This is based on #15934 + #15935. The non-base commits are:


If a setting like pruning, port mapping, or a network proxy is enabled in the GUI, it will now be stored in the bitcoin persistent setting file in the datadir and shared with bitcoind, instead of being stored as Qt settings which end up in the the windows registry or platform specific config files and are ignored by bitcoind.


This is marked work-in-progress because it needs an end-to-end test checking settings read, write, and modification.

@DrahtBot

This comment has been minimized.

Copy link
Contributor

DrahtBot commented May 2, 2019

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #16215 (gui: Refactor wallet controller activities by promag)
  • #16205 (Refactor: Replace fprintf with tfm::format by MarcoFalke)
  • #16115 (On bitcoind startup, write config args to debug.log by LarryRuane)
  • #16097 (WIP: Prevent meaningless negating of arguments by hebasto)
  • #16069 (test: move-only: Split large tests into smaller compile units by MarcoFalke)
  • #15934 (Separate settings merging from parsing by ryanofsky)
  • #15450 ([GUI] Create wallet menu option by achow101)
  • #14866 (Improve property evaluation way in bitcoin.conf by AkioNak)
  • #14045 (refactor: Fix the chainparamsbase -> util circular dependency by Empact)
  • #13818 (More intuitive GUI settings behavior when -proxy is set by Sjors)
  • #10102 ([experimental] Multiprocess bitcoin by ryanofsky)
  • #8994 (Testchains: Introduce custom chain whose constructor... by jtimon)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request May 3, 2019
Persistent settings are used in followup PRs bitcoin#15936 to unify gui settings
between bitcoin-qt and bitcoind, and bitcoin#15937 to add a load_on_startup flag to
the loadwallet RPC and maintain a dynamic list of wallets that should be loaded
on startup that also can be shared between bitcoind and bitcoin-qt.
@ryanofsky ryanofsky force-pushed the ryanofsky:pr/qtset branch from e7bf93f to a50315a May 3, 2019
ryanofsky added 7 commits Mar 5, 2019
Implement merging of settings from different sources (command line and config
file) separately from parsing code in system.cpp, so it is easier to add new
sources.

Document current inconsistent merging behavior without changing it.

This commit only adds new settings code without using it. The next commit calls
the new code to replace existing code in system.cpp.
Get rid of settings merging code mixed in settings parsing code in
util/system.cpp repeated 5 places, inconsistently:

- ArgsManagerHelper::GetArg
- ArgsManagerHelper::GetNetBoolArg
- ArgsManager::GetArgs
- ArgsManager::IsArgNegated
- ArgsManager::GetUnsuitableSectionOnlyArgs

Having settings merging code separated from parsing simplifies parsing somewhat
(for example negated values can simply be represented as false values instead
of partially cleared or emply placeholder lists).

Having settings merge happen one place instead of 5 makes it easier to add new
settings sources and harder to introduce new inconsistencies in the way
settings are merged.

This commit does not change behavior in any way.
Persistent settings are used in followup PRs #15936 to unify gui settings
between bitcoin-qt and bitcoind, and #15937 to add a load_on_startup flag to
the loadwallet RPC and maintain a dynamic list of wallets that should be loaded
on startup that also can be shared between bitcoind and bitcoin-qt.
If a bitcoind setting like pruning, port mapping, or a network proxy is enabled
in the GUI, it will now be stored in the bitcoin persistent setting file and
shared with bitcoind, instead of being stored as Qt settings backed by the
windows registry or platform specific config files.
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request May 7, 2019
Persistent settings are used in followup PRs bitcoin#15936 to unify gui settings
between bitcoin-qt and bitcoind, and bitcoin#15937 to add a load_on_startup flag to
the loadwallet RPC and maintain a dynamic list of wallets that should be loaded
on startup that also can be shared between bitcoind and bitcoin-qt.
@ryanofsky ryanofsky force-pushed the ryanofsky:pr/qtset branch 2 times, most recently from f625d1a to 0ab41dd May 7, 2019
@jonasschnelli

This comment has been minimized.

Copy link
Member

jonasschnelli commented May 18, 2019

Concept ACK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.