rpc: Return more specific reject reason for submitblock #13983

The second commit in #13439 made the TODO in the first commit impossible to solve.

The meaning of fNewBlock changed from "This is the first time we process this block" to "We are about to write the new valid block".

So whenever fNewBlock is true, the block was valid. And whenever the fNewBlock is false, the block is either valid or invalid. If it was valid and not new, we know it is a "duplicate". In all other cases, the BIP22ValidationResult() will return the reason why it is invalid.

<!--e57a25ab6845829454e8d69fc972939a-->No more conflicts as of last run.

Concept ACK, don't know enough of the submit logic here to be confident that this is correct but the tests look convincing.

Yeah, submitblock had zero test coverage previously, which is why this was missed during review I guess.

Personally I don't care about the specificness of the return code for invalid blocks (since in normal operation you'd never submit invalid blocks), but we might want to backport this if some miner cares.

CC @TheBlueMatt @luke-jr

utACK fa6ab8ada14dd265e224496440ab0b5d99dfd0ef. I'm not very familiar with this logic, but the code change is simple and matches the description, and the new test coverage is good.

Two possible suggestions:

• Maybe add the new test cases in one commit, and then make the code change in a second commit so the test diff can reflect the changed behavior.
• Maybe add release note mentioning the change.

@MarcoFalke, are you planning on adding release notes for this change or is it too minor to mention?

@ryanofsky I believe the changes here only affect the return value of invalid blocks. I'd be surprised to see someone submitting invalid blocks in production. Also, I believe this restores the behavior that existed before 0.17.0.

If someone feels strongly they could backport that to 0.17.0, but at this point it might be too late to get in.