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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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

Fix typos and use consistent comment style.

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.