http: Make class fields private and make HTTPResponse a struct #35829

1. 
   hodlinator commented at 11:49 AM on July 28, 2026: contributor

   The new HTTP server implementation does not fully adhere to common C++ practice such as described in Core Guidelines[^1][^2]. Beyond motivations such as information hiding and enforcing invariants, exposing data through accessor methods also implies adding lock annotations.

   Commits:

   • Makes HTTPResponse a struct since it is used that way. (https://github.com/bitcoin/bitcoin/pull/35182#discussion_r3336757663)
   • HTTPRequest:
     • Saner return type for GetHeader() (old type was mirroring the now removed libevent-wrapper).
     • Make fields private.
   • Simplifies boolean logic in HTTPServer::DisconnectClients(). (https://github.com/bitcoin/bitcoin/pull/35182#discussion_r3336757663)
   • Extraction of HTTPServer functions into HTTPRemoteClient: Refactors HTTPRemoteClient to be more self-contained rather than having HTTPServer reach into the fields of other objects. (https://github.com/bitcoin/bitcoin/pull/35182#discussion_r3339543447, #35182 (review))
   • Severely narrows http_bitcoin namespace and renames it to bitcoin_http (https://github.com/bitcoin/bitcoin/pull/35182#discussion_r3264510816)

   Follow-up to #35182.

   [^1]: Core Guidelines: C.2: Use class if the class has an invariant; use struct if the data members can vary independently - https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c2-use-class-if-the-class-has-an-invariant-use-struct-if-the-data-members-can-vary-independently [^2]: Core Guidelines: C.9: Minimize exposure of members - https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c9-minimize-exposure-of-members

2. DrahtBot added the label RPC/REST/ZMQ on Jul 28, 2026
3. 
   DrahtBot commented at 11:49 AM on July 28, 2026: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   <!--006a51241073e994b41acfe9ec718e94-->

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35829.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline and AI policy for information on the review process.

   Type	Reviewers
   Concept ACK	winterrdog

   If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #29700 (kernel, refactor: return error status on all fatal errors by ryanofsky)
   • #26022 (Add util::ResultPtr class by ryanofsky)
   • #25722 (refactor: Use util::Result class for wallet loading by ryanofsky)
   • #25665 (refactor: Add util::Result failure types and ability to merge result values by ryanofsky)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

   <!--5faf32d7da4f0f540f40219e4f7537a3-->

4. 
   pinheadmz commented at 7:51 PM on July 28, 2026: member

   Light code review, changes all look fine. It's all code style cleanups (thanks 😬) though without any obvious gain. Will review more thoroughly after more concept a-c-ks

5. DrahtBot added the label Needs rebase on Jul 29, 2026
6. hodlinator force-pushed on Aug 10, 2026
7. DrahtBot removed the label Needs rebase on Aug 10, 2026
8. 
   hodlinator commented at 1:05 PM on August 10, 2026: contributor

   Thanks for the interest @pinheadmz!

   I've updated the PR description to include more of a "Why-preamble". + Resolved conflicts with master.

9. fanquake added this to the milestone 32.0 on Aug 17, 2026
10. 
    fanquake commented at 9:20 AM on August 17, 2026: member

    Can you rebase? Added to the 32 milestone, because it'd be good to get all these sort of "cleanups" done in the same release cycle.

11. DrahtBot added the label Needs rebase on Aug 17, 2026
12. 
    pinheadmz commented at 11:03 AM on August 17, 2026: member

    Wanna add this too (make m_body private) #35735 (review) when you rebase since #35735 was merged

13. hodlinator force-pushed on Aug 17, 2026
14. DrahtBot removed the label Needs rebase on Aug 17, 2026
15. hodlinator force-pushed on Aug 17, 2026
16. DrahtBot added the label CI failed on Aug 17, 2026
17. 
    hodlinator commented at 9:33 PM on August 17, 2026: contributor

    Rebased and worked on it a bit more. The new HTTPRequest state and test from #35735 took some time to resolve. Agree with the sentiment from #35829 (comment) that it would be nice to include in the initial release of the new HTTP code.

18. DrahtBot removed the label CI failed on Aug 17, 2026
19. hodlinator renamed this:
    http: Make class fields private or convert to struct
    http: Make class fields private and make HTTPResponse a struct
    on Aug 18, 2026
20. refactor: Make HTTPResponse a struct since all fields are public cb2c90a817
21. refactor: Make HTTPRequest::GetHeader() return saner optional type

    No need to stick to weird old API from libevent-wrapper days.

    966de1753b
22. refactor: Make HTTPRequest fields private

    Makes sense since they are only set by methods in the class itself, and already had accessors for most fields.

    76521da240
23. refactor: Simplify boolean logic in HTTPServer::DisconnectClients() 3b22789eb9
24. refactor: Extract HTTPRemoteClient::MaybeDisconnect() from HTTPServer::DisconnectClients() 4f126700b3
25. refactor: Extract Send() and Receive() into HTTPRemoteClient from HTTPServer 6f2a21d22c
26. refactor: Replace HTTPServer::MaybeDispatchRequestsFromClient() with HTTPRemoteClient::ReadRequests() 523a53ee18
27. refactor(test): Upper case DummyClient::Receive() as per developer-notes.md

    Also avoid repetitive HTTPRequest::State::-prefixes inside http_request_state_tests test case.

    dd7eb456a4
28. refactor: Expose HTTPRemoteClient fields to tests through methods

    Enables making the fields private later.

    5b7f2afac9
29. refactor: Make HTTPRemoteClient fields private

    Also makes ReadRequest() an internal method.

    5bc16beb80
30. refactor: Drastically narrow scope of http_bitcoin namespace and rename it to bitcoin_http

    http_bitcoin was mostly used during #35182 to distinguish from http_libevent counterpart:
    - The http_libevent namespace was introduced around the legacy code in 89c54ae4cbc8e58921551d5f1a90eb4683106ccb.
    - The http_bitcoin namespace was introduced in 68b5d289d19c42de9bebf54a0555053d29721111 and extended in subsequent commits.
    - The http_libevent namespace together with code it contained was removed in 8c1eea0777c586ce58a500bbea509cd39e4f3507.
    
    bitcoin_http is a better name as it is Bitcoin Core's implementation of the HTTP protocol, not HTTP protocol's implementation of bitcoin 402 payment required codes or anything like that.
    The namespace only remains for a few constants and a type which don't have HTTP in their names.

    3dae698004
31. hodlinator force-pushed on Aug 18, 2026
32. sedited requested review from pinheadmz on Aug 18, 2026
33. 
    winterrdog commented at 8:42 AM on August 19, 2026: contributor

    concept ACK

Contributors

hodlinatorDrahtBotpinheadmzfanquakewinterrdog

Review Requested
pinheadmz

Milestone
32.0