Change NULLFAIL => SIG_NULLFAIL. #9754

pull richardkiss wants to merge 1 commits into bitcoin:master from richardkiss:feature/unify_sig_nullfail changing 6 files +21 −21

richardkiss commented at 10:42 PM on February 13, 2017: contributor

In src/test/script_tests.cpp the ScriptErrorDesc entries all take the form {SCRIPT_ERR_XXX, "XXX"} except for { SCRIPT_ERR_SIG_NULLFAIL, "NULLFAIL" }. This patch corrects this oversight by replacing NULLFAIL with SIG_NULLFAIL.

The alternative direction is in pull request https://github.com/bitcoin/bitcoin/pull/9342
Change NULLFAIL => SIG_NULLFAIL. bb19843220
fanquake added the label Validation on Feb 14, 2017
richardkiss commented at 6:59 PM on March 23, 2017: contributor

@paveljanik any comments on this one?
richardkiss commented at 6:59 PM on March 23, 2017: contributor

@kallewoof any thoughts?
richardkiss commented at 6:27 PM on March 28, 2017: contributor

@jl2012 It looks like your code is what I changed... do you approve of this pull request? It fixes an exception to naming, and it's helpful for me, as it makes things simpler for me importing these tests into pycoin https://github.com/richardkiss/pycoin, and I expect it would be helpful to others importing these tests into dynamic languages.
jl2012 commented at 8:57 AM on March 29, 2017: contributor

@richardkiss I think NULLDUMMY and some other flags have the same issue. The VERIFY flags and ERR flags are just not 1-to-1

in src/test/data/script_tests.json:1512 in bb19843220

1509 |  ],
1510 |  [
1511 |      "0x09 0x300602010102010101",
1512 |      "0x21 0x038282263212c609d9ea2a6e3e172de238d8c39cabd5ac1ca10646e23fd5f51508 CHECKSIG NOT",
1513 | -    "DERSIG,NULLFAIL",
1514 | -    "NULLFAIL",

richardkiss commented at 10:06 PM on April 4, 2017:

There is no error code named NULLFAIL. It's SIG_NULLFAIL.

richardkiss commented at 10:07 PM on April 4, 2017: contributor

Without this change, the generated file src/test/data/script_tests.json has errors.

There are a couple of tests that enforce NULLFAIL checking (around line 1504), and subsequently fail with SIG_NULLFAIL. However, the JSON file suggests they should fail with the (non-existent) NULLFAIL error.

This reduces the usefulness of the JSON file to those who want to use it. (I realize bitcoind doesn't use it, or it would have the same problem.)
fanquake commented at 8:04 AM on November 28, 2017: member

Going to close this. Needs a rebase, and hasn't generated any discussion again.
fanquake closed this on Nov 28, 2017
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#9342 Change SIG_NULLFAIL => NULLFAIL.