fix(p2p): disconnect a peer if hello is not received in 30s

hanabi1224 commented 3 months ago

Summary of changes

This PR makes a few improvements to the p2p hello protocol handling logic

Changes introduced in this pull request:

disconnect a peer if a hello is not received in 30 seconds (to allow the other peer to re-connect and send another hello)
mark a peer full_peer right after receiving and validating hello
add peer selection fallback logic to mitigate CI stateless node test timeout issue.

Reference issue to close (if applicable)

Closes

Change checklist

[x] I have performed a self-review of my own code,
[x] I have made corresponding changes to the documentation,
[x] I have added tests that prove my fix is effective or that my feature works (if possible),
[x] I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

ruseinov commented 3 months ago

I wish we had some tests for some of this newly introduced logic. It looks good to me, but I can't help but wonder when all of this complexity is going to regress and blow up on us. And how are we going to notice that. What do you think? Perhaps some of the state-independent pieces of logic could be moved to helpers and tested? We could potentially test the bigger picture too, but I am unsure how mockable all of this currently is.

ruseinov commented 3 months ago

I'll approve this, let's create an issue to cover this with some tests as a follow-up.

ChainSafe / forest