T3075
Depends on D13525
Details
Details
- Reviewers
Fabien - Group Reviewers
Restricted Project - Commits
- rABC791bbfba4ea3: [ecashaddrjs] Support encoding outputScript to address
Review new functions and new unit tests
npm test
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| web/ecashaddrjs/src/cashaddr.js | ||
|---|---|---|
| 492 ↗ | (On Diff #39130) | |
| 502 ↗ | (On Diff #39130) | Are you going to use this getTypeFromOutputScript function anywhere else ? Otherwise since it's not exported nor useful for testing you should just inline it into encodeOutputScript |
| web/ecashaddrjs/test/cashaddr.js | ||
| 224 ↗ | (On Diff #39130) | Please check with an invalid hash length also |
Comment Actions
unit test covering bad hash length, exporting getTypeAndHashFromOutputScript function
| web/ecashaddrjs/src/cashaddr.js | ||
|---|---|---|
| 502 ↗ | (On Diff #39130) | I think it's better for code readability to have it split out. This also follows the existing standard of the repo, which does the same for prefixToUint5Array, getTypeBits, getHashSizeBits, toUint5Array. It's true that getTypeFromOutputScript is used only one time, but it's possible this could be expanded on with future methods. This is also true of other functions (used only one time), e.g. getTypeBits and getHashSizeBits ....hm the main reason though is that this could be a useful function to export, so I'll go ahead and do that. e.g. type and hash are what you need to make a chronik call, so if you want to do that based on an outputScript, this is the function to use. |
Comment Actions
if getTypeAndHashFromOutputScript is exported, should validate for hash length and have a unit test