test: refactor: add `script_util` helper for creating bare multisig scripts #23305

pull theStack wants to merge 1 commits into bitcoin:master from theStack:202110-test-add_helper_for_bare_multisig_scripts changing 5 files +23 −18

theStack commented at 8:59 PM on October 18, 2021: member
This PR is a follow-up to #22363 and #23118 and introduces a helper keys_to_multisig_script for creating bare multisig outputs in the form of
```
OP_K PubKey1 PubKey2 ... PubKeyN OP_N OP_CHECKMULTISIG
```
The function takes a list of pubkeys (both hex- and byte-strings are accepted due to the script_util.check_key helper being used internally) and optionally a threshold k. If no threshold is passed, a n-of-n multisig output is created, with n being the number of passed pubkeys.
DrahtBot added the label Tests on Oct 18, 2021
practicalswift commented at 7:43 AM on October 19, 2021: contributor

Concept ACK

The canonical bare multisig construction in keys_to_multisig_script looks good. FWIW see #23285 for some non-canonical bare multisig constructions we also consider IsStandard :)
in test/functional/test_framework/script_util.py:46 in 31ca61b6c4 outdated
```
  42 | @@ -41,6 +43,17 @@ def key_to_p2pk_script(key):
  43 |      return CScript([key, OP_CHECKSIG])
  44 |  
  45 |  
  46 | +def keys_to_multisig_script(keys, *, m=None):
```
MarcoFalke commented at 8:22 AM on October 19, 2021:

nit: Could use k(-of-n)? See https://github.com/bitcoinops/bitcoinops.github.io/blob/master/STYLE.md#spelling

def keys_to_multisig_script(keys, *, k=None):
theStack commented at 11:10 AM on October 19, 2021:

Good idea, thanks for the hint. Done.
theStack force-pushed on Oct 19, 2021
theStack commented at 11:11 AM on October 19, 2021: member

Rebased on master and changed threshold parameter name from "m" to "k", to match the optech style guide (as suggested by MarcoFalke).
DrahtBot commented at 2:52 AM on October 20, 2021: member



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

No conflicts as of last run.
DrahtBot added the label Needs rebase on Oct 25, 2021
test: add script_util helper for creating bare multisig scripts 4718897ce3
theStack force-pushed on Oct 26, 2021
theStack commented at 12:53 PM on October 26, 2021: member

Rebased on master.
DrahtBot removed the label Needs rebase on Oct 26, 2021
shaavan approved
shaavan commented at 6:18 PM on October 26, 2021: contributor
utACK 4718897ce3a7c728ff7aebbadabcc8ed7a0b8d6e

This PR declares a new helper function keys_to_multisig_script in the file script_util.py, which serves two purposes:
1. Helps simplify the code; each time, a bare multisig script needs to be created.
2. Reduce the number of times different files have to import CScript and OP_CHECKMULTISIG as dependencies.
rajarshimaitra commented at 6:51 AM on October 27, 2021: contributor

tACK https://github.com/bitcoin/bitcoin/pull/23305/commits/4718897ce3a7c728ff7aebbadabcc8ed7a0b8d6e

Verified that no other bare multisig cases exists.
MarcoFalke merged this on Oct 27, 2021
MarcoFalke closed this on Oct 27, 2021
sidhujag referenced this in commit a6072b3e2d on Oct 27, 2021
DrahtBot locked this on Nov 13, 2022

Contributors

Labels

Linked (view graph)

#22363 test: refactor: use `script_util` helpers for creating P2{PKH,SH,WPKH,WSH} scripts #23118 test: refactor: introduce `script_util` helper for creating P2PK scripts #28312 test: fix `keys_to_multisig_script` (P2MS) helper for n/k > 16

test: refactor: add `script_util` helper for creating bare multisig scripts #23305

Conflicts