Page MenuHomePhabricator

Revert "add flags to VerifySignature and sigcache"
AcceptedPublic

Authored by markblundeberg on Fri, Jun 7, 04:35.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Maniphest Tasks
T653: Clean up past upgrades
Summary

This reverts D2373, which was only done in the first place so that a
pre-activation 64-byte ECDSA signature could not fill the cache to
masquerade as a Schnorr signature post-activation. Now that the
upgrade is past and enable_schnorr flag is gone, this extra technical
complication is pointless (and it is unlikely to ever be useful again).

Depends on D3332

Test Plan

make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
revsigcache
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6336
Build 10719: Bitcoin ABC Teamcity Staging
Build 10718: arc lint + arc unit

Event Timeline

markblundeberg created this revision.Fri, Jun 7, 04:35
Herald added a reviewer: Restricted Project. · View Herald TranscriptFri, Jun 7, 04:35
markblundeberg edited the summary of this revision. (Show Details)Fri, Jun 7, 04:40
jasonbcox added inline comments.
src/script/sigcache.cpp
53 ↗(On Diff #9235)

I realize this ordering is going back to what it used to be. But that begs the question why it was changed in the first place?

markblundeberg added inline comments.Fri, Jun 7, 20:15
src/script/sigcache.cpp
53 ↗(On Diff #9235)

IIRC I changed it in order to match the order of its caller, VerifySignature. Perhaps we can keep that order, just take out flags.

An alternative order that would also make sense is hash,pubkey,sig, which would match order of hashing.

deadalnix accepted this revision.Wed, Jun 12, 17:15

This is correct as long as we "pretend" schnorr always was activated.

This revision is now accepted and ready to land.Wed, Jun 12, 17:15
markblundeberg edited the summary of this revision. (Show Details)Fri, Jun 14, 04:47
markblundeberg updated this revision to Diff 9428.

rebased onto split parent