Page MenuHomePhabricator

Split the decoding of cashaddr in a pure decode step + a Destination creation step
ClosedPublic

Authored by deadalnix on Nov 8 2017, 17:04.

Details

Summary

This allows for much more in depth testing, even for destination we don't have support for.

Test Plan

Added a bunch of unittests.

Diff Detail

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

Event Timeline

dagurval requested changes to this revision.Nov 9 2017, 11:03

Mostly nits, except for CashAddressContent::type which must be initialized.

src/cashaddrenc.cpp
89 ↗(On Diff #1680)

CashAddressContent::type is unitialized. You could add a constructor to CashAddressContent.

Also, just return { }?

src/cashaddrenc.h
16 ↗(On Diff #1680)

Suggest calling this CashAddrContent for consistency

src/test/cashaddrenc_tests.cpp
181 ↗(On Diff #1680)

How about a comment about what pair contains?

189 ↗(On Diff #1680)

Suggest naming the magic number 12. For example declare variable const size_t prefixLen = params.CashAddrPrefix() + 1 outside loop

This revision now requires changes to proceed.Nov 9 2017, 11:03
src/cashaddrenc.cpp
89 ↗(On Diff #1680)

It is initialized. If the number of element of the initializer list is less than the number of element in the aggregate, the extra element are value-initializer , which means zeroed for PODs and default constructed for non POD.

src/test/cashaddrenc_tests.cpp
189 ↗(On Diff #1680)

It is 8 for the version + 4 in order to get the right rounding for the division.

deadalnix edited edge metadata.

Various nits

This revision is now accepted and ready to land.Nov 9 2017, 13:28
This revision was automatically updated to reflect the committed changes.