vulpemventures / marina

Liquid Wallet browser extension
MIT License
37 stars 19 forks source link

marina.signTransaction: support passing both psetv0 and v2 #400

Closed tiero closed 1 year ago

tiero commented 2 years ago

try/catch with Psbt an Pset classes, before returning error to user

Randy808 commented 8 months ago

Is this still something on the roadmap?

screenshot
tiero commented 8 months ago

this seems to be the Transaction Version, not PSET/PSBT in particular. A reason to not use version 2?

Randy808 commented 8 months ago

Sorry for the late response but here's the full context:

After the marina api change I tried to make a shim wrapper around the utxos returned from 'marina.getCoins()' to match the old format. It seemed to work okay until I serialized the Psbt (the class for psetv0) and sent it to marina for signing. This is what it looks like before serialization via 'pset.toBase64()':

Screenshot 2024-01-18 at 2 02 38 PM

I also added the longer base64-encoded psbt in the attached file here.

When 'marina.signTransaction' is called it goes to the sign-pset.tsx file and tries to decode the base64 with the Pset class contained in the liquidjs-lib repo: https://github.com/vulpemventures/marina/blob/aba89e66d969ed31383bc8a9a15efe86cebf2f05/src/extension/popups/sign-pset.tsx#L127

In the 'fromBase64' method in Pset it calls 'this.fromBuffer': https://github.com/vulpemventures/liquidjs-lib/blob/69e3e9ee3483000d36c79b9a6cd6ac7a43532872/src/psetv2/pset.js#L72

And in the 'fromBuffer' method it calls 'sanityCheck' after failing to decode a KeyPair: https://github.com/vulpemventures/liquidjs-lib/blob/69e3e9ee3483000d36c79b9a6cd6ac7a43532872/src/psetv2/globals.js#L43

And in the sanity check, the error about the txVersion always gets called: https://github.com/vulpemventures/liquidjs-lib/blob/69e3e9ee3483000d36c79b9a6cd6ac7a43532872/src/psetv2/globals.js#L153

I assume this is stemming from a difference in parsing between psetv0 and psetv2.

Either way this will always get hit whenever we're in the first 'catch' of 'fromBuffer' since the PsetGlobal is initialized without any arguments. The error message being incorrect should be addressed in a separate issue though: https://github.com/vulpemventures/liquidjs-lib/blob/69e3e9ee3483000d36c79b9a6cd6ac7a43532872/src/psetv2/globals.js#L37

Edit: I can add a try-catch to check Psbt parsing before marina attempts to parse the base64 pset using Pset at the place pointed to by the snippet above: https://github.com/vulpemventures/marina/blob/aba89e66d969ed31383bc8a9a15efe86cebf2f05/src/extension/popups/sign-pset.tsx#L127

I also changed psetv1 to psetv0 since I'm not too familiar with the spec for versioning psets and I guess v1 was skipped 😅

We can switch to the psetv2 and stay within the Vulpem ecosystem of libraries but last time I tried to build the secp256k1-zkp repo I ran into this issue: https://github.com/vulpemventures/secp256k1-zkp/issues/52