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

Syncstorewjb #19284

Merged
merged 3 commits into from
Feb 18, 2025
Merged

Syncstorewjb #19284

merged 3 commits into from
Feb 18, 2025

Conversation

wjblanke
Copy link
Contributor

@wjblanke wjblanke commented Feb 18, 2025

Purpose:

Fix peer_has_block recording

Current Behavior:

Doesn't check hash before recording block

New Behavior:

Check hash before recording block

Testing Notes:

None

@wjblanke wjblanke requested a review from a team as a code owner February 18, 2025 01:36
@wjblanke wjblanke changed the base branch from main to release/2.5.2 February 18, 2025 01:37
@wjblanke wjblanke added the Fixed Required label for PR that categorizes merge commit message as "Fixed" for changelog label Feb 18, 2025
@wjblanke wjblanke requested review from arvidn and emlowe February 18, 2025 02:37
@emlowe emlowe requested a review from almogdepaz February 18, 2025 02:59
@almogdepaz
Copy link
Contributor

almogdepaz commented Feb 18, 2025

we have a bad peak cache we used for chip0013 that is currently not used, i think we can identify the peaks that fail due to bad genesis and use the bad peak cache this will help us identify WP's that belong to these bad peaks and avoid validating the WP for those peaks.

we would still need to request the WP since for new peaks that are not in the cache we need the WP to see if the last two epochs contained a block that is in the bad cache in order to fail

im not sure this is needed because the WP should fail on this pretty fast since it is the first thing we validate and that happens before all the vdf heavy lifting , lmk what you think

@emlowe
Copy link
Contributor

emlowe commented Feb 18, 2025

With this change, we still ask for the WP, but we ask it from the correct peer - ie the peer that has the heavier weight peak. Without this, we would ask for the WP from a random selection of all our peers, since all the peers have that block height even though it's the wrong header hash - and since most of those peers produce a WP we accept, there is no error in WP.

Copy link
Contributor

File Coverage Missing Lines
chia/full_node/full_node.py 50.0% lines 781
Total Missing Coverage
2 lines Unknown 50%

@pmaslana pmaslana merged commit 683d0ac into release/2.5.2 Feb 18, 2025
867 of 868 checks passed
@pmaslana pmaslana deleted the syncstorewjb branch February 18, 2025 17:47
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants