net processing: Remove hash and fValidatedHeaders from QueuedBlock #22141
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MarkBlockAsInFlight is always called with a non-null pindex. Just get the block hash from that pindex inside the function.
Since headers-first syncing, we only ever request a block if we've already validated its headers. Therefore QueuedBlock.fValidatedHeaders is always set to true. Remove it.
nBlocksInFlightValidHeaders always has the same value as nBlocksInFlight, since we only download blocks with valid headers.
-BEGIN VERIFY SCRIPT-
ren() { sed -i "s:\<$1\>:$2:g" $(git grep -l "\<$1\>" ./src ./test); }
ren nPeersWithValidatedDownloads m_peers_downloading_from
-END VERIFY SCRIPT-
It's redundant with CBlockIndex::GetBlockHash()
MarkBlockAsReceived() should not be used for both removing the block from mapBlocksInFlight and checking whether it was in the map.
-BEGIN VERIFY SCRIPT-
ren() { sed -i "s:\<$1\>:$2:g" $(git grep -l "\<$1\>" ./src ./test); }
ren MarkBlockAsInFlight BlockRequested
ren MarkBlockAsReceived RemoveBlockRequest
-END VERIFY SCRIPT-
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
mjdietzx
reviewed
Jun 7, 2021
mjdietzx
reviewed
Jun 7, 2021
|
Concept ACK |
|
crACK 2f4ad6b |
|
Thanks for the review @mjdietzx. I've answered your review questions. Feel free to ask more if there's anything that's still unclear about this PR. |
maflcko
approved these changes
Jun 9, 2021
There was a problem hiding this comment.
review ACK 2f4ad6b 📊
Show signature and timestamp
Signature:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
review ACK 2f4ad6b7efa408b8a858e87499bf6cfcdf936d73 📊
-----BEGIN PGP SIGNATURE-----
iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUhXzwv9HniRZ2DZea8vtfP9W0XCvKLSaLzohfoLcy8LV+kuq0kzasQQWYfqy6s+
9ZUkOV/pxBsrl55Pfk/MvhbHOMw2t4Y/2RP7foRm/GZVbF2W5eMkg58L3R0CIXsi
k4/dxINftIkNObRMtsYSjNWBwVaYiMHbAB8CHYH+B43/yE3otQ8QweT8IG7/r9VV
ZWSkw9P2Z7tUQw9FeiaOYxteDK/W+dxec0m3+iIvOJeloc/UHKXM+wI15E/KoSK/
S9ttkHCRzius/M3a54JqmgkA73vnDwcjQe7NLVphl2f/UbIgf2cE2PH/BIiL4k72
tNye+KKxMqknxsdMRjQXVs6biQp2crxu/pNSUS5kw6z6vvb0SQgF6zZtWzs2c77D
1r+Zo0eHhzxyLodbxwgtZzZmr5HvMzHMnAqhp/7aDWTqpeGRYeGC0Klqko9wXipB
dxs8nre5S/5XnJHt5DJ3DHiVLRGk0g7xldCDrv0A2KAJOI8whbHt8aEp1SojfM5I
0eVnuNvO
=itrW
-----END PGP SIGNATURE-----
Timestamp of file with hash f0a6a932acf4e1e73bf07c8f0433dd6b510a91f4061198f5afafddba6da52c6d -
Merged
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jun 10, 2021
…s from QueuedBlock 2f4ad6b scripted-diff: rename MarkBlockAs functions (John Newbery) 2c45f83 [net processing] Tidy up MarkBlockAsReceived() (John Newbery) 6299350 [net processing] Add IsBlockRequested() function (John Newbery) 4e90d2d [net processing] Remove QueuedBlock.hash (John Newbery) 156a19e scripted-diff: rename nPeersWithValidatedDownloads (John Newbery) b03de9c [net processing] Remove CNodeState.nBlocksInFlightValidHeaders (John Newbery) b4e29f2 [net processing] Remove QueuedBlock.fValidatedHeaders (John Newbery) 85e058b [net processing] Remove unnecessary hash arg from MarkBlockAsInFlight() (John Newbery) Pull request description: The QueuedBlock struct contains a `fValidatedHeaders` field that indicates whether we have already validated a header for the requested block. Since headers-first syncing, we only request blocks where the header is already validated, so `fValidatedHeaders` is always true. Remove it and clean up the logic that uses that field. Likewise, QueuedBlock contains a `hash` field that is set to the block hash. Since headers-first syncing, we always have a CBlockIndex, which contains the block hash, so remove the redundant `hash` field. Tidy up the logic and rename functions to better indicate what they're doing. ACKs for top commit: mjdietzx: crACK 2f4ad6b sipa: utACK 2f4ad6b MarcoFalke: review ACK 2f4ad6b 📊 Tree-SHA512: 3d31d2bcb4d35d0fdb7c1da624c2878203218026445e8f76c4a2df68cc7183ce0e7d0c47c7c0a3242e55efaca7c9f5532b683cf6ec7c03d23fa83764fdb82fd2
gwillen
pushed a commit
to ElementsProject/elements
that referenced
this pull request
Jun 1, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The QueuedBlock struct contains a
fValidatedHeadersfield that indicates whether we have already validated a header for the requested block. Since headers-first syncing, we only request blocks where the header is already validated, sofValidatedHeadersis always true. Remove it and clean up the logic that uses that field.Likewise, QueuedBlock contains a
hashfield that is set to the block hash. Since headers-first syncing, we always have a CBlockIndex, which contains the block hash, so remove the redundanthashfield.Tidy up the logic and rename functions to better indicate what they're doing.