Page MenuHomePhabricator
Differential D1576

Refactor IsValidSignatureEncoding
ClosedPublic
Actions

Authored by deadalnix on Jul 17 2018, 14:34.

Details

Reviewers
jasonbcox
Group Reviewers
Restricted Project
Commits
rSTAGINGdd7800b4f4f5: Refactor IsValidSignatureEncoding
rABCdd7800b4f4f5: Refactor IsValidSignatureEncoding
Summary

Mostly reorder stuff and add a lot of comments to explain what it is doing.

Depends on D1571

Test Plan
make check

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

deadalnix created this revision.Jul 17 2018, 14:34
Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 17 2018, 14:34
teamcity added a subscriber: teamcity.Jul 17 2018, 14:37

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

teamcity added a comment.Jul 17 2018, 15:40

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

Harbormaster completed remote builds in B2889: Diff 4333.Jul 17 2018, 15:40
deadalnix added a child revision: D1577: Ignore sighash when checking for DER encoding..Jul 17 2018, 15:59
jasonbcox requested changes to this revision.Jul 20 2018, 05:18
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/script/sigencoding.cpp
44 ↗(On Diff #4333)

A compound what?

46 ↗(On Diff #4333)

Merge with this comment so we don't have mixed comment styles

52 ↗(On Diff #4333)

Make doxygen-style and explain not just what is being removed, but what sig[1] represents such that this comparison makes sense.

63 ↗(On Diff #4333)

Mixed comment styles

81 ↗(On Diff #4333)

Doxygen-style

96 ↗(On Diff #4333)

Doxygen-style

97 ↗(On Diff #4333)

our -> out

106 ↗(On Diff #4333)

Mixed comment styles

132 ↗(On Diff #4333)

Doxygen-style

144 ↗(On Diff #4333)

Doxygen-style

146 ↗(On Diff #4333)

our -> out

This revision now requires changes to proceed.Jul 20 2018, 05:18
deadalnix added inline comments.Jul 23 2018, 10:08
src/script/sigencoding.cpp
44 ↗(On Diff #4333)

http://luca.ntop.org/Teaching/Appunti/asn1.html

deadalnix updated this revision to Diff 4372.Jul 23 2018, 12:31

Fix typos and use consistent comment style.

teamcity added a comment.Jul 23 2018, 12:33

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

teamcity added a comment.Jul 23 2018, 13:38

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

Harbormaster failed remote builds in B2919: Diff 4372!Jul 23 2018, 13:38
jasonbcox accepted this revision.Jul 23 2018, 17:21
jasonbcox added inline comments.
src/script/sigencoding.cpp
44 ↗(On Diff #4333)

Better explanation: https://bitcoin.stackexchange.com/questions/12554/why-the-signature-is-always-65-13232-bytes-long
0x30 is a generic "compound structure". I guess this comment is fine, but I would prefer it say "compound structure" or something similar.

This revision is now accepted and ready to land.Jul 23 2018, 17:21
Closed by commit rABCdd7800b4f4f5: Refactor IsValidSignatureEncoding (authored by deadalnix). · Explain WhyJul 23 2018, 17:25
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
src/
script/
114 lines

Diff 4375

View Options

src/script/sigencoding.cpp

Loading...