Consolidate hash parsing into core_io.h #13420

1. 
   Empact commented at 9:51 am on June 8, 2018: member

   This consolidates separate hash parsing functions from rpc/server.cpp, rest.cpp, and core_read.cpp into a low-level parser ParseHashStr in core_read.cpp, and an rpc-level ParseHash in rpc/server.cpp.

   Note the rpc parser calls through to the core_io parser.

   Behavior changes are:

   • Hashes are consistently validated to 64 characters, the expected length for 256 bits hexidecimal. Because all newly-tested calls were against txids or blockhashes, this is safe.
   • prioritisetransaction now throws an RPC_INVALID_PARAMETER rather than std::runtime_error on bad hash. This is in the context of rpc, so seems more appropriate.
   • A bunch of calls that did not error meaningfully on bad hashes now do.
2. Empact force-pushed on Jun 8, 2018
3. Empact force-pushed on Jun 8, 2018
4. Empact force-pushed on Jun 8, 2018
5. Empact force-pushed on Jun 8, 2018
6. Drop ParseHashUV in favor of calling ParseHashStr

   The one existing call in bitcoin-tx already validates the call will
   pass via checkObject.

   fcadb98d89
7. Empact force-pushed on Jun 8, 2018
8. Empact force-pushed on Jun 8, 2018
9. Empact force-pushed on Jun 8, 2018
10. Empact force-pushed on Jun 8, 2018
11. Empact force-pushed on Jun 8, 2018
12. Empact force-pushed on Jun 8, 2018
13. Empact force-pushed on Jun 8, 2018
14. Empact force-pushed on Jun 8, 2018
15. Empact force-pushed on Jun 8, 2018
16. Consolidate hash parsing in core_io.h

    This consolidates separate hash parsing functions from
    rpc/server.cpp, rest.cpp, and core_read.cpp into a low-level
    parser in core_read.cpp, and an rpc-level parser in rpc/server.cpp.
    
    Note the rpc parser calls through to the core_io parser.
    
    Behavior changes are:
    * Hashes are consistently validated to 64 characters, the expected
      length for 256 bits hexidecimal. Because all newly-tested calls
      were against txids, this is safe.
    * prioritisetransaction now throws an RPC_INVALID_PARAMETER rather
      than std::runtime_error on bad hash. This is in the context of rpc,
      so seems more appropriate.

    edbacc6c97
17. Use ParseHash consistently

    These calls were previously unvalidated. As of this change, only
    hashes in test and qt are unvalidated.

    81d0eb8af4
18. Empact force-pushed on Jun 8, 2018
19. 
    Empact commented at 12:37 pm on June 8, 2018: member

    TODO: add tests for the new rpc errors on the various calls that now consistently validate input
20. 
    DrahtBot commented at 5:18 pm on June 8, 2018: member

    • #13399 (rpc: Add submitblockheader by MarcoFalke)
    • #10973 (Refactor: separate wallet from node by ryanofsky)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

21. 
    MarcoFalke commented at 5:39 pm on June 8, 2018: member

    Concept ACK.
22. 
    Empact commented at 5:49 pm on June 8, 2018: member

    Going to split this up to ease review.
23. Empact closed this on Jun 8, 2018

    ---

24. MarcoFalke referenced this in commit 2b770080a4 on Jun 15, 2018
25. Empact deleted the branch on Jul 2, 2018
26. MarcoFalke referenced this in commit 37612099ec on Sep 24, 2018
27. PastaPastaPasta referenced this in commit 1c002dd1b6 on Jul 7, 2020
28. pravblockc referenced this in commit 6faf808461 on Jul 23, 2021
29. MarcoFalke locked this on Sep 8, 2021

Contributors
Empact DrahtBot MarcoFalke