Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scriptpubkey_to_address() does not give clear error messages #354

Open
Randy808 opened this issue Dec 2, 2022 · 2 comments
Open

scriptpubkey_to_address() does not give clear error messages #354

Randy808 opened this issue Dec 2, 2022 · 2 comments

Comments

@Randy808
Copy link

Randy808 commented Dec 2, 2022

After attempting to use the library function scriptpubkey_to_address from the Python distribution, I got the error message ValueError: Invalid argument

This was the line I used to invoke the function:
wallycore.scriptpubkey_to_address(bytes([0x6a]), wallycore.WALLY_NETWORK_LIQUID)

I tried looking through the repo to see if there's any examples of scriptpubkey_to_address and didn't find much in the code aside from this (which linked the python call to 'wally_scriptpubkey_to_address'):

int ret = ::wally_scriptpubkey_to_address(scriptpubkey.data(), scriptpubkey.size(), network, output);

I did find an example in one of the commits matching the search result:
399906a#diff-a1dde0a251f92d169d953a7e3b12d4f97d98a5985abf5b1a0e48075c062e708c

The standalone example file in that commit can be found here (although it's not in master anymore):
https://github.com/ElementsProject/libwally-core/blob/399906aba358a16f05b233f619fa08aec6bfe116/src/swig_python/contrib/address.py

When I tried running this line using data from the test case it worked:
wallycore.scriptpubkey_to_address(wallycore.hex_to_bytes("76a914bef5a2f9a56a94aab12459f72ad9cf8cf19c7bbe88ac"), wallycore.WALLY_NETWORK_LIQUID)

So then I decided to look into the implementation for 'wally_scriptpubkey_to_address':

int wally_scriptpubkey_to_address(const unsigned char *scriptpubkey, size_t scriptpubkey_len,

There's a line that tries to determine the script 'type':

if ((ret = wally_scriptpubkey_get_type(scriptpubkey, scriptpubkey_len, &type)) != WALLY_OK) {

And then only processes the types WALLY_SCRIPT_TYPE_P2PKH and WALLY_SCRIPT_TYPE_P2SH

The function that gets the types ('wally_scriptpubkey_get_type') can be found here:

int wally_scriptpubkey_get_type(const unsigned char *bytes, size_t bytes_len,

Although a type is returned for OP_RETURN, it doesn't accept it in 'wally_scriptpubkey_to_address'

The error I get from the call could tell me it wasn't supported instead of the generic 'WALLY_EINVAL' that's returned as the default catch-all:

return WALLY_EINVAL;

@Randy808
Copy link
Author

Randy808 commented Dec 2, 2022

For more context, I originally tried the function call in nodejs:
image

And then tried in Python:
Screenshot 2022-12-02 at 1 54 21 PM

@jgriffiths
Copy link
Contributor

jgriffiths commented Dec 2, 2022

Hi @Randy808

There is no address format for OP_RETURN outputs AFAIU, so passing one to this function is always going to be invalid. I think the documentation should state that only p2pkh and p2sh addresses are valid inputs.

The address API was added while I was on sabbatical and would not have passed review had I reviewed the calls tbh. There should be no need in a better API for the caller to treat different address types manually, but for now that's the state of things.

Wally has very limited error reporting capabilities at present, since initially it was not going to have higher level APIs like PSBT. I intend to extend the error reporting to give optional human readable error codes in due course.

Given that this call will only ever support p2pkh/p2sh, the caller is always going to have to check the script type before calling, so I'm not sure that returning e.g. WALLY_ERROR for an unsupported script type helps much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants