Details

Reviewers

Group Reviewers

Restricted Project

Commits

rSTAGING0b22d28659e5: Add test case for CheckSignatureEncoding for proper DER encoding
rABC0b22d28659e5: Add test case for CheckSignatureEncoding for proper DER encoding

Summary

This ensures IsValidSignatureEncoding is properly checked.

The test time is atrocious because IsValidSignatureEncoding assumes the presence of the sighash type which causes a combinotary explosion, and also uncover a SRP violation. I would like to refactor these to detangle the two, but obviously doing so safely require a test so here we are.

Depends on D1570

Test Plan

make check

Diff Detail

Repository

rABC Bitcoin ABC

Branch

sigcheckderencoding

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 2910
Build 3924: Bitcoin ABC Buildbot (legacy)
Build 3923: arc lint + arc unit

Event Timeline

deadalnix created this revision.Jul 16 2018, 12:47

Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 16 2018, 12:47

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

deadalnix added a child revision: D1572: Use sliced to remove the sighash byte from a signature to check for low S.Jul 16 2018, 13:20

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

Harbormaster completed remote builds in B2881: Diff 4317.Jul 16 2018, 13:40

deadalnix added a child revision: D1576: Refactor IsValidSignatureEncoding.Jul 17 2018, 14:34

jasonbcox added a subscriber: jasonbcox.Jul 19 2018, 02:21

jasonbcox added inline comments.

src/test/sigencoding_tests.cpp
190 ↗	(On Diff #4317)	Is this referring to the `// Non canonical length for S.` case? If not, I don't understand which case this is referring to. It might be worth making it explicit as it's own list to pass in here (as a local variable).
203 ↗	(On Diff #4317)	Same as above.