P2P: Update p2p-auto-bp-peer for Savanna #1244
Conversation
…ck proposers instead of just a few. Remove requirement to be configured as a block producer.
…ssue with the test not actually testing anything because python peers = peers.sort() meant that peers==None.
…avanna head is so close to lib and a node can temporarily fall back into head catchup on slight block delays.
plugins/net_plugin/include/eosio/net_plugin/auto_bp_peering.hpp
Outdated
Show resolved
Hide resolved
plugins/net_plugin/include/eosio/net_plugin/auto_bp_peering.hpp
Outdated
Show resolved
Hide resolved
…h its implementation.
plugins/net_plugin/include/eosio/net_plugin/auto_bp_peering.hpp
Outdated
Show resolved
Hide resolved
|
Note:start |
| flat_set<account_name> pending_configured_bps; | ||
| flat_set<account_name> active_configured_bps; |
There was a problem hiding this comment.
I think boost::unordered_flat_set would be faster, although it may not make a significant difference and you wouldn't be able to use the std::set_* algos.
Oh and you wouldn't be able to compare them in tests. Forget it then.
| mock_connections_manager connections; | ||
| std::vector<std::string> p2p_addresses{"0.0.0.0:9876"}; | ||
| const std::string& get_first_p2p_address() const { return *p2p_addresses.begin(); } | ||
|
|
||
| bool in_sync() { return is_in_sync; } | ||
| bool not_lib_catchup() { return is_not_lib_catchup; } |
There was a problem hiding this comment.
I don't like these negative booleans personally, for example this is harder to parse for me:
plugin.is_not_lib_catchup = true;
than:
plugin._lib_catchup = false;
I think it would be better to have:
bool lib_catchup() { return _lib_catchup; }
| "proda"_n, "prodb"_n, "prodc"_n, "prodd"_n, "prode"_n, "prodf"_n, | ||
| "prodg"_n, "prodh"_n, "prodi"_n, "prodj"_n, /*"prodk"_n,*/ "prodl"_n, | ||
| "prodm"_n, "prodn"_n, "prodo"_n, "prodp"_n, "prodq"_n, "prodr"_n, | ||
| "prods"_n, /*"prodt"_n,*/ "produ"_n }; |
There was a problem hiding this comment.
couple items commented out here and above. Maybe consider removing them; it's not clear if it's intentional or not
There was a problem hiding this comment.
I think it is intentional, the list is named producers_minus_prodkt and prodk and prodt are commented out for clarity.
There was a problem hiding this comment.
ah true for this new one the name does make it clearer. But the two above don't have such an indication though, and they weren't previously commented out; only newly commented out in this PR, that made those more sus to me
There was a problem hiding this comment.
Updated comments: a1f55d6
Change
p2p-auto-bp-peerto remain connected to the current active producer schedule instead of just the next 2 in the schedule. With Savanna, it is important to have connections to all finalizers. Currently block proposers are also block finalizers.Block producers can configure their BP canary p2p node with
p2p-auto-bp-peerentries for all p2p nodes they know are associated with corresponding block producer. As the active producer schedule changes, thenet_pluginwill connect to peers that are in the pending producer schedule and disconnect from peers when they drop out of the active schedule.Resolves #1227