Merge bitcoin/bitcoin#27631: test: avoid sporadic MINIMALDATA failure in feature_taproot.py (fixes #27595)

54877253c807dac7a3720b2c3d1d989c410259a7 test: avoid sporadic MINIMALDATA failure in feature_taproot.py (fixes #27595) (Sebastian Falbesoner)

Pull request description:

  The functional test feature_taproot.py fails in some rare cases on the execution of the following `"branched_codesep"` spending script (can be reproduced via `$ ./test/functional/feature_taproot.py --randomseed 9048710178866422833` on master / 137a98c5a22e058ed7a7997a0a4dbd75301de51e):

  https://github.com/bitcoin/bitcoin/blob/9d85c03620bf660cfa7d13080f5c0b191579cbc3/test/functional/feature_taproot.py#L741

  The problem occurs if the first data-push (having random content with a random length in the range [0, 510]) has a length of 1 and the single byte has value of [1...16] or [-1]; in this case, the data-push is not minimally encoded by test framework's CScript class (i.e. doesn't use the special op-codes OP_1...OP_16 or OP_1NEGATE) and the script interpreter throws an SCRIPT_ERR_MINIMALDATA error:

  ```
  test_framework.authproxy.JSONRPCException: non-mandatory-script-verify-flag (Data push larger than necessary) (-26)
  ```

  Background: the functional test framework's CScript class translates passed bytes/bytearrays always to data pushes using OP_PUSHx/OP_PUSHDATA{1,2,4} op-codes (see `CScript.__coerce_instance(...)`). E.g. the expression `CScript(bytes([1]))` yields `bytes([OP_PUSH1, 1])` instead of the minimal-encoded `bytes([OP_1])`.

  Fix this by adapting the random-size range to [2,...], i.e. never pass byte-arrays below length two to be pushed.

  Closes #27595.

ACKs for top commit:
  instagibbs:
    ACK https://github.com/bitcoin/bitcoin/pull/27631/commits/54877253c807dac7a3720b2c3d1d989c410259a7
  sipa:
    utACK 54877253c807dac7a3720b2c3d1d989c410259a7
  achow101:
    ACK 54877253c807dac7a3720b2c3d1d989c410259a7

Tree-SHA512: 3ffad89b2c3985c20702242192e744c9b10188bff880efaf3c38424a00fa07bd4608d8c948678ff9cdbb4e1e5b06696c7f55407ee10bb05edbb3ee03aa599cdc

1 files changed, 5 insertions, 1 deletions

diff --git a/test/functional/feature_taproot.py b/test/functional/feature_taproot.py
index b37bfd28ae..ec1e580369 100755
--- a/test/functional/feature_taproot.py
+++ b/test/functional/feature_taproot.py
@@ -739,7 +739,11 @@ def spenders_taproot_active():
         scripts = [
             ("pk_codesep", CScript(random_checksig_style(pubs[1]) + bytes([OP_CODESEPARATOR]))),  # codesep after checksig
             ("codesep_pk", CScript(bytes([OP_CODESEPARATOR]) + random_checksig_style(pubs[1]))),  # codesep before checksig
-            ("branched_codesep", CScript([random_bytes(random.randrange(511)), OP_DROP, OP_IF, OP_CODESEPARATOR, pubs[0], OP_ELSE, OP_CODESEPARATOR, pubs[1], OP_ENDIF, OP_CHECKSIG])),  # branch dependent codesep
+            ("branched_codesep", CScript([random_bytes(random.randrange(2, 511)), OP_DROP, OP_IF, OP_CODESEPARATOR, pubs[0], OP_ELSE, OP_CODESEPARATOR, pubs[1], OP_ENDIF, OP_CHECKSIG])),  # branch dependent codesep
+            # Note that the first data push in the "branched_codesep" script has the purpose of
+            # randomizing the sighash, both by varying script size and content. In order to
+            # avoid MINIMALDATA script verification errors caused by not-minimal-encoded data
+            # pushes (e.g. `OP_PUSH1 1` instead of `OP_1`), we set a minimum data size of 2 bytes.
         ]
         random.shuffle(scripts)
         tap = taproot_construct(pubs[0], scripts)