net: deduplicate private broadcast state and snapshot types

MatthewAura444 commented at 6:52 pm on April 6, 2026: none

Follow-up cleanup in the private broadcast subsystem.

This change removes duplication between the internal queue state (SendStatus, TxSendStatus) and the snapshot types exported to PeerManager / RPC / tests (PeerSendInfo, TxBroadcastInfo).

Remove PeerSendInfo, reuse SendStatus directly in TxBroadcastInfo
Simplify TxBroadcastInfo to {tx, TxSendStatus status}
Simplify GetBroadcastInfo() from field-by-field copy to direct snapshot
JSON output of getprivatebroadcastinfo is unchanged

No behavioral changes intended.

DrahtBot added the label P2P on Apr 6, 2026

DrahtBot commented at 6:53 pm on April 6, 2026: contributor

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

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34707 (net: keep finished private broadcast txs in memory by andrewtoth)

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.

andrewtoth commented at 7:13 pm on April 6, 2026: contributor

Looks like this was pulled out of the conversation here #34707 (review).

cc @vasild

fanquake commented at 2:32 pm on April 7, 2026: member

https://github.com/bitcoin/bitcoin/actions/runs/24045821228/job/70255455947?pr=35016#step:9:1647:

0[ 21%] Building CXX object src/CMakeFiles/bitcoin_node.dir/rpc/mempool.cpp.o
1cd /home/admin/actions-runner/_work/bitcoin/bitcoin/ci_build/src && /usr/bin/ccache /usr/bin/clang++ -DABORT_ON_FAILED_ASSUME -DBOOST_MULTI_INDEX_DISABLE_SERIALIZATION -DBOOST_NO_CXX98_FUNCTION_BASE -DDEBUG -DDEBUG_LOCKCONTENTION -DDEBUG_LOCKORDER -DENABLE_EMBEDDED_ASMAP=1 -DENABLE_ZMQ=1 -DRPC_DOC_CHECK -I/home/admin/actions-runner/_work/bitcoin/bitcoin/ci_build/src -I/home/admin/actions-runner/_work/bitcoin/bitcoin/src -I/home/admin/actions-runner/_work/bitcoin/bitcoin/src/leveldb/include -I/home/admin/actions-runner/_work/bitcoin/bitcoin/src/minisketch/include -I/home/admin/actions-runner/_work/bitcoin/bitcoin/src/univalue/include -Wno-error=unused-member-function -O0 -ftrapv -g3 -std=c++20 -fPIC -fmacro-prefix-map=/home/admin/actions-runner/_work/bitcoin/bitcoin/src=. -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection -Wall -Wextra -Wgnu -Wcovered-switch-default -Wformat -Wformat-security -Wvla -Wshadow-field -Wthread-safety -Wloop-analysis -Wredundant-decls -Wunused-member-function -Wdate-time -Wconditional-uninitialized -Woverloaded-virtual -Wsuggest-override -Wimplicit-fallthrough -Wunreachable-code -Wdocumentation -Wself-assign -Wundef -Wno-unused-parameter -Werror -MD -MT src/CMakeFiles/bitcoin_node.dir/rpc/mempool.cpp.o -MF CMakeFiles/bitcoin_node.dir/rpc/mempool.cpp.o.d -o CMakeFiles/bitcoin_node.dir/rpc/mempool.cpp.o -c /home/admin/actions-runner/_work/bitcoin/bitcoin/src/rpc/mempool.cpp  -O3 -g2
2/home/admin/actions-runner/_work/bitcoin/bitcoin/src/rpc/mempool.cpp:187:86: error: no member named 'time_added' in 'PrivateBroadcast::TxBroadcastInfo'
3  187 |                 o.pushKV("time_added", TicksSinceEpoch<std::chrono::seconds>(tx_info.time_added));
4      |                                                                              ~~~~~~~ ^
5/home/admin/actions-runner/_work/bitcoin/bitcoin/src/rpc/mempool.cpp:189:49: error: no member named 'peers' in 'PrivateBroadcast::TxBroadcastInfo'
6  189 |                 for (const auto& peer : tx_info.peers) {
7      |                                         ~~~~~~~ ^
82 errors generated.

DrahtBot added the label CI failed on Apr 7, 2026

DrahtBot commented at 2:34 pm on April 7, 2026: contributor

🚧 At least one of the CI tasks failed. Task test ancestor commits: https://github.com/bitcoin/bitcoin/actions/runs/24045821228/job/70255455947 LLM reason (✨ experimental): CI failed due to a build error while compiling rpc/mempool.cpp.o for bitcoin_node (CMake/gmake returned non-zero).

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

net: deduplicate private broadcast snapshot types

Remove the PeerSendInfo struct and simplify TxBroadcastInfo to reuse the canonical internal state types (SendStatus, TxSendStatus) instead of maintaining parallel snapshot-only types with duplicated fields.

GetBroadcastInfo() is simplified from a field-by-field copy loop to a direct state snapshot.

No behavioral changes. No field renames.

b894bb95ef

test: update private broadcast tests for deduplicated types

Update snapshot access patterns to use TxBroadcastInfo.status.send_statuses instead of the removed .peers field. Add a snapshot_structural_invariant test that verifies time_added preservation, send status count, and confirmed state round-trip through the snapshot.

887fa7e51f

MatthewAura444 force-pushed on Apr 7, 2026

MatthewAura444 commented at 6:01 pm on April 7, 2026: none

Fixed. The CI failure was due to a non-atomic commit split — the first commit changed the TxBroadcastInfo layout while rpc/mempool.cpp was only updated in the second commit. Squashed them into one so each commit builds cleanly.

I also noticed this PR overlaps with the dedup suggested by @vasild in #34707 (comment). Happy to rebase on top of #34707 or close this if it becomes redundant after that PR lands.

net: deduplicate private broadcast state and snapshot types #35016

Reviews

Conflicts