Option to consider bare multisig scripts in TX outputs non-standard#3939
Option to consider bare multisig scripts in TX outputs non-standard#3939laanwj merged 1 commit intobitcoin:masterfrom
Conversation
|
P2SH tests should probably be updated to avoid using bare multisig in IsStandard tests (not the intention of the test): c5127b7 |
|
rebased |
|
This option needs documentation in HelpMessage() (or is it ondocumented on purpose?) |
|
+1 @laanwj yes, needs docs |
src/script.h
Outdated
There was a problem hiding this comment.
Nit: Nowhere else we use msig, can you use multisig?
There was a problem hiding this comment.
@Diapolo I don't mind s/msig/multisig/ but I do dislike long symbols. Maybe RELAY_MULTISIG or somesuch.
There was a problem hiding this comment.
I think a SCRIPT_VERIFY flag is the wrong place for this. The other flags either are consensus critical, or may be in a plausible future soft-fork. They also are all evaluated during script execution. Multisig outputs on the other hand are just an IsStandard() rule and are evaluated via the Solver() template matcher. Additionally being purely a policy rule they would be inappropriate in a libconsensuscore library, again unlike the other script verification flags.
There was a problem hiding this comment.
Yes, IsStandard needs a separate set of flags.
|
Needs rebase and nit fix. |
…TX outputs First and foremost, this defaults to OFF. This option lets a node consider such transactions non-standard, meaning they will not be relayed or mined by default, but other miners are free to mine these as usual.
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p3939_3da434a2ef9ac76a0ad4a33921773a9ac8f10ab7/ for binaries and test log. |
|
All nits addressed. |
|
ACK |
|
Untested ACK |
1 similar comment
|
Untested ACK |
3da434a Introduce option to disable relay/mining of bare multisig scripts in TX outputs (Jeff Garzik)
First and foremost, this defaults to OFF. Perhaps consider changing the default if the community feels strongly about the issue and/or a security issue strongly encourages a change.
This option lets a node consider such transactions non-standard.
This impacts parties using CheckMultiSig for multisig and parties using CheckMultiSig for raw data transport.
This also makes a sigops-related issue harder to express in practice.
WARNING: An issue @gavinandresen raised has not yet been tested. Do not pull at this time.