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

Handle attempts to get block headers at invalid heights #3683

Merged
merged 3 commits into from
Dec 31, 2021

Conversation

yeastplume
Copy link
Member

Twofold 'belt and suspender' fix for #3681:

  • First, add an error check into get_header_hash_by_height in txhashset to return an error if the header height requested is too large for pmmr::insertion_to_pmmr_index to handle. Note there's no error handling in the core pmmr functions (which all assume arguments are correct, rightly or wrongly), so this check is placed as close to it as possible.
  • Second, manually handle negative heights from the difficulty iterator in get_server_stats code. Note the call to get_header_by_height is now needed here due to earlier performance optimizations on DifficultyIterator that removed the deserialization of proof nonces and left iterator entries unable to calculate their hash.

Either fix on its own would work, but get_header_by_height should be returning an error for invalid heights, while the call to it from get_server_stats shouldn't be attempting to provide invalid heights via an incorrect cast. (Note that casting from i64 to u64 is allowed in rust regardless of value, meaning that -59i64 to u64 results in 18446744073709551557).

@yeastplume yeastplume requested review from phyro and tromp December 30, 2021 15:02
Copy link
Contributor

@tromp tromp left a comment

Choose a reason for hiding this comment

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

see single in-line comment

@phyro
Copy link
Member

phyro commented Dec 30, 2021

I have run the initial sync that broke before and it no longer does. Apart from the minor comment from tromp, the changes look good 👍 I was not able to sync the node, but I now suspect the problem is on my end with the disk.

Copy link

@Hecim1984 Hecim1984 left a comment

Choose a reason for hiding this comment

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

🤑

@yeastplume yeastplume merged commit 2237f42 into mimblewimble:master Dec 31, 2021
@yeastplume yeastplume deleted the initial_sync_issue branch January 20, 2022 10:00
bayk added a commit to mwcproject/mwc-node that referenced this pull request Jun 21, 2024
…hts (mimblewimble#3683)

* handle attempts to get block headers at invalid heights
* compare requested height against header pmmr size
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants