[TierTwo] Do not ask for single MN items if the node is in initial sync state #2622
Conversation
This file contains 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
…chronization state. if it happens before or while the node's tier two initial synchronization is being executed, the peer will move to single items requests instead of requesting the full list. Plus, decrease the DoS score for a bad mnb signature during the initial synchronization process, so we don't end up banning pre v5.3.3 nodes too fast for an invalidly serialized mnb (this issue was solved in bitcoin#2611 removing the mnb BIP155 flag, this is only an extra guard to give a bit more time to non-updated peers to upgrade while the network is upgrading).
Fuzzbawls
approved these changes
Oct 31, 2021
furszy
added a commit
that referenced
this pull request
Nov 2, 2021
e43a239 TierTwo: do not ask for single MN items if the node is in initial synchronization state. (furszy) 1a3f7e0 [GUI] Force argument insertion when switching languages (Fuzzbawls) 9ea042c refactor: rename DsegUpdate to RequestMnList (furszy) 60767c0 TierTwo: when the INV is received, remove it from every peer AskFor set. (furszy) 6ece735 net: do not relay INV messages to not successfully connected peers. (furszy) ff181f1 TierTwo: Only relay data if we aren't on the initial synchronization phase. (furszy) bf5f78e net_processing: improve INV processing for tier two elements. (furszy) a5ff7f8 Masternode: clean possible invalidly stored seen MN broadcasts at startup. (furszy) e024421 Masternode: remove own mnb validation skip. (furszy) 3dec0cd Masternode-sync: Count local MN, in case of have it, for fallback sync issue. (furszy) a729fc9 Masternode: pre-v6.0 guard against an invalidly stored BIP155 masternode addr that causes a signature verification failure. (furszy) 66568c6 Masternode: remove error-prone 'isBIP155Addr' flag, use directly the addr isCompat function. (furszy) df1b3cc test, governance sync: increase tier two network gossip movement (furszy) 15f5a70 test: add tier two resync from scratch test case. (furszy) ee7cc08 TierTwo sync: raise mn list sync time to 40 seconds. (furszy) 2240602 net bugfix: clear waiting for response INVs from the "askedFor" set when the item is received. (furszy) 5dd052c tier two: do not request single missing mn entries until the initial sync process is completed. (furszy) 156b6a9 MasternodeMan: remove mainnet only check for the "already asked mn list sync" DoS validation. (furszy) be5bf14 Raise max mnlist sync threshold, increase budget sync finish timeout and document masternode-sync process. (furszy) c1c1044 Bump protocol version to 70924. (furszy) 09c72c5 budget sync: if budget finalization fails, not only request missing proposals, request proposals' votes as well. (furszy) 0726c16 budget sync: do not block budget full sync requests forever. (furszy) 95bbbd5 mnsync: randomize the sync requests order so the node try to sync from different peers in case of needing it. (furszy) bbb28fe mnsync: do not mark get_mn_list peer sync fulfilled if the request wasn't done. (furszy) 6d285b2 Test: add test case for budget data re-synchronization. (furszy) 2bde6fc [RPC] Add cleanbudget command. (furszy) Pull request description: Backports #2611 for v5.3.3 So we do a release focused on solving tier two network synchronization and re-synchronization issues that are mainly affecting the superblock payments window and causing network forks. Update: Added #2617 and #2622 as well. ACKs for top commit: random-zebra: utACK e43a239 then Fuzzbawls: ACK e43a239 Tree-SHA512: ea7e1730caaefc48a49edd7610bd9874fdcbabd6365a818313f3b418eb77fd95b0bdeb32bacdfdeefee8dddb3edc422ebaa2e85a1c93df51ee66fc64f40a68bb
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Solving a last edge case race condition for the tier two initial sync process:
if a
fbvoteorfbsmessage is received before or while we are synchronizing the MN list, there can be a race where the node will move to single items requests instead of requesting the full list (which is not what we want in the initial sync process as single items requests sync is a reactive approach while full list requests is a proactive one).Plus, decreased the DoS score for a bad mnb signature during the initial synchronization process, so we don't end up banning pre v5.3.3 nodes too fast for an invalidly serialized mnb (this issue was solved in #2611 removing the mnb BIP155 flag, this is only an extra guard to give a bit more time to non-updated peers to upgrade while the network is upgrading).