search

bcoin-org / bcoin

Javascript bitcoin library for node.js and browsers
https://bcoin.io
Other
2.98k stars 811 forks source link

Adds descriptor module with getdescriptorinfo rpc #1152

Closed Vasu-08 closed 11 months ago

Vasu-08 commented 1 year ago

This pr implements getdescriptorinfo rpc by adding a descriptor module.

If the argument string of getdescriptorinfo() rpc is a valid string it is parsed into a AbstractDescriptor object. Along with that various parts of the string, such as origin information, keys (Public/Private), and derivation path (along with hardened or unhardened information) are parsed into PubKeyProvider object respectively. If the descriiptor string contains a checksum it will be validiated and if a checksum is not present we will calculate one.

Note: The input descriptor string doesn't require checksum to be present.

The AbstractDescriptor class has PKDescriptor, PKHDescriptor, WPKHDescriptor, AddressDescriptor, MultisigDescriptor, WSHDescriptor, SHDescriptor, RawDescriptor, ComboDescriptor as its subclasses.

The KeyProvider class will have ConstPubkeyProvider (for parsing constant keys with/without origin info) and HDKeyProvider (for parsing extended (hd - bip32) keys with/without origin info) as its subclasses.

Functions for parsing descriptor string will be present as global function in parser.js. Checksum functions are implemented in common.js.

Functions checking isRange(), isSolvable(), hasPrivateKeys() etc. will be present in the AbstractDescriptor class.

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 90.93% and project coverage change: +0.69 :tada:

Comparison is base (bb0375e) 69.56% compared to head (5b09c3f) 70.26%.

:exclamation: Current head 5b09c3f differs from pull request most recent head f388594. Consider uploading reports for the commit f388594 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1152 +/- ## ========================================== + Coverage 69.56% 70.26% +0.69% ========================================== Files 159 174 +15 Lines 26607 27367 +760 ========================================== + Hits 18509 19229 +720 - Misses 8098 8138 +40 ``` | [Impacted Files](https://app.codecov.io/gh/bcoin-org/bcoin/pull/1152?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcoin-org) | Coverage Δ | | |---|---|---| | [lib/bcoin.js](https://app.codecov.io/gh/bcoin-org/bcoin/pull/1152?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcoin-org#diff-bGliL2Jjb2luLmpz) | `0.00% <0.00%> (ø)` | | | [lib/descriptor/index.js](https://app.codecov.io/gh/bcoin-org/bcoin/pull/1152?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcoin-org#diff-bGliL2Rlc2NyaXB0b3IvaW5kZXguanM=) | `0.00% <0.00%> (ø)` | | | [lib/descriptor/type/combo.js](https://app.codecov.io/gh/bcoin-org/bcoin/pull/1152?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcoin-org#diff-bGliL2Rlc2NyaXB0b3IvdHlwZS9jb21iby5qcw==) | `89.65% <89.65%> (ø)` | | | [lib/descriptor/type/addr.js](https://app.codecov.io/gh/bcoin-org/bcoin/pull/1152?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcoin-org#diff-bGliL2Rlc2NyaXB0b3IvdHlwZS9hZGRyLmpz) | `90.90% <90.90%> (ø)` | | | [lib/hd/keyorigininfo.js](https://app.codecov.io/gh/bcoin-org/bcoin/pull/1152?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcoin-org#diff-bGliL2hkL2tleW9yaWdpbmluZm8uanM=) | `92.00% <92.00%> (ø)` | | | [lib/descriptor/abstractdescriptor.js](https://app.codecov.io/gh/bcoin-org/bcoin/pull/1152?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcoin-org#diff-bGliL2Rlc2NyaXB0b3IvYWJzdHJhY3RkZXNjcmlwdG9yLmpz) | `92.06% <92.06%> (ø)` | | | [lib/descriptor/type/pk.js](https://app.codecov.io/gh/bcoin-org/bcoin/pull/1152?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcoin-org#diff-bGliL2Rlc2NyaXB0b3IvdHlwZS9way5qcw==) | `92.30% <92.30%> (ø)` | | | [lib/descriptor/type/sh.js](https://app.codecov.io/gh/bcoin-org/bcoin/pull/1152?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcoin-org#diff-bGliL2Rlc2NyaXB0b3IvdHlwZS9zaC5qcw==) | `92.30% <92.30%> (ø)` | | | [lib/node/rpc.js](https://app.codecov.io/gh/bcoin-org/bcoin/pull/1152?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcoin-org#diff-bGliL25vZGUvcnBjLmpz) | `41.86% <92.30%> (+0.47%)` | :arrow_up: | | [lib/descriptor/type/pkh.js](https://app.codecov.io/gh/bcoin-org/bcoin/pull/1152?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcoin-org#diff-bGliL2Rlc2NyaXB0b3IvdHlwZS9wa2guanM=) | `92.85% <92.85%> (ø)` | | | ... and [9 more](https://app.codecov.io/gh/bcoin-org/bcoin/pull/1152?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcoin-org) | | ... and [4 files with indirect coverage changes](https://app.codecov.io/gh/bcoin-org/bcoin/pull/1152/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bcoin-org)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

theanmolsharma commented 1 year ago

BUG Report:

Incorrect position in pubkeyProvider object.

Replication:

On parsing this descriptor. The pos value in pubkeyProvider object is et to 0 for all 3 keys. Screenshot is attached below.

Screenshot 2023-06-09 at 8 38 38 PM
Vasu-08 commented 1 year ago

BUG Report:

Incorrect position in pubkeyProvider object.

Replication:

  • Parse the following descriptor: sh(wsh(sortedmulti(2,[e7dd1c50/48\'/1\'/10\'/1\']tpubDEh4Cah3FYZ7ePUoCcmTBSRuySQjo24F6RPZ6fM7xcDPUqLZgo2j9vwZkafwMUUvFJ9Foa5wXcwuft2Dv1UQNYXpQ81KMG5aFv1RzNbCvag/0/*,[e7dd1c50/48\'/1\'/13\'/1\']tpubDFkbvhzqd71DQHRYZze7MvRwvv2xFkbibRdW96Juo2uJrHAULe5CqW1QvGGHSsQSzoPUpF5ncedLDq79i8nQMQnyELX2ZSBH48kABC7Wabs/0/*,[aedb3d12/48\'/1\'/0\'/1\']tpubDEbuxto5Kftus28NyPddiEev2yUhzZGpkpQdCK732KBge5FJDhaMdhG1iVw3rMJ2qvABkaLR9HxobkeFkmQZ4RqQgN1KJadDjPn9ANBLo8V/0/*)))

On parsing this descriptor. The pos value in pubkeyProvider object is et to 0 for all 3 keys. Screenshot is attached below.

Screenshot 2023-06-09 at 8 38 38 PM

Fixed

theanmolsharma commented 1 year ago

I think we can get by without passing context in fromString() in various descriptor types. I think we can make it an optional parameter with the default value TOP

Vasu-08 commented 1 year ago

I think we can get by without passing context in fromString() in various descriptor types. I think we can make it an optional parameter with the default value TOP

Yup this should be handled internally.

pinheadmz commented 1 year ago

See https://github.com/bitcoin/bitcoin/pull/26076

We might need to switch our hardened marker in all outputs to h instead of '.

I modified the bitcoin core descriptor test to output a JSON blob and all those vectors passed your tests except where bitcoin core used h in the output