From 0c41fc3fa52ad16923afbd0ec18b9c1b3ded8036 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Tue, 22 Aug 2023 03:06:15 +0200 Subject: [PATCH 1/3] test: fix `keys_to_multisig_script` (P2MS) helper for n/k > 16 The helper assumes that the n and k values have to be provided as a single byte push operation, which is only possible for values up to 16. Fix that by passing the numbers directly to the CScript list, where it's automatically converted to minimally-encoded pushes (see class method `CScript.__coerce_instance`, branch `isinstance(other, int)`). In case of 17..20, this means that the data-pushes are done with two bytes using OP_PUSH1 (0x01), e.g. for n=20: 0x01,0x14 --- test/functional/test_framework/script_util.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/functional/test_framework/script_util.py b/test/functional/test_framework/script_util.py index 62894cc0f44..ee8199c2daf 100755 --- a/test/functional/test_framework/script_util.py +++ b/test/functional/test_framework/script_util.py @@ -5,7 +5,6 @@ """Useful Script constants and utils.""" from test_framework.script import ( CScript, - CScriptOp, OP_0, OP_CHECKMULTISIG, OP_CHECKSIG, @@ -49,10 +48,8 @@ def keys_to_multisig_script(keys, *, k=None): if k is None: # n-of-n multisig by default k = n assert k <= n - op_k = CScriptOp.encode_op_n(k) - op_n = CScriptOp.encode_op_n(n) checked_keys = [check_key(key) for key in keys] - return CScript([op_k] + checked_keys + [op_n, OP_CHECKMULTISIG]) + return CScript([k] + checked_keys + [n, OP_CHECKMULTISIG]) def keyhash_to_p2pkh_script(hash): From 0570d2c204ec7f10af6bd8e48c23318a48fefc10 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Tue, 22 Aug 2023 03:17:00 +0200 Subject: [PATCH 2/3] test: add unit test for `keys_to_multisig_script` --- .../feature_framework_unit_tests.py | 1 + test/functional/test_framework/script_util.py | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/test/functional/feature_framework_unit_tests.py b/test/functional/feature_framework_unit_tests.py index f03f084bed8..14d83f8a707 100755 --- a/test/functional/feature_framework_unit_tests.py +++ b/test/functional/feature_framework_unit_tests.py @@ -27,6 +27,7 @@ TEST_FRAMEWORK_MODULES = [ "crypto.ripemd160", "crypto.secp256k1", "script", + "script_util", "segwit_addr", "wallet_util", ] diff --git a/test/functional/test_framework/script_util.py b/test/functional/test_framework/script_util.py index ee8199c2daf..855f3b8cf59 100755 --- a/test/functional/test_framework/script_util.py +++ b/test/functional/test_framework/script_util.py @@ -3,9 +3,13 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Useful Script constants and utils.""" +import unittest + from test_framework.script import ( CScript, OP_0, + OP_15, + OP_16, OP_CHECKMULTISIG, OP_CHECKSIG, OP_DUP, @@ -122,3 +126,19 @@ def check_script(script): if isinstance(script, bytes) or isinstance(script, CScript): return script assert False + + +class TestFrameworkScriptUtil(unittest.TestCase): + def test_multisig(self): + fake_pubkey = bytes([0]*33) + # check correct encoding of P2MS script with n,k <= 16 + normal_ms_script = keys_to_multisig_script([fake_pubkey]*16, k=15) + self.assertEqual(len(normal_ms_script), 1 + 16*34 + 1 + 1) + self.assertTrue(normal_ms_script.startswith(bytes([OP_15]))) + self.assertTrue(normal_ms_script.endswith(bytes([OP_16, OP_CHECKMULTISIG]))) + + # check correct encoding of P2MS script with n,k > 16 + max_ms_script = keys_to_multisig_script([fake_pubkey]*20, k=19) + self.assertEqual(len(max_ms_script), 2 + 20*34 + 2 + 1) + self.assertTrue(max_ms_script.startswith(bytes([1, 19]))) # using OP_PUSH1 + self.assertTrue(max_ms_script.endswith(bytes([1, 20, OP_CHECKMULTISIG]))) From 5cf0a1f230389ef37e0ff65de5fc98394f32f60c Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Wed, 23 Aug 2023 23:34:43 +0200 Subject: [PATCH 3/3] test: add `createmultisig` P2MS encoding test for all n (1..20) --- test/functional/rpc_createmultisig.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/functional/rpc_createmultisig.py b/test/functional/rpc_createmultisig.py index fdac3623d3c..37656341d26 100755 --- a/test/functional/rpc_createmultisig.py +++ b/test/functional/rpc_createmultisig.py @@ -12,6 +12,7 @@ from test_framework.address import address_to_scriptpubkey from test_framework.descriptors import descsum_create, drop_origins from test_framework.key import ECPubKey from test_framework.messages import COIN +from test_framework.script_util import keys_to_multisig_script from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_raises_rpc_error, @@ -69,6 +70,16 @@ class RpcCreateMultiSigTest(BitcoinTestFramework): # Check that bech32m is currently not allowed assert_raises_rpc_error(-5, "createmultisig cannot create bech32m multisig addresses", self.nodes[0].createmultisig, 2, self.pub, "bech32m") + self.log.info('Check correct encoding of multisig script for all n (1..20)') + for nkeys in range(1, 20+1): + keys = [self.pub[0]]*nkeys + expected_ms_script = keys_to_multisig_script(keys, k=nkeys) # simply use n-of-n + # note that the 'legacy' address type fails for n values larger than 15 + # due to exceeding the P2SH size limit (520 bytes), so we use 'bech32' instead + # (for the purpose of this encoding test, we don't care about the resulting address) + res = self.nodes[0].createmultisig(nrequired=nkeys, keys=keys, address_type='bech32') + assert_equal(res['redeemScript'], expected_ms_script.hex()) + def check_addmultisigaddress_errors(self): if self.options.descriptors: return