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
Branch
slicedderencoding
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 2889
Build 3882: Bitcoin ABC Buildbot (legacy)
Build 3881: arc lint + arc unit

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

A compound what?

46

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

52

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

63

Mixed comment styles

81

Doxygen-style

96

Doxygen-style

97

our -> out

106

Mixed comment styles

132

Doxygen-style

144

Doxygen-style

146

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

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.