Export helper function cvToHex and parseReadOnlyResponse

stacks-archive / stacks-transactions-js

The JavaScript library for generating Stacks 2.0 transactions

19 stars 17 forks source link

Export helper function cvToHex and parseReadOnlyResponse #126

Closed friedger closed 3 years ago

friedger commented 3 years ago

Is your feature request related to a problem? Please describe. I want to use the read-only function call of the blockstack-api-client library. It requires to provide arguments as hex encoded string. The response needs to converted to a clarity value.

I want to use the hex encoded string to make rpc calls using curl

Describe the solution you'd like

Export function cvToHex and parseReadOnlyResponse by this library.
Rename parseReadOnlyResponse to hexToCv

Describe alternatives you've considered

copy paste the code from cvToHex - duplicate code is not good in general.
adjust the client library so that clarity values can be passed as read-only function arguments - it is not possible to specify this specific argument and result handling in openapi (that is used to generate the client)

yknl commented 3 years ago

If we rename parseReadOnlyResponse to hexToCv it should also check if 0x prefix is present. Right now it assumes the hex string is prefixed with 0x since that's the response from the blockchain API.

Also would be great to have the PR opened against the new stack.js monorepo PR as well since this library will be migrated to that repo. https://github.com/blockstack/blockstack.js/pull/807

friedger commented 3 years ago

If we check for 0x prefix we should also have an option for cvToHex to omit the prefix.

My understanding of hex is that it has a prefix. Everything else is not a hex

We should just make sure that parsing invalid hex strings fails meaningfully.

yknl commented 3 years ago

Meaningful failure in lieu of checking for prefix works for me 👍

friedger commented 3 years ago

As discussed with yknl, we should not rename the function parseReadOnlyResponse but also add helper functions for map entry function.

friedger commented 3 years ago

I missed the transactions result of the blockchain api. There we need also to decode a hex string. Defining a specific helper method for that is too much, I think. Therefore, I would add a hexToCV helper method.

friedger commented 3 years ago

@yknl PR for monorepo: https://github.com/blockstack/blockstack.js/pull/826

friedger commented 3 years ago

Fixed in stacks.js https://github.com/blockstack/stacks.js/pull/826