Page MenuHomePhabricator

document MANDATORY_SCRIPT_VERIFY_FLAGS accurately
ClosedPublic

Authored by markblundeberg on Dec 9 2019, 22:37.

Details

Summary

Currently MANDATORY_SCRIPT_VERIFY_FLAGS is used in weird ways and also
its documentation is a misleading lie. I'd like to fix the craziness,
which starts with documenting it accurately.

Test Plan

read

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markblundeberg created this revision.Dec 9 2019, 22:37
Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 9 2019, 22:37
deadalnix requested changes to this revision.Dec 10 2019, 03:48
deadalnix added inline comments.
src/script/standard.h
55 ↗(On Diff #14708)

I don't think a listing of where this is used is a good idea ever. These place will be changed over time, but this comment will stay, providing completely misleading information.

The main purpose of this set of flag is that, if you receive a transaction that is not valid as per one of these, then you can ban the peer.

This revision now requires changes to proceed.Dec 10 2019, 03:48
markblundeberg added inline comments.Dec 10 2019, 03:57
src/script/standard.h
55 ↗(On Diff #14708)

Yeah my plan is to fix all those other places which are misusing these flags, really it should only be used for a single purpose which is the one you describe (the first one in my list). In fact I'd suggest we don't even need this flagset, really we just ought to pay attention to what is in STANDARD_NOT_MANDATORY_VERIFY_FLAGS.

Nonetheless, the current comment describing this flag is simply wrong.

smaller comment

deadalnix accepted this revision.Sun, Jan 12, 14:02
This revision is now accepted and ready to land.Sun, Jan 12, 14:02