feat(l2): show peer count in monitor when in based mode #3668
Conversation
Lines of code reportTotal lines added: Detailed view |
| @@ -40,7 +50,7 @@ impl NodeStatusTable { | |||
| .get_latest_block_number() | |||
| .await | |||
| .map_err(|_| MonitorError::GetLatestBlock)?; | |||
| let follower_nodes = "NaN"; // TODO: Implement follower nodes retrieval | |||
| let follower_nodes = l2_client.peer_count().await?; | |||
There was a problem hiding this comment.
nit: peers could also be syncing.
Maybe active_peers is more accurate.
| @@ -53,7 +63,11 @@ impl NodeStatusTable { | |||
| "Last Known Block:".to_string(), | |||
| last_known_block.to_string(), | |||
| ), | |||
| ("Peers:".to_string(), follower_nodes.to_string()), | |||
| if is_based { | |||
There was a problem hiding this comment.
why is this needed? In general it seems an antipattern to have a is_based flag spread across the codebase.
There was a problem hiding this comment.
I agree, part of the problem is due to the fact that the table rows are rendered (read rows elements are formatted into String) in refresh_items instead of in the SatefulWidget implementation, where we have the state of the widget that knows whether we are on based or not, and can render the Peers row or not, reducing all the flag spread thought this code to a single if.
We could either file an issue or tackle this in this PR, what do you think?
There was a problem hiding this comment.
it's ok to add a ticket and fix later
There was a problem hiding this comment.
cc @iovoid file an issue explaining this and link this thread.
Motivation
Currently peer count is a placeholder ('NaN') so we want an actual implementations.
The peer count of the L2 should only be shown in based bode, because in other (non-based) modes peers aren't needed and having more isn't necessarily better.
Description
Adds a EthClient method to query net_peerCount, and uses it to get the peer count from the local node. Only displays the value if based mode is enabled.
Closes #3516