This is a reformulation of #12463 which aims to be more boring and easier to review. A significant part of the change is now a scripted-diff and the other bits work up to that.
This is best reviewed on a per-commit basis.
The end result is removing Invalid, and removing 2 args from DoS. Renaming DoS to Reject is a protective measure against undetected merge conflicts, given the call flexibility of DoS.
This allows us to remove a rarely-used DoS argument while
making the calls more explict and easily identifiable.
This enables simplification of DoS to occur without separately handling
different call signatures.
-BEGIN VERIFY SCRIPT-
find ./src -type f \( -name *.cpp -o -name *.h \) -exec sed -i -E 's/\b\.Invalid\(([^)])/\.DoS\(0, \1/' {} \;
-END VERIFY SCRIPT-
Functionally, this was just passing through false each time.
Better to separate the calls which allows for simplifying DoS.
And removed Invalid, unused since 2 commits back
This is setting the stage for the next change...
Renaming protects against silent merge conflicts, the argument can be dropped because it is always false,
and was circumvented in the previous commit.
-BEGIN VERIFY SCRIPT-
find ./src -type f \( -name *.cpp -o -name *.h \) -exec sed -i -E 's/\bDoS\(([[:digit:]]+), false,/Reject\(\1,/' {} \;
find ./src -type f \( -name *.cpp -o -name *.h \) -exec sed -i -E 's/\bInvalid\(/Reject\(/' {} \;
sed -i 's/bool DoS(int level, bool ret,/bool Reject(int level,/' src/consensus/validation.h
-END VERIFY SCRIPT-Test failure is unrelated.
Concept ACK
Strong concept ack - dramatically improves readiblity - if you are already going through the trouble of such an expansive change does it make sense to distinguish (via method name) for a reject with ban points and a reject with 0 score (maybe state.Reject(code,text) state.Punish(points,code,text)? Should further improve readibility / grepability
@skeees Your comment is making me think I should retain Invalid and DoS, and instead do the removals and adjustments on them. @sipa expressed concern about the optional arguments resulting in missed merge conflicts. I think I'll start by opening a PR to remove those optional arguments, as they aren't relied upon as-is. #12463 (comment)