Page MenuHomePhabricator

remove effect of SCRIPT_ENABLE_SCHNORR flag
ClosedPublic

Authored by markblundeberg on Jun 14 2019, 04:13.

Details

Summary

Originally, the SCRIPT_ENABLE_SCHNORR flag was added first without any
effect, then later diffs put in the functionality. This Diff removes the
effect of the SCRIPT_ENABLE_SCHNORR flag, but keeps the flag around.
Some tests have to be removed accordingly since they were specifically
testing flag-off behaviour that is no longer true.

(effectively this is a backing-out of D2455 and D2469)

Depends on D3253

Test Plan

make check
test_runner.py --extended

  • IBD with -checkpoints=0 and -assumevalid=0 on testnet
  • IBD with -checkpoints=0 and -assumevalid=0 on mainnet

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.Jun 14 2019, 04:13
Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 14 2019, 04:13
jasonbcox added inline comments.
src/script/sigencoding.cpp
180 ↗(On Diff #9425)

The double-negative doesn't help. I think the parentheses can be reworded or removed all together.

markblundeberg edited the test plan for this revision. (Show Details)Jun 21 2019, 16:00

simple rebase

update per comment

markblundeberg marked an inline comment as done.Jun 21 2019, 16:09
markblundeberg added a comment.EditedJun 23 2019, 16:17

Note: in principle in this Diff, I could take out the two remaining *uses* of this flag as well, from MANDATORY_SCRIPT_VERIFY_FLAGS and from GetNextBlockScriptFlags (aka GetBlockScriptFlags). At the moment they are removed in the follow-up diff D3332.

If I did this, then D3332 would be basically a pure test-cleanup diff that quite manifestly doesn't touch any consensus code.

Note: in principle in this Diff, I could take out the two remaining *uses* of this flag as well, from MANDATORY_SCRIPT_VERIFY_FLAGS and from GetNextBlockScriptFlags (aka GetBlockScriptFlags). At the moment they are removed in the follow-up diff D3332.
If I did this, then D3332 would be basically a pure test-cleanup diff that quite manifestly doesn't touch any consensus code.

Yeah, that's what I would do, basically just squash D3331 and D3332 together. Seems unnecessary to have a mid-state with a flag that does nothing.

On the other hand, it's not very important, so whatever way others prefer is fine with me.

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