add Zcash address encoding support #4
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Should be able to deal with incomplete P2SH/P2WSH inputs when allowIncomplete is set * add additional fixes to afk11's pull request
* add testnet bip49 example * Should be able to deal with incomplete P2SH/P2WSH inputs when allowIncomplete is set * remove redundant baddress.toOutputScript call from tests * set p2sh=true only if redeemScriptType is set * testing/integration/examples: isolate to addresses/transactions examples, use public broadcast endpoints * tests/integration: add BIP32 serialization and multi-child examples * README: re-generate examples list * README: drop Contributors * README: add bech32 * README: move BCoin to Alternative * README: drop insight * README: add helperbit * LICENSE: 2017 too * add from/toBech32 * add Bech32 support to toOutputScript/fromOutputScript * README/tests: add BIP173/BIP16 SegWit address examples * tests: add P2WPK, P2WSH spend example * tests: resist txn-mempool-conflicts * tests/txb: add P2WSH(multisig), incomplete fixture * txbuilder: refactor branches for readability * add witnessPubKeyHash compressed policy * templates/pubkey: only canonical pubkeys to encode * TxBuilder: restrict uncompressed keyPairs for P2WPK and P2WSH * script: use asMinimalOP for ASM/decompile * Fix the integration url's to latest version also some of the urls were broken * add bech32 fixture * Fixed Segwit links The links were redirecting to 404 - I've modified them so they adhere to the beginning of the respective Segwit unit tests. * Fixed some README links Found some more links that were displaced by the Segwit unit tests and fixed them * typescript instructions on README closes: bitcoinjs#815 * README: cleanup typescript help * Add witness is true to signing * Add test for witness = true edge case during multisigning * TransactionBuilder: collect witnessValue as input.value, and match it * Fix txb.__overMaximumFees for segwit * Add test case * Fix absurd fee in fixture * buildstack - don't return op_0 * multisig.input.encodestack - replace OP_0 (permitted by partialSignature) with EMPTY_BUFFER * update CHANGELOG * 3.2.0 * tests: script tests can validate template fixtures too * match scriptHash types 1 for 1, ignore classify order * add fixture to verify input type classification (cherry picked from commit 8f9d8b7) * respond to Jonathan Underwood's comments (cherry picked from commit 8126ca2) * tests/fixtures: amend truncated outputHex * README: add notes about ES5, Node LTS feature tracking * package: rm contributors field, outdated, update wallet estimate * rm bscript circular dependencies * txbuilder: fix canSign returning true for missing witness value * address/txbuilder: require templates to prevent undefined exports * tests: add passing and failing tests for witness*.input.encode/decode * witnessScriptHash: fixed implementation * tests: add failing staged transaction building example bitcoinjs#901 * txbuilder: apply input.value before prepareInput * s/checkP2shInput/checkP2SHInput * 3.2.1 * ECSignature: add toRSBuffer/fromRSBuffer * TxBuilder: add support for RSBuffer type keyPairs and .publicKey * 3.3.0 * tests: txb for TxBuilder, Tx for Transaction * increase max feerate sanity check from 1000 to 2500 * 3.3.1 * opt-in bitcoin-cash support in transaction_builder * TransactionBuilder.fromTransaction & Bitcoin Cash Adds an extra parameter to fromTransaction, which tells the library to expect a value property to be added on each txin which uses bitcoin cash's sighashtype * bitcoin gold support * package: rename to bitcoinforksjs-lib
Melvillian
suggested changes
Mar 13, 2018
| @@ -12,10 +12,13 @@ function fromBase58Check (address) { | |||
|
|
|||
| // TODO: 4.0.0, move to "toOutputScript" | |||
| if (payload.length < 21) throw new TypeError(address + ' is too short') | |||
| if (payload.length > 21) throw new TypeError(address + ' is too long') | |||
| if (payload.length > 22) throw new TypeError(address + ' is too long') | |||
|
|
|||
| var version = payload.readUInt8(0) | |||
There was a problem hiding this comment.
readability: We can make this more readable by creating variables for the 21 length and 22 length cases and then if/elseing on those. Right now an arbitrary reader of this code will have no idea why or when it's length 21 or length 22. Something like
// zcash addresses use a 2-byte prefix, all other coins we deal with use only 1 byte.
var isZCashAddress = payload.length === 22;
var version;
if (isZCashAddress) {
version = payload.readUInt16BE(0)
} else {
version = payload.readUInt8(0)
}
var hash = payload.slice(payload.length - 20)
|
|
||
| var payload = Buffer.allocUnsafe(21) | ||
| payload.writeUInt8(version, 0) | ||
| hash.copy(payload, 1) | ||
| try { |
There was a problem hiding this comment.
same here, use variable names to differentiate between the different lengths, otherwise future readers will in the best case waste time going through git blame to figure out why this changed, and in the worse case not know whether they can change this code or not.
OttoAllmendinger
pushed a commit
that referenced
this pull request
Sep 15, 2021
OttoAllmendinger
pushed a commit
that referenced
this pull request
Sep 15, 2021
|
@arik-so is this PR still active? 😄 |
|
Pretty sure Jorge commandeered it in a group of separate PRs. Feel free to close. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
hey Alex, please review diff