Page MenuHomePhabricator
Differential D1570

Add test for CheckSignatureEncoding to verify low S is checked properly.
ClosedPublic
Actions

Authored by deadalnix on Jul 15 2018, 22:55.

Details

Reviewers
jasonbcox
Group Reviewers
Restricted Project
Commits
rSTAGING608519dce2d0: Add test for CheckSignatureEncoding to verify low S is checked properly.
rABC608519dce2d0: Add test for CheckSignatureEncoding to verify low S is checked properly.
Summary

Increase the unit test coverage for CheckSignatureEncoding.

Depends on D1567

Test Plan
make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
checksigencodinglows
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 2893
Build 3890: Bitcoin ABC Buildbot (legacy)
Build 3889: arc lint + arc unit

Event Timeline

deadalnix created this revision.Jul 15 2018, 22:55
Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 15 2018, 22:55
teamcity added a subscriber: teamcity.Jul 15 2018, 22:57

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

teamcity added a comment.Jul 15 2018, 23:39

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

Harbormaster failed remote builds in B2878: Diff 4314!Jul 15 2018, 23:39
deadalnix added a child revision: D1571: Add test case for CheckSignatureEncoding for proper DER encoding.Jul 16 2018, 12:47
jasonbcox accepted this revision.Jul 17 2018, 17:56
jasonbcox added a subscriber: jasonbcox.

Accepted, with feedback on a comment

src/test/sigencoding_tests.cpp
100 ↗(On Diff #4314)

Since CheckSignatureEncodingWithSigHashType checks more than just the correct forkId, I think this comment should be more along the lines of:
// Minimal signature is valid with any flags

This revision is now accepted and ready to land.Jul 17 2018, 17:56
deadalnix added inline comments.Jul 17 2018, 19:33
src/test/sigencoding_tests.cpp
100 ↗(On Diff #4314)

That comment would be incorrect because depending on forkid enforcement, and forkid flag, it may be invalid.

deadalnix updated this revision to Diff 4340.Jul 17 2018, 19:41

rebase

Harbormaster failed remote builds in B2892: Diff 4340!Jul 17 2018, 19:42
deadalnix updated this revision to Diff 4341.Jul 17 2018, 19:54

rebase

teamcity added a comment.Jul 17 2018, 19:56

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

Closed by commit rABC608519dce2d0: Add test for CheckSignatureEncoding to verify low S is checked properly. (authored by deadalnix). · Explain WhyJul 17 2018, 19:56
This revision was automatically updated to reflect the committed changes.
teamcity added a comment.Jul 17 2018, 20:36

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

Harbormaster completed remote builds in B2893: Diff 4341.Jul 17 2018, 20:36

Revision Contents
Changeset List

PathSize
src/
test/
32 lines

Diff 4341

View Options

src/test/sigencoding_tests.cpp

Loading...