Page MenuHomePhabricator

Rename SCRIPT_ENABLE_CHECKDATASIG to SCRIPT_VERIFY_CHECKDATASIG_SIGOPS.
ClosedPublic

Authored by Mengerian on May 25 2019, 03:28.

Details

Summary

The new name is more descriptive of what the flag actually does.

This is a straight rename with no other changes.

Test Plan

make check

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

In the review of D3100, @markblundeberg had suggested renaming the flag. This Diff does that.

src/test/checkdatasig_tests.cpp
153 ↗(On Diff #8869)

Now that the flag is renamed, this kind of thing looks silly (as it should!). :-)
Part of why I wanted to see this renaming is that it reveals all these usages.

Removing these silly parts can be done in separate Diff though.

255 ↗(On Diff #8869)

Does belong here of course...

src/test/data/script_tests.json
244 ↗(On Diff #8869)

ditto for plainly visible silliness, as with all other instances in script_tests

src/test/script_tests.cpp
39 ↗(On Diff #8869)

Oopsie! Keep this commented. :-)

src/validation.cpp
718 ↗(On Diff #8869)

Hmm this ought to have been removed in previous diff, ah well.

1663 ↗(On Diff #8869)

(this of course stays)

LGTM, except just recomment that line in script_tests.cpp .

Mengerian added inline comments.
src/test/script_tests.cpp
39 ↗(On Diff #8869)

Oops, will re-comment that

Mengerian marked an inline comment as done.

Re-comment auto-test generating #define
Rebase on master

Mengerian added a task: Restricted Maniphest Task.May 25 2019, 18:32
deadalnix requested changes to this revision.May 26 2019, 20:00
deadalnix added inline comments.
src/test/data/script_tests.json
244 ↗(On Diff #8876)

No need to rename flags in there.

src/test/scriptflags.cpp
34 ↗(On Diff #8876)

No need to rename this flag

This revision now requires changes to proceed.May 26 2019, 20:00
Mengerian edited the summary of this revision. (Show Details)

Don't rename flag in script tests.
Rebase on master.

This revision is now accepted and ready to land.May 27 2019, 14:30