bitcoinjs / bitcoinjs-lib

A javascript Bitcoin library for node.js and browsers.
MIT License
5.7k stars 2.11k forks source link

Error when deserializing a PSBT #2114

Closed antondalgren closed 4 months ago

antondalgren commented 4 months ago

When trying to deserialize a PSBT that only has one output or one input defined crashes.

See example below.

import {Psbt} from "bitcoinjs-lib";

const psbt = new Psbt();
psbt.addOutput({
  address: "bc1qfxm76mpx6z8eg5jg2du2gr9020j27a94k8u7vx",
  value: 1,
})

const hex = psbt.toHex();
Psbt.fromHex(hex); // RangeError: Trying to access beyond buffer length
jasonandjay commented 4 months ago

image After debugging, I found the reason ParsePSBT will set vinLen to 1, but actual no input

jasonandjay commented 4 months ago

When constructing PSBT verion = 2 inputLen = 0 outputLen = 1 output[0].value = 1 So we get 2000000000101... of Unsigned Transaction after hex();

When parsing PSBT we get version = 2 marker = 0 flag = 1

which led to error result of this transaction is witness

and then get vinLen = 1, but actual vinLen = 0

@junderw Do you have any suggestions on this?

junderw commented 4 months ago

Yeah. When parsing a transaction, a non-segwit tx with 1 output will have 0x00 and 0x01 for txinlen and txoutlen, and since the transaction parsing logic is matching on that specific pattern for segwit, there's really nothing we can do.

Solution: Don't make a Psbt with 1 output and 0 inputs.

antondalgren commented 4 months ago

Well... That's sad :( Thank you for taking the time to debug this for me.

I guess it's not possible to differentiate a segwit transaction from a non-segwit transaction based on the bytes following 0x0001?

One thing to add could be to throw an error when trying to serialize a non-segwit tx since it's not possible to deserialize it. It could be used to add context to why it's not possible to do that.

junderw commented 4 months ago

Well... That's sad

We could probably create some sort of hack for Psbt specifically (since Psbt internal transactions have the invariant that they contain no input scripts or witnesses, so they would only be serialized as non-segwit), but it would increase complexity and add some invariants to the library in exchange for.......... what?

I don't understand the use case, so I'm comparing having to add something that is definitely annoying against losing the ability to make something that, as far as I know, no one needs.

antondalgren commented 4 months ago

Yeah, I understand you. I wouldn't like creating a hack either. Let's just leave this as is.

Thanks!