P2P: Add gossip of BP peers by heifner · Pull Request #1265 · AntelopeIO/spring

heifner · 2025-03-21T19:09:48Z

Adds new gossip_bp_peers_message message to P2P protocol for gossip of available "BP" peers. Normally these peers would be canary relay nodes to the block producer and block finalizer nodeos.

Gossip connection information is only shared with nodes that are connected to another "BP" node that has a peer key registered on-chain.
Block producers should register a new peer key on chain. See Add support for storing public keys on-chain that will be used to validate a network peer's identity. VaultaFoundation/system-contracts#185
Block producers then run the canary relay node(s) with the private key of the peer key and their block producer account.
For example, block producer defproducera.
- Register a public peer key on change via regpeerkey with values: defproducera EOS6y94JK7Dsg3iJCrnvdbyLPZJvaAhfBSU44eU8jkPd4VHX9CJzp
- Add the following to their canary relay node.

       p2p-producer-peer = defproducera
       signature-provider = EOS6y94JK7Dsg3iJCrnvdbyLPZJvaAhfBSU44eU8jkPd4VHX9CJzp=KEY:5J6Z64Jqexample
       p2p-auto-bp-peer = p2p.peer.com:9876

The p2p-producer-peer matches the producer-name of the BP. The key provided to signature-provider is the public/private key of the on-chain key specified with regpeerkey. The p2p-auto-bp-peer is a known BP peer. This is not technically needed if another BP peer has your endpoint listed in their p2p-auto-bp-peer list.

p2p-auto-bp-peer can be specified for as many known bp peers. These will be used along with gossip peers as producers rotate into the producer schedule. They are not gossiped out to other peers. Only this nodes connection info along with other received gossip BP info is gossiped to other peers.

Resolves #1245

…_host_port_remainder()

…er than anything in net_plugin. Empty p2p-server-address should not be allowed.

…peers

…chedule they can be connected to.

…peers Update to latest system contract changes.

…ng on receive of a block before read-only trx could be executed.

spoonincode · 2025-04-16T23:32:36Z


-   void update_peer_keys(fc::time_point deadline) {
+   void update_peer_keys() {
+      // if syncing or replaying old blocks don't bother updating peer keys


good call I didn't think of this when reviewing other PR

spoonincode · 2025-04-18T22:14:35Z

I don't think it was changed in this PR but looking through the code I see it creates and verifies canonical signatures. Maybe consider turning that off (passing false) now from the start of this feature

spoonincode · 2025-04-18T22:21:09Z

+         try {
+            std::optional<peer_info_t> peer_info = cc.get_peer_info(peer.producer_name);
+            if (peer_info && peer_info->key) {
+               public_key_type pk(peer.sig, peer.digest());


I think we want to immediately kick out SIG_WA. We did make the contract disallow PUB_WA.

…_and_connect.

greg7mdp · 2025-04-22T18:44:31Z

+
+      fc::lock_guard g(gossip_bps.mtx);
+      auto& sig_idx = gossip_bps.index.get<by_sig>();
+      for (auto i = msg.peers.begin(); i != msg.peers.end() && !fatal_error;) {


I don't think we need to exit the loop early if fatal_error is true (it is after all an optimization for a very unlikely case).

And if we don't, we can use std::erase instead of that for loop which makes the code clearer I feel.

But up to you what you prefer!

nit: Maybe invalid_message would be more descriptive than fatal_error.

I think it is important to exit early on a bad public_key. Seems like those could be crafted to be expensive to evaluate. As soon as one is determined to be invalid, don't bother trying to verify signature on any others.

Seems like those could be crafted to be expensive to evaluate.

I don't think it is a very likely attack vector from one of the top-50 producers, but OK.

If the message is not from one of the top-50 we know about then we will not consider that a valid message and will disconnect.

So this means that the attack with invalid signatures is not very likely, right. Not advocating for a change, but just that these messages are among a trusted group (all trusted to see the IP of all block producers, so if there was a rogue element it could probably do more damage by ddos'ing producers as they are scheduled to produce than by forging public keys that take time to validate).

Well anyone could send a crafted gossip_bp_peers_message we have to validate to know it is from a valid producer.

This conversation did make me realize that it was a bit too strict. Just because a peer is no longer in the top producers does not mean we should fatal disconnect from them.

Well anyone could send a crafted gossip_bp_peers_message we have to validate to know it is from a valid producer.

Hum, I wonder if the whole message (the std::vector<bp_peer> + sender's name) should not be signed by the peer sending it. Then the first thing we could do is validate that the signature matches the public key of one of the IPs registered for that producer, so we authenticate that it comes from one of the current top-50 producers.

We could certainly do that. Not sure it is worth the extra processing. Open to what others think.

…sip connection

heifner added 20 commits March 12, 2025 15:22

GH-1245 Add gossip_bp_peers_message

7f66aad

GH-1245 Move max_p2p_address_length to net_utils and enforce in split…

4a48bba

…_host_port_remainder()

GH-1245 Move net_logger_impl to net_logger.hpp

73ba970

GH-1245 Move buffer_factory into buffer_factor.hpp

d51c6ed

GH-1245 Add tracking and validation of gossip_bp_peers_message.

414c22d

GH-1245 Refactor some functionality into auto_bp_peering.hpp

1f97d9d

GH-1245 The gethostname() seems to be testing this python script rath…

b337c1a

…er than anything in net_plugin. Empty p2p-server-address should not be allowed.

GH-1245 Support both manually configured bp peers and gossip bp peers.

b4abdc7

GH-1245 Remove extra loop

a18b4fc

Merge remote-tracking branch 'spring/gh_1183' into GH-1245-gossip-bp-…

c6bed3b

…peers

GH-1245 Add is_bp_gossip_peer to connection_status

8c98362

GH-1245 Integrate peer_keys_db

1a788ea

GH-1245 Add integration test for bp gossip peering

dec942d

GH-1245 Do not include sig in digest

b4cf089

GH-1245 Add peerkeys table and actions to system contract

e8cbb49

GH-1245 Update to match system contract

77bf31d

GH-1245 Track active_schedule so when new peers are received in the s…

ac93b63

…chedule they can be connected to.

GH-1245 Fix startup missing disconnect

8e7e6bd

Merge remote-tracking branch 'spring/gh_1183' into GH-1245-gossip-bp-…

8f0c5c4

…peers Update to latest system contract changes.

GH-1245 Reduce test runtime

292e4c2

heifner added enhancement New feature or request OCI Work exclusive to OCI team labels Mar 21, 2025

heifner linked an issue Mar 21, 2025 that may be closed by this pull request

Add P2P gossip_bp_peer message #1245

Closed

This comment was marked as outdated.

Sign in to view

greg7mdp reviewed Mar 21, 2025

View reviewed changes

Comment thread plugins/net_plugin/include/eosio/net_plugin/auto_bp_peering.hpp

Base automatically changed from gh_1183 to main March 24, 2025 19:17

greg7mdp reviewed Mar 24, 2025

View reviewed changes

Comment thread plugins/net_plugin/include/eosio/net_plugin/auto_bp_peering.hpp Outdated

GH-1245 Simplify and add comment

b17234d

greg7mdp reviewed Mar 25, 2025

View reviewed changes

Comment thread plugins/net_plugin/include/eosio/net_plugin/auto_bp_peering.hpp Outdated

heifner added 4 commits April 16, 2025 10:54

GH-1245 enable-stale-production so production restarts on node restart

65f3c9e

GH-1245 Integrate peer key retrieval by read-only trx. Required waiti…

dff3da9

…ng on receive of a block before read-only trx could be executed.

Merge remote-tracking branch 'spring/main' into GH-1245-gossip-bp-peers

e835f95

GH-1245 Update test for simplified system contract

9e487c0

heifner requested review from greg7mdp and spoonincode April 16, 2025 16:53

heifner added 2 commits April 16, 2025 12:14

1245 Improve test robustness in face of temp duplicate connections

b629769

Merge branch 'main' into GH-1245-gossip-bp-peers

b8d8f22

spoonincode reviewed Apr 16, 2025

View reviewed changes

greg7mdp reviewed Apr 18, 2025

View reviewed changes

Comment thread libraries/chain/controller.cpp

Comment thread libraries/chain/include/eosio/chain/peer_keys_db.hpp Outdated

Comment thread plugins/net_plugin/include/eosio/net_plugin/auto_bp_peering.hpp Outdated

Merge remote-tracking branch 'spring/main' into GH-1245-gossip-bp-peers

0721d18

spoonincode reviewed Apr 18, 2025

View reviewed changes

heifner added 6 commits April 19, 2025 08:22

GH-1245 Use peer_name_set_t throughout

596135d

GH-1245 Verify not webauthn sig and don't check for canonical

f816643

GH-1245 Add and use a new endpoint type instead of using string

3b8145f

GH-1245 Fix host/port parsing when there is only one colon

e70d33b

GH-1245 Fix auto_bp_peering_test.py to check for correct host name

436e188

Merge remote-tracking branch 'spring/main' into GH-1245-gossip-bp-peers

67e038f

greg7mdp reviewed Apr 22, 2025

View reviewed changes

Comment thread plugins/net_plugin/include/eosio/net_plugin/auto_bp_peering.hpp

greg7mdp reviewed Apr 22, 2025

View reviewed changes

Comment thread plugins/net_plugin/include/eosio/net_plugin/auto_bp_peering.hpp

Comment thread plugins/net_plugin/include/eosio/net_plugin/auto_bp_peering.hpp

GH-1245 Reduce logging on expected already connected calls to resolve…

f778d71

…_and_connect.

greg7mdp reviewed Apr 22, 2025

View reviewed changes

GH-1245 Rename fatal_error to invalid_message

75c63e3

greg7mdp approved these changes Apr 22, 2025

View reviewed changes

heifner added 2 commits April 22, 2025 17:06

GH-1245 Don't consider a peer outside of top producers a fatal error

4fe408f

GH-1245 Add debug log statement when connection first becoming bp gos…

107647b

…sip connection

spoonincode approved these changes Apr 23, 2025

View reviewed changes

heifner merged commit 02bc051 into main Apr 23, 2025
36 checks passed

heifner deleted the GH-1245-gossip-bp-peers branch April 23, 2025 11:49

Conversation

heifner commented Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

spoonincode commented Apr 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greg7mdp Apr 22, 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!

greg7mdp Apr 22, 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!

greg7mdp Apr 23, 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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

heifner commented Mar 21, 2025 •

edited

Loading

greg7mdp Apr 22, 2025 •

edited

Loading

greg7mdp Apr 22, 2025 •

edited

Loading

greg7mdp Apr 23, 2025 •

edited

Loading