Skip to content
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

fix(comms+dht): mark peers as online inbound connection,join #5741

Merged

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Sep 5, 2023

Description

  • mark peers as online inbound connection and on join msg
  • fix DHT peer query logic

Motivation and Context

Peers were not being marked as online whenever we have proof that they are online, specifically in the inbound connection and join msg.

The peer query to exclude peers if we have attempted them but never succeeded was incorrect and filtered out most peers. This was changed to

 // we have tried to connect to this peer, and we have never made a successful attempt at connection
if peer.last_connect_attempt().is_some() && peer.last_seen().is_none() {
    return false;
}

How Has This Been Tested?

Manually, node reconnects quickly to peers on startup

What process can a PR reviewer use to test or verify this change?

Breaking Changes

  • None
  • Requires data directory on base node to be deleted
  • Requires hard fork
  • Other - Please specify

@ghpbot-tari-project ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Sep 5, 2023
@github-actions
Copy link

github-actions bot commented Sep 5, 2023

Test Results (CI)

1 199 tests   1 199 ✔️  10m 10s ⏱️
     38 suites         0 💤
       1 files           0

Results for commit 837bed2.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Sep 5, 2023

Test Results (Integration tests)

27 tests   27 ✔️  12m 41s ⏱️
11 suites    0 💤
  2 files      0

Results for commit 837bed2.

♻️ This comment has been updated with latest results.

@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Sep 6, 2023
@SWvheerden SWvheerden merged commit e8413ea into tari-project:development Sep 6, 2023
@sdbondi sdbondi deleted the comms-offline-peer-fixes branch September 19, 2023 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-acks_required Process - Requires more ACKs or utACKs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants