ipc: Serialize null CTransactionRef as empty Data#242
ipc: Serialize null CTransactionRef as empty Data#242Sjors wants to merge 2 commits intobitcoin-core:masterfrom
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
|
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
force-pushed
the
2026/02/serialize-null
branch
from
fb2622f to
e8bcca3
Compare
|
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
force-pushed
the
2026/02/serialize-null
branch
from
e8bcca3 to
423a789
Compare
|
Added missing |
Reference and rationale: bitcoin/bitcoin#34020 (comment) Empty `Data` can be a valid representation for some types, so this behavior should not be generalized. In `List(Data)` contexts, Cap'n Proto's current C++ API does not let us distinguish null vs empty `Data`. Given those constraints, this commit is narrowly scoped: interpret empty `Data` as null only for `CTransactionRef`-like pointer paths where empty serialization is known to be safe. The implementation avoids making generic pointer-type assumptions and introduces a targeted has-field customization path. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Sjors
force-pushed
the
2026/02/serialize-null
branch
from
423a789 to
413f915
Compare
2 participants
Taken from: bitcoin/bitcoin#34020 (comment)
Empty
Datacan be a valid representation for some types, so this behavior should not be generalized.In
List(Data)contexts, Cap'n Proto's current C++ API does not let us distinguish null vs emptyData.Given those constraints, this commit is narrowly scoped: interpret empty
Dataas null only forCTransactionRef-like pointer paths where empty serialization is known to be safe. The implementation avoids making generic pointer-type assumptions and introduces a targeted has-field customization path.