[ecashaddrjs] Support returning hash160 as string instead of uint8array
ClosedPublic
Actions

Authored by bytesofman on Mar 29 2023, 04:54.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC424f228023ec: [ecashaddrjs] Support returning hash160 as string instead of uint8array

Summary

T3055

ecashaddrjs returns the hash of an address (aka hash160) as a uint8array. This is used in zero apps in the monorepo: Cashtab, alias-server, and ecash-telegram-bot all need special util functions to convert this to a string (since this is the format needed by chronik).

So that backward compatibility is not impacted, returning hash160 as a string from cashaddr.decode is a user-specified parameter that defaults to returning the old data.

Webpack updates are required to support the Buffer type in the browser. The webpack config is updated in the same way that it is in Cashtab to support the use of Buffer from nodejs modules in the browser. This was tested by installing this module into create-react-app and running the new functions.

Test Plan

Review new helper functions, changes to existing encode and decode functions, and accompanying unit tests.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

return-hash-as-string-ecashaddrjs

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 22800
Build 45220: Build Diff
Build 45219: arc lint + arc unit

Event Timeline

bytesofman created this revision.Mar 29 2023, 04:54

Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 29 2023, 04:54

bytesofman requested review of this revision.Mar 29 2023, 04:54

Harbormaster completed remote builds in B22800: Diff 39010.Mar 29 2023, 04:54

Need to confirm that the webpack build works okay in react apps.

Cashtab (and other react apps in general) need to use special webpack settings to support the Buffer type, which works fine in nodejs.

It's possible this is why cashaddrjs returned uint8array types.

Actions

Determine if there is evidence that cashaddrjs purposefully excluded Buffer

I can't find anything in the git repo but the Buffer build error is probably why this was excluded. Anyway, it needs to be handled.

Prove that you can build this in webpack and use it in a webapp with default settings (e.g. CRA default) without throwing a buffer error.

Step 1 -- established that the Buffer error is thrown if you don't do anything

If you add this module to a fresh create react app and then add
const test = cashaddr.decode('ecash:qrcuqadqrzp2uztjl9wn5sthepkg22majylrprqkuk', true

);

to App.js, 

you get
{F6602633}

Note: no error is thrown if you use the methods without requesting string return or passing a string for input.

Confirmed in testing that the "Buffer not defined" error will throw in the browser without webpack building the usual buffer shims. However, it only throws if you call using the new methods, i.e. try encode while passing a string param, or try to decode while passing true.

So, not a breaking change.

But -- should try and fix it in webpack here before landing this diff.

Update webpack config and browser path

Harbormaster completed remote builds in B22849: Diff 39069.Mar 30 2023, 16:50

bytesofman edited the summary of this revision. (Show Details)Mar 30 2023, 17:00

Fabien added a subscriber: Fabien.Mar 30 2023, 17:25

Fabien added inline comments.

web/ecashaddrjs/package.json
6 ↗	(On Diff #39069)	Why is that needed ?

bytesofman added inline comments.Mar 30 2023, 18:12

web/ecashaddrjs/package.json
6 ↗	(On Diff #39069)	If a user runs `npm i ecashaddrjs` in an app that is not nodejs, like a react app, it will install `dist/cashaddrjs.min.js` -- which is now built with webpack to resolve the buffer error described above