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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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