search

Bitcoin-com / slp-sdk

Simple Ledger Protocol SDK powered by BITBOX
https://developer.bitcoin.com/slp
MIT License
20 stars 32 forks source link

Fix isTokenUtxo to non SLP transactions #85

Closed thr0wn closed 4 years ago

thr0wn commented 4 years ago

Hi :)

I found that SLP.Utils.isTokenUtxo does not handle validations like SLP.Utils.tokenUtxoDetails. More specifically, the following code of SLP.Utils.isTokenUtxo does not handle null cases of x:

validations = validations.map((x: any) => x.valid)

I think that it should be done like in https://github.com/Bitcoin-com/slp-sdk/blob/master/src/Util.ts#L462, correct?

By the way, thanks for your effort here at slp-sdk, it is a really helpful library!

christroutner commented 4 years ago

Thanks for the PR! It looks like a great addition to the repository, and mirrors a similar PR I just created for tokenUtxoDetails() a couple days ago.

And thanks for alerting us to the issue. We just caught it ourselves.

The only thing we need to do before accepting the PR is for the Developer Services team to review the implications to SLP-aware wallets interacting with isTokenUtxo() if we merge this PR.

We recently got clarity: SLPDB returns null for a BCH-only TXID AND it returns null for SLP transactions that it hasn't processed yet.

That's the main thing we need to take into consideration.

christroutner commented 4 years ago

Going to merge this PR. Thank you @thr0wn!

After this PR is merged, I'll also refactor this function to match the refactoring done to tokenUtxoDetails() in PR #86. The new behavior of this function will return confident false or true. It will only return null if SLPDB has not processed the UTXO yet and has not yet made a determination of its validity.

christroutner commented 4 years ago

Created Issue #87 to capture future work.