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

feat: reduce timeouts and increase bans #5882

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

SWvheerden
Copy link
Collaborator

Description

Reduces the latency of timeouts from 30 seconds to 15 seconds
Increases the ban periods

Motivation and Context

Reduces the latency of the timeouts from 30 secs to 15 secs: #5860

Increase the short ban to 4 mins up from 30 seconds. The short ban is used for timeouts etc. and is used to ensure the base node can select a new node to sync from etc. But if this is too short, or under the block time, timeouts can be used maliciously by nodes. For example a node can say it has a better pow, let your node try and sync to it, wait for the timeouts, get banned for 30 seconds. This means your node waits the 30 seconds -> 1 mins if needs more than 1 call, which is likely. This means you node rejects all new blocks for 1 min, and then you ban the peer for 30 secs. They can then submit their new block to you after the 30 secs. This makes it longer than the block time. This means the bad node is excluded from all new nodes for 2 blocks on avg.

Fixes: : #5860
Fixes: #5867

@github-actions
Copy link

Test Results (CI)

1 243 tests   1 243 ✔️  9m 38s ⏱️
     39 suites         0 💤
       1 files           0

Results for commit a1b5a4b.

@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 Oct 27, 2023
@github-actions
Copy link

Test Results (Integration tests)

34 tests   34 ✔️  14m 3s ⏱️
11 suites    0 💤
  2 files      0

Results for commit a1b5a4b.

@SWvheerden
Copy link
Collaborator Author

Need to test and verify these changes on system level once merged in.

Copy link
Contributor

@hansieodendaal hansieodendaal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Oct 30, 2023
@SWvheerden SWvheerden merged commit df9bc9a into tari-project:development Oct 30, 2023
14 checks passed
@SWvheerden SWvheerden deleted the sw_change_periods branch October 30, 2023 13:45
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.

Revise default ban period
3 participants