Page MenuHomePhabricator

[script_tests] improve test coverage of minimal push rules
ClosedPublic

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

Details

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

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

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.

This revision now requires changes to proceed.Jun 24 2019, 19:16
This revision is now accepted and ready to land.Jul 16 2019, 22:06