Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stdscript: Add num required sigs support. #2805

Merged
merged 2 commits into from Nov 17, 2021

Conversation

davecgh
Copy link
Member

@davecgh davecgh commented Nov 14, 2021

This is rebased on #2803.

This adds support to the stdscript package for determining the number of signatures required by a given script version and script for the known standard types along with an implementation for version 0 scripts.

Full test coverage is included.

Note that this functionality is currently part of txscript.ExtractPkScriptAddrs, but that is going to be removed in favor of the new stdscript package in upcoming PRs so it needs to be made available prior to that.

Aside from that, the primary reasons for splitting it out from address extraction are:

  • The number of required signatures is only used in a small number of cases, specifically for RPC results and signing, while all the other cases end up ignoring it
  • Providing separate analysis methods results in an API that is better able to deal with multiple script versions without requiring major module version bumps
BenchmarkDetermineRequiredSigs
------------------------------
v0_complex_non_standard                        31580193    33.47 ns/op   0 B/op   0 allocs/op
malformed_v0_script_that_does_not_parse        35297439    33.45 ns/op   0 B/op   0 allocs/op
v0_p2pk-ecdsa-secp256k1_uncompressed          135813928     8.72 ns/op   0 B/op   0 allocs/op
v0_p2pk-ed25519                               100000000    11.17 ns/op   0 B/op   0 allocs/op
v0_p2pk-schnorr-secp256k1_compressed_even      68248903    17.14 ns/op   0 B/op   0 allocs/op
v0_p2pkh-ecdsa-secp256k1                       85467650    14.27 ns/op   0 B/op   0 allocs/op
v0_p2pkh-ed25519                               54437568    21.10 ns/op   0 B/op   0 allocs/op
v0_p2pkh-schnorr-secp256k1                     49283944    24.22 ns/op   0 B/op   0 allocs/op
v0_p2sh                                        58491218    19.14 ns/op   0 B/op   0 allocs/op
v0_multisig_1-of-1_compressed_pubkey           10233825   118.70 ns/op   0 B/op   0 allocs/op
v0_nulldata_no_data_push                       42779635    26.65 ns/op   0 B/op   0 allocs/op
v0_stake_submission_p2pkh-ecdsa-secp256k1      41388444    27.88 ns/op   0 B/op   0 allocs/op
v0_stake_submission_p2sh                       40624120    29.19 ns/op   0 B/op   0 allocs/op
v0_stake_gen_p2pkh-ecdsa-secp256k1             39675847    29.56 ns/op   0 B/op   0 allocs/op
v0_stake_gen_p2sh                              35297232    30.90 ns/op   0 B/op   0 allocs/op
v0_stake_revoke_p2pkh-ecdsa-secp256k1          38717295    30.44 ns/op   0 B/op   0 allocs/op
v0_stake_revoke_p2sh                           36837500    33.05 ns/op   0 B/op   0 allocs/op
v0_stake_change_p2pkh-ecdsa-secp256k1          36365949    32.56 ns/op   0 B/op   0 allocs/op
v0_stake_change_p2sh                           35291002    34.11 ns/op   0 B/op   0 allocs/op
v0_treasury_add                                37483249    32.74 ns/op   0 B/op   0 allocs/op
v0_treasury_generation_p2pkh-ecdsa-secp256k1   34267209    34.95 ns/op   0 B/op   0 allocs/op
v0_treasury_generation_p2sh                    33676647    35.41 ns/op   0 B/op   0 allocs/op

@davecgh davecgh force-pushed the stdscript_determine_required_sigs branch from a085dce to c0d509c Compare November 15, 2021 23:27
Copy link
Member

@matheusd matheusd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double checked that the required sig count from the old ExtractPkScriptAddrs and the new DetermineRequiredSigs are the same (modulo 2 multisig cases that are non-standard anyway).

@davecgh
Copy link
Member Author

davecgh commented Nov 16, 2021

... (modulo 2 multisig cases that are non-standard anyway).

Right. The new stdscript standardness checks have been tightened so that multisigs that contain malformed public keys are no longer considered standard, so those will now require 0 signatures, as is the case with all non standard scripts.

Thanks for confirming.

This adds support for determining the number of signatures required by a
given script version and script for the known standard types along with
an implementation for version 0 scripts.

Full test coverage is included.
BenchmarkDetermineRequiredSigs
------------------------------
v0_complex_non_standard                        31580193    33.47 ns/op   0 B/op   0 allocs/op
malformed_v0_script_that_does_not_parse        35297439    33.45 ns/op   0 B/op   0 allocs/op
v0_p2pk-ecdsa-secp256k1_uncompressed          135813928     8.72 ns/op   0 B/op   0 allocs/op
v0_p2pk-ed25519                               100000000    11.17 ns/op   0 B/op   0 allocs/op
v0_p2pk-schnorr-secp256k1_compressed_even      68248903    17.14 ns/op   0 B/op   0 allocs/op
v0_p2pkh-ecdsa-secp256k1                       85467650    14.27 ns/op   0 B/op   0 allocs/op
v0_p2pkh-ed25519                               54437568    21.10 ns/op   0 B/op   0 allocs/op
v0_p2pkh-schnorr-secp256k1                     49283944    24.22 ns/op   0 B/op   0 allocs/op
v0_p2sh                                        58491218    19.14 ns/op   0 B/op   0 allocs/op
v0_multisig_1-of-1_compressed_pubkey           10233825   118.70 ns/op   0 B/op   0 allocs/op
v0_nulldata_no_data_push                       42779635    26.65 ns/op   0 B/op   0 allocs/op
v0_stake_submission_p2pkh-ecdsa-secp256k1      41388444    27.88 ns/op   0 B/op   0 allocs/op
v0_stake_submission_p2sh                       40624120    29.19 ns/op   0 B/op   0 allocs/op
v0_stake_gen_p2pkh-ecdsa-secp256k1             39675847    29.56 ns/op   0 B/op   0 allocs/op
v0_stake_gen_p2sh                              35297232    30.90 ns/op   0 B/op   0 allocs/op
v0_stake_revoke_p2pkh-ecdsa-secp256k1          38717295    30.44 ns/op   0 B/op   0 allocs/op
v0_stake_revoke_p2sh                           36837500    33.05 ns/op   0 B/op   0 allocs/op
v0_stake_change_p2pkh-ecdsa-secp256k1          36365949    32.56 ns/op   0 B/op   0 allocs/op
v0_stake_change_p2sh                           35291002    34.11 ns/op   0 B/op   0 allocs/op
v0_treasury_add                                37483249    32.74 ns/op   0 B/op   0 allocs/op
v0_treasury_generation_p2pkh-ecdsa-secp256k1   34267209    34.95 ns/op   0 B/op   0 allocs/op
v0_treasury_generation_p2sh                    33676647    35.41 ns/op   0 B/op   0 allocs/op
@davecgh davecgh force-pushed the stdscript_determine_required_sigs branch from a92e9de to 69a4b62 Compare November 17, 2021 00:20
@davecgh davecgh merged commit 69a4b62 into decred:master Nov 17, 2021
@davecgh davecgh deleted the stdscript_determine_required_sigs branch November 17, 2021 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants