Skip to content

proxy-types: add CustomHasField hook to map Cap'n Proto values to null C++ values#242

Merged
ryanofsky merged 2 commits intobitcoin-core:masterfrom
Sjors:2026/02/serialize-null
Mar 9, 2026
Merged

proxy-types: add CustomHasField hook to map Cap'n Proto values to null C++ values#242
ryanofsky merged 2 commits intobitcoin-core:masterfrom
Sjors:2026/02/serialize-null

Conversation

@Sjors
Copy link
Copy Markdown
Member

@Sjors Sjors commented Feb 20, 2026 •
edited
Loading

Taken from: bitcoin/bitcoin#34020 (comment)

Let applications override CustomHasField so they can decide to treat certain capnproto values as being unset. For example, when converting List(Data) to vector<shared_ptr<CTransaction>>, mapping empty Data fields to null pointers.

This safe to do in special cases, like in this example because serialized CTransaction representations are never empty. It is also useful to do in this case because Cap'n Proto doesn't currently provide any API for distinguishing between unset and empty data values in a list (although they can be distinguished on the wire).

@DrahtBot
Copy link
Copy Markdown

DrahtBot commented Feb 20, 2026 •
edited
Loading

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 <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #243 (mpgen: support primitive std::optional struct fields 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.

@Sjors
Copy link
Copy Markdown
Member Author

Sjors commented Feb 20, 2026 •
edited
Loading

So far I just took bitcoin/bitcoin@b147783 verbatim, minus the IPC tests which go into Bitcoin Core. Let me know if you need more adjustments. Looks like the LLM found some typos.

@Sjors Sjors force-pushed the 2026/02/serialize-null branch from fb2622f to e8bcca3 Compare February 20, 2026 09:48
@Sjors
Copy link
Copy Markdown
Member Author

Sjors commented Feb 20, 2026 •
edited
Loading

Whipped up a test inspired by the one in bitcoin/bitcoin@b147783 on the Bitcoin Core side.

I fixed the typos and also added a commit to fix two missing includes, that would otherwise need to be added in the test.

@Sjors Sjors force-pushed the 2026/02/serialize-null branch from e8bcca3 to 423a789 Compare February 20, 2026 10:06
@Sjors
Copy link
Copy Markdown
Member Author

Sjors commented Feb 20, 2026

Added missing #include <vector> to mp/gen.cpp.

@Sjors Sjors mentioned this pull request Feb 20, 2026
Draft
@Sjors Sjors force-pushed the 2026/02/serialize-null branch from 423a789 to 413f915 Compare February 21, 2026 07:59
@DrahtBot DrahtBot mentioned this pull request Feb 22, 2026
Open
ryanofsky
ryanofsky approved these changes Feb 25, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

Code review ACK 413f915. Code here looks good. I think PR title & description need to be updated (also title & description of main commit) because they are referencing CTransactionRef which is not in this repository and also not affected by this change (it requires the CustomHasField(TypeList<CTransaction>...) overload in the original commit).

A good summary of changes here might be "Add CustomHasValue overload to allow mapping non-null Cap'n Proto values to null C++ values" and probably the LLM can turn that into a good title & description not referring to CTransactionRef

Also wondering if you'd want this added to bitcoin/bitcoin#34422 if merged (seems reasonable)

include/mp/proxy-types.h Outdated
Comment on lines +196 to +197
//! requires function parameters, there's no way to call the function
//! constructing values for each of the parameters. Similarly there's no way to
Copy link
Copy Markdown
Collaborator

@ryanofsky ryanofsky Feb 25, 2026

Choose a reason for hiding this comment

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

In commit "ipc: Serialize null CTransactionRef as empty Data" (413f915)

I think there is a missing word and this supposed to say "call the function without constructing values"

@Sjors
Copy link
Copy Markdown
Member Author

Sjors commented Feb 27, 2026

Also wondering if you'd want this added to bitcoin/bitcoin#34422 if merged (seems reasonable)

That's not necessary. I need this for bitcoin/bitcoin#34020 which can wait for v32.

@Sjors Sjors changed the title ipc: Serialize null CTransactionRef as empty Data proxy-types: add CustomHasField hook to map Cap'n Proto values to null C++ values Feb 27, 2026
@Sjors Sjors force-pushed the 2026/02/serialize-null branch from 413f915 to 2d3cc77 Compare February 27, 2026 18:05
@Sjors
Copy link
Copy Markdown
Member Author

Sjors commented Feb 27, 2026

Tweaked the commit message and add the missing "without".

@ryanofsky
Copy link
Copy Markdown
Collaborator

ryanofsky commented Feb 27, 2026

Code review ACK 2d3cc77, with commit message rewrite and comment fix since last review.

FWIW I think new commit message is clear, but new PR description is pretty random and doesn't say what the change does. Not important, but something like this might be better:

Let applications override CustomHasField so they can decide to treat certain capnproto values as being unset. For example, when converting List(Data) to vector<shared_ptr<CTransaction>>, mapping empty Data fields to null pointers.
This safe to do in special cases, like in this example because serialized CTransaction representations are never empty. It is also useful to do in this case because Cap'n Proto doesn't currently provide any API for distinguishing between unset and empty data values in a list (although they can be distinguished on the wire).

Sjors and others added 2 commits March 4, 2026 18:40
@Sjors
refactor: add missing includes to mp/type-data.h
8c2f102
@Sjors @ryanofsky
proxy-types: add CustomHasField hook for nullable decode paths
97d8770 
Add a CustomHasField(TypeList<...>, InvokeContext&, const Input&) extension
point used by ReadField/CustomReadField to decide whether an input should
materialize a C++ value. The default behavior remains input.has().

This enables targeted mappings from specific non-null Cap'n Proto values to
null C++ values (for example empty Data in List(Data), where Cap'n Proto C++
cannot currently distinguish null vs empty), without CTransactionRef-specific
logic in libmultiprocess.

Apply the hook across nullable read paths and add a test covering
round-tripping data pointers including null values.

Originally proposed here:
bitcoin/bitcoin#34020 (comment)

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
@Sjors Sjors force-pushed the 2026/02/serialize-null branch from 2d3cc77 to 97d8770 Compare March 4, 2026 21:41
@Sjors
Copy link
Copy Markdown
Member Author

Sjors commented Mar 4, 2026

Rebased for easier subtree updating in bitcoin/bitcoin#34020. No other changes.

@ryanofsky
Copy link
Copy Markdown
Collaborator

ryanofsky commented Mar 5, 2026

Code review ACK 97d8770. Confirmed no changes since last rebase.

@Sjors could you maybe review #246? I want to bump the version after bitcoin/bitcoin#28722 was merged. Then merge newer PR's like this.

@ryanofsky ryanofsky merged commit 8a5e3ae into bitcoin-core:master Mar 9, 2026
10 checks passed
@Sjors Sjors deleted the 2026/02/serialize-null branch March 10, 2026 10:52
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Mar 11, 2026
@ryanofsky
Squashed 'src/ipc/libmultiprocess/' changes from 1868a84451f..789ac8c…
9b78b60 
…86d5

789ac8c86d5 test: m_on_cancel called after request finishes
5aa8c36cdd3 test: getParams() called after request cancel
7f954aa5eac race fix: m_on_cancel called after request finishes (bitcoin#34782)
88b4d85099d race fix: getParams() called after request cancel (bitcoin#34777)
4fa90c68015 race fix: worker thread destroyed before it is initialized (bitcoin#34711, bitcoin#34756)
22bec918c97 Merge bitcoin-core/libmultiprocess#247: type-map: Work around LLVM 22 "out of bounds index" error
8a5e3ae6ed2 Merge bitcoin-core/libmultiprocess#242: proxy-types: add CustomHasField hook to map Cap'n Proto values to null C++ values
e8d35246918 Merge bitcoin-core/libmultiprocess#246: doc: Bump version 8 > 9
97d877053b6 proxy-types: add CustomHasField hook for nullable decode paths
8c2f10252c9 refactor: add missing includes to mp/type-data.h
b1638aceb40 doc: Bump version 8 > 9
f61af487217 type-map: Work around LLVM 22 "out of bounds index" error

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 789ac8c86d5532438351e06296e7139565ba60d7
@ryanofsky ryanofsky mentioned this pull request Mar 11, 2026
Open
Sjors added a commit to Sjors/bitcoin that referenced this pull request Mar 11, 2026
@Sjors
Squashed 'src/ipc/libmultiprocess/' changes from 1868a84451..22bec918c9
9a92945 
22bec918c9 Merge bitcoin-core/libmultiprocess#247: type-map: Work around LLVM 22 "out of bounds index" error
8a5e3ae6ed Merge bitcoin-core/libmultiprocess#242: proxy-types: add CustomHasField hook to map Cap'n Proto values to null C++ values
e8d3524691 Merge bitcoin-core/libmultiprocess#246: doc: Bump version 8 > 9
97d877053b proxy-types: add CustomHasField hook for nullable decode paths
8c2f10252c refactor: add missing includes to mp/type-data.h
b1638aceb4 doc: Bump version 8 > 9
f61af48721 type-map: Work around LLVM 22 "out of bounds index" error

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 22bec918c97d32660c4694c3a8b5af4cdbb88481
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Mar 12, 2026
@ryanofsky
Squashed 'src/ipc/libmultiprocess/' changes from 1868a84451f..3f28bca…
473e097 
…5174

3f28bca5174 test: worker thread destroyed before it is initialized
789ac8c86d5 test: m_on_cancel called after request finishes
5aa8c36cdd3 test: getParams() called after request cancel
7f954aa5eac race fix: m_on_cancel called after request finishes (bitcoin#34782)
88b4d85099d race fix: getParams() called after request cancel (bitcoin#34777)
4fa90c68015 race fix: worker thread destroyed before it is initialized (bitcoin#34711, bitcoin#34756)
22bec918c97 Merge bitcoin-core/libmultiprocess#247: type-map: Work around LLVM 22 "out of bounds index" error
8a5e3ae6ed2 Merge bitcoin-core/libmultiprocess#242: proxy-types: add CustomHasField hook to map Cap'n Proto values to null C++ values
e8d35246918 Merge bitcoin-core/libmultiprocess#246: doc: Bump version 8 > 9
97d877053b6 proxy-types: add CustomHasField hook for nullable decode paths
8c2f10252c9 refactor: add missing includes to mp/type-data.h
b1638aceb40 doc: Bump version 8 > 9
f61af487217 type-map: Work around LLVM 22 "out of bounds index" error

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 3f28bca5174f2a231b7fff6772411f8689f3d7e7
@Sjors Sjors mentioned this pull request Mar 12, 2026
Closed
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Mar 12, 2026
@ryanofsky
Squashed 'src/ipc/libmultiprocess/' changes from 1868a84451f..2fb97e8…
4522fc6 
…cca9

2fb97e8cca9 race fix: m_on_cancel called after request finishes
846a43aafb4 test: m_on_cancel called after request finishes
e69b6bf3f4e race fix: getParams() called after request cancel
75c5425173f test: getParams() called after request cancel
f09731e242f race fix: worker thread destroyed before it is initialized
88cacd4239f test: worker thread destroyed before it is initialized
22bec918c97 Merge bitcoin-core/libmultiprocess#247: type-map: Work around LLVM 22 "out of bounds index" error
8a5e3ae6ed2 Merge bitcoin-core/libmultiprocess#242: proxy-types: add CustomHasField hook to map Cap'n Proto values to null C++ values
e8d35246918 Merge bitcoin-core/libmultiprocess#246: doc: Bump version 8 > 9
97d877053b6 proxy-types: add CustomHasField hook for nullable decode paths
8c2f10252c9 refactor: add missing includes to mp/type-data.h
b1638aceb40 doc: Bump version 8 > 9
f61af487217 type-map: Work around LLVM 22 "out of bounds index" error

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 2fb97e8cca9feb1df70cf29b2a9895bea2c4c49c
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Mar 26, 2026
@ryanofsky
Squashed 'src/ipc/libmultiprocess/' changes from 1868a84451f..ff0eed1…
c7cdd9d 
…bf18

ff0eed1bf18 refactor: Use loop variable in type-context.h
ff1d8ba172a refactor: Move type-context.h getParams() call closer to use
1dbc59a4aa3 race fix: m_on_cancel called after request finishes
1643d05ba07 test: m_on_cancel called after request finishes
f5509a31fcc race fix: getParams() called after request cancel
4a60c39f24a test: getParams() called after request cancel
f11ec29ed20 race fix: worker thread destroyed before it is initialized
a1d643348f4 test: worker thread destroyed before it is initialized
e606fd84a8c Merge bitcoin-core/libmultiprocess#264: ci: reduce nproc multipliers
336023382c4 ci: reduce nproc multipliers
b090beb9651 Merge bitcoin-core/libmultiprocess#256: ci: cache gnu32 nix store
be8622816da ci: cache gnu32 nix store
975270b619c Merge bitcoin-core/libmultiprocess#263: ci: bump timeout factor to 40
09f10e5a598 ci: bump timeout factor to 40
db8f76ad290 Merge bitcoin-core/libmultiprocess#253: ci: run some Bitcoin Core CI jobs
55a9b557b19 ci: set Bitcoin Core CI test repetition
fb0fc84d556 ci: add TSan job with instrumented libc++
0f29c38725b ci: add Bitcoin Core IPC tests (ASan + macOS)
3f64320315d Merge bitcoin-core/libmultiprocess#262: ci: enable clang-tidy in macOS job, use nullptr
cd9f8bdc9f0 Merge bitcoin-core/libmultiprocess#258: log: add socket connected info message and demote destroy logs to debug
b5d6258a42f Merge bitcoin-core/libmultiprocess#255: fix: use unsigned char cast and sizeof in LogEscape escape sequence
d94688e2c32 Merge bitcoin-core/libmultiprocess#251: Improved CustomBuildField for std::optional in IPC/libmultiprocess
a9499fad755 mp: use nullptr with pthread_threadid_np
f499e37850f ci: enable clang-tidy in macOS job
98f1352159d log: add socket connected info message and demote destroy logs to debug
554a481ea73 fix: use unsigned char cast and sizeof in LogEscape escape sequence
1977b9f3f65 Use std::forward in CustomBuildField for std::optional to allow move semantics, resolves FIXME
22bec918c97 Merge bitcoin-core/libmultiprocess#247: type-map: Work around LLVM 22 "out of bounds index" error
8a5e3ae6ed2 Merge bitcoin-core/libmultiprocess#242: proxy-types: add CustomHasField hook to map Cap'n Proto values to null C++ values
e8d35246918 Merge bitcoin-core/libmultiprocess#246: doc: Bump version 8 > 9
97d877053b6 proxy-types: add CustomHasField hook for nullable decode paths
8c2f10252c9 refactor: add missing includes to mp/type-data.h
b1638aceb40 doc: Bump version 8 > 9
f61af487217 type-map: Work around LLVM 22 "out of bounds index" error

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: ff0eed1bf183627c89007e71d631f039bd61bfb5
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Mar 26, 2026
@ryanofsky
Squashed 'src/ipc/libmultiprocess/' changes from 1868a84451f..2fb97e8…
b4753d7 
…cca9

2fb97e8cca9 race fix: m_on_cancel called after request finishes
846a43aafb4 test: m_on_cancel called after request finishes
e69b6bf3f4e race fix: getParams() called after request cancel
75c5425173f test: getParams() called after request cancel
f09731e242f race fix: worker thread destroyed before it is initialized
88cacd4239f test: worker thread destroyed before it is initialized
22bec918c97 Merge bitcoin-core/libmultiprocess#247: type-map: Work around LLVM 22 "out of bounds index" error
8a5e3ae6ed2 Merge bitcoin-core/libmultiprocess#242: proxy-types: add CustomHasField hook to map Cap'n Proto values to null C++ values
e8d35246918 Merge bitcoin-core/libmultiprocess#246: doc: Bump version 8 > 9
97d877053b6 proxy-types: add CustomHasField hook for nullable decode paths
8c2f10252c9 refactor: add missing includes to mp/type-data.h
b1638aceb40 doc: Bump version 8 > 9
f61af487217 type-map: Work around LLVM 22 "out of bounds index" error

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 2fb97e8cca9feb1df70cf29b2a9895bea2c4c49c
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Mar 27, 2026
@ryanofsky
Squashed 'src/ipc/libmultiprocess/' changes from 1868a84451f..ff0eed1…
d45e185 
…bf18

ff0eed1bf18 refactor: Use loop variable in type-context.h
ff1d8ba172a refactor: Move type-context.h getParams() call closer to use
1dbc59a4aa3 race fix: m_on_cancel called after request finishes
1643d05ba07 test: m_on_cancel called after request finishes
f5509a31fcc race fix: getParams() called after request cancel
4a60c39f24a test: getParams() called after request cancel
f11ec29ed20 race fix: worker thread destroyed before it is initialized
a1d643348f4 test: worker thread destroyed before it is initialized
e606fd84a8c Merge bitcoin-core/libmultiprocess#264: ci: reduce nproc multipliers
336023382c4 ci: reduce nproc multipliers
b090beb9651 Merge bitcoin-core/libmultiprocess#256: ci: cache gnu32 nix store
be8622816da ci: cache gnu32 nix store
975270b619c Merge bitcoin-core/libmultiprocess#263: ci: bump timeout factor to 40
09f10e5a598 ci: bump timeout factor to 40
db8f76ad290 Merge bitcoin-core/libmultiprocess#253: ci: run some Bitcoin Core CI jobs
55a9b557b19 ci: set Bitcoin Core CI test repetition
fb0fc84d556 ci: add TSan job with instrumented libc++
0f29c38725b ci: add Bitcoin Core IPC tests (ASan + macOS)
3f64320315d Merge bitcoin-core/libmultiprocess#262: ci: enable clang-tidy in macOS job, use nullptr
cd9f8bdc9f0 Merge bitcoin-core/libmultiprocess#258: log: add socket connected info message and demote destroy logs to debug
b5d6258a42f Merge bitcoin-core/libmultiprocess#255: fix: use unsigned char cast and sizeof in LogEscape escape sequence
d94688e2c32 Merge bitcoin-core/libmultiprocess#251: Improved CustomBuildField for std::optional in IPC/libmultiprocess
a9499fad755 mp: use nullptr with pthread_threadid_np
f499e37850f ci: enable clang-tidy in macOS job
98f1352159d log: add socket connected info message and demote destroy logs to debug
554a481ea73 fix: use unsigned char cast and sizeof in LogEscape escape sequence
1977b9f3f65 Use std::forward in CustomBuildField for std::optional to allow move semantics, resolves FIXME
22bec918c97 Merge bitcoin-core/libmultiprocess#247: type-map: Work around LLVM 22 "out of bounds index" error
8a5e3ae6ed2 Merge bitcoin-core/libmultiprocess#242: proxy-types: add CustomHasField hook to map Cap'n Proto values to null C++ values
e8d35246918 Merge bitcoin-core/libmultiprocess#246: doc: Bump version 8 > 9
97d877053b6 proxy-types: add CustomHasField hook for nullable decode paths
8c2f10252c9 refactor: add missing includes to mp/type-data.h
b1638aceb40 doc: Bump version 8 > 9
f61af487217 type-map: Work around LLVM 22 "out of bounds index" error

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: ff0eed1bf183627c89007e71d631f039bd61bfb5
ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Mar 27, 2026
@ryanofsky
Squashed 'src/ipc/libmultiprocess/' changes from 1868a84451f..70f632b…
db2dc76 
…da8f

70f632bda8f Merge bitcoin-core/libmultiprocess#265: ci: set LC_ALL in shell scripts
8e8e564259a Merge bitcoin-core/libmultiprocess#249: fixes for race conditions on disconnects
05d34cc2ec3 ci: set LC_ALL in shell scripts
e606fd84a8c Merge bitcoin-core/libmultiprocess#264: ci: reduce nproc multipliers
ff0eed1bf18 refactor: Use loop variable in type-context.h
ff1d8ba172a refactor: Move type-context.h getParams() call closer to use
1dbc59a4aa3 race fix: m_on_cancel called after request finishes
1643d05ba07 test: m_on_cancel called after request finishes
f5509a31fcc race fix: getParams() called after request cancel
4a60c39f24a test: getParams() called after request cancel
f11ec29ed20 race fix: worker thread destroyed before it is initialized
a1d643348f4 test: worker thread destroyed before it is initialized
336023382c4 ci: reduce nproc multipliers
b090beb9651 Merge bitcoin-core/libmultiprocess#256: ci: cache gnu32 nix store
be8622816da ci: cache gnu32 nix store
975270b619c Merge bitcoin-core/libmultiprocess#263: ci: bump timeout factor to 40
09f10e5a598 ci: bump timeout factor to 40
db8f76ad290 Merge bitcoin-core/libmultiprocess#253: ci: run some Bitcoin Core CI jobs
55a9b557b19 ci: set Bitcoin Core CI test repetition
fb0fc84d556 ci: add TSan job with instrumented libc++
0f29c38725b ci: add Bitcoin Core IPC tests (ASan + macOS)
3f64320315d Merge bitcoin-core/libmultiprocess#262: ci: enable clang-tidy in macOS job, use nullptr
cd9f8bdc9f0 Merge bitcoin-core/libmultiprocess#258: log: add socket connected info message and demote destroy logs to debug
b5d6258a42f Merge bitcoin-core/libmultiprocess#255: fix: use unsigned char cast and sizeof in LogEscape escape sequence
d94688e2c32 Merge bitcoin-core/libmultiprocess#251: Improved CustomBuildField for std::optional in IPC/libmultiprocess
a9499fad755 mp: use nullptr with pthread_threadid_np
f499e37850f ci: enable clang-tidy in macOS job
98f1352159d log: add socket connected info message and demote destroy logs to debug
554a481ea73 fix: use unsigned char cast and sizeof in LogEscape escape sequence
1977b9f3f65 Use std::forward in CustomBuildField for std::optional to allow move semantics, resolves FIXME
22bec918c97 Merge bitcoin-core/libmultiprocess#247: type-map: Work around LLVM 22 "out of bounds index" error
8a5e3ae6ed2 Merge bitcoin-core/libmultiprocess#242: proxy-types: add CustomHasField hook to map Cap'n Proto values to null C++ values
e8d35246918 Merge bitcoin-core/libmultiprocess#246: doc: Bump version 8 > 9
97d877053b6 proxy-types: add CustomHasField hook for nullable decode paths
8c2f10252c9 refactor: add missing includes to mp/type-data.h
b1638aceb40 doc: Bump version 8 > 9
f61af487217 type-map: Work around LLVM 22 "out of bounds index" error

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 70f632bda8f80449b6240f98da768206a535a04e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ryanofsky ryanofsky ryanofsky approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Sjors @DrahtBot @ryanofsky