Page MenuHomePhabricator

Refactor IsValidSignatureEncoding

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



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

Depends on D1571

Test Plan
make check

Diff Detail

rABC Bitcoin ABC
Automatic diff as part of commit; lint not applicable.
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.
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)


96 ↗(On Diff #4333)


97 ↗(On Diff #4333)

our -> out

106 ↗(On Diff #4333)

Mixed comment styles

132 ↗(On Diff #4333)


144 ↗(On Diff #4333)


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
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.
44 ↗(On Diff #4333)

Better explanation:
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.