`net_plugin` cleanup by heifner · Pull Request #205 · Wire-Network/wire-sysio

heifner · 2026-02-17T21:05:17Z

Summary

Replace generic notice_message / request_message (which abused select_ids<T> template with overloaded field semantics) with three self-documenting protocol types: peer_status_notice, block_request_message,
block_nack_request_message
Remove id_list_modes enum, modes_str(), select_ids<T> template, make_block_id() hack, and all mode-switch validation logic in handlers
Remove unused chain_size_message
Remove old proto versions that are no longer needed

Motivation

The old notice_message and request_message types used a confusing generic select_ids<T> template where field semantics were overloaded based on context:

Old field	What it actually carried
`known_trx.pending`	`fork_db_root_num`
`known_trx.mode`	`none` or `last_irr_catch_up` (never actually about transactions)
`known_blocks.pending`	`fork_db_head_num`
`known_blocks.ids[0]`	`fork_db_head_id`
`known_blocks.ids[1]`	`make_block_id(earliest_available_block_num)` (a fake block ID encoding a block number)
`req_trx`	Always dead — mode set to `none`, ids always empty
`req_blocks.mode`	`catch_up` for head catchup, `normal` for nack recovery, `none` for stop signal

Wire is a fresh chain with no legacy peers to support — now is the time to make the protocol self-documenting.

New types

struct peer_status_notice {
   bool          lib_sync{false};        // true = peer is far behind (was last_irr_catch_up mode)
   block_id_type fork_db_root_id;        // was encoded in known_trx.pending as a block number
   block_id_type fork_db_head_id;        // was known_blocks.ids[0]
   uint32_t      earliest_available_block_num{0}; // was fake block_id in known_blocks.ids[1]
};

struct block_request_message {
   block_id_type my_head_id;             // was req_blocks.ids[0] with mode=catch_up
};

struct block_nack_request_message {
   block_id_type target_id;              // was req_blocks.ids[0] with mode=normal
   block_id_type my_head_id;             // was req_blocks.ids[1]
};

- chain_size_message not used. - Two enum values not used. - No need for savanna block transition handling - No need to support pre-Leap 5.0 clients epoch time in microseconds - Simplify notice_message handling

…a raw number

…ypes The generic select_ids<T> template used by notice_message and request_message overloaded field semantics based on context: known_trx.pending secretly carried fork_db_root_num, req_trx was always dead, transaction IDs were never populated, and id_list_modes meant different things in notices vs requests. Replace with three self-documenting types: - peer_status_notice: carries fork_db_root_id, fork_db_head_id, earliest_available_block_num, and lib_sync flag - block_request_message: carries my_head_id for catchup requests - block_nack_request_message: carries target_id and my_head_id for gap recovery after block_notice_message This also removes the make_block_id() hack that encoded block numbers into fake block_id_type values, the id_list_modes enum, modes_str(), select_ids<T>, and all the mode-switch validation logic in the handlers.

Copilot

Pull request overview

This PR modernizes the net_plugin protocol by removing legacy abstractions and replacing them with self-documenting message types. The changes are specifically designed for Wire, a fresh blockchain with no legacy peer compatibility requirements.

Changes:

Replaced generic notice_message and request_message with three specific types: peer_status_notice, block_request_message, and block_nack_request_message
Simplified protocol versioning to a single base version (version 1) removing all legacy version support
Removed unused protocol types and backward compatibility code (chain_size_message, id_list_modes, select_ids, generation checks, epoch normalization)

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
plugins/net_plugin/include/sysio/net_plugin/protocol.hpp	Defined new self-documenting message types, removed legacy types and enums, updated net_message variant and msg_type_t enum
plugins/net_plugin/src/net_plugin.cpp	Updated message handlers to use new types, simplified protocol version handling, removed legacy compatibility code (generation checks, epoch normalization, proper_svnn_block_seen)
tests/trx_generator/trx_provider.cpp	Updated to use `to_index()` function instead of hardcoded message index, added protocol.hpp include
tests/trx_generator/CMakeLists.txt	Added net_plugin include directory to support protocol.hpp usage

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-18T22:54:28Z

plugins/net_plugin/include/sysio/net_plugin/protocol.hpp

-   };
-
   struct handshake_message {
      uint16_t                   network_version = 0; ///< incremental value above a computed base


The comment "incremental value above a computed base" is outdated. With the new protocol version scheme (proto_version_t::base = 1), the network_version field now directly contains the protocol version value without any base offset calculation.

Suggested change

uint16_t network_version = 0; ///< incremental value above a computed base

uint16_t network_version = 0; ///< protocol version value (no base offset)

brianjohnson5972

Just a couple of cleanup items, but approved otherwise

brianjohnson5972 · 2026-02-19T20:18:48Z

plugins/net_plugin/src/net_plugin.cpp

+         start_sync(c, root_num);
+      } else {
+         peer_dlog( p2p_blk_log, c, "sync_manager got catch_up peer_status_notice" );
+         peer_ilog( p2p_blk_log, c, "peer_status_notice, head_num {}, id {}...",


I think head_num is only used here, can move it down here and avoid the call in the other conditional. Of could just call it in the peer_ilog and only call it if logging.

brianjohnson5972 · 2026-02-19T20:20:53Z

plugins/net_plugin/src/net_plugin.cpp

+      uint32_t head_num = block_header::num_from_id(msg.fork_db_head_id);
+      uint32_t root_num = block_header::num_from_id(msg.fork_db_root_id);
+      if (msg.lib_sync) {
+         peer_dlog( p2p_blk_log, c, "sync_manager got lib_sync peer_status_notice" );


root_num only used in the lib_sync path, can avoid calling for other path.

brianjohnson5972 · 2026-02-20T19:18:06Z

plugins/net_plugin/src/net_plugin.cpp

-         req.req_blocks.ids.emplace_back( chain_info.fork_db_head_id );
+         block_request_message req;
+         req.my_head_id = chain_info.fork_db_head_id;
+         c->enqueue( req );


Moving the enqueue here and/or changing the message type means that peer_syncing_from_us is no longer being set false and I was wondering if that was a problem. It looks like it is not, because it doesn't seem like peer_syncing_from_us is ever read, just written. If I'm right then remove it in other locations.

peer_syncing_from_us is used in many places including:

bool connection::current() const { return (connected() && !peer_syncing_from_us); }

So yes, this is a real issue. Very good! Thanks! I'll push up a fix. Instead of adding a new message type or hacking it into the new message types, I can just send a handshake; as that causes the peer to re-evaluates sync state and clears peer_syncing_from_us.

…mote peer_syncing_from_us. When verify_catchup determines we already have the peer's announced block, the old request_message with mode=none signaled the remote peer to clear peer_syncing_from_us. The protocol simplification removed that signal path, leaving the remote peer stuck with peer_syncing_from_us=true until the next handshake exchange. This caused current() to return false, temporarily excluding the peer from block broadcasts. Send a handshake instead so the remote peer re-evaluates sync state through recv_handshake, which clears peer_syncing_from_us on the appropriate path.

heifner added 4 commits February 17, 2026 13:31

Remove the unneeded proto versions since launching a new chain.

c699a92

Remove dead code.

9c65492

- chain_size_message not used. - Two enum values not used. - No need for savanna block transition handling - No need to support pre-Leap 5.0 clients epoch time in microseconds - Simplify notice_message handling

Include net_plugin protocol so transaction_message which is not just …

83159fc

…a raw number

heifner marked this pull request as ready for review February 18, 2026 13:06

brianjohnson5972 requested a review from Copilot February 18, 2026 22:49

Copilot started reviewing on behalf of brianjohnson5972 February 18, 2026 22:50 View session

Copilot AI reviewed Feb 18, 2026

View reviewed changes

heifner added 2 commits February 19, 2026 08:10

Merge branch 'feature/net-plugin-perf' into feature/net-plugin-cleanup

c0cf3cd

Fix comment

4aa80c8

brianjohnson5972 approved these changes Feb 20, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

`net_plugin` cleanup#205

`net_plugin` cleanup#205
heifner wants to merge 7 commits intofeature/net-plugin-perffrom
feature/net-plugin-cleanup

heifner commented Feb 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 18, 2026

Uh oh!

heifner Feb 19, 2026

Uh oh!

brianjohnson5972 left a comment

Uh oh!

brianjohnson5972 Feb 19, 2026

Uh oh!

brianjohnson5972 Feb 19, 2026

Uh oh!

brianjohnson5972 Feb 20, 2026

Uh oh!

heifner Feb 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	uint16_t network_version = 0; ///< incremental value above a computed base
	uint16_t network_version = 0; ///< protocol version value (no base offset)

Comments

Conversation

heifner commented Feb 17, 2026

Summary

Motivation

New types

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

heifner Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

brianjohnson5972 left a comment

Choose a reason for hiding this comment

Uh oh!

brianjohnson5972 Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

brianjohnson5972 Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

brianjohnson5972 Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

heifner Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants