doc: Add comments to FIELD_* constants in proxy.h by ViniciusCestarii · Pull Request #279 · bitcoin-core/libmultiprocess

ViniciusCestarii · 2026-05-26T20:03:03Z

Add documentation comments to the five FIELD_* bit flag constants in proxy.h, which were previously undocumented

DrahtBot · 2026-05-26T20:03:12Z

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	ryanofsky

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#288 (Create support branch for CI scripts, documentation, and examples by ryanofsky)
#274 (Add nonunix platform support 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.

ryanofsky

Code review d706480. These comments are helpful but could be misleading in a few ways so I left some recommendations below.

ryanofsky · 2026-06-02T20:08:07Z

+static constexpr int FIELD_IN = 1;        //!< Field is read from the Cap'n Proto Params struct (client -> server).
+static constexpr int FIELD_OUT = 2;       //!< Field is read from the Cap'n Proto Results struct (server -> client).


In commit "doc: Add comments to FIELD_* constants in proxy.h" (d706480)

I think it would be better to say "present" instead of "read". "read" is accurate but these flags are used to decide whether to write fields as well, so "present" reflects the wider context

ryanofsky · 2026-06-02T20:27:53Z

-static constexpr int FIELD_OPTIONAL = 4;
-static constexpr int FIELD_REQUESTED = 8;
-static constexpr int FIELD_BOXED = 16;
+static constexpr int FIELD_IN = 1;        //!< Field is read from the Cap'n Proto Params struct (client -> server).


In commit "doc: Add comments to FIELD_* constants in proxy.h" (d706480)

I think it would be better to move these comments to the Accessor struct below where they are used, and written as preceding comments (//!) trailing comments (//!<) so there is more space for the descriptions.

Could keep //!< comments here referring to the Accessor struct below though.

ryanofsky · 2026-06-02T20:34:28Z

-static constexpr int FIELD_BOXED = 16;
+static constexpr int FIELD_IN = 1;        //!< Field is read from the Cap'n Proto Params struct (client -> server).
+static constexpr int FIELD_OUT = 2;       //!< Field is read from the Cap'n Proto Results struct (server -> client).
+static constexpr int FIELD_OPTIONAL = 4;  //!< Field has a `has<Field>` sibling in the Cap'n Proto schema; value may be absent and presence must be checked via `has()`.


In commit "doc: Add comments to FIELD_* constants in proxy.h" (d706480)

Would replace the word "schema" with "struct" to be more specific. Also has<Field> looks like c++ template syntax which could be confusing. And I'm not sure the "presence must be checked" comment is meaningful because it seems focused on reading the field when the flag controls how the field needs to be written as well as read.

Would suggest something more like "Field has a companion has{Name} boolean field in the Cap'n Proto struct. This used to represent optional primitive values (e.g. C++ std::optional<int>) because Cap'n Proto doesn't allow primitive fields to be unset."

ryanofsky · 2026-06-02T20:41:00Z

+static constexpr int FIELD_IN = 1;        //!< Field is read from the Cap'n Proto Params struct (client -> server).
+static constexpr int FIELD_OUT = 2;       //!< Field is read from the Cap'n Proto Results struct (server -> client).
+static constexpr int FIELD_OPTIONAL = 4;  //!< Field has a `has<Field>` sibling in the Cap'n Proto schema; value may be absent and presence must be checked via `has()`.
+static constexpr int FIELD_REQUESTED = 8; //!< Field has a `want<Field>` sibling in the Cap'n Proto Params struct; client opts in to receiving it.


In commit "doc: Add comments to FIELD_* constants in proxy.h" (d706480)

This doesn't really say what the flag is used for. Maybe would write "Results field has a companion want{Name} boolean field in the Params struct. Used for optional output parameters (e.g. C++ int*) and set to true if the caller passed a non-null and wants the result."

ryanofsky · 2026-06-02T20:45:41Z

+static constexpr int FIELD_OUT = 2;       //!< Field is read from the Cap'n Proto Results struct (server -> client).
+static constexpr int FIELD_OPTIONAL = 4;  //!< Field has a `has<Field>` sibling in the Cap'n Proto schema; value may be absent and presence must be checked via `has()`.
+static constexpr int FIELD_REQUESTED = 8; //!< Field has a `want<Field>` sibling in the Cap'n Proto Params struct; client opts in to receiving it.
+static constexpr int FIELD_BOXED = 16;    //!< Field is a Cap'n Proto pointer type (struct, list, text, data, interface); value may be absent and presence must be checked via `has()`.


In commit "doc: Add comments to FIELD_* constants in proxy.h" (d706480)

I think absense/presence is not really relevant here. Would suggest something more like "Field is a Cap'n Proto pointer type (struct, list, text, data, interface) as opposed to a primitive type (bool, int, float, enum)."

ViniciusCestarii · 2026-06-03T13:32:06Z

Forced-push e863c6c to add suggested changes

ryanofsky

Code review ACK e863c6c. Thanks for the comments!

ryanofsky · 2026-06-09T19:54:15Z

 template <typename Field, int flags>
 struct Accessor : public Field
 {
+    //! Field is present from the Cap'n Proto Params struct (client -> server).


In commit "doc: Add comments to FIELD_* constants in proxy.h" (e863c6c)

IMO "present in" would sound more natural than "present from" but this seems ok

I agree. Thanks for the feedback

8412fcdc65 Merge bitcoin-core/libmultiprocess#295: Mark Waiter m_cv as guarded by m_mutex 1593ee2d18 Merge bitcoin-core/libmultiprocess#294: test: Add passDouble smoke test 9885d7dd33 Merge bitcoin-core/libmultiprocess#286: proxy-client: fix TSan data race in clientDestroy fa35501c4f Mark Waiter m_cv as guarded by m_mutex faaedb11f8 test: Add passDouble smoke test 733c64318d Merge bitcoin-core/libmultiprocess#292: type-number: fix clang-tidy modernize-use-nullptr 9cc3479ab3 Merge bitcoin-core/libmultiprocess#291: cmake: Add `mp_headers` custom target 201abd9e3a Merge bitcoin-core/libmultiprocess#289: cmake: make target_capnp_sources use CURRENT dirs 99820c8aec Merge bitcoin-core/libmultiprocess#279: doc: Add comments to FIELD_* constants in proxy.h 73b985540c Merge bitcoin-core/libmultiprocess#278: doc: Fix and expand design.md e7e91b2e23 Merge bitcoin-core/libmultiprocess#277: Add std::unordered_set support and a helper BuildList to dedup list build handlers 91a951f59a tidy fix: modernize-use-nullptr 16362f42d0 cmake: Add `mp_headers` custom target 615a94fe3a cmake: document ONLY_CAPNP option in target_capnp_sources 90982f75c6 mpgen: iwyu changes required by previous commit 25bb3e67f3 proxy-client: fix TSan data race in clientDestroy 620f297f31 cmake: make target_capnp_sources use CURRENT dirs 9de4b885aa test: use camelCase + $Proxy.name for FooStruct fields 011b91793d type: add std::unordered_set support 20d19b9644 proxy: add BuildList helper and dedup map/set/vector build handlers e863c6cdf6 doc: Add comments to FIELD_* constants in proxy.h 18db0ab957 doc: Fix and expand design.md 61de697536 Merge bitcoin-core/libmultiprocess#273: proxy-client: tolerate exceptions from remote destroy during cleanup 9cec9d6ca5 Merge bitcoin-core/libmultiprocess#243: mpgen: support primitive std::optional struct fields 4aaff11374 Merge bitcoin-core/libmultiprocess#238: cmake, ci: updates for recent nixpkgs 2ac55a56b5 Merge bitcoin-core/libmultiprocess#218: Better error and log messages 6de92e1c73 proxy-client: tolerate exceptions from remote destroy during cleanup 90be8354d4 test: regression for ~ProxyClient destroy after peer disconnect 3c69d125a1 Merge bitcoin-core/libmultiprocess#260: event loop: tolerate unexpected exceptions in `post()` callbacks b8a48c65e6 event loop: tolerate unexpected exceptions in `post()` callbacks f787863d2c Merge bitcoin-core/libmultiprocess#270: doc: Bump version 10 > 11 a22f602910 doc: Bump version 10 > 11 4eae445d6d debug: Add TypeName() function and log statements for Proxy objects being created and destroyed f326c5b1b7 logging: Add better logging on IPC server-side failures 6dbfa56a04 mpgen: support primitive std::optional struct fields 8d1277deb5 mpgen refactor: add AccessorType function db716bbcba mpgen refactor: Move field handling code to FieldList class db7acb3ce2 ci: Fix shell.nix compatibility with CMake 4.0 91a7759a9a cmake: Fix IWYU in nix by adding CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES git-subtree-dir: src/ipc/libmultiprocess git-subtree-split: 8412fcdc659e1379f9b4dea896c26bc1c5f3afa8

ryanofsky reviewed Jun 2, 2026

View reviewed changes

doc: Add comments to FIELD_* constants in proxy.h

e863c6c

ViniciusCestarii force-pushed the doc/field-flags-comments branch from d706480 to e863c6c Compare June 3, 2026 13:30

This was referenced Jun 3, 2026

Add nonunix platform support #274

Open

Create support branch for CI scripts, documentation, and examples #288

Open

ryanofsky reviewed Jun 9, 2026

View reviewed changes

ryanofsky merged commit 99820c8 into bitcoin-core:master Jun 9, 2026
12 of 13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: Add comments to FIELD_* constants in proxy.h#279

doc: Add comments to FIELD_* constants in proxy.h#279
ryanofsky merged 1 commit into
bitcoin-core:masterfrom
ViniciusCestarii:doc/field-flags-comments

ViniciusCestarii commented May 26, 2026

Uh oh!

DrahtBot commented May 26, 2026 •

edited

Loading

Uh oh!

ryanofsky left a comment

Uh oh!

ryanofsky Jun 2, 2026

Uh oh!

ryanofsky Jun 2, 2026

Uh oh!

ryanofsky Jun 2, 2026

Uh oh!

ryanofsky Jun 2, 2026

Uh oh!

ryanofsky Jun 2, 2026

Uh oh!

ViniciusCestarii commented Jun 3, 2026

Uh oh!

ryanofsky left a comment

Uh oh!

ryanofsky Jun 9, 2026

Uh oh!

ViniciusCestarii Jun 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		static constexpr int FIELD_IN = 1; //!< Field is read from the Cap'n Proto Params struct (client -> server).
		static constexpr int FIELD_OUT = 2; //!< Field is read from the Cap'n Proto Results struct (server -> client).

Conversation

ViniciusCestarii commented May 26, 2026

Uh oh!

DrahtBot commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Conflicts

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

ryanofsky Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

ryanofsky Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

ryanofsky Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

ryanofsky Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

ryanofsky Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

ViniciusCestarii commented Jun 3, 2026

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

ryanofsky Jun 9, 2026

Choose a reason for hiding this comment

Uh oh!

ViniciusCestarii Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

DrahtBot commented May 26, 2026 •

edited

Loading