[script_tests] improve test coverage of minimal push rules
ClosedPublic
Actions

Authored by markblundeberg on Jun 16 2019, 16:24.

Details

Reviewers

deadalnix
jasonbcox
Fabien
Mengerian

Group Reviewers

Restricted Project

Commits

rSTAGING734897bad886: [script_tests] improve test coverage of minimal push rules
rABC734897bad886: [script_tests] improve test coverage of minimal push rules

Summary

It looks like the MINIMALDATA flag is destined to be a consensus rule so
it needs to be tested properly.

Some of the features of MINIMALDATA push restrictions were not being tested fully, namely there apparently were only tests for scriptSig.

Test Plan

make check

Diff Detail

Repository

rABC Bitcoin ABC

Branch

arcpatch-D3350

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 6846
Build 11739: Bitcoin ABC Buildbot (legacy)
Build 11738: arc lint + arc unit

Event Timeline

markblundeberg created this revision.Jun 16 2019, 16:24

Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 16 2019, 16:24

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

markblundeberg planned changes to this revision.Jun 16 2019, 16:24

oops, some tests duplicated

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B6371: Diff 9482.Jun 16 2019, 16:31

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B6372: Diff 9483.Jun 16 2019, 16:37

markblundeberg edited the summary of this revision. (Show Details)Jun 17 2019, 03:01

What about including the script_tests.cpp changes in the script_build test?

In D3350#79433, @jasonbcox wrote:

What about including the script_tests.cpp changes in the script_build test?

In this case they're pure json tests; the script_build C++ code only builds the subset of json tests that go in the 'Automatically generated test cases' section, further down in the json file, and it only really makes sense to use for tests that involve tx signatures since it's a bit heavy for the compiler to build them.

src/test/data/script_tests.json
1642 ↗	(On Diff #9483)	(down here)

jasonbcox requested changes to this revision.Jun 24 2019, 19:16

jasonbcox added inline comments.

src/test/data/script_tests.json
1515 ↗	(On Diff #9483)	Instead of separating this from the similar test section above, it could be included under its scriptSig variant: `["0x01 0x81", "DROP 1"...` In a similar vein, all of the existing scriptSig variants should be tested as scriptPubKey variants as well.