fix: don't assume that successful weightFromArg links to valid index

[kwvg]

Additional Information

dash#7068 introduced a regression where Qt clients with no GUI settings are unable to run at all due to failed weight to index conversion. This was covered in 9cd9d44 but made an incorrect assumption that earlier iterations of the fix (see ef34868) did not make.

The assumption made that resulted in this regression was that if the argument to weight conversion is successful, then the weight to index conversion will also be successful.

The earlier iteration of the fix checked the return value of the index conversion and set a sane default if it was invalid. The final iteration set the default weight value to the sane weight, expecting weightFromArg() to fail (hence using the sane weight), not supportedWeightToIndex() (which might not be able to process even a valid weight from argument).

This was not caught in testing as over the course of working on dash#6833, my GUI settings were migrated to settings.json, which has precedence over QSettings and therefore, the client always ran with known good Montserrat parameters.

Fix can be verified by deleting your QSettings parameters and running the client anew.

Breaking Changes

None expected.

Checklist

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas (note: N/A)
• I have added or updated relevant unit/integration/functional/e2e tests (note: N/A)
• I have made corresponding changes to the documentation (note: N/A)
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[coderabbitai]

Walkthrough

The data() method mapping for FontWeightNormal and FontWeightBold in optionsmodel.cpp has been modified to validate computed font weights using IsValidWeight before converting to an index. If the computed weight is invalid, the method now falls back to the registry's default weight for the corresponding style via a lambda. Previously, weights were computed and converted directly to indices without an explicit validity check.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly and specifically describes the main bug fix: correcting an invalid assumption that successful weight conversion guarantees a valid index.
Description check	✅ Passed	The description provides comprehensive context explaining the regression, the root cause, how it was introduced, and how to verify the fix.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 297c9cc and 8bba1d2.

📒 Files selected for processing (1)

• src/qt/optionsmodel.cpp

🧰 Additional context used

📓 Path-based instructions (2)

src/**/*.{cpp,h,hpp,cc}

📄 CodeRabbit inference engine (CLAUDE.md)

Dash Core implementation must be written in C++20, requiring at least Clang 16 or GCC 11.1

Files:

• src/qt/optionsmodel.cpp

src/qt/**/*.{cpp,h}

📄 CodeRabbit inference engine (CLAUDE.md)

GUI implementation in src/qt/ must use Qt 5

Files:

• src/qt/optionsmodel.cpp

🧠 Learnings (3)

📓 Common learnings

Learnt from: kwvg
Repo: dashpay/dash PR: 7068
File: src/qt/guiutil_font.h:68-77
Timestamp: 2025-12-22T15:42:51.679Z
Learning: In the Dash Core Qt codebase (src/qt/guiutil_font.h), kwvg intentionally uses `assert()` in setter methods like `SetWeightBold` and `SetWeightNormal` to enforce invariants (e.g., `assert(m_weights.count(m_font))`). This fail-fast approach is preferred over defensive checks with fallbacks when an inconsistent state indicates a programming error that should be caught during development, rather than a runtime condition requiring graceful handling.

📚 Learning: 2025-12-22T15:42:51.679Z

Learnt from: kwvg
Repo: dashpay/dash PR: 7068
File: src/qt/guiutil_font.h:68-77
Timestamp: 2025-12-22T15:42:51.679Z
Learning: In the Dash Core Qt codebase (src/qt/guiutil_font.h), kwvg intentionally uses `assert()` in setter methods like `SetWeightBold` and `SetWeightNormal` to enforce invariants (e.g., `assert(m_weights.count(m_font))`). This fail-fast approach is preferred over defensive checks with fallbacks when an inconsistent state indicates a programming error that should be caught during development, rather than a runtime condition requiring graceful handling.

Applied to files:

• src/qt/optionsmodel.cpp

📚 Learning: 2025-12-17T13:58:19.813Z

Learnt from: kwvg
Repo: dashpay/dash PR: 7072
File: src/qt/walletcontroller.cpp:520-528
Timestamp: 2025-12-17T13:58:19.813Z
Learning: In Dash Qt wallet code, prefer using existing Qt signals/slots (e.g., ShowProgress signals) for long-running operations instead of adding explicit QProgressDialog dialogs. This minimizes upstream deviations and simplifies backports. Apply this pattern to wallet-related Qt components (e.g., operations like rescanning) and rely on signal-based progress updates rather than bespoke dialogs.

Applied to files:

• src/qt/optionsmodel.cpp

🧬 Code graph analysis (1)

src/qt/optionsmodel.cpp (1)

src/qt/guiutil_font.cpp (2)

• weightFromArg (168-176)
• weightFromArg (168-168)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Lint / Run linters
• GitHub Check: Build container / Build container (arm64)
• GitHub Check: Build container / Build container (amd64)

🔇 Additional comments (2)

src/qt/optionsmodel.cpp (2)

566-575: LGTM! Correct fix for weight-to-index validation.

The lambda correctly validates that the weight from settings is both successfully converted AND supported by the current font before converting to an index. The fallback to GetWeightNormalDefault() is appropriate for cases where the stored weight is not supported, fixing the regression where clients with missing QSettings could fail to start.

---

576-585: LGTM! Consistent validation for bold weight.

The implementation mirrors the FontWeightNormal fix, correctly validating both the weight conversion and font support before using it. The fallback to GetWeightBoldDefault() ensures robust behavior when stored weights are invalid or unsupported.

---

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 and usage tips.}

[UdjinM6]

utACK 8bba1d2

[PastaPastaPasta]

utACK 8bba1d2