Page MenuHomePhabricator

Refactor IsValidSignatureEncoding
ClosedPublic

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

Details

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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; 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
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)
deadalnix updated this revision to Diff 4372.Jul 23 2018, 12:31

Fix typos and use consistent comment style.

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
This revision was automatically updated to reflect the committed changes.