fix: RPC endpoint always return stacks canonical height

[Jiloc]

Description

The X-Canonical-Stacks-Tip-Height header is intended to be included in all succesfull RPC responses, and this has mostly been the case until epoch 2.5. However, over time this behavior was inconsistently applied, as the header had to be manually added in each endpoint's implementation. This made it easy to forget during implementation or review of new endpoints.

This PR refactors the code to centralize the logic for setting this header, ensuring it is automatically included in all relevant responses. So we eliminate the need to manually include it in each endpoint and write dedicated tests for it for each new endpoint. The presence of the header continues to be covered by existing integration tests for various endpoints (e.g.,

stacks-core/stackslib/src/net/api/tests/getinfo.rs

Lines 90 to 93 in 9d53951

	assert_eq!(
	response.preamble().get_canonical_stacks_tip_height(),
	Some(1)
	);

)

The following endpoints were previously missing the header and are now covered:

• /v2/blocks/:block_id
• /v2/headers/:height
• /v2/mempool/query
• /v2/microblock/*
• /v3/tenures/fork_info
• /v3/blocks/:block_id
• /v3/blocks/height/:block_height
• /v3/health
• /v3/sortitions/*
• /v3/tenures/:block_id
• /v3/tenures/info
• /v3/tenures/tip/:consensus_hash
• /v3/transaction/:txid

This change also lays the groundwork for a follow-up PR that will use this header to determine the highest Stacks tip height among peers via their RPC responses.

Other changes:

• the various HttpPreambleExtensions functions to set the header now expect a u64 instead of a u32, avoiding the u64 -> u32 casting.
• add a is_success function to HttpResponsePreamble to easily check the response header.

Applicable issues

• fixes #

Additional info (benefits, drawbacks, caveats)

Checklist

• Test coverage for new or modified code paths
• Changelog is updated
• Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
• New clarity functions have corresponding PR in clarity-benchmarking repo
• New integration test(s) added to bitcoin-tests.yml

[fdefelici]

Overall looks fine!

Waiting for open remarks to be addressed before approving

[fdefelici]

The new commit a95d3e7 centralizes the get_canonical_stacks_tip_height() response header check in the TestRPC facility. This opens an opportunity to clean up the RPC endpoint tests regarding canonical height assertions (let's call it height assert for brevity).

Currently, each RPC endpoint should checks for the height assert in their tests. This approach has proved to be error-prone. As a result, about 60% of RPC (e.g., callreadonly.rs) perform this check in their tests, while 40% (e.g., getblock.rs) do not. Ideally, these inconsistencies should be addressed in this PR.

Given the new commit, I propose the following improvement to our test strategy:

• Remove all individual height assert checks from the 60% of RPC endpoint tests that currently include them.
• Introduce a dedicated test specifically for the height assert, which can target one or a few RPC endpoints (or just a mocked one if we could) and verify that the height assert matches the expected value.

This approach centralizes the height assert check and relieves all RPC endpoint tests (including any future additions) from having to perform this verification individually.

What do you think?

[Jiloc]

The new commit a95d3e7 centralizes the get_canonical_stacks_tip_height() response header check in the TestRPC facility. This opens an opportunity to clean up the RPC endpoint tests regarding canonical height assertions (let's call it height assert for brevity).

Currently, each RPC endpoint should checks for the height assert in their tests. This approach has proved to be error-prone. As a result, about 60% of RPC (e.g., callreadonly.rs) perform this check in their tests, while 40% (e.g., getblock.rs) do not. Ideally, these inconsistencies should be addressed in this PR.

Given the new commit, I propose the following improvement to our test strategy:

• Remove all individual height assert checks from the 60% of RPC endpoint tests that currently include them.
• Introduce a dedicated test specifically for the height assert, which can target one or a few RPC endpoints (or just a mocked one if we could) and verify that the height assert matches the expected value.

This approach centralizes the height assert check and relieves all RPC endpoint tests (including any future additions) from having to perform this verification individually.

What do you think?

I agree, this sounds like an improvement. I also had the same concern in the back of my mind. If we leave the tests as they are, future developers adding new endpoints might wonder whether they should include the height assert based on the mixed patterns in existing tests.

[fdefelici]

LGMT!

[codecov]

Codecov Report

❌ Patch coverage is 91.34615% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.60%. Comparing base (a9e2828) to head (72caf0c).
⚠️ Report is 164 commits behind head on develop.

Files with missing lines	Patch %	Lines
stackslib/src/net/httpcore.rs	66.66%	2 Missing ⚠️
stackslib/src/net/api/fastcallreadonly.rs	0.00%	1 Missing ⚠️
stackslib/src/net/api/getattachment.rs	0.00%	1 Missing ⚠️
stackslib/src/net/api/getconstantval.rs	0.00%	1 Missing ⚠️
stackslib/src/net/api/getdatavar.rs	0.00%	1 Missing ⚠️
stackslib/src/net/download/epoch2x.rs	66.66%	1 Missing ⚠️
...download/nakamoto/tenure_downloader_unconfirmed.rs	0.00%	1 Missing ⚠️
stackslib/src/net/tests/httpcore.rs	97.82%	1 Missing ⚠️

❌ Your project status has failed because the head coverage (64.60%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #6344      +/-   ##
===========================================
+ Coverage    62.44%   64.60%   +2.15%     
===========================================
  Files          552      555       +3     
  Lines       351797   350866     -931     
===========================================
+ Hits        219676   226672    +6996     
+ Misses      132121   124194    -7927     

Files with missing lines	Coverage Δ
stackslib/src/chainstate/stacks/db/mod.rs	82.36% <ø> (+3.96%)	⬆️
stackslib/src/net/api/callreadonly.rs	93.24% <100.00%> (+11.18%)	⬆️
stackslib/src/net/api/get_tenures_fork_info.rs	68.61% <ø> (-0.54%)	⬇️
stackslib/src/net/api/getaccount.rs	91.41% <100.00%> (+9.09%)	⬆️
stackslib/src/net/api/getattachmentsinv.rs	79.61% <100.00%> (-0.13%)	⬇️
stackslib/src/net/api/getblock.rs	73.80% <ø> (ø)
stackslib/src/net/api/getblock_v3.rs	79.21% <ø> (ø)
stackslib/src/net/api/getblockbyheight.rs	78.22% <ø> (ø)
stackslib/src/net/api/getclaritymarfvalue.rs	85.95% <100.00%> (-0.12%)	⬇️
stackslib/src/net/api/getclaritymetadata.rs	89.72% <100.00%> (+42.10%)	⬆️
... and 61 more

... and 341 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a9e2828...72caf0c. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.