Decouple peer-processing-logic from block-connection-logic + BIP61 removal. #2464
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
furszy
changed the title
furszy
changed the title
furszy
force-pushed
the
2021_remove_processBlock_state
branch
6 times, most recently
from
4b233a8 to
8320fd6
Compare
furszy
force-pushed
the
2021_remove_processBlock_state
branch
2 times, most recently
from
edc5520 to
82ea177
Compare
Author
|
Rebased on master, lint-python warnings fixed. Ready to go. |
furszy
force-pushed
the
2021_remove_processBlock_state
branch
from
82ea177 to
e1c01c4
Compare
Author
|
And.. the rebase came with few needed changes. All good now, ready. |
furszy
force-pushed
the
2021_remove_processBlock_state
branch
from
e1c01c4 to
b5afa79
Compare
Backport of btc@fa25f43ac5692082dba3f90456c501eb08f1b75c and btc@b1b0cfecb639ce44be280c7a45a41a19e893c401
…invalid arriving block. 1) The actual peer misbehaving check is being performed inside PeerLogicValidation::BlockChecked. 2) The CValidationState inputted to ProcessNewBlock does not return the error nor invalidity reason if the block is marked as invalid during the activate best chain process (thus why BlockChecked exists).
…s in State() call and mapBlockIndex access.
…ocessNewBlock. Any block processing error is being notified by the BlockChecked signal.
Aside from the code responsibilities division improvement, this adds the good property of checking block spam filter for blocks that were rejected during the connection process as well (before, was only called if AcceptBlock rejected the block).
Plus pass pblock as const reference.
Plus minor cleanup, removing unused parameter.
…and made static as them are only used there.
…ect GetCommand call. The array is duplicated with the NetMsgType constants
…and evo_deterministicmns_tests files.
furszy
force-pushed
the
2021_remove_processBlock_state
branch
from
b5afa79 to
f16f422
Compare
Author
|
Updated per zebra's feedback, squashing all the updates in their respective commits, and rebased it on master so it has the correct bench raw files generation. To minimize the review mental burden --> |
Do not loop over every protocol message type, only walk through the tier two possible messages.
furszy
force-pushed
the
2021_remove_processBlock_state
branch
from
f16f422 to
8e9f2bc
Compare
Author
|
updated, added the vector change. |
random-zebra
added a commit
to random-zebra/PIVX
that referenced
this pull request
Jul 25, 2021
broken due to merges of PIVX-Project#2438 + PIVX-Project#2464
random-zebra
added a commit
that referenced
this pull request
Jul 25, 2021
4155b48 [Tests] Fix ProcessNewBlock signature in budget_tests.cpp (random-zebra) Pull request description: broken due to merges of #2438 + #2464 Top commit has no ACKs. Tree-SHA512: 1df1a7a45be0cdaca9409f2698220dfcf025df2fe784d28bd9d7b084a73718d4fccfad5def3296992f5bb4e78c0fea5860651b28c6e61f1c121a08e4df1e1535
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Continuation of #2463. Another error prone topic.
Essentially focused in the following points:
Remove duplicate and incorrect peer misbehaving:
-1) The peer misbehaving score set for an invalid arriving block is being performed inside
PeerLogicValidation::BlockChecked.-2) The
CValidationStateparam inProcessNewBlockdoes not return the error nor the invalidity reason if the block is marked invalid during the activate best chain process (block connection), an emptyCValidationStateis set. TheBlockCheckedsignal is used instead to notify the block invalidity reason.Get rid off
CValidationStateparam inProcessNewBlockand use onlyBlockChecked(own version of ae22357)Removal of BIP61 (p2p: Remove BIP61 reject messages bitcoin/bitcoin#15437 and test: Remove REJECT message code bitcoin/bitcoin#18609).
-- BIP61 could be disabled first by default before its removal like upstream did (or moved to its own standalone PR), but.. i don't think that worth to continue maintaining it, most likely no one is using it and if someone is actually using it, isn't good to have any piece of software depending on it. --
Move block spam filter check to
BlockChecked. Following the same peer-processing-logic / block-connection-logic division. Improves the spam protection as now blocks rejected during the connection process are going to pass through the spam filter check as well (before, only blocks rejected duringAcceptBlockwere passing through it).Plus, this improvement let me do some further cleanup and remove the extra
fAcceptedflag fromProcessNewBlock.Create
GetDataMsgenum and replaceppszTypeNamearray for directGetCommandcall.The array was duplicated with the
NetMsgTypeconstants