Page MenuHomePhabricator
Differential D3063

Make tests independent of whether CHECKDATASIG is included in mandatory or standard flags
ClosedPublic
Actions

Authored by Mengerian on May 17 2019, 16:49.

Details

Reviewers
deadalnix
Fabien
markblundeberg
Group Reviewers
Restricted Project
Maniphest Tasks
T653: Clean up past upgrades
Commits
rSTAGINGe048372368b8: Make tests independent of whether CHECKDATASIG is included in mandatory or…
rABCe048372368b8: Make tests independent of whether CHECKDATASIG is included in mandatory or…
Summary

Currently, in addition to testing the functioning of CHECKDATASIG,
sigopcount_tests.cpp and checkdatasig_tests.cpp implicitly test that
CHECKDATASIG is not included in standard or mandatory flags.

Change this to make the CHECKDATASIG tests independent of whether CHECKDATASIG
is included in standard or mandatory flags. To maintain test coverage, also
add a test to explicitly test whether CHECKDATASIG is included in standard or
mandatory flags.

Test Plan

make check - passes
Try adding CHECKDATASIG to standard flags - make check - test fails.
Try adding CHECKDATASIG to mandatory flags - make check - test fails.

Diff Detail

Event Timeline

Mengerian created this revision.May 17 2019, 16:49
Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 17 2019, 16:49
teamcity added a comment.May 17 2019, 16:50

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

Mengerian added a task: T653: Clean up past upgrades.May 17 2019, 17:01
teamcity added a comment.May 17 2019, 17:04

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

Harbormaster completed remote builds in B5883: Diff 8714.May 17 2019, 17:04
Mengerian abandoned this revision.May 17 2019, 17:16

I realized it makes more sense to combine the code and test changes together in one Diff.

Mengerian updated this revision to Diff 8752.May 20 2019, 20:59

Add test for CHECKDATASIG inclusion in standard or mandatory flags.

teamcity added a comment.May 20 2019, 21:00

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

Mengerian retitled this revision from Make tests independent of whether CHECKDATASIG is included in flag set to Make tests independent of whether CHECKDATASIG is included in mandatory or standard flags.May 20 2019, 21:02
Mengerian edited the summary of this revision. (Show Details)
Mengerian edited the test plan for this revision. (Show Details)
Mengerian added a reviewer: markblundeberg.
Mengerian mentioned this in D3069: Add CHECKDATASIG to standard flags..May 20 2019, 21:06
Harbormaster completed remote builds in B5910: Diff 8752.May 20 2019, 21:15
teamcity added a comment.May 20 2019, 21:15

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

Mengerian added a child revision: D3069: Add CHECKDATASIG to standard flags..May 20 2019, 21:16
markblundeberg added a comment.May 20 2019, 22:37

I'm happy with this general direction. Given the severity of consequences of not including it in standardness, the checkdatasig_inclusion_in_standard_and_mandatory_flags is nice to have as a fool-proof check. :)

src/test/checkdatasig_tests.cpp
260 ↗(On Diff #8752)

nit: I would have done
BOOST_CHECK(!(STANDARD_SCRIPT_VERIFY_FLAGS & SCRIPT_ENABLE_CHECKDATASIG))

Mengerian updated this revision to Diff 8762.May 20 2019, 23:21

Simplify boolean logic per Mark's suggestion.

teamcity added a comment.May 20 2019, 23:22

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

Harbormaster failed remote builds in B5913: Diff 8762!May 20 2019, 23:27
teamcity added a comment.May 20 2019, 23:27

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

Mengerian planned changes to this revision.May 21 2019, 00:01

need to fix build

Mengerian updated this revision to Diff 8769.May 21 2019, 01:48

Fix stupid mistake.

teamcity added a comment.May 21 2019, 01:49

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

teamcity added a comment.May 21 2019, 02:03

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

Harbormaster completed remote builds in B5918: Diff 8769.May 21 2019, 02:03
deadalnix accepted this revision.May 21 2019, 21:19
This revision is now accepted and ready to land.May 21 2019, 21:19
Closed by commit rABCe048372368b8: Make tests independent of whether CHECKDATASIG is included in mandatory or… (authored by Mengerian). · Explain WhyMay 21 2019, 23:15
This revision was automatically updated to reflect the committed changes.
Mengerian added a commit: rABCe048372368b8: Make tests independent of whether CHECKDATASIG is included in mandatory or….
Mengerian added a commit: rSTAGINGe048372368b8: Make tests independent of whether CHECKDATASIG is included in mandatory or….Sep 18 2019, 23:40
markblundeberg mentioned this in D4679: add SCRIPT_VERIFY_MINIMALDATA to mandatory flags.Dec 12 2019, 09:25

Revision Contents
Changeset List

PathSize
src/
test/
11 lines
32 lines

Diff 8769

View Options

src/test/checkdatasig_tests.cpp

Loading...
View Options

src/test/sigopcount_tests.cpp

Loading...