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: Introduce pkg infra for std scripts. #2656
Conversation
82e4066
to
a715dd4
Compare
Just reviewing the Readme and trying to make sure I understand the basics correctly.
If I'm not grossly misunderstanding things, then I don't have any critical feedback. It's well-written! The main question I had while reading it was about Thanks for your work! |
My answer does not purport to be authoritative, @davecgh will still need to answer, but I will note the following chaincfg parameter: Lines 351 to 353 in 70483c8
It is false on mainnet, and true on all the other networks. Miners are free to include transactions with non-standard output scripts, but I believe the above answers your question w.r.t. relay. A miner is unlikely to be relayed such a txn, but they can include them if they want. Stakeholder approval probably doesn't enter into it unless some set of stakeholders have defined some custom voting policy that I'm not aware of. |
@matthawkins90 Thanks for the feedback. Your summary is pretty good. I'd probably just add that they're standard too because they're useful and widely used for everyday actions like sending coins. What @chappjc said is all correct.
Mainnet nodes will refuse to accept and relay transactions with them to other nodes. In practice, this means it is extremely difficult to get them mined unless you work directly with a miner who is willing to accept them because the miner will typically never see them if they don't relay through the network. Voting doesn't enter into it at all. Miners are free to include any scripts that are valid by consensus.
I don't believe there is a graphical way in the explorer, but it's pretty easy to write a SQL query against the database that powers dcrdata if you have it setup locally to get that information. That said, I should also note that none of this prevents more complex redeem scripts (the target of a pay-to-script-hash script which is standard) with arbitrary constraints and opcodes from being relayed and mined even though they would be non standard as public key scripts. The difference really boils down to the fact that non-standard scripts (public key scripts, remember) could otherwise be used to play games for free, whereas it actually costs money to try and play such games with redeem scripts and similarly they can't be used to DDoS because you can't mutate them after the fact. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Partial review, will finish later in the day. Looks great so far 👍
3f840ac
to
da78957
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work as always, well documented with extensive test coverage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great to me. I just had a few minor stylistic/typo comments inline. The documentation is excellent, and I found the API to be intuitive and straightforward to follow.
da78957
to
29f862f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
In the comment of commit d001101 there is a trailing first multisig
that doesn't look intended to me.
29f862f
to
61f73a6
Compare
Nice catch. It was leftover from a squash. Fixed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woop! Tested all commits via run_tests.sh
.
Similar to the code for handling standard addresses prior to the recent updates which implemented the stdaddr package, the current code for handling standard scripts implemented in txscript was written many years ago prior a wide variety of changes and several new features added by Decred. As a result, it entirely lacks support for some features and supports others in a roundabout and non-intuitive way. Specifically, it does not support or provide a clean path to enable support for different script versions. Finally, the code being implemented in txscript (which was necessary back when the original code was implemented, but no longer is due to changes since) has led to confusion regarding what is considered standard and what is considered consensus which has tripped up several contributors over the years. This is part of a series of commits that aims to resolve all of the aforementioned issues by introducing a new package named stdscript which reworks the way standard scripts are handled. For the time being, the package is introduced into the internal staging area for initial review. The following provides an overview of some of the key features of the new design: - Supports versioned scripts - Provides the ability to efficiently extract version-specific relevant data, such as public keys, public keys hashes, and script hashes from standard scripts - Allows callers to choose between using version-specific or dynamic type determination methods - Implements several new types, as compared to the existing code, which leads to an easier to use API - Clearly denotes that these scripts are a standardized construction the must not be used directly in consensus code - Provides additional convenience methods for working with typical version 0 atomic swap and multisignature scripts In order to help ease the review process, this commit only contains the overall generic infrastructure without adding support for any specific script types. Each supported version 0 script type will be added in future commits.
This adds support for detecting version 0 pay-to-pubkey-ecdsa-secp256k1 scripts as standard and extracting the associated public key. Both compressed and uncompressed keys are supported. Full test coverage is included. This is part of a series of commits to fully implement the stdscript package.
BenchmarkIsPubKeyScript ----------------------- v0_complex_non_standard 235694011 5.058 ns/op 0 allocs/op v0_p2pk-ecdsa-secp256k1_uncompressed 248714095 4.550 ns/op 0 allocs/op v0_p2pk-ecdsa-secp256k1_compressed_even 268054593 4.481 ns/op 0 allocs/op v0_p2pk-ecdsa-secp256k1_compressed_odd 269792724 4.472 ns/op 0 allocs/op This is part of a series of commits to fully implement the stdscript package.
This adds support for detecting version 0 pay-to-pubkey-ed25519 scripts as standard and extracting the associated public key. Full test coverage is included. This is part of a series of commits to fully implement the stdscript package.
BenchmarkIsPubKeyEd25519Script ------------------------------ v0_complex_non_standard 276328269 4.445 ns/op 0 allocs/op v0_p2pk-ed25519 205478853 5.837 ns/op 0 allocs/op This is part of a series of commits to fully implement the stdscript package.
This adds support for detecting version 0 pay-to-pubkey-schnorr-secp256k1 scripts as standard and extracting the associated public key. Note that consensus only allows compressed public keys for this signature type so only compressed public keys are detected and extracted. Full test coverage is included. This is part of a series of commits to fully implement the stdscript package.
BenchmarkIsPubKeySchnorrSecp256k1Script --------------------------------------- v0_complex_non_standard 182614185 6.565 ns/op 0 allocs/op v0_p2pk-schnorr-secp256k1_compressed_even 138353696 8.369 ns/op 0 allocs/op v0_p2pk-schnorr-secp256k1_compressed_odd 138506664 8.620 ns/op 0 allocs/op This is part of a series of commits to fully implement the stdscript package.
This adds support for detecting version 0 pay-to-pubkey-hash-ecdsa-secp256k1 scripts as standard and extracting the associated public key hash. Full test coverage is included. This is part of a series of commits to fully implement the stdscript package.
BenchmarkIsPubKeyHashScript --------------------------- v0_complex_non_standard 896964224 1.289 ns/op 0 allocs/op v0_p2pkh-ecdsa-secp256k1 623920810 1.905 ns/op 0 allocs/op This is part of a series of commits to fully implement the stdscript package.
This adds support for detecting version 0 pay-to-pubkey-hash-ed25519 scripts as standard and extracting the associated public key hash along with signature type. Full test coverage is included. This is part of a series of commits to fully implement the stdscript package.
BenchmarkIsStakeSubmissionScriptHashScript ------------------------------------------ v0_complex_non_standard 930421526 1.284 ns/op 0 allocs/op v0_stake_submission_p2sh 529045244 2.273 ns/op 0 allocs/op This is part of a series of commits to fully implement the stdscript package.
This adds support for detecting version 0 stake generation pay-to-pubkey-hash scripts as standard and extracting the associated public key hash. Full test coverage is included. This is part of a series of commits to fully implement the stdscript package.
BenchmarkIsStakeGenPubKeyHashScript ----------------------------------- v0_complex_non_standard 311106744 3.842 ns/op 0 allocs/op v0_stake_gen_p2pkh-ecdsa-secp256k1 217513142 5.305 ns/op 0 allocs/op This is part of a series of commits to fully implement the stdscript package.
This adds support for detecting version 0 stake generation pay-to-script-hash scripts as standard and extracting the associated script hash. Full test coverage is included. This is part of a series of commits to fully implement the stdscript package.
BenchmarkIsStakeGenScriptHashScript ----------------------------------- v0_complex_non_standard 748967828 1.612 ns/op 0 allocs/op v0_stake_gen_p2sh 536599674 2.234 ns/op 0 allocs/op This is part of a series of commits to fully implement the stdscript package.
This adds support for detecting version 0 stake revocation pay-to-pubkey-hash scripts as standard and extracting the associated public key hash. Full test coverage is included. This is part of a series of commits to fully implement the stdscript package.
BenchmarkIsStakeRevocationPubKeyHashScript ------------------------------------------ v0_complex_non_standard 311770504 3.842 ns/op 0 allocs/op v0_stake_revoke_p2pkh-ecdsa-secp256k1 228088618 5.316 ns/op 0 allocs/op This is part of a series of commits to fully implement the stdscript package.
This adds support for detecting version 0 stake revocation pay-to-script-hash scripts as standard and extracting the associated script hash. Full test coverage is included. This is part of a series of commits to fully implement the stdscript package.
BenchmarkIsStakeRevocationScriptHashScript ------------------------------------------ v0_complex_non_standard 725635868 1.631 ns/op 0 allocs/op v0_stake_revoke_p2sh 522431919 2.571 ns/op 0 allocs/op This is part of a series of commits to fully implement the stdscript package.
This adds support for detecting version 0 stake change pay-to-pubkey-hash scripts as standard and extracting the associated public key hash. Full test coverage is included. This is part of a series of commits to fully implement the stdscript package.
BenchmarkIsStakeChangePubKeyHash -------------------------------- v0_complex_non_standard 311063840 3.794 ns/op 0 allocs/op v0_stake_change_p2pkh-ecdsa-secp256k1 228116065 5.355 ns/op 0 allocs/op This is part of a series of commits to fully implement the stdscript package.
This adds support for detecting version 0 stake change pay-to-script-hash scripts as standard and extracting the associated script hash. Full test coverage is included. This is part of a series of commits to fully implement the stdscript package.
BenchmarkIsStakeChangeScriptHash -------------------------------- v0_complex_non_standard 688796947 1.626 ns/op 0 allocs/op v0_stake_change_p2sh 503956689 2.282 ns/op 0 allocs/op This is part of a series of commits to fully implement the stdscript package.
This adds support for detecting version 0 treasury add scripts as standard. Full test coverage is included. This is part of a series of commits to fully implement the stdscript package.
BenchmarkIsTreasuryAddScript ---------------------------- v0_complex_non_standard 905938653 1.328 ns/op 0 allocs/op v0_treasury_add 1000000000 0.9594 ns/op 0 allocs/op This is part of a series of commits to fully implement the stdscript package.
This adds support for detecting version 0 treasury generation pay-to-pubkey-hash scripts as standard and extracting the associated public key hash. Full test coverage is included. This is part of a series of commits to fully implement the stdscript package.
BenchmarkIsTreasuryGenPubKeyHashScript -------------------------------------- v0_complex_non_standard 317927872 3.843 ns/op 0 allocs/op v0_treasury_gen_p2pkh-ecdsa-secp256k1 225968358 5.261 ns/op 0 allocs/op This is part of a series of commits to fully implement the stdscript package.
This adds support for detecting version 0 treasury generation pay-to-script-hash scripts as standard and extracting the associated script hash. Full test coverage is included. This is part of a series of commits to fully implement the stdscript package.
BenchmarkIsTreasuryGenScriptHashScript -------------------------------------- v0_complex_non_standard 814816624 1.310 ns/op 0 allocs/op v0_treasury_generation_p2sh 520297904 2.274 ns/op 0 allocs/op This is part of a series of commits to fully implement the stdscript package.
BenchmarkDetermineScriptType ---------------------------- v0_complex_non_standard 23581336 49.37 ns/op 0 allocs/op v0_p2pk-ecdsa-secp256k1_uncomp 168650203 7.077 ns/op 0 allocs/op v0_p2pk-ed25519 85811743 14.22 ns/op 0 allocs/op v0_p2pk-schnorr-secp256k1_comp_even 66687412 17.95 ns/op 0 allocs/op v0_p2pkh-ecdsa-secp256k1 79932855 15.51 ns/op 0 allocs/op v0_p2pkh-ed25519 49981048 24.53 ns/op 0 allocs/op v0_p2pkh-schnorr-secp256k1 37498006 32.34 ns/op 0 allocs/op v0_p2sh 52141040 23.39 ns/op 0 allocs/op v0_multisig_1-of-1_compressed_pubkey 13393005 88.78 ns/op 0 allocs/op v0_nulldata_no_data_push 40623846 30.05 ns/op 0 allocs/op v0_stake_sub_p2pkh-ecdsa-secp256k1 35731618 35.09 ns/op 0 allocs/op v0_stake_sub_p2sh 32320881 37.48 ns/op 0 allocs/op v0_stake_gen_p2pkh-ecdsa-secp256k1 31518481 37.78 ns/op 0 allocs/op v0_stake_gen_p2sh 29044929 41.45 ns/op 0 allocs/op v0_stake_revoke_p2pkh-ecdsa-secp256k1 26891238 40.93 ns/op 0 allocs/op v0_stake_revoke_p2sh 25999124 46.65 ns/op 0 allocs/op v0_stake_change_p2pkh-ecdsa-secp256k1 27856704 44.25 ns/op 0 allocs/op v0_stake_change_p2sh 25236273 47.53 ns/op 0 allocs/op v0_treasury_add 28225464 42.33 ns/op 0 allocs/op v0_treasury_gen_p2pkh-ecdsa-secp256k1 22417670 53.44 ns/op 0 allocs/op v0_treasury_gen_p2sh 23529964 52.03 ns/op 0 allocs/op This is part of a series of commits to fully implement the stdscript package.
This adds a convenience method to create a standard version 0 ecdsa multisignature redemption script that requires a threshold of signatures from a set of public keys. Full test coverage is included. This is part of a series of commits to fully implement the stdscript package.
This adds support for identifying if a redeem script is a version 0 atomic swap redeem script for an original pay-to-script-hash script as a convenience method for consumers of the package. Note that this is different than most other methods in the package in that it applies to the redeem script as opposed to the public key script and consequently there is no new script type introduced nor does the overall script determination detect it. This is the case because a redeem script is the final push of a signature script that is itself a redemption of a pay-to-script-hash script. Full test coverage is included. This is part of a series of commits to fully implement the stdscript package.
BenchmarkExtractAtomicSwapDataPushes ------------------------------------ v0_complex_not_atomic_swap 15944958 76.34 ns/op 0 B/op 0 allocs/op v0_normal_valid_atomic_swap 3660967 335.9 ns/op 96 B/op 1 allocs/op This part of a series of commits to fully implement the stdscript package.
This adds an example for determining the script type of a public key script in terms of standardness. This part of a series of commits to fully implement the stdscript package.
This adds an example for extracting the public key hash from a p2pkh-ecdsa-secp256k1 script. This part of a series of commits to fully implement the stdscript package.
This adds an example for extracting the script hash from a pay-to-script-hash script. This part of a series of commits to fully implement the stdscript package.
61f73a6
to
62950c2
Compare
Similar to the code for handling standard addresses prior to the recent updates which implemented the
stdaddr
package, the current code for handling standard scripts implemented intxscript
was written many years ago prior a wide variety of changes and several new features added by Decred. As a result, it entirely lacks support for some features and supports others in a roundabout and non-intuitive way.Specifically, it does not support or provide a clean path to enable support for different script versions.
Finally, the code being implemented in
txscript
(which was necessary back when the original code was implemented, but no longer is due to changes since) has led to confusion regarding what is considered standard and what is considered consensus which has tripped up several contributors over the years.This aims to resolve all of the aforementioned issues by introducing a new package named
stdscript
which reworks the way standard scripts are handled.It is split up into a series of commits to help ease the review process such that the initial generic infrastructure is introduced first, followed by each supported version 0 script and associated benchmark, followed by examples, and finally what I hope is a fairly complete
README.md
that describes the architecture. Comprehensive tests are included.For the time being, the package is introduced into the internal staging area for initial review.
The following provides an overview of some of the key features of the new design:
Additionally, this includes a few changes to the standardness policy versus the existing code as follows:
These changes do not affect consensus in any way since consensus independently enforces all details related to the associated opcodes as required, but they help ensure the typical standard scripts occupy less space on chain and further restrict cases that don't make sense. Further, all known software already creates scripts that conform to these more stringent requirements and has since launch, so these changes are unlikely to even be noticed in practice.
Test Coverage: