rpc: Return more specific reject reason for submitblock #13983

1. 
   MarcoFalke commented at 5:49 pm on August 15, 2018: member

   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.

2. MarcoFalke added the label RPC/REST/ZMQ on Aug 15, 2018
3. MarcoFalke force-pushed on Aug 15, 2018
4. MarcoFalke renamed this:
   rpc: Return more reject reject reason for submitblock
   rpc: Return more specific reject reason for submitblock
   on Aug 15, 2018
5. rpc: Return more specific reject reason for submitblock fa6ab8ada1
6. MarcoFalke force-pushed on Aug 15, 2018
7. 
   DrahtBot commented at 6:23 pm on August 15, 2018: member
8. 
   laanwj commented at 3:43 pm on August 21, 2018: member

   Concept ACK, don’t know enough of the submit logic here to be confident that this is correct but the tests look convincing.
9. laanwj added the label Mining on Aug 21, 2018
10. 
    MarcoFalke commented at 8:34 pm on August 22, 2018: member

    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

11. ryanofsky approved
12. 
    ryanofsky commented at 9:18 pm on September 12, 2018: member

    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.
13. MarcoFalke referenced this in commit 962c302710 on Sep 13, 2018
14. MarcoFalke merged this on Sep 13, 2018
15. MarcoFalke closed this on Sep 13, 2018

    ---

16. MarcoFalke deleted the branch on Sep 13, 2018
17. 
    ryanofsky commented at 6:43 pm on September 14, 2018: member

    @MarcoFalke, are you planning on adding release notes for this change or is it too minor to mention?
18. 
    MarcoFalke commented at 6:50 pm on September 14, 2018: member

    @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.

19. MarcoFalke locked this on Sep 8, 2021

Contributors
MarcoFalke DrahtBot laanwj ryanofsky

Labels
RPC/REST/ZMQ Mining