feat: dynamically populate nodeinfo#663
feat: dynamically populate nodeinfo#663jking-aus wants to merge 9 commits intosigp:release-v1.0.0from
Conversation
jking-aus
changed the title
jking-aus
changed the title
dknopik
left a comment
dknopik
left a comment
There was a problem hiding this comment.
I don't see how this mechanism of updating the subnets in our metadata (generating it anytime ti is needed) is superior to our current approach (updating it as we subscribe/unsubscribe to subnets)
| /// Fetches node version information from beacon and execution nodes | ||
| async fn fetch_node_versions<T: SlotClock>( | ||
| beacon_nodes: &BeaconNodeFallback<T>, | ||
| execution_http_urls: &[SensitiveUrl], | ||
| execution_ws_url: &SensitiveUrl, | ||
| ) -> Result<(String, String), String> { |
There was a problem hiding this comment.
IMO it is unclear if we actually want to do this. e.g. Go-SSV just leaves the node version in the handshake empty
| pub static HANDSHAKE_SUCCESSFUL: LazyLock<Result<IntCounter>> = LazyLock::new(|| { | ||
| try_create_int_counter( | ||
| "libp2p_handshake_successful_total", | ||
| "Total count of successful handshakes", | ||
| ) | ||
| }); | ||
|
|
||
| pub static HANDSHAKE_FAILED: LazyLock<Result<IntCounter>> = LazyLock::new(|| { | ||
| try_create_int_counter( | ||
| "libp2p_handshake_failed_total", | ||
| "Total count of failed handshakes", | ||
| ) | ||
| }); | ||
|
|
||
| pub static HANDSHAKE_SUBNET_MATCHES: LazyLock<Result<IntGaugeVec>> = LazyLock::new(|| { | ||
| try_create_int_gauge_vec( | ||
| "libp2p_handshake_subnet_matches", | ||
| "Count of successful handshakes by number of matching subnets", | ||
| &["match_count"], | ||
| ) | ||
| }); |
There was a problem hiding this comment.
Metrics for the handshake is not really what this PR advertises to do, please open a separate PR
| // Get initial subnets from peer_manager and populate metadata | ||
| let mut node_metadata = node_metadata; | ||
| node_metadata.subnets = subnets_to_hex(behaviour.peer_manager.needed_subnets()); |
There was a problem hiding this comment.
this will effectively set the same, all empty subnet string as before, as the peer_manager will have no needed subnets until the subnet tracker sends join events.
It’s not - close this if that’s happening already. |
3 participants
Issue Addressed
Addresses #645
Proposed Changes
Gets required subnets from peermanager and updates the handshake
Gets node versions on startup and errors if not present
Additional Info
Will add preferences for own subets in peer selection