Details

Reviewers

deadalnix
Mengerian

Group Reviewers

Restricted Project

Maniphest Tasks

T653: Clean up past upgrades

Commits

rSTAGING0fdba130937b: remove effect of SCRIPT_ENABLE_SCHNORR flag
rABC0fdba130937b: remove effect of SCRIPT_ENABLE_SCHNORR flag

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

markblundeberg created this revision.Jun 14 2019, 04:13

Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 14 2019, 04:13

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

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

Harbormaster completed remote builds in B6333: Diff 9425.Jun 14 2019, 04:22

markblundeberg added a task: T653: Clean up past upgrades.Jun 14 2019, 04:23

markblundeberg added a child revision: D3332: remove SCRIPT_ENABLE_SCHNORR flag and clean up tests.Jun 14 2019, 04:25

markblundeberg mentioned this in D3253: Remove Schnorr activation.Jun 14 2019, 04:26

markblundeberg added a child revision: D3264: sigencoding_tests: improve test coverage.Jun 14 2019, 04:35

jasonbcox added a subscriber: jasonbcox.Jun 15 2019, 00:18

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.

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

simple rebase

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

update per comment

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

markblundeberg marked an inline comment as done.Jun 21 2019, 16:09

Harbormaster completed remote builds in B6428: Diff 9567.Jun 21 2019, 16:16

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

Harbormaster completed remote builds in B6429: Diff 9568.Jun 21 2019, 16:19

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.

In D3331#80339, @markblundeberg wrote:

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

Closed by commit rABC0fdba130937b: remove effect of SCRIPT_ENABLE_SCHNORR flag (authored by Mark Lundeberg <36528214+markblundeberg@users.noreply.github.com>). · Explain WhyJun 26 2019, 00:52

This revision was automatically updated to reflect the committed changes.

Mark Lundeberg <36528214+markblundeberg@users.noreply.github.com> added a commit: rABC0fdba130937b: remove effect of SCRIPT_ENABLE_SCHNORR flag.

Mark Lundeberg <36528214+markblundeberg@users.noreply.github.com> added a commit: rSTAGING0fdba130937b: remove effect of SCRIPT_ENABLE_SCHNORR flag.Sep 19 2019, 00:33