[PastaPastaPasta]

Issue being fixed or feature implemented

If settings.json contains a "lang", "font-family", "font-scale", "font-weight-normal" or "font-weight-bold" value of an unexpected JSON type (an object or an array, e.g. after a hand edit or a bad merge of the file), dash-qt is terminated at startup by an uncaught exception (std::terminate, exit 134). It should instead show the existing "Could not read setting ... Settings file ... might be corrupt or invalid." error, which is what already happens for the same mistake in prune, proxy, onion, dbcache and the other settings handled by the getOption() loop in OptionsModel::Init.

The conversion helpers (SettingToString / SettingToInt) fall back to UniValue::get_str() for anything that is not null, a bool or a number, so an object or array throws UniValue::type_error. OptionsModel::Init already catches that for the settings read in its getOption() loop (added with bitcoin-core/gui#602), but a few reads happen outside that loop:

• "lang", read at the top of OptionsModel::Init (same as upstream), and also read earlier through gArgs.GetArg("-lang") in GetLangTerritory(), which is called again right after common::InitConfig has loaded settings.json.
• The Dash font settings (font-family, font-scale, font-weight-normal, font-weight-bold), read before the loop once fonts are loaded.
• When -font-family is given on the command line and the weight is not, Init does not read the stored font-weight-normal / font-weight-bold at all. A malformed weight then gets through Init, and GuiMain throws on it a moment later from gArgs.IsArgSet("-font-weight-normal") / gArgs.GetIntArg(...), which also see settings.json.

None of these call sites is inside a try block. The only catch in GuiMain starts after the options model has been created, so the exception ends in std::terminate.

Why this is a real problem

Code path at c14104b:

• SettingToString / SettingToInt fall back to get_str(): src/util/system.cpp#L529-L536, #L554-L561
• Unprotected "lang" read: src/qt/optionsmodel.cpp#L208
• Unprotected font reads: src/qt/optionsmodel.cpp#L258-L302; the stored weights are skipped when only -font-family is overridden: #L281-L282, #L297-L298
• GuiMain reading the weights through gArgs after Init: src/qt/bitcoin.cpp#L700-L701, #L709-L710
• The loop that does handle type errors: src/qt/optionsmodel.cpp#L355-L370
• GetLangTerritory() reading -lang through gArgs: src/qt/bitcoin.cpp#L122, called again after InitConfig at #L621 / #L644
• createOptionsModel only handles a false return, not an exception: src/qt/bitcoin.cpp#L248-L265, called at #L687, before the try at #L777

Reproduction with dash-qt built from unmodified c14104b (macOS, --with-gui, debug build):

dash-qt with {"lang": {}} in settings.json (unfixed)

$ cat $DATADIR/regtest/settings.json
{"lang": {}}
$ timeout 60 src/qt/dash-qt -regtest -datadir=$DATADIR -nosplash
std::terminate() called due to unhandled exception
Exception: type=UniValue::type_error, what="JSON value of type object is not of expected type string"
exit status: 134

No debug.log is created: the abort happens in the second initTranslations() call, before the node is created.

dash-qt with {"font-weight-normal": []} in settings.json and -font-family on the command line (unfixed)

$ cat $DATADIR/regtest/settings.json
{"font-weight-normal": []}
$ timeout 20 src/qt/dash-qt -regtest -datadir=$DATADIR -nosplash -font-family=Montserrat
std::terminate() called due to unhandled exception
Exception: type=UniValue::type_error, what="JSON value of type array is not of expected type string"
exit status: 134

The new OptionTests cases, run against the unfixed OptionsModel::Init:

test_dash-qt (unfixed), default minimal platform

$ src/qt/test/test_dash-qt
********* Start testing of OptionTests *********
PASS   : OptionTests::initTestCase()
PASS   : OptionTests::migrateSettings()
PASS   : OptionTests::integerGetArgBug()
SKIP   : OptionTests::invalidFontSetting() AppTests cannot initialize fonts with the 'minimal' platform plugin.
FAIL!  : OptionTests::invalidLangSetting() Caught unhandled exception
   Loc: [qtestcase.cpp(1966)]
Totals: 3 passed, 1 failed, 1 skipped, 0 blacklisted, 9ms
********* Finished testing of OptionTests *********
exit status: 134

test_dash-qt (unfixed), QT_QPA_PLATFORM=cocoa so fonts are loaded

$ QT_QPA_PLATFORM=cocoa src/qt/test/test_dash-qt
********* Start testing of OptionTests *********
PASS   : OptionTests::initTestCase()
PASS   : OptionTests::migrateSettings()
PASS   : OptionTests::integerGetArgBug()
FAIL!  : OptionTests::invalidFontSetting() Caught unhandled exception
   Loc: [qtestcase.cpp(1966)]
Totals: 3 passed, 1 failed, 0 skipped, 0 blacklisted, 9ms
********* Finished testing of OptionTests *********
libc++abi: terminating due to uncaught exception of type UniValue::type_error: JSON value of type array is not of expected type string
exit status: 134

Why it matters

The user gets a crash with no explanation instead of an error that names the bad setting and the file. The way out (start with -nosettings or fix settings.json by hand) is not discoverable from an abort. It only affects startup of dash-qt with a malformed local settings.json; dashd is not affected and nothing reaches this path from the network.

What was done?

• OptionsModel::Init: the "lang" read and the four font reads are wrapped in try/catch and report the error the same way as the existing getOption() loop. The error formatting is moved into a small local lambda (setting_error) so the loop and the new call sites share one message; the translatable string is unchanged.
• OptionsModel::Init: the stored font weights are now always read (and so checked), and the existing condition then picks either that value or the font's default weight as before. This closes the -font-family path above.
• GetLangTerritory(): if reading -lang throws, keep the language picked from QSettings / the system locale. Startup then continues to OptionsModel::Init, which reports the bad "lang" value through the normal error dialog.
• OptionTests: invalidLangSetting and invalidFontSetting put an object/array into the persistent settings and check that Init returns false with the expected error. invalidFontSetting also covers the weights with -font-family set on the command line. The font case needs loaded fonts, so like updateFontsWithPixelSizedWidget it is skipped on the minimal platform.

Why this is the correct minimal fix

The root cause is that these reads sit outside the error handling that bitcoin-core/gui#602 added for exactly this kind of type error, so the fix puts them under the same handling and reuses the same message. For the font weights only the SettingToInt read is inside the try.

The font settings are not simply added to the getOption() loop: that loop also calls addOverriddenOption(), which the font blocks already do, so the overridden-options list would get duplicate entries. Reading the stored weights unconditionally does not change which weight is applied; it only makes sure that a value GuiMain will read through gArgs anyway has been checked first.

Fixing only OptionsModel::Init is not enough for "lang": GetLangTerritory() reads the same value first and would still abort before Init runs. A type check there would need to duplicate which UniValue types SettingToString accepts, while catching the conversion error keeps the existing -lang behaviour (strings, -nolang, numbers) unchanged. Making the conversion helpers non-throwing or widening GuiMain's try would be larger changes; the latter would also only swap the abort for a generic "runaway exception" message.

Behaviour for valid settings is unchanged. If the bold weight is malformed after the normal weight has been reset via forceSetting, that partial state is not undone; Init returns false and startup stops anyway.

The "lang" handling exists the same way in Bitcoin Core and could also be sent upstream.

How Has This Been Tested?

macOS arm64, --with-gui --enable-debug --enable-werror, depends Qt 5.

• Before the fix: the two dash-qt reproductions and the two test_dash-qt runs above (abort, exit 134).

• With the try/catch changes but without always reading the stored weights, the -font-family cases of invalidFontSetting fail (cocoa):

  FAIL!  : OptionTests::invalidFontSetting() '!OptionsModel{m_node}.Init(error)' returned FALSE. ()
     Loc: [qt/test/optiontests.cpp(135)]
  

• After the fix, src/qt/test/test_dash-qt (default minimal platform): all suites pass; in OptionTests, invalidLangSetting passes and invalidFontSetting is skipped as expected.

  SKIP   : OptionTests::invalidFontSetting() AppTests cannot initialize fonts with the 'minimal' platform plugin.
  PASS   : OptionTests::invalidLangSetting()
  ...
  Totals: 8 passed, 0 failed, 2 skipped, 0 blacklisted, 11ms
  

• After the fix, QT_QPA_PLATFORM=cocoa src/qt/test/test_dash-qt: all OptionTests pass, including both new cases.

  PASS   : OptionTests::invalidFontSetting()
  PASS   : OptionTests::invalidLangSetting()
  ...
  Totals: 10 passed, 0 failed, 0 skipped, 0 blacklisted, 15ms
  

• After the fix, dash-qt with {"lang": {}}: no abort; it reports the error and shows the dialog (the process was then killed by timeout while the modal dialog was open):

  $ timeout 20 src/qt/dash-qt -regtest -datadir=$DATADIR -nosplash
  Error: Could not read setting "lang", JSON value of type object is not of expected type string.
  Settings file "<datadir>/regtest/settings.json" might be corrupt or invalid.
  exit status: 124
  

• After the fix, dash-qt with {"font-weight-normal": []} and -font-family=Montserrat: same, no abort:

  $ timeout 20 src/qt/dash-qt -regtest -datadir=$DATADIR -nosplash -font-family=Montserrat
  Error: Could not read setting "font-weight-normal", JSON value of type array is not of expected type string.
  Settings file "<datadir>/regtest/settings.json" might be corrupt or invalid.
  exit status: 124
  

• contrib/devtools/clang-format-diff.py only suggests reflowing the moved tr() line and the test lambdas/strings, which follow the style of the surrounding code; test/lint/lint-whitespace.py is clean.

Breaking Changes

None.

Checklist:

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 25 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 83ce4edf-f8c2-4a56-be48-e1c2f4ed9eec

📥 Commits

Reviewing files that changed from the base of the PR and between 8819507 and cdeadac.

📒 Files selected for processing (4)

• src/qt/bitcoin.cpp
• src/qt/optionsmodel.cpp
• src/qt/test/optiontests.cpp
• src/qt/test/optiontests.h

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[thepastaclaw]

✅ Final review complete — no blockers (commit cdeadac) · triage: low

[thepastaclaw]

Final validation — Phase 1 + Phase 2

Static verification at the exact head found no actionable in-scope defects: malformed language and font settings now reach the existing settings-error dialog, including stored font weights when the font family is overridden, without changing valid-setting behavior. The regression tests follow existing Qt test initialization and settings-reset patterns. No builds or tests were run in this lane; the supplied CI snapshot shows successful preliminary checks and formatting, queued arm64 container jobs, and no completed application build or test results.

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: low by gpt-6.1-sol (effort low) — This is a contained Qt startup error-handling fix with focused regression tests, affecting neither critical surfaces nor intricate cross-subsystem behavior.
• Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer
• Phase 1 model: glm-5.3-flash — zai quota: 5h 99% left, weekly 69% left; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer

Out-of-scope follow-up suggestions (1)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

• dash-qt still terminates on malformed settings.json values for other settings (e.g. proxy, prune, splash) via earlier unprotected gArgs conversions — Outside the PR's narrow scope, not a false positive: src/qt/bitcoin.cpp:678 invokes parameterSetup before OptionsModel initialization and the try block at line 782; InitParameterInteraction reads proxy and prune at src/init.cpp:1020 and :1083, and the splash conversion at bitcoin.cpp:686 is also unprotected. These paths are already present at c14104b and remain unchanged. The PR specifically fixes lang and the four font settings, does not worsen or depend on these adjacent paths, and does not require broader startup exception handling to achieve that goal. This pre-existing local-settings issue does not meet the exceptional follow-up bar for retention.
  • Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.

[PastaPastaPasta]

approved