Qt: defer Spark GUI callbacks during validation - #1950
Conversation
Use try-locks in the queued incoming-fund scan, automatic mint check and Spark address-book refresh so they do not block the GUI on cs_main. Retry the startup scan and retain address choices while refresh is busy. Keep Spark proof verification serialized and preserve historical batching. Add Qt regressions for lock contention and the deferred scan retry. Alternative to #1940 for the GUI responsiveness issue.
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@CodeAnt-AI review |
🤖 CodeAnt AI — Review Status
|
User descriptionPR intentionAlternative to #1940 for the Qt freeze during Spark batch verification. Long validation holds Code changes briefUse existing Two production files, 21 additions and 3 deletions. Qt regression tests cover chain/wallet lock contention, retry without another block notification, and preservation of address choices. Independent of #1940, based on master Validation: Local configuration command (PowerShell, MSYS2 UCRT compiler on PATH): C:\msys64\ucrt64\bin\cmake.exe -G Ninja -S . -B build-qt -DCMAKE_C_COMPILER=C:/msys64/ucrt64/bin/cc.exe -DCMAKE_CXX_COMPILER=C:/msys64/ucrt64/bin/c++.exe -DCMAKE_MAKE_PROGRAM=C:/msys64/ucrt64/bin/ninja.exe -DCMAKE_BUILD_TYPE=Debug -DBUILD_GUI=ON -DBUILD_TESTS=ON -DBUILD_GUI_TESTS=ON -DWITH_ZMQ=OFF -DWITH_QRENCODE=OFF -DWITH_DBUS=OFFCodeAnt-AI DescriptionKeep the Qt interface responsive while Spark validation is running What Changed
Impact
💡 Usage GuideChecking Your Pull RequestEvery time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later. Talking to CodeAnt AIGot a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask: This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code. ExamplePreserve Org Learnings with CodeAntYou can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input: This helps CodeAnt AI learn and adapt to your team's coding style and standards. ExampleRetrigger reviewAsk CodeAnt AI to review the PR again, by typing: Check Your Repository HealthTo analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health. |
AddressBookPage requires a QWidget parent argument. Supply nullptr in the Spark GUI contention test so the Linux Qt test target compiles.
A shared refresh can finish when another page succeeds. Schedule the busy page's retry directly so its address choices still update, and verify the retry without another block notification.
reubenyap
left a comment
There was a problem hiding this comment.
No additional actionable issues were found at 82c9f8f5c628d7b9f16a54eee15ce698b3ff91cf.
I traced the three GUI callbacks through their callers and lock ordering, checked timer lifetimes, retry behavior, and address-choice preservation, reviewed the contention tests and existing discussion, and confirmed the current Linux Debug/RelWithDebInfo Qt tests passed. git diff --check also passes.
PR intention
Alternative to #1940 for the Qt freeze during Spark batch verification. Long validation holds
cs_main, while the queued incoming-fund scan, auto-mint activation check, and Spark address-book refresh can wait for that lock on the GUI thread.Code changes brief
Use existing
TRY_LOCKand Qt timer patterns to defer these automatic callbacks. The startup scan and each busy address-book page retry afterMODEL_UPDATE_DELAY(250 ms); auto-mint retries on its next timer tick. Address choices remain intact while busy. Each page retries independently because the shared refresh may finish when another page succeeds.Two production files, 25 additions and 3 deletions. Core proof verification remains serialized, preserving master's historical batching and recent-block verification behavior. Independent of #1940, based on master
4f0c771462b2f327e3a7ff7bf2532bc33c727713; deferred-proof drain and recovery-marker improvements remain separate work.Qt regression tests cover chain/wallet lock contention, automatic retries without another block notification, and preservation of address choices.
Integration with #1914 at
425c6dbwas checked in an isolated test merge. Production code merges cleanly. Resolve the two conflicts insrc/qt/test/CMakeLists.txtandsrc/qt/test/test_main.cppby retaining both test suites and #1914's resource/settings setup. The combined GUI has not been built or exercised.Validation at
82c9f8f(CI run):cd build && ctest --output-on-failurepassed 94/94 tests in each, includingtest_firo-qt.qa/pull-tester/rpc-tests.py -extendedpassed in both configurations.git diff origin/master...HEAD --checkandgit merge-tree --write-tree --messages origin/master origin/pr1950-thorough-reviewpassed against the current base. The resolved Qt: Redesign wallet UI with light/dark theme support #1914 integration diff check also passed.Local Qt development files are unavailable, so no local GUI build/test was run. A live batch-sync GUI run has not been reproduced. Keep this draft until the reported hang is verified fixed; these callbacks are not proven to be its only cause.