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
Branch
remschnorr2
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 6333
Build 10713: Bitcoin ABC Buildbot (legacy)
Build 10712: arc lint + arc unit

Event Timeline

jasonbcox added inline comments.
src/script/sigencoding.cpp
180

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

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.

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