Page MenuHomePhabricator

Use the new cashaddr format
AbandonedPublic

Authored by dagurval on Sep 16 2017, 21:05.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Summary

This replaces the existing base58 address format with the new base32 cashaddr format.

Test Plan

Updated unittest.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
bech32-addr
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 878
Build 878: arc lint + arc unit

Event Timeline

Differentiate between p2kh and p2sh with version instead of prefix.

Uses version == 0 for p2kh, version == 8 for p2sh. Lower 3 bits are reserved.

deadalnix requested changes to this revision.Sep 17 2017, 13:06
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/base58.cpp
6 ↗(On Diff #1384)

We should rename this as this is clearly not bech32 anymore. cashaddress maybe ?

216 ↗(On Diff #1384)

You can make these uint8_t which would remove the need for cast.

226 ↗(On Diff #1384)

unsigned char => uint8_t

253 ↗(On Diff #1384)

braces around ifs.

327 ↗(On Diff #1384)

Let's not add more reference to global state. Pass the params down.

331 ↗(On Diff #1384)

dito

340 ↗(On Diff #1384)

dito

src/bech32.cpp
34 ↗(On Diff #1384)

Please format like:

/**
 * blablabla
 */

You can put the comment on one line and let clang-format do its magic.

42 ↗(On Diff #1384)

dito

54 ↗(On Diff #1384)

dito

68 ↗(On Diff #1384)

dito

78 ↗(On Diff #1384)

dito

111 ↗(On Diff #1384)
if (c0 & 1) {
  // k(x) = {29}x^5 + {22}x^4 + {20}x^3 + {21}x^2 + {29}x + {18}
  c ^= 0x3b6a57b2;
}
144 ↗(On Diff #1384)

Hash the lower 5 bits of all characters + a zero.

163 ↗(On Diff #1384)

Comment on the previous line.

199 ↗(On Diff #1384)

Use ':' as a separator.

src/chainparams.cpp
198 ↗(On Diff #1384)

bitcoincash.

362 ↗(On Diff #1384)

Not sure, but something more specific.

479 ↗(On Diff #1384)

dito

This revision now requires changes to proceed.Sep 17 2017, 13:06
dagurval edited edge metadata.
dagurval marked 11 inline comments as done.

formatting, renamed to cashaddr, changed seperator to :, other nits based on feedback

dagurval added inline comments.
src/base58.cpp
327 ↗(On Diff #1384)

This is a huge change all over the code base. Can we put this one on the wish list?

deadalnix requested changes to this revision.Sep 27 2017, 12:44

I think this diff should be splet as right now, it is doing 2 things at once:

  • Implement the format
  • Integrate the format.

Let's focus on the first one and do the second one in another diff. We don't want any problem with one to slow down the other and vice versa.

src/base58.cpp
327 ↗(On Diff #1384)

OK

src/cashaddr.cpp
1

Add a compyright notice for the bitcoin developers

35

We will change that one slightly, but we can use it in the meantime.

126

uint8_t, no unsigned char . There are numerous others.

130

We should use prefix instead of HRP

141

We discussed this already, just go throught the lower 5 bits of all char and then a 0. There is no point checksuming the higher bits as they are constants.

156

+8

158

uint64_t

172

CashAddress

184

CashAddress

190

braces

194

braces

src/chainparams.h
89

Prefix

104

dito

src/test/cashaddr_tests.cpp
1

This needs to be changed to test the format. We should probably rewrite it anyway rather than backporting the one from core.

src/utilstrencodings.h
145

This would deserve a few tests.

This revision now requires changes to proceed.Sep 27 2017, 12:44

I'll update this diff when D575 or D576 have landed

dagurval edited edge metadata.

Added support for encoding CTxDestination with base58 (the old way). Rebased on top of D576.

dagurval retitled this revision from [WIP] Use bech32 for addresses instead of base58. to [WIP] Use new cashaddress format.Oct 9 2017, 20:10
dagurval edited the summary of this revision. (Show Details)
dagurval retitled this revision from [WIP] Use new cashaddress format to Use the new cashaddr format.Oct 14 2017, 20:58
dagurval edited the summary of this revision. (Show Details)
dagurval edited the test plan for this revision. (Show Details)

There are various self contained changes here that could go in now.

The first obvious one is the chain param change.

Another one is the logic for cashaddr key extraction, that can be added and tested as a standalone change without being wired in the existing logic.