Option to consider bare multisig scripts in TX outputs non-standard #3939
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)
7 participants
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.