Page MenuHomePhabricator

add new encoding checker for Schnorr sigs
Needs RevisionPublic

Authored by markblundeberg on Sat, Jun 8, 21:46.

Details

Reviewers
deadalnix
Mengerian
Group Reviewers
Restricted Project
Maniphest Tasks
T528: Add Schnorr support to OP_CHECKMULTISIG (new mechanics)
Summary

Part of effort to add new CHECKMULTISIG mode.

Depends on D3264

Test Plan

make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
schnorrenconly
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6236
Build 10519: Bitcoin ABC Teamcity Staging
Build 10518: arc lint + arc unit

Event Timeline

markblundeberg created this revision.Sat, Jun 8, 21:46
Herald added a reviewer: Restricted Project. · View Herald TranscriptSat, Jun 8, 21:46
Mengerian added inline comments.Sat, Jun 8, 22:14
src/script/script_error.cpp
88 ↗(On Diff #9262)

And the comment something like: "Signature must be validly encoded Schnorr, or null, in this operation".

It's not really checking for ECDSA, but that it's a validly encoded Schnorr.

src/script/script_error.h
66 ↗(On Diff #9262)

I think it would be better to call it something like "SCRIPT_ERR_SIG_BADSCHNORR".

markblundeberg planned changes to this revision.Sat, Jun 8, 23:21
markblundeberg added inline comments.
src/script/script_error.cpp
88 ↗(On Diff #9262)

Good points

markblundeberg marked 2 inline comments as done.Sun, Jun 9, 14:45
markblundeberg updated this revision to Diff 9274.

update per comments

deadalnix requested changes to this revision.Wed, Jun 12, 13:06
deadalnix added inline comments.
src/script/sigencoding.cpp
183

The fact you duplicated logic is a tell the design isn't quite right.

This revision now requires changes to proceed.Wed, Jun 12, 13:06