Depends on D631
Details
- Reviewers
schancel deadalnix - Group Reviewers
Restricted Project - Commits
- rSTAGINGa9e694c25063: Support for encoding addresses using cashaddr.
rABCa9e694c25063: Support for encoding addresses using cashaddr.
Added unittests.
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- 10-23-cashaddr
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 1023 Build 1023: arc lint + arc unit
Event Timeline
src/cashaddrenc.cpp | ||
---|---|---|
18 | Does this basically operate as a closure then? | |
52 | What's the point of constructing this in such a way if we have a single-use function that takes params? | |
59 | This was added in another patch already? | |
src/cashaddrenc.h | ||
15 | I assume this needs chainparams to add the appropriate prefix and whatnot? | |
src/test/cashaddrenc_tests.cpp | ||
25 | Maybe we should generate a bunch of these? |
A few nits, but I think the double padding problem can be an issue. This needs to be accounted for and tested. Except that, it's good.
src/cashaddrenc.cpp | ||
---|---|---|
40 | We need to make sure we don't double pad when encoding/decoding. This needs more logic and a test for it. | |
69 | Please use uint8_t | |
80 | I think it'd be preferable to use std::copy but not a big deal. |
- Convert entire data-part with ConvertBits. Used to convert the hash only.
- Ignore extra padding
- Added test to encode/decode bunch of random addresses.
- Explicitly check that we get the expected size when converting bits 8 <-> 5 and back.
- Use std::copy instead of memcpy
src/cashaddrenc.cpp | ||
---|---|---|
52 | This is how boost suggests working with boost::variants http://www.boost.org/doc/libs/1_64_0/doc/html/variant.html#variant.motivation | |
src/cashaddrenc.h | ||
15 | Yes, it needs the address prefix. |
There should be a tests using actual addresses in there. If nothing else, they can be used as test vectors.
src/test/cashaddrenc_tests.cpp | ||
---|---|---|
92 ↗ | (On Diff #1636) | braces |
src/test/cashaddrenc_tests.cpp | ||
---|---|---|
162 ↗ | (On Diff #1655) | Yes no, that was stupid. |