Page MenuHomePhabricator

add CScript::IsMinimalPushOnly
AbandonedPublic

Authored by markblundeberg on Sun, Jun 9, 02:28.

Details

Reviewers
jasonbcox
Mengerian
deadalnix
Group Reviewers
Restricted Project
Maniphest Tasks
T667: add rule for **minimal** push only in scriptSig
Summary

Part of effort to get a scriptsig push restriction consensuse rule that is actually effective in reducing third party malleability, see T667.

Depends on D3268 D3269

Test Plan

make check

Diff Detail

Event Timeline

markblundeberg created this revision.Sun, Jun 9, 02:28
Herald added a reviewer: Restricted Project. · View Herald TranscriptSun, Jun 9, 02:28

hmmm looks like a build system failure ... failed during secp compilation.

jasonbcox requested changes to this revision.Mon, Jun 10, 17:12

Needs a summary. What's the goal here?

This revision now requires changes to proceed.Mon, Jun 10, 17:12
markblundeberg edited the summary of this revision. (Show Details)Mon, Jun 10, 17:48

Needs a summary. What's the goal here?

OK, I've added it, see T667 for extended summary of the plan.

markblundeberg requested review of this revision.Mon, Jun 10, 17:49
deadalnix requested changes to this revision.Tue, Jun 11, 23:43

There is already logic to check if a push is minimal or not, duplicating indicate the approach is wrong.

This revision now requires changes to proceed.Tue, Jun 11, 23:43
markblundeberg requested review of this revision.EditedWed, Jun 12, 00:07

There is already logic to check if a push is minimal or not, duplicating indicate the approach is wrong.

Note this function is not just for checking minimal pushes, rather it checks that a script is push-only and only uses minimal pushes.

This does reuse the existing CheckMinimalPush function, however, currently there is no consensus code that requires minimal pushes aside from BIP34 & the segwit recovery exemption to P2SH, but those use special-purpose methods instead of CheckMinimalPush. So, this adds the requisite tests to ensure that minimal push enforcement is safe for consensus layer.

Note that to enforce a restriction on scriptSig alone, in general some action needs to be done in VerifyScript (not just EvalScript) since EvalScript doesn't know whether it's running a scriptSig or scriptPubKey or whatever. And there is no need to make any consensus rules about push opcodes outside of the scriptSig.

Here are some alternative methods to the same effect:

  • During VerifyScript execution, use the consensus SCRIPTSIGMINPUSHONLY flag to force on the (non-consensus) MINIMALDATA flag, but only for the the scriptSig call to EvalScript
  • Add an additional boolean flag argument to EvalScript named fIsScriptSig or fRequireMinimalPush or something like that.
  • Split the overly-broad MINIMALDATA flag into two parts, one which deals with minimal push enforcement and one which deals with number encoding. Then, adopt the minimal push flag at consensus layer even though it unnecessarily restricts push forms in scriptPubKeys and redeemScripts.

Let me know if you prefer any of these other approaches or have a better one in mind to achieve the same goal.

markblundeberg planned changes to this revision.Thu, Jun 13, 22:31
markblundeberg abandoned this revision.Wed, Jun 19, 15:52