leather-io / extension

Leather browser extension
https://leather.io
MIT License
295 stars 140 forks source link

Error listing SRC-20 tokens on software wallet with openstamp.io #5548

Closed 314159265359879 closed 3 months ago

314159265359879 commented 3 months ago

I have not been able to reproduce this issue. I went through several checks with the user, who used 6.42.1 and disabled other extensions on Chrome.

This error pops up when the user tries to buy any SRC token on the marketplace. And when they try to list an SRC-20 token, The list with the amount and unit price and confirm... then the error shows.

They see this error: image

Error: Unknown address type=ms
    at Object.encode (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/90.js:217047:19)
    at getAddressFromOutScript (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/90.js:177440:93)
    at chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/index.js:19731:77
    at [http://Array.map](https://t.co/3aoIrbrvL1) (<anonymous>)
    at chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/index.js:19726:19
    at Object.useMemo (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/90.js:147957:189)
    at exports.useMemo (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/90.js:152406:48)
    at useParsedOutputs (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/index.js:19725:27)
    at usePsbtDetails (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/index.js:19873:25)
    at PsbtSigner (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/index.js:19965:7)
    at Nh (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/90.js:147940:137)
    at Vk (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/90.js:148069:337)
    at Uk (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/90.js:148059:389)
    at Tk (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/90.js:148059:320)
    at Ik (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/90.js:148059:180)
    at Nk (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/90.js:148050:88)
    at Ek (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/90.js:148052:300)
    at jg (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/90.js:147907:105)
    at Wk (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/90.js:148062:470)
    at Pk (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/90.js:148060:398)
    at Gk (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/90.js:148049:269)
    at J (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/90.js:158036:203)
    at MessagePort.R (chrome-extension://ldinpeekobnhjjdofggfgjlcehhmanlj/90.js:158037:128)
markmhendrickson commented 3 months ago

It may be worth contacting the developer since the PSBT may be badly formatted?

314159265359879 commented 3 months ago

This is reproducible with wallet version 6.42.1 after all, I thought I already tested this with the latest wallet but it turned out to be on 6.42.0, and there it was still fine. This broke in the latest release (with the testnet fix):

  1. go to openstamp.io
  2. Connect wallet that has SRC20 (dev1)
  3. Go to assets, click list
  4. click "list" Enter amount (i.e. 100)
  5. add price (i.e.) 300000 sats
  6. confirm
  7. See error (as displayed above)

The same error is displayed when trying to transfer tokens.

https://github.com/leather-wallet/extension/assets/33360391/9c4bc8aa-7ba8-49fc-afd0-e22fa99c5f4b

openstamp.io also mentioned reporting another signing issue earlier which came up from wallet version 6.41.1: #5015

fbwoolf commented 3 months ago

@markmhendrickson @kyranjamie This is a prob again w/ the signer lib not supporting the 'ms' address type. We created a function getAddressFromOutScript to get around it, but if you call Address.encode() now, it is going to hit this error. I am trying to understand exactly where this changed, but in a recent commit there is now an added function for what we are doing here, but again, it isn't supporting the type 'ms' and hits the same error:

Current error using our custom function: Screenshot 2024-06-26 at 12 55 04 PM

Using his function hits same error: Screenshot 2024-06-26 at 1 05 15 PM

Commit adding the new function, but not release yet ...I just copied it to test: https://github.com/paulmillr/scure-btc-signer/commit/e1460341e88b161e99999b31697ab1f4401e0192

Hitting this error (trying to pinpoint when changed): https://github.com/paulmillr/scure-btc-signer/blob/bae4ac5ee1b442b20046052ecedd33ca32827e1a/src/payment.ts#L717

I don't know how to proceed here to support how these Stamps txs are being constructed? I think we still need the address for type 'ms' so we can't just ignore this? For type 'unknown' we just return 'unknown' to display in the UI, can we do the same for this case? 🤔 I wouldn't advise doing this.

The other issue linked above is hitting a similar error in the lib here: https://github.com/paulmillr/scure-btc-signer/blob/bae4ac5ee1b442b20046052ecedd33ca32827e1a/src/payment.ts#L244

I believe these txs just aren't being constructed in a way that the signer lib supports? These errors are in place for a reason?

kyranjamie commented 3 months ago

Thanks for the research @fbwoolf.

The simplest fix here might be to just downgrade @scure/btc-signer for now, if this unblocks the issue?

I wonder if it might be a good idea for us to use both signer libraries, with bitcoinjs-lib as the fallback? This sounds like a bit of a hack, but might make for a more robust signing process. Developers use both libraries, so us using both would avoid any library-specific edge cases. We could get analytics on how/why txs fail with one and not the other.

I'm inclined not to straight swap btc-signer for bitcoinjs-lib as afaik this lib has issues with RN owing to its use of Buffer, and might have the reverse problem of some edge case txs that work in signer but not bitcoinjs.

fbwoolf commented 3 months ago

The fix is here in the mono repo. I need to install the change into the extension. We simply had an outdated fn in the mono repo and installed it back into the extension: https://github.com/leather-io/mono/pull/253