Depends on D631
Details
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
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? | |
Comment Actions
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. | |
Comment Actions
- 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. | |
Comment Actions
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. |