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

Taproot address validation #1919

Closed
DaniSchenk opened this issue May 19, 2023 · 4 comments
Closed

Taproot address validation #1919

DaniSchenk opened this issue May 19, 2023 · 4 comments

Comments

@DaniSchenk
Copy link

In v6.0.1 I was able to validate taproot addresses like this:

import * as bitcoinjs from "bitcoinjs-lib";
const value = "bc1pxe5uh7cst3p3qzsuzng0r94uv0tn4wfzum2jflph0km6sjjjp5cqc4906u";

const isBitcoin = !!bitcoinjs.address.toOutputScript(value, bitcoinjs.networks.bitcoin);

I got a warning (Sending to a future segwit version address can lead to loss of funds. End users MUST be warned carefully...) but it validated the address.

In v6.1.0 I had to adjust to validation successfully. Otherwise, I got the error No ECC Library provided. You must call initEccLib() with a valid TinySecp256k1Interface instance if I did not initEccLib(). Based on this comment #1889 (comment) I installed a new lib to make it work again.

import * as bitcoinjs from "bitcoinjs-lib";
import * as ecc from "tiny-secp256k1";
const value = "bc1pxe5uh7cst3p3qzsuzng0r94uv0tn4wfzum2jflph0km6sjjjp5cqc4906u";

bitcoinjs.initEccLib(ecc);
const isBitcoin = !!bitcoinjs.address.toOutputScript(value, bitcoinjs.networks.bitcoin);

So a minor version change introduced a breaking change on my side. I'm curious if this is a fix, feature or bug 🤷‍♂️

@junderw
Copy link
Member

junderw commented May 19, 2023

I think the decision was made that anyone relying on the insecure warning-filled "validation" (it wasn't really validating anything other than the bech32 checksum, which is the whole reason for the warning) should have their app broken rather than including a backwards compatible way for them to continue not validating the p2tr address properly... or bumping major and any project who just never wants to bump major will never switch to proper validation.

TBH I personally was against including the future segwit version "(not-so-much) validation" logic, but there was a clarification in the BIP that stated that all future segwit versions "MUST NOT" hard fail... so our hands were semi-tied.

I understand that a strict interpretation of semver would state that this change needed to be a major bump, but in this specific case we decided against it.

We should probably document this decision in the warning itself so people who see that warning all the time know that if we add a new segwit version in the future and there's a more strict validation requirement, their app might break on minor bump.

@DaniSchenk
Copy link
Author

DaniSchenk commented May 22, 2023

Thank you so much for your explanation @junderw.

I would like to validate if a user provided string is a bitcoin address that can be used as a tx recipient in general. Since you seem unconvinced about the "validation", do you suggest another approach to test if a string is a bitcoin address?

@junderw
Copy link
Member

junderw commented May 22, 2023

That's the thing, a bech32m address with a future segwit version (v2~v16) IS a "valid" bitcoin address according to the BIP.

The only time you should see those warnings is when a new version of segwit is available but your bitcoinjs-lib's version didn't support that segwit version.

I say "valid" with quotes because it's a higher chance of resulting in an unspendable output, but technically the definition of a "valid" address doesn't match 1:1 to a "fully spendable output"

ie. 1BitcoinEaterAddressDontSendf59kuE is a "valid" address, but coins sent to it are virtually unspendable.

Perhaps the address API should return an object with a warnings array of strings that should be shown to a user.

@tansanDOTeth
Copy link

In v6.0.1 I was able to validate taproot addresses like this:

import * as bitcoinjs from "bitcoinjs-lib";
const value = "bc1pxe5uh7cst3p3qzsuzng0r94uv0tn4wfzum2jflph0km6sjjjp5cqc4906u";

const isBitcoin = !!bitcoinjs.address.toOutputScript(value, bitcoinjs.networks.bitcoin);

I got a warning (Sending to a future segwit version address can lead to loss of funds. End users MUST be warned carefully...) but it validated the address.

In v6.1.0 I had to adjust to validation successfully. Otherwise, I got the error No ECC Library provided. You must call initEccLib() with a valid TinySecp256k1Interface instance if I did not initEccLib(). Based on this comment #1889 (comment) I installed a new lib to make it work again.

import * as bitcoinjs from "bitcoinjs-lib";
import * as ecc from "tiny-secp256k1";
const value = "bc1pxe5uh7cst3p3qzsuzng0r94uv0tn4wfzum2jflph0km6sjjjp5cqc4906u";

bitcoinjs.initEccLib(ecc);
const isBitcoin = !!bitcoinjs.address.toOutputScript(value, bitcoinjs.networks.bitcoin);

So a minor version change introduced a breaking change on my side. I'm curious if this is a fix, feature or bug 🤷‍♂️

Thanks! I couldn't figure out what was happening, and this helped me progress. The fix is not obvious at all.

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

4 participants