Merge #18378: Bugfix & simplify bn2vch using int.to_bytes

a733ad514a Add bn2vch test to functional tests (Pieter Wuille)
a3ad6459b7 Simplify bn2vch using int.to_bytes (Pieter Wuille)

Pull request description:

  Alternative to #18374, fixing the incorrect padding added sometimes in `bn2vch`.

  Since we're using Python 3.2+, a much simpler implementation of `bn2vch` is possible using `int.to_bytes`.

  This also adds a "functional" test for bn2vch, in a new "framework_test_script.py", where the "framework_test_" prefix is intended for tests of the framework itself.

ACKs for top commit:
  laanwj:
    nice, ACK a733ad514a
  jnewbery:
    Tested ACK a733ad514a.

Tree-SHA512: aeacc4e7fd84279023d38e8b4a5175fb16d7b3a7f93c61b9dcb59cd9927547732983c76f28564b62e37088399fc0121b38a514d73b0ea38b3983836539e9ca90

test/functional/framework_test_script.py

@@ -0,0 +1,44 @@
+#!/usr/bin/env python3
+# Copyright (c) 2020 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Tests for test_framework.script."""
+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.script import bn2vch
+from test_framework.util import assert_equal
+
+def test_bn2vch():
+    assert_equal(bn2vch(0), bytes([]))
+    assert_equal(bn2vch(1), bytes([0x01]))
+    assert_equal(bn2vch(-1), bytes([0x81]))
+    assert_equal(bn2vch(0x7F), bytes([0x7F]))
+    assert_equal(bn2vch(-0x7F), bytes([0xFF]))
+    assert_equal(bn2vch(0x80), bytes([0x80, 0x00]))
+    assert_equal(bn2vch(-0x80), bytes([0x80, 0x80]))
+    assert_equal(bn2vch(0xFF), bytes([0xFF, 0x00]))
+    assert_equal(bn2vch(-0xFF), bytes([0xFF, 0x80]))
+    assert_equal(bn2vch(0x100), bytes([0x00, 0x01]))
+    assert_equal(bn2vch(-0x100), bytes([0x00, 0x81]))
+    assert_equal(bn2vch(0x7FFF), bytes([0xFF, 0x7F]))
+    assert_equal(bn2vch(-0x8000), bytes([0x00, 0x80, 0x80]))
+    assert_equal(bn2vch(-0x7FFFFF), bytes([0xFF, 0xFF, 0xFF]))
+    assert_equal(bn2vch(0x80000000), bytes([0x00, 0x00, 0x00, 0x80, 0x00]))
+    assert_equal(bn2vch(-0x80000000), bytes([0x00, 0x00, 0x00, 0x80, 0x80]))
+    assert_equal(bn2vch(0xFFFFFFFF), bytes([0xFF, 0xFF, 0xFF, 0xFF, 0x00]))
+
+    assert_equal(bn2vch(123456789), bytes([0x15, 0xCD, 0x5B, 0x07]))
+    assert_equal(bn2vch(-54321), bytes([0x31, 0xD4, 0x80]))
+
+class FrameworkTestScript(BitcoinTestFramework):
+    def setup_network(self):
+        pass
+
+    def set_test_params(self):
+        self.num_nodes = 0
+
+    def run_test(self):
+        test_bn2vch()
+
+if __name__ == '__main__':
+    FrameworkTestScript().main()

test/functional/test_framework/script.py

@@ -27,38 +27,14 @@ def hash160(s):
 
 def bn2vch(v):
     """Convert number to bitcoin-specific little endian format."""
-    # The top bit is used to indicate the sign of the number. If there
-    # isn't a spare bit in the bit length, add an extension byte.
-    have_ext = False
-    ext = bytearray()
-    if v.bit_length() > 0:
-        have_ext = (v.bit_length() & 0x07) == 0
-        ext.append(0)
-
-    # Is the number negative?
-    neg = False
-    if v < 0:
-        neg = True
-        v = -v
-
-    # Convert the int to bytes
-    v_bin = bytearray()
-    bytes_len = (v.bit_length() + 7) // 8
-    for i in range(bytes_len, 0, -1):
-        v_bin.append((v >> ((i - 1) * 8)) & 0xff)
-
-    # Add the sign bit if necessary
-    if neg:
-        if have_ext:
-            ext[0] |= 0x80
-        else:
-            v_bin[0] |= 0x80
-
-    v_bytes = ext + v_bin
-    # Reverse bytes ordering for LE
-    v_bytes.reverse()
-
-    return bytes(v_bytes)
+    # We need v.bit_length() bits, plus a sign bit for every nonzero number.
+    n_bits = v.bit_length() + (v != 0)
+    # The number of bytes for that is:
+    n_bytes = (n_bits + 7) // 8
+    # Convert number to absolute value + sign in top bit.
+    encoded_v = 0 if v == 0 else abs(v) | ((v < 0) << (n_bytes * 8 - 1))
+    # Serialize to bytes
+    return encoded_v.to_bytes(n_bytes, 'little')
 
 _opcode_instances = []
 class CScriptOp(int):

test/functional/test_runner.py

@@ -222,6 +222,7 @@ BASE_SCRIPTS = [
     'rpc_help.py',
     'feature_help.py',
     'feature_shutdown.py',
+    'framework_test_script.py',
     # Don't append tests at the end to avoid merge conflicts
     # Put them in a random line within the section that fits their approximate run-time
 ]
@@ -614,7 +615,7 @@ class TestResult():
 def check_script_prefixes():
     """Check that test scripts start with one of the allowed name prefixes."""
 
-    good_prefixes_re = re.compile("(example|feature|interface|mempool|mining|p2p|rpc|wallet|tool)_")
+    good_prefixes_re = re.compile("^(example|feature|interface|mempool|mining|p2p|rpc|wallet|tool|framework_test)_")
     bad_script_names = [script for script in ALL_SCRIPTS if good_prefixes_re.match(script) is None]
 
     if bad_script_names: