Page MenuHomePhabricator
Differential D13656

[ecashaddrjs] Add function to support conversion to legacy format
ClosedPublic
Actions

Authored by bytesofman on Apr 10 2023, 17:45.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABCadd063177a26: [ecashaddrjs] Add function to support conversion to legacy format
Summary

T3096

Going forward, we will use utxo-lib and bitcoinjs-message for javascript applications that create transactions and sign / verify msgs.

These libraries are robust and well maintained. However, they require address input format to be legacy.

Include a toLegacy function in ecashaddrjs to support future implementation of above libraries.

Test Plan

npm test

Diff Detail

Repository
rABC Bitcoin ABC
Branch
add-tolegacy
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 23176
Build 45971: Build Diff
Build 45970: arc lint + arc unit

Event Timeline

bytesofman created this revision.Apr 10 2023, 17:45
Herald added a reviewer: Restricted Project. · View Herald TranscriptApr 10 2023, 17:45
bytesofman requested review of this revision.Apr 10 2023, 17:45
Harbormaster completed remote builds in B23174: Diff 39509.Apr 10 2023, 17:45
bytesofman updated this revision to Diff 39510.Apr 10 2023, 17:46

Move new unit tests after existing unit tests

Harbormaster completed remote builds in B23175: Diff 39510.Apr 10 2023, 17:46
bytesofman planned changes to this revision.Apr 10 2023, 18:11

add a unit test case for failure to convert

bytesofman updated this revision to Diff 39512.Apr 10 2023, 18:25

failure case unit tests

Harbormaster completed remote builds in B23176: Diff 39512.Apr 10 2023, 18:25
Fabien requested changes to this revision.Apr 10 2023, 19:18
Fabien added a subscriber: Fabien.
Fabien added inline comments.
modules/ecashaddrjs/src/cashaddr.js
526

It would be more robust to check isTestnet as !isMainnet. E.g. ecregtest prefix are testnet wrt cash address format.
Also we tend to add test nets over time but not main nets.

This revision now requires changes to proceed.Apr 10 2023, 19:18
bytesofman updated this revision to Diff 39520.Apr 10 2023, 22:27

Improve check for mainnet or testnet

Harbormaster completed remote builds in B23184: Diff 39520.Apr 10 2023, 22:27
bytesofman added inline comments.Apr 10 2023, 22:27
modules/ecashaddrjs/src/cashaddr.js
526
var VALID_PREFIXES = [
    'ecash',
    'bitcoincash',
    'simpleledger',
    'etoken',
    'ectest',
    'bchtest',
    'bchreg',
];

E.g. ecregtest prefix are testnet wrt cash address format.

Is this a prefix that should be added to VALID_PREFIXES ?

is bchreg a testnet address?

Fabien accepted this revision.Apr 11 2023, 04:56
Fabien added inline comments.
modules/ecashaddrjs/src/cashaddr.js
526

ecregtest and bchreg are both used for regression testing, so not a live chain. Supporting them depends on how you plan to use the lib, but I think you should have none or both. Both are testnet addresses as per address encoding.

This revision is now accepted and ready to land.Apr 11 2023, 04:56
Closed by commit rABCadd063177a26: [ecashaddrjs] Add function to support conversion to legacy format (authored by bytesofman). · Explain WhyApr 11 2023, 13:59
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABCadd063177a26: [ecashaddrjs] Add function to support conversion to legacy format.

Revision Contents
Changeset List

PathSize
modules/
ecashaddrjs/
63 lines
3 lines
src/
31 lines
test/
71 lines

Diff 39512

View Options

modules/ecashaddrjs/package-lock.json

Loading...
View Options

modules/ecashaddrjs/package.json

Loading...
View Options

modules/ecashaddrjs/src/cashaddr.js

Loading...
View Options

modules/ecashaddrjs/test/cashaddr.js

Loading...