Improve CScriptNum error reporting #15074

1. 
   jl2012 commented at 6:02 pm on January 1, 2019: contributor

   Instead of reporting “unknown error”, more specific messages are returned for CScriptNum related errors.

   Also replaced throw with assert in case opcode is negative or bigger than 0xff, which should never happen

2. Improve CScriptNum error reporting 50605f0c34
3. laanwj added the label Consensus on Jan 2, 2019
4. 
   DrahtBot commented at 4:42 pm on January 3, 2019: member

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #15045 ([test] Apply maximal flags to tx_valid tests and minimal flags to tx_invalid tests by jl2012)

   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.

5. in src/script/interpreter.cpp:1078 in 50605f0c34

   1071@@ -1072,6 +1072,11 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
   1072                 return set_error(serror, SCRIPT_ERR_STACK_SIZE);
   1073         }
   1074     }
   1075+    catch (ScriptError runtime_error)
   1076+    {
   1077+        return set_error(serror, runtime_error);
   1078+
   

   ---

   ---

   

   practicalswift commented at 9:27 am on January 5, 2019:

   Nit: Remove redundant newline at end of block :-)

   ---

   

   laanwj commented at 2:03 pm on January 15, 2019:

   Also our catch syntax is } catch (ScriptError runtime_error) { — but let’s first get Concept ACKs on this before nitting it
6. 
   meshcollider commented at 4:06 am on April 14, 2019: contributor

   utACK https://github.com/bitcoin/bitcoin/pull/15074/commits/50605f0c343daa3f93c7740af202b6778a378f1a
7. fanquake added the label Needs Conceptual Review on Aug 29, 2019
8. fanquake added this to the "Chasing Concept ACK" column in a project

   ---

9. 
   fanquake commented at 7:41 am on September 16, 2019: member

   Going to take this over in a new PR.
10. fanquake closed this on Sep 16, 2019

    ---

11. fanquake removed this from the "Chasing Concept ACK" column in a project

    ---

12. fanquake removed the label Needs Conceptual Review on Sep 16, 2019
13. fanquake referenced this in commit 87a8cbf5dc on Sep 16, 2019
14. DrahtBot locked this on Dec 16, 2021

Contributors
jl2012 DrahtBot practicalswift laanwj meshcollider fanquake

Labels
Consensus