Skip to content

Qt: avoid blocking on Spark activation height - #1946

Open
navidR wants to merge 1 commit into
firoorg:masterfrom
navidR:dev/navidr/qt-spark-height-nonblocking
Open

Qt: avoid blocking on Spark activation height#1946
navidR wants to merge 1 commit into
firoorg:masterfrom
navidR:dev/navidr/qt-spark-height-nonblocking

Conversation

@navidR

@navidR navidR commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Use ClientModel's nonblocking cached-height accessor for Spark send controls, leaving transaction creation's locked chain-height checks unchanged. Initialize the cache and client-model wiring before use, exclude header-only heights, and add Qt regression coverage. This addresses the height-query stall observed during Debug replay of 1,024 Spark spends; separate startup and replay stalls remain.

Use the existing nonblocking ClientModel height accessor for Spark send controls. Seed the cache at startup, keep header updates separate, and attach the client model before initializing the send view.

Add Qt coverage for block/header cache separation and decreasing heights. Exclude the test source from AUTOUIC to avoid mistaking ui_interface.h for a generated form header.
@codeant-ai

codeant-ai Bot commented Sep 4, 2026

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Reviewed your PR ce28c3f Sep 04, 2026 · 21:03 21:06

@codeant-ai

codeant-ai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-04T21:05:31.561761Z ce28c3f PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: ff1943a6-ba62-4d26-8f4b-c65d533a0b1a

📥 Commits

Reviewing files that changed from the base of the PR and between 4f0c771 and ce28c3f.

📒 Files selected for processing (6)
  • src/qt/clientmodel.cpp
  • src/qt/test/CMakeLists.txt
  • src/qt/test/test_sendcoinsentry.cpp
  • src/qt/test/test_sendcoinsentry.h
  • src/qt/walletmodel.cpp
  • src/qt/walletview.cpp

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.


Summary by CodeRabbit

  • Bug Fixes

    • Corrected wallet block-height tracking so header-only updates no longer affect the reported blockchain height.
    • Improved initialization of the displayed block count to reflect the current best block when available.
    • Ensured wallet features that depend on the upcoming block height use the client’s synchronized height.
    • Improved wallet and client model synchronization, including cases where a wallet model is not yet available.
  • Tests

    • Added coverage verifying that header notifications do not alter the cached block height.

Walkthrough

The Qt client model now separates block-tip height from header height. Wallet activation checks read the client model cache, and wallet setup propagates the client model consistently. A regression test covers block-tip and header-only notifications.

Changes

Client height flow

Layer / File(s) Summary
ClientModel cache semantics and regression coverage
src/qt/clientmodel.cpp, src/qt/test/test_sendcoinsentry.cpp, src/qt/test/test_sendcoinsentry.h, src/qt/test/CMakeLists.txt
cachedNumBlocks initializes from the connection manager and changes only for block tips. The test verifies that header-only notifications update cachedBestHeaderHeight without changing cachedNumBlocks.
Wallet height integration
src/qt/walletmodel.cpp, src/qt/walletview.cpp
Versioned Spark-spend checks use the client model height. WalletView sets the client model on the wallet model when setWalletModel runs.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to ce28c

Spark send activation checks now use a nonblocking cached block height while keeping header-only heights separate. The supplied coverage exercises the corrected cache behavior, and no merge-blocking risk remains.

Suggested reviewers: levonpetrosyan93, reubenyap

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 5 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: avoiding blocking on Spark activation-height checks in Qt.
Description check ✅ Passed The description explains the intent, implementation, regression coverage, and known scope limits. It does not use the template headings, but it provides the required information and is mostly complete…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 5 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

@codeant-ai codeant-ai Bot added the size:M This PR changes 30-99 lines, ignoring generated files label Sep 4, 2026
@codeant-ai

codeant-ai Bot commented Sep 4, 2026

Copy link
Copy Markdown

User description

Use ClientModel's nonblocking cached-height accessor for Spark send controls, leaving transaction creation's locked chain-height checks unchanged. Initialize the cache and client-model wiring before use, exclude header-only heights, and add Qt regression coverage. This addresses the height-query stall observed during Debug replay of 1,024 Spark spends; separate startup and replay stalls remain.


CodeAnt-AI Description

Prevent Spark send controls from stalling while checking activation height

What Changed

  • Spark versioned-spend checks now use the cached confirmed block height without waiting on chain locks
  • Header-only updates no longer make Spark features appear activated before the corresponding block is connected
  • The height cache is initialized at startup and remains connected when wallet and client models are set up
  • Added coverage for ignoring header heights and handling decreasing block heights

Impact

✅ Fewer Spark send stalls
✅ Correct activation status during header sync
✅ Reliable Spark controls during startup and replay

💡 Usage Guide

Checking Your Pull Request

Every 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 AI

Got 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:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You 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:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To 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.

@reubenyap reubenyap left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed head ce28c3fe98f951d0b8f0a1c80e80aef4f37c6736. No additional actionable issues were found. I traced confirmed-height initialization and block/header notifications through versionedSparkSpendsAllowed(), verified wallet/client wiring precedes model use, and checked the focused Qt regression and build integration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants