HomePhabricator

fix scriptSig analysis in sign.cpp

Description

fix scriptSig analysis in sign.cpp

Summary:
This is used for analyzing partially unsigned transactions, and exhibits
weird behaviour where it allows any opcodes (including CPU-intensive ones)
to be run, and then it analyses the aftermath.

Also, this is a misuse of MANDATORY_SCRIPT_VERIFY_FLAGS as it here we
shouldn't care about how invalid transactions are sometimes banned/not
banned in mempool acceptance. In Core they have STRICTENC here which
is pretty weird, since it doesn't really have any effect (invalidly
encoded signatures still get through since they are just pushed).

This changes the evaluation to only proceed if the script is properly
push-only, and uses neutral flags to build a stack out of the push
opcodes.

(In principle we could enforce MINIMALDATA, but, it doesn't hurt to
allow nonstandard pushes since all the pushes will get canonicalized by
PushAll after signing.)

Test Plan: ninja check-all

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D4907

Details

Provenance
Mark Lundeberg <markblundeberg@users.noreply.github.com>Authored on Dec 12 2019, 01:58
markblundebergPushed on Jan 13 2020, 00:15
Reviewer
Restricted Project
Differential Revision
D4907: fix scriptSig analysis in sign.cpp
Parents
rABC0306d7c919d7: [mempool_tests] add sigop counting check in TestPackageAccounting
Branches
Unknown
Tags
Unknown

Event Timeline

Mark Lundeberg <markblundeberg@users.noreply.github.com> committed rABCc95c9b9fa75c: fix scriptSig analysis in sign.cpp (authored by Mark Lundeberg <markblundeberg@users.noreply.github.com>).Jan 13 2020, 00:15