Page MenuHomePhabricator

[script_tests] improve coverage of minimal number encoding
ClosedPublic

Authored by markblundeberg on Jun 15 2019, 00:28.

Details

Summary

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

A couple of sections are labelled "Test every numeric-accepting opcode
for correct handling of the numeric minimal encoding rule" and a few
number-accepting opcodes have been added since then, so for completeness
the tests now include all number-accepting opcodes. Some of these are
redundant but it's good to have them all collected in one place.

(Also there was an oversight in one OP_NUM2BIN test which supposedly failed
due to nonminimal number, but actually failed due to too-long number. With
a shorter non-minimal number the test actually passes.)

Test Plan

make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
patch
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 6362
Build 10771: Bitcoin ABC Buildbot (legacy)
Build 10770: arc lint + arc unit

Event Timeline

Also I find it a bit odd that we have a known error that triggers UNKNOWN_ERROR (this is because it works by throwing an exception instead of doing the usual s_error mechanism). Would be good to make it clearer, but that can be a separate diff.

This revision is now accepted and ready to land.Jun 25 2019, 20:25