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

refactor new_peak_timelord #16881

Merged
merged 31 commits into from
Jul 29, 2024
Merged

refactor new_peak_timelord #16881

merged 31 commits into from
Jul 29, 2024

Conversation

almogdepaz
Copy link
Contributor

@almogdepaz almogdepaz commented Nov 20, 2023

Purpose:

improving the flow that handles a new peak

Current Behavior:

we would skip a new peak for the wrong condition in some edge cases
we would also handle peaks we should skip

New Behavior:

skip less heavy peaks even if we dont have unfinished blocks in the cache

Testing Notes:

added a test case for receiving a less heavy peak while we dont have unfinished blocks in the cache

@almogdepaz almogdepaz added timelord Fixed Required label for PR that categorizes merge commit message as "Fixed" for changelog labels Nov 20, 2023
@almogdepaz almogdepaz requested a review from fchirica November 20, 2023 11:56
@almogdepaz almogdepaz marked this pull request as ready for review November 20, 2023 16:47
@almogdepaz almogdepaz requested a review from a team as a code owner November 20, 2023 16:47
@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Nov 22, 2023
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Dec 6, 2023
Copy link
Contributor

github-actions bot commented Dec 6, 2023

Conflicts have been resolved. A maintainer will review the pull request shortly.

Copy link

coveralls-official bot commented Dec 6, 2023

Pull Request Test Coverage Report for Build 8144610631

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 195 of 200 (97.5%) changed or added relevant lines in 5 files are covered.
  • 1481 unchanged lines in 65 files lost coverage.
  • Overall coverage increased (+0.3%) to 91.165%

Changes Missing Coverage Covered Lines Changed/Added Lines %
chia/simulator/block_tools.py 4 5 80.0%
tests/blockchain/test_blockchain.py 5 6 83.33%
tests/timelord/test_new_peak.py 158 161 98.14%
Files with Coverage Reduction New Missed Lines %
chia/consensus/block_record.py 1 95.24%
chia/daemon/client.py 1 84.38%
chia/daemon/keychain_proxy.py 1 64.98%
chia/daemon/server.py 1 88.56%
chia/farmer/farmer.py 1 72.67%
chia/full_node/mempool.py 1 99.66%
chia/rpc/rpc_client.py 1 98.91%
chia/server/server.py 1 81.12%
chia/util/json_util.py 1 88.46%
chia/wallet/db_wallet/db_wallet_puzzles.py 1 98.44%
Totals Coverage Status
Change from base Build 7830053184: 0.3%
Covered Lines: 97886
Relevant Lines: 107364

💛 - Coveralls

fchirica
fchirica previously approved these changes Dec 11, 2023
Copy link
Contributor

This PR has been flagged as stale due to no activity for over 60 days. It will not be automatically closed, but it has been given a stale-pr label and should be manually reviewed by the relevant parties.

Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Mar 14, 2024
# Conflicts:
#	chia/_tests/timelord/test_new_peak.py
@github-actions github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Mar 25, 2024
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

fchirica
fchirica previously approved these changes Apr 8, 2024
Copy link
Contributor

This PR has been flagged as stale due to no activity for over 60 days. It will not be automatically closed, but it has been given a stale-pr label and should be manually reviewed by the relevant parties.

@github-actions github-actions bot added the stale-pr Flagged as stale and in need of manual review label May 24, 2024
emlowe
emlowe previously approved these changes Jul 17, 2024
@emlowe emlowe removed stale-pr Flagged as stale and in need of manual review coverage-diff labels Jul 17, 2024
@emlowe
Copy link
Contributor

emlowe commented Jul 17, 2024

coverage exemption

@Starttoaster
Copy link
Contributor

Last CI run is stale, going to close and re-open to run again.

@Starttoaster
Copy link
Contributor

Please check the latest CI run. Some seemingly non-flakey failed jobs. As an aside, this PR was fairly clearly stale and imo should not have been approved without a new CI run. @almogdepaz @emlowe FYI

@Starttoaster Starttoaster requested a review from emlowe July 18, 2024 18:53
@emlowe emlowe dismissed stale reviews from fchirica and themself via 6ac100b July 24, 2024 20:05
Copy link
Contributor

File Coverage Missing Lines
chia/_tests/timelord/test_new_peak.py 98.1% lines 357, 365-366
chia/simulator/block_tools.py 80.0% lines 1073
Total Missing Coverage
200 lines 4 lines 98%

@Starttoaster Starttoaster merged commit 1c1e030 into main Jul 29, 2024
374 of 375 checks passed
@Starttoaster Starttoaster deleted the refactor_new_peak_timelord branch July 29, 2024 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed Required label for PR that categorizes merge commit message as "Fixed" for changelog timelord
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants