p2p: support new msg broadcast features; by galaio · Pull Request #3043 · bnb-chain/bsc

galaio · 2025-04-22T14:12:12Z

Description

This PR aims to implement the msg broadcast mechanism of bnb-chain/BEPs#563.

For Message Propagation

Some connection features will be enabled between validators to optimize network congestion and reduce message latency.

Transaction: it will not be propagated between validators.
Block: it will directly broadcast to all other connected validators in this network.
Vote: same as block.

It will make a more stable network between validators. It also allows adding more normal nodes in DirectBroadcastList.

Example

You should set the config as below:

[Node]
EnableBroadcastFeature = true

[Node.P2P]
DirectBroadcastList =["nodeid1", "nodeid2"]

Changes

Notable changes:

p2p: support new msg broadcast features;
...

eth/handler.go

buddh0 · 2025-04-23T08:17:18Z

need a better var name than EnableBroadcastFeature,
‘EnableEnhancedBroadcast’ ？

Copilot

Pull Request Overview

This PR implements new peer-to-peer broadcast features and related configuration updates to support a more efficient message propagation mechanism for block and vote messages between validators. The key changes include adding new configuration fields for direct broadcast and proxied validator lists, updating peer and handler logic in both the P2P and ETH subsystems to handle these new features, and extending system contract upgrade mechanisms with the maxwell upgrade details.

Reviewed Changes

Copilot reviewed 19 out of 22 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
p2p/enode/node.go	Added parsing for V4 public key to support new node ID formation.
p2p/enode/idscheme.go	Introduced V4NodeIDFromPublicKey to generate node IDs from public keys.
p2p/config_toml.go, p2p/config.go	Extended configuration with DirectBroadcastList and ProxyedValidatorList.
node/config.go	Added EnableBroadcastFeatures flag to node config.
eth/protocols/eth/peer.go	Exposed NodeID method on Peer for use in broadcast logic.
eth/peerset*.go, eth/peerset_test.go	Updated peer set logic along with comprehensive tests for new features.
eth/handler.go	Integrated broadcast feature flags and peer selection adjustments.
eth/ethconfig/* and eth/backend.go	Updated configuration marshalling and shutdown handling to reflect new P2P fields.
consensus/parlia/*	Extended StakeHub and ABI definitions to accommodate new validator node ID support.
core/systemcontracts/upgrade.go	Added maxwell upgrade entries as part of the system contract upgrade suite.

Files not reviewed (3)

core/systemcontracts/maxwell/chapel/StakeHubContract: Language not supported
core/systemcontracts/maxwell/mainnet/StakeHubContract: Language not supported
core/systemcontracts/maxwell/rialto/StakeHubContract: Language not supported

Comments suppressed due to low confidence (1)

p2p/config_toml.go:34

[nitpick] Consider renaming 'ProxyedValidatorList' to 'ProxiedValidatorList' for clarity and consistency with standard spelling.

ProxyedValidatorList []enode.ID       `toml:",omitempty"`

eth/handler.go

eth/peerset.go

MatusKysel

except inefficiencies with slices it's all good

chore: renaming some vars & fields;

Copilot

Pull Request Overview

This PR implements new message broadcast features for the p2p network including enhanced direct broadcasting and specialized handling for transaction propagation for validators. Key changes include:

New atomic flags in the Peer struct for full, direct, and no-tx broadcast modes.
Updates to enode, config, and handler modules to support broadcast feature configuration.
Modifications to node configuration and chain command logic to integrate the new broadcast mechanisms.

Reviewed Changes

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

Show a summary per file

File	Description
p2p/peer.go	Adds new atomic flags for various broadcast modes
p2p/enode/idscheme.go	Introduces V4NodeIDFromPublicKey for V4 public key processing
p2p/config_toml.go	Adds new configuration fields for broadcast features
p2p/config.go	Updates configuration struct with broadcast node IDs
node/config.go	Adds new flag EnableBroadcastFeatures with documentation
eth/handler.go	Integrates broadcast features in block propagation logic
eth/peerset.go	Updates peer set enabling broadcast features
cmd/geth/chaincmd.go	Adjusts node configuration creation to support sentry nodes

Comments suppressed due to low confidence (1)

p2p/config_toml.go:34

[nitpick] Consider renaming 'ProxyedValidatorNodeIDs' to 'ProxiedValidatorNodeIDs' for improved clarity and consistency with standard English usage.

ProxyedValidatorNodeIDs []enode.ID       `toml:",omitempty"`

Copilot · 2025-04-29T09:10:05Z

node/config.go

+	// EnableBroadcastFeatures enable directly broadcast feature + no tx broadcast feature,
+	// it mainly used for validators, the validators can be recognized by StakHub contract
+	// Notice: validator, sentryNode need to set this flag to true.


[nitpick] Consider revising the comment for clarity and correct grammar, e.g., 'Used mainly for validators, which can be recognized by the StakHub contract. Note: Validator and sentry nodes need to set this flag to true.'

Suggested change

// EnableBroadcastFeatures enable directly broadcast feature + no tx broadcast feature,

// it mainly used for validators, the validators can be recognized by StakHub contract

// Notice: validator, sentryNode need to set this flag to true.

// EnableBroadcastFeatures enables the direct broadcast feature and disables the transaction broadcast feature.

// Used mainly for validators, which can be recognized by the StakHub contract.

// Note: Validator and sentry nodes need to set this flag to true.

zzzckck · 2025-04-29T12:03:26Z

eth/handler.go

+	EnableBroadcastFeatures  bool
+	DirectBroadcastNodeIDs   []enode.ID
+	ProxyedValidatorNodeIDs  []enode.ID
 }


how about rename:

ProxyedValidatorNodeIDs => ValidatorNodeIDs DirectBroadcastNodeIDs => EVNNodeIdsWhitelist // (EVN = Enhanced Validator Network)

zzzckck · 2025-04-29T23:18:17Z

eth/peerset.go

+	}
+	for _, peer := range ps.peers {
+		nodeID := peer.NodeID()
+		if slices.Contains(directNodeIDs, nodeID) || slices.Contains(proxyedNodeIDs, nodeID) {


direct broadcast only for proxyedNodeIDs?
directNodeIDs is for EVNWhilteList, right?

It's necessary to EnableDirectBroadcast for proxyedNodeIDs IMO

buddh0 · 2025-04-30T06:42:19Z

eth/backend.go

 		EventMux:                 eth.eventMux,
 		RequiredBlocks:           config.RequiredBlocks,
 		DirectBroadcast:          config.DirectBroadcast,
+		EnableBroadcastFeatures:  stack.Config().EnableBroadcastFeatures,


how about rename
EnableBroadcastFeatures --> EnableEVNBroadcastFeatures?

buddh0 · 2025-04-30T07:23:18Z

eth/handler.go

+	if !ok {
+		return false
+	}
+	if parlia.ConsensusAddress() == block.Coinbase() {


check mining, maybe It's a backup validator.

It's ok, the backup validator won't produce block. Here only block producer need full broadcast to all other validators.

zzzckck · 2025-04-30T12:05:53Z

eth/handler.go

 			active++
 		case <-h.handlerDoneCh:
 			active--
+		case <-updateTicker.C:


why use a periodical timer here?

zzzckck · 2025-04-30T12:29:19Z

eth/handler.go

+		broadcastChecker := func(peer *ethPeer) bool {
+			return peer.EnableDirectBroadcast.Load()
+		}
+		if h.needMoreDirectBroadcastPeers(block) {
+			log.Debug("needMoreDirectBroadcastPeers", "number", block.NumberU64(), "hash", block.Hash())
+			broadcastChecker = func(peer *ethPeer) bool {
+				return peer.EnableDirectBroadcast.Load() || peer.EnableFullBroadcast.Load()
+			}
+		}
+
+		var morePeers []*ethPeer
+		for i := len(transfer); i < len(peers); i++ {
+			if broadcastChecker(peers[i]) {
+				log.Debug("add extra direct broadcast peer", "peer", peers[i].ID())
+				morePeers = append(morePeers, peers[i])
+			}
+		}
+		for _, peer := range morePeers {
+			peer.AsyncSendNewBlock(block, td)
+		}


better to refactor this code to make it more readable.

zzzckck · 2025-04-30T13:14:22Z

eth/peerset.go

+	}
+	for _, peer := range ps.peers {
+		nodeID := peer.NodeID()
+		if slices.Contains(directNodeIDs, nodeID) || slices.Contains(proxyedNodeIDs, nodeID) {


Suggested change

if slices.Contains(directNodeIDs, nodeID) || slices.Contains(proxyedNodeIDs, nodeID) {

if slices.Contains(proxyedNodeIDs, nodeID) {

zzzckck · 2025-04-30T13:14:58Z

eth/peerset.go

+			peer.EnableDirectBroadcast.Store(true)
+		}
+		_, isValNodeID := valNodeIDMap[nodeID]
+		if isValNodeID {


Suggested change

if isValNodeID {

if isValNodeID || slices.Contains(directNodeIDs, nodeID) {

zzzckck · 2025-04-30T13:16:25Z

eth/peerset.go

+		}
+		// if the peer is in the valNodeIDs and not in the proxyedList, enable the no tx broadcast feature
+		// the node also need to forward tx to the proxyedList
+		if isValNodeID && !slices.Contains(proxyedNodeIDs, nodeID) {


1.Add a warning if validator configure it self as EVN node.
2.In the future: to check itself whether belong to EnhancedValidatorNetwork or not.

zzzckck · 2025-04-30T13:18:01Z

eth/peerset.go

+			peer.EnableFullBroadcast.Store(true)
+		}
+		// if the peer is in the valNodeIDs and not in the proxyedList, enable the no tx broadcast feature
+		// the node also need to forward tx to the proxyedList


Suggested change

// the node also need to forward tx to the proxyedList

// the node also need to forward tx to the proxyedList

if slices.Contains(proxyedNodeIDs, nodeID) {

log.Warn("validator Nodeid is registerred", "id", nodeID)

}

galaio requested review from MatusKysel, buddh0 and zzzckck April 22, 2025 14:12

galaio force-pushed the new-broadcast branch from 0ebfcaf to 1d76d41 Compare April 23, 2025 07:40

buddh0 reviewed Apr 23, 2025

View reviewed changes

eth/handler.go Outdated Show resolved Hide resolved

galaio force-pushed the new-broadcast branch 6 times, most recently from 29675a6 to 683cc58 Compare April 28, 2025 07:15

MatusKysel requested a review from Copilot April 28, 2025 09:08

Copilot AI reviewed Apr 28, 2025

View reviewed changes

MatusKysel reviewed Apr 28, 2025

View reviewed changes

eth/handler.go Outdated Show resolved Hide resolved

MatusKysel reviewed Apr 28, 2025

View reviewed changes

eth/peerset.go Outdated Show resolved Hide resolved

MatusKysel reviewed Apr 28, 2025

View reviewed changes

galaio force-pushed the new-broadcast branch from 683cc58 to e4aa951 Compare April 28, 2025 15:57

MatusKysel previously approved these changes Apr 29, 2025

View reviewed changes

galaio dismissed MatusKysel’s stale review via 78b356e April 29, 2025 07:05

galaio force-pushed the new-broadcast branch from 78b356e to 0aa75a6 Compare April 29, 2025 08:47

galaio added 9 commits April 29, 2025 17:03

p2p: support new msg broadcast features;

70fe530

p2p: opt broadcast mechanism in validator network;

9e95268

p2p: support validator NodeIDs;

b178c19

p2p: support validator NodeIDs;

9c8cdcc

p2p: support validator NodeIDs;

a5d3578

tool: init network support sentry node;

17e212b

p2p: opt proxyed & direct broadcast mechanism;

045ec36

chore: renaming some vars & fields;

chore: fix lint issues;

b0fe9e5

chore: opt validator nodeID filter;

c4a9cb7

galaio force-pushed the new-broadcast branch from 0aa75a6 to c4a9cb7 Compare April 29, 2025 09:04

MatusKysel requested a review from Copilot April 29, 2025 09:09

Copilot AI reviewed Apr 29, 2025

View reviewed changes

MatusKysel approved these changes Apr 29, 2025

View reviewed changes

zzzckck reviewed Apr 29, 2025

View reviewed changes

buddh0 reviewed Apr 30, 2025

View reviewed changes

buddh0 approved these changes Apr 30, 2025

View reviewed changes

zzzckck reviewed Apr 30, 2025

View reviewed changes

eth/handler.go

active++

case <-h.handlerDoneCh:

active--

case <-updateTicker.C:

Copy link

Collaborator

zzzckck Apr 30, 2025

Choose a reason for hiding this comment

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

why use a periodical timer here?

zzzckck reviewed Apr 30, 2025

View reviewed changes

zzzckck merged commit 21f6888 into bnb-chain:develop Apr 30, 2025
7 checks passed

galaio mentioned this pull request May 6, 2025

chore: renaming for evn and some optmization; #3070

Merged

This was referenced May 16, 2025

release: prepare for release v1.5.13 #3095

Merged

Draft release v1.5.13 #3094

Merged

galaio added a commit to galaio/bsc that referenced this pull request May 29, 2025

p2p: support new msg broadcast features; (bnb-chain#3043)

3f72268

galaio added a commit to galaio/bsc that referenced this pull request May 29, 2025

p2p: support new msg broadcast features; (bnb-chain#3043)

0887260

	if slices.Contains(directNodeIDs, nodeID) \|\| slices.Contains(proxyedNodeIDs, nodeID) {
	if slices.Contains(proxyedNodeIDs, nodeID) {

	if isValNodeID {
	if isValNodeID \|\| slices.Contains(directNodeIDs, nodeID) {

Comments

Conversation

galaio commented Apr 22, 2025

Description

For Message Propagation

Example

Changes

Uh oh!

Uh oh!

buddh0 commented Apr 23, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

MatusKysel left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zzzckck Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

buddh0 Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zzzckck Apr 29, 2025 •

edited

Loading

buddh0 Apr 30, 2025 •

edited

Loading