-
Notifications
You must be signed in to change notification settings - Fork 41
Added log if initial peers are invalid #1703
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
OCaml Reference Validation ResultsRepository: https://github.com/MinaProtocol/mina.git Click to see full validation output |
✓ Code Reference Verification PassedAll code references in the documentation have been verified successfully! Total references checked: 1 The documentation is in sync with the codebase on the |
9033737 to
b7212dd
Compare
35f70b1 to
adf6fba
Compare
adf6fba to
e8a0d18
Compare
e8a0d18 to
6f9b84b
Compare
51216b4 to
38dc53f
Compare
38dc53f to
c25cfb5
Compare
|
Looking at it now. |
node/src/reducer.rs
Outdated
| crate::core::error!( | ||
| crate::core::log::system_time(); | ||
| summary = "Exiting", | ||
| error = "Invalid initial peers" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are continuously printing on the stdout:
2025-12-04T16:42:16.552063Z ERROR node::reducer: time="1764866536552054251" summary="Exiting" error="Invalid initial peers"
2025-12-04T16:42:16.656957Z ERROR node::reducer: time="1764866536656948167" summary="Exiting" error="Invalid initial peers"
2025-12-04T16:42:16.761831Z ERROR node::reducer: time="1764866536761823042" summary="Exiting" error="Invalid initial peers"
2025-12-04T16:42:16.866726Z ERROR node::reducer: time="1764866536866719208" summary="Exiting" error="Invalid initial peers"
2025-12-04T16:42:16.971191Z ERROR node::reducer: time="1764866536971179084" summary="Exiting" error="Invalid initial peers"
2025-12-04T16:42:17.076161Z ERROR node::reducer: time="1764866537076153209" summary="Exiting" error="Invalid initial peers"
2025-12-04T16:42:17.181250Z ERROR node::reducer: time="1764866537181236750" summary="Exiting" error="Invalid initial peers"
2025-12-04T16:42:17.286159Z ERROR node::reducer: time="1764866537286148292" summary="Exiting" error="Invalid initial peers"
2025-12-04T16:42:17.391037Z ERROR node::reducer: time="1764866537391026792" summary="Exiting" error="Invalid initial peers"
2025-12-04T16:42:17.496219Z ERROR node::reducer: time="1764866537496203709" summary="Exiting" error="Invalid initial peers"
2025-12-04T16:42:17.601182Z ERROR node::reducer: time="1764866537601171959" summary="Exiting" error="Invalid initial peers"
2025-12-04T16:42:17.706175Z ERROR node::reducer: time="1764866537706161792" summary="Exiting" error="Invalid initial peers"
2025-12-04T16:42:17.811069Z ERROR node::reducer: time="1764866537811057709" summary="Exiting" error="Invalid initial peers"
However, it does not say which peers to remove. I know from the command I run for my test, but a user might be confused. Can you make it more explicit, please?
node/src/action.rs
Outdated
| RpcEffectful(RpcEffectfulAction), | ||
|
|
||
| WatchedAccounts(WatchedAccountsAction), | ||
| Exit(ExitAction), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no documentation for the others. Could you add a one-liner please?
Also, why do you call it Exit? By reading the code, I would have expected that it is an action to kill the node. But it is simply writing an error message. It is inconsistent with what we would expect by reading the code.
| if p2p.ready_peers().is_empty() && kad_state.has_bootstraped { | ||
| if let Some(tip) = &state.transition_frontier.best_tip_breadcrumb() { | ||
| // TODO: this might need to change in future | ||
| if tip.height() == 296372 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does this value come from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure where it comes from exactly, but when adding:
dbg!(state
.transition_frontier
.best_tip_breadcrumb()
.map(|t| t.height()));To check timeouts action and running node without internet connecting that is the value that gets printed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To anyone reading this code, this is effectively a random number as there is no explanation or description. In general, one should not have any numbers like this in the middle of a file but use a named constant instead. I don't know the best way to name, this, but anything is better than no name.
const KNOWN_PROBLEMATIC_BLOCK_HEIGHT: usize = 296_372;
// ...
if tip_height == KNOWN_PROBLEMATIC_BLOCK_HEIGHT {
// ...
CHANGELOG.md
Outdated
| Dashboard, documenting all endpoints and specific data fields used by the | ||
| frontend ([#1566](https://github.com/o1-labs/mina-rust/issues/1566)) | ||
|
|
||
| - **FEATURE**: Add logging if initial peers are invalid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - **FEATURE**: Add logging if initial peers are invalid | |
| - **Feature**: Add logging if an initial peer is invalid |
FEATURE -> Feature: there is no need to capitalize the word.
Plural -> singular: one initial peer is enough to have a log.
dannywillems
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments
c25cfb5 to
692c5af
Compare
692c5af to
1f220e4
Compare
464245a to
1a66221
Compare
1a66221 to
1ab39d6
Compare
|
Rebasing. I will have a look today. |
1ab39d6 to
09e0180
Compare
|
|
||
| crate::core::error!( | ||
| crate::core::log::system_time(); | ||
| summary = "Exiting", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The log says:
2025-12-15T23:50:48.379914Z ERROR node::reducer: time="1765842648379890834" summary="Exiting" error="Invalid initial peers" initial_peers="/ip4/5.5.5.5/tcp/8302/p2p/12D3KooWAMKZM7ysxRka2RfQWxCXXFTopjZ1eZcwtSi4VPgcdont"
2025-12-15T23:50:48.488578Z ERROR node::reducer: time="1765842648488538709" summary="Exiting" error="Invalid initial peers" initial_peers="/ip4/5.5.5.5/tcp/8302/p2p/12D3KooWAMKZM7ysxRka2RfQWxCXXFTopjZ1eZcwtSi4VPgcdont"
2025-12-15T23:50:48.597485Z ERROR node::reducer: time="1765842648597445917" summary="Exiting" error="Invalid initial peers" initial_peers="/ip4/5.5.5.5/tcp/8302/p2p/12D3KooWAMKZM7ysxRka2RfQWxCXXFTopjZ1eZcwtSi4VPgcdont"
2025-12-15T23:50:48.706593Z ERROR node::reducer: time="1765842648706558709" summary="Exiting" error="Invalid initial peers" initial_peers="/ip4/5.5.5.5/tcp/8302/p2p/12D3KooWAMKZM7ysxRka2RfQWxCXXFTopjZ1eZcwtSi4VPgcdont"
2025-12-15T23:50:48.815594Z ERROR node::reducer: time="1765842648815556709" summary="Exiting" error="Invalid initial peers" initial_peers="/ip4/5.5.5.5/tcp/8302/p2p/12D3KooWAMKZM7ysxRka2RfQWxCXXFTopjZ1eZcwtSi4VPgcdont"
2025-12-15T23:50:48.925051Z ERROR node::reducer: time="1765842648925017709" summary="Exiting" error="Invalid initial peers" initial_peers="/ip4/5.5.5.5/tcp/8302/p2p/12D3KooWAMKZM7ysxRka2RfQWxCXXFTopjZ1eZcwtSi4VPgcdont"
2025-12-15T23:50:49.031912Z ERROR node::reducer: time="1765842649031889709" summary="Exiting" error="Invalid initial peers" initial_peers="/ip4/5.5.5.5/tcp/8302/p2p/12D3KooWAMKZM7ysxRka2RfQWxCXXFTopjZ1eZcwtSi4VPgcdont"
2025-12-15T23:50:49.140310Z ERROR node::reducer: time="1765842649140283709" summary="Exiting" error="Invalid initial peers" initial_peers="/ip4/5.5.5.5/tcp/8302/p2p/12D3KooWAMKZM7ysxRka2RfQWxCXXFTopjZ1eZcwtSi4VPgcdont"
2025-12-15T23:50:49.249481Z ERROR node::reducer: time="1765842649249446125" summary="Exiting" error="Invalid initial peers" initial_peers="/ip4/5.5.5.5/tcp/8302/p2p/12D3KooWAMKZM7ysxRka2RfQWxCXXFTopjZ1eZcwtSi4VPgcdont"
2025-12-15T23:50:49.356963Z ERROR node::reducer: time="1765842649356923751" summary="Exiting" error="Invalid initial peers" initial_peers="/ip4/5.5.5.5/tcp/8302/p2p/12D3KooWAMKZM7ysxRka2RfQWxCXXFTopjZ1eZcwtSi4VPgcdont"
2025-12-15T23:50:49.465792Z ERROR node::reducer: time="1765842649465760376" summary="Exiting" error="Invalid initial peers" initial_peers="/ip4/5.5.5.5/tcp/8302/p2p/12D3KooWAMKZM7ysxRka2RfQWxCXXFTopjZ1eZcwtSi4VPgcdont"
2025-12-15T23:50:49.574145Z ERROR node::reducer: time="1765842649574116709" summary="Exiting" error="Invalid initial peers" initial_peers="/ip4/5.5.5.5/tcp/8302/p2p/12D3KooWAMKZM7ysxRka2RfQWxCXXFTopjZ1eZcwtSi4VPgcdont"
2025-12-15T23:50:49.680850Z ERROR node::reducer: time="1765842649680816250" summary="Exiting" error="Invalid initial peers" initial_peers="/ip4/5.5.5.5/tcp/8302/p2p/12D3KooWAMKZM7ysxRka2RfQWxCXXFTopjZ1eZcwtSi4VPgcdont"
2025-12-15T23:50:49.791452Z ERROR node::reducer: time="1765842649791414375" summary="Exiting" error="Invalid initial peers" initial_peers="/ip4/5.5.5.5/tcp/8302/p2p/12D3KooWAMKZM7ysxRka2RfQWxCXXFTopjZ1eZcwtSi4VPgcdont"
2025-12-15T23:50:49.903637Z ERROR node::reducer: time="1765842649903603709" summary="Exiting" error="Invalid initial peers" initial_peers="/ip4/5.5.5.5/tcp/8302/p2p/12D3KooWAMKZM7ysxRka2RfQWxCXXFTopjZ1eZcwtSi4VPgcdont"
2025-12-15T23:50:50.012286Z ERROR node::reducer: time="1765842650012267167" summary="Exiting" error="Invalid initial peers" initial_peers="/ip4/5.5.5.5/tcp/8302/p2p/12D3KooWAMKZM7ysxRka2RfQWxCXXFTopjZ1eZcwtSi4VPgcdont"
2025-12-15T23:50:50.120740Z ERROR node::reducer: time="1765842650120712542" summary="Exiting" error="Invalid initial peers" initial_peers="/ip4/5.5.5.5/tcp/8302/p2p/12D3KooWAMKZM7ysxRka2RfQWxCXXFTopjZ1eZcwtSi4VPgcdont"
but it does not actually exits. @0xMimir can you fix that please?
| if let Some(p2p) = state.p2p.ready() { | ||
| if let Some(kad_state) = &p2p.network.scheduler.discovery_state { | ||
| if p2p.ready_peers().is_empty() && kad_state.has_bootstraped { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should almost never had this much nesting in code. Please flatten out at least some of these conditionals:
| if let Some(p2p) = state.p2p.ready() { | |
| if let Some(kad_state) = &p2p.network.scheduler.discovery_state { | |
| if p2p.ready_peers().is_empty() && kad_state.has_bootstraped { | |
| let is_bootstrapped_and_empty = state | |
| .p2p | |
| .ready() | |
| .map(|state| { | |
| let has_bootstrapped = state | |
| .netwowrk | |
| .scheduler | |
| .discovery_state | |
| .map(|s| s.has_bootstraped) | |
| .unwrap_or_default(); | |
| state.ready_peers().is_empty() && has_bootstrapped | |
| }) | |
| .unwrap_or_default(); | |
| if is_bootstrapped_and_empty { |
| if p2p.ready_peers().is_empty() && kad_state.has_bootstraped { | ||
| if let Some(tip) = &state.transition_frontier.best_tip_breadcrumb() { | ||
| // TODO: this might need to change in future | ||
| if tip.height() == 296372 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To anyone reading this code, this is effectively a random number as there is no explanation or description. In general, one should not have any numbers like this in the middle of a file but use a named constant instead. I don't know the best way to name, this, but anything is better than no name.
const KNOWN_PROBLEMATIC_BLOCK_HEIGHT: usize = 296_372;
// ...
if tip_height == KNOWN_PROBLEMATIC_BLOCK_HEIGHT {
// ...
Issue #1641