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 871
Build 871: 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

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

216

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

226

unsigned char => uint8_t

253

braces around ifs.

327

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

331

dito

340

dito

src/bech32.cpp
34

Please format like:

/**
 * blablabla
 */

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

42

dito

54

dito

68

dito

78

dito

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

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

163

Comment on the previous line.

199

Use ':' as a separator.

src/chainparams.cpp
198

bitcoincash.

362

Not sure, but something more specific.

479

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

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

OK

src/cashaddr.cpp
1 ↗(On Diff #1393)

Add a compyright notice for the bitcoin developers

35 ↗(On Diff #1393)

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

126 ↗(On Diff #1393)

uint8_t, no unsigned char . There are numerous others.

130 ↗(On Diff #1393)

We should use prefix instead of HRP

141 ↗(On Diff #1393)

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 ↗(On Diff #1393)

+8

158 ↗(On Diff #1393)

uint64_t

172 ↗(On Diff #1393)

CashAddress

184 ↗(On Diff #1393)

CashAddress

190 ↗(On Diff #1393)

braces

194 ↗(On Diff #1393)

braces

src/chainparams.h
89 ↗(On Diff #1393)

Prefix

104 ↗(On Diff #1393)

dito

src/test/cashaddr_tests.cpp
1 ↗(On Diff #1393)

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 ↗(On Diff #1393)

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.