consensus: Witness program push must be minimal

[dergoegge]

According to BIP 141 the witness program push must be minimal: "Then, a byte L between 0x02 (push of 2 bytes) and 0x28 (push of 40 bytes) inclusive".

Libbitcoin incorrectly also matches on non-minimal pushes, i.e. any type of push between 2 and 40 bytes matches as a witness program:

https://github.com/libbitcoin/libbitcoin-system/blob/590919dca75df4cf22f9ac69d44c86aac5f0c92c/include/bitcoin/system/impl/chain/script.ipp#L86-L93

The corresponding logic in Bitcoin Core: https://github.com/bitcoin/bitcoin/blob/bc87ad98543299e1990ee1994d0653df3ac70093/src/script/script.cpp#L243-L257.

This difference can be demonstrated by the following test case:

libbitcoin test case

```patch diff --git a/test/chain/script.hpp b/test/chain/script.hpp index c985f8e57..00e5d3c23 100644 --- a/test/chain/script.hpp +++ b/test/chain/script.hpp @@ -522,7 +522,8 @@ const script_test_list valid_context_free_scripts { "[42424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242]", "[2.42424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242] equal", "basic push signedness check" }, { "[1.42424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242]", "[2.42424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242424242] equal", "basic pushdata1 signedness check" }, { "0x00", "size 0 equal", "basic op_0 execution" }, - { "", "0 1 equal size 0 equal", "boolean encoding" } + { "", "0 1 equal size 0 equal", "boolean encoding" }, + { "[4242]", "zero [1.4242424242424242424242]", "non-minimal push should not match as witness program" } }}; // These are always invalid. ```

Bitcoin Core test case

```patch diff --git a/src/test/data/tx_valid.json b/src/test/data/tx_valid.json index 70df0d0f69..aeecfcec5a 100644 --- a/src/test/data/tx_valid.json +++ b/src/test/data/tx_valid.json @@ -5,6 +5,9 @@ ["serializedTransaction, excluded verifyFlags]"], ["Objects that are only a single string (like this one) are ignored"], +["non-minimal push should not match as witness program"], +[[["0000000000000000000000000000000000000000000000000000000000000000", 0, "0x00 0x4c 0x08 0x4242424242424242"]], "020000000100000000000000000000000000000000000000000000000000000000000000000000000000ffffffff0100000000000000000000000000", "STRICTENC,LOW_S,SIGPUSHONLY,MINIMALDATA,DISCOURAGE_UPGRADABLE_NOPS,CLEANSTACK,MINIMALIF,NULLFAIL,DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM,WITNESS_PUBKEYTYPE,CONST_SCRIPTCODE,TAPROOT,DISCOURAGE_UPGRADABLE_PUBKEYTYPE,DISCOURAGE_OP_SUCCESS,DISCOURAGE_UPGRADABLE_TAPROOT_VERSION"], + ["The following is 23b397edccd3740a74adb603c9756370fafcde9bcc4483eb271ecad09a94dd63"], ["It is of particular interest because it contains an invalidly-encoded signature which OpenSSL accepts"], ["See http://r6.ca/blog/20111119T211504Z.html"], ```

[evoskuil]

Thanks again Niklas, this looks correct!

[evoskuil]

Resolved.