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

Adjust ban times for DL dat files #17021

Merged
merged 2 commits into from
Dec 8, 2023
Merged

Conversation

emlowe
Copy link
Contributor

@emlowe emlowe commented Dec 8, 2023

Currently, if a file is downloaded, but fails to insert properly, we ban that server for 7 days. In this case, the files can fail to insert for a variety of reasons, such as a truncated download, or disk problem that is likely not a result of a malicious actor.

This ban is overly long and has caused some problems with both CADT and Labrador.

This PR uses the logic for missing files to set a progressive ban per attempt with a maximum of one hour. This is using an existing function for this and so has very little risk. We would like to get into 2.1.2 so we can update CADT with this change

Change made in agreement with Zach and Ken.

@emlowe emlowe requested a review from a team as a code owner December 8, 2023 19:37
@emlowe emlowe requested a review from fchirica December 8, 2023 19:49
Copy link
Contributor

@wjblanke wjblanke left a comment

Choose a reason for hiding this comment

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

aok

@emlowe emlowe added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label Dec 8, 2023
Copy link
Contributor

github-actions bot commented Dec 8, 2023

File Coverage Missing Lines
chia/data_layer/download_data.py 0.0% lines 203
Total Missing Coverage
3 lines Unknown 66%

Copy link

Pull Request Test Coverage Report for Build 7145543163

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

  • 2 of 3 (66.67%) changed or added relevant lines in 3 files are covered.
  • 8 unchanged lines in 6 files lost coverage.
  • Overall coverage increased (+0.02%) to 90.429%

Changes Missing Coverage Covered Lines Changed/Added Lines %
chia/data_layer/download_data.py 0 1 0.0%
Files with Coverage Reduction New Missed Lines %
chia/daemon/client.py 1 85.0%
chia/rpc/rpc_server.py 1 88.7%
chia/server/node_discovery.py 1 80.07%
chia/simulator/setup_nodes.py 1 98.31%
chia/full_node/full_node.py 2 85.52%
tests/core/util/test_lockfile.py 2 91.09%
Totals Coverage Status
Change from base Build 7110364154: 0.02%
Covered Lines: 93748
Relevant Lines: 103630

💛 - Coveralls

@emlowe
Copy link
Contributor Author

emlowe commented Dec 8, 2023

coverage diff exemption

@cmmarslender cmmarslender merged commit cfab3a4 into release/2.1.2 Dec 8, 2023
583 of 585 checks passed
@cmmarslender cmmarslender deleted the EL.dl-download-bans branch December 8, 2023 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changed Required label for PR that categorizes merge commit message as "Changed" for changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants