PSBT internal transaction property getters

[lukechilds]

This PR exposes some properties of the transaction internal to the PSBT in a readonly way that prevents mutation.

const psbt = bitcoin.Psbt.fromBase64("cHNidP8BAFUCAAAAASeaIyOl37UfxF8iD6WLD8E+HjNCeSqF1+Ns1jM7XLw5AAAAAAD/////AaBa6gsAAAAAGXapFP/pwAYQl8w7Y28ssEYPpPxCfStFiKwAAAAAAAEBIJVe6gsAAAAAF6kUY0UgD2jRieGtwN8cTRbqjxTA2+uHIgIDsTQcy6doO2r08SOM1ul+cWfVafrEfx5I1HVBhENVvUZGMEMCIAQktY7/qqaU4VWepck7v9SokGQiQFXN8HC2dxRpRC0HAh9cjrD+plFtYLisszrWTt5g6Hhb+zqpS5m9+GFR25qaAQEEIgAgdx/RitRZZm3Unz1WTj28QvTIR3TjYK2haBao7UiNVoEBBUdSIQOxNBzLp2g7avTxI4zW6X5xZ9Vp+sR/HkjUdUGEQ1W9RiED3lXR4drIBeP4pYwfv5uUwC89uq/hJ/78pJlfJvggg71SriIGA7E0HMunaDtq9PEjjNbpfnFn1Wn6xH8eSNR1QYRDVb1GELSmumcAAACAAAAAgAQAAIAiBgPeVdHh2sgF4/iljB+/m5TALz26r+En/vykmV8m+CCDvRC0prpnAAAAgAAAAIAFAACAAAA=");

psbt.version
// 2
psbt.locktime
// 0
psbt.inputs
// [
//   {
//     hash: <Buffer 27 9a 23 23 a5 df b5 1f c4 5f 22 0f a5 8b 0f c1 3e 1e 33 42 79 2a 85 d7 e3 6c d6 33 3b 5c bc 39>,
//     index: 0,
//     script: <Buffer >,
//     sequence: 4294967295,
//     witness: []
//   }
// ]
psbt.outputs
// [
//   {
//     script: <Buffer 76 a9 14 ff e9 c0 06 10 97 cc 3b 63 6f 2c b0 46 0f a4 fc // 42 7d 2b 45 88 ac>,
//     value: 199908000
//   }
// ]

One issue is that it's a little confusing that we now have two sets of inputs/outputs arrays. One for the PSBT data and one for the internal transaction data:

psbt.inputs
// [
//   {
//     hash: <Buffer 27 9a 23 23 a5 df b5 1f c4 5f 22 0f a5 8b 0f c1 3e 1e 33 42 79 2a 85 d7 e3 6c d6 33 3b 5c bc 39>,
//     index: 0,
//     script: <Buffer >,
//     sequence: 4294967295,
//     witness: []
//   }
// ]
psbt.data.inputs
// [
//   {
//     witnessUtxo: {
//       script: <Buffer a9 14 63 45 20 0f 68 d1 89 e1 ad c0 df 1c 4d 16 ea 8f 14 c0 db eb 87>,
//       value: 199909013
//     },
//     partialSig: [ [Object] ],
//     redeemScript: <Buffer 00 20 77 1f d1 8a d4 59 66 6d d4 9f 3d 56 4e 3d bc 42 f4 c8 47 74 e3 60 ad a1 68 16 a8 ed 48 8d 56 81>,
//     witnessScript: <Buffer 52 21 03 b1 34 1c cb a7 68 3b 6a f4 f1 23 8c d6 e9 7e 71 67 d5 69 fa c4 7f 1e 48 d4 75 41 84 43 55 bd 46 21 03 de 55 d1 e1 da c8 05 e3 f8 a5 8c 1f bf ... 21 more bytes>,
//     bip32Derivation: [ [Object], [Object] ]
//   }
// ]

Maybe that's ok or maybe we should call it psbt.txInputs to make it a little clearer?

[junderw]

Thoughts:

• maybe we should add setters for version and locktime that call setVersion setLocktime etc.
• scriptSig and witness should not be given for inputs. Those are dealt with in finalScriptSig and finalWitnessScript
• perhaps we should add address to outputs using fromOutputScript and this.network
• txInputs, txOutputs should be good names instead of inputs and outputs.
• add dpew(this, 'txInputs', true, false); etc in the constructor for everything to make sure it shows up. (true, false is enumerable but not writable)

[lukechilds]

How important is it that the getters are enumerable?

Maybe I'm missing something but it seems a bit awkward to get working with getters.

If we do dpew(this, 'version', true, false); then version is now enumerable but it's just undefined, the getters/setters don't work. I think because Object.defineProperty overrides them? Same happens if I just do: Object.defineProperty(this, 'version', { enumerable: true });

We can just not use the nice get/set syntax and instead define them in Object.defineProperty in the constructor:

constructor()
  Object.defineProperty(this, 'version', {
    enumerable: true,
    get: (): number => this.__CACHE.__TX.version,
    set: (version: number): Psbt => this.setVersion(version),
  });
}

This works, version is now enumerable and the getters/setters work.

However this confuses TypeScript. It doesn't understand what's happening in Object.defineProperty so if you try to do stuff with version TypeScript complains that: Property 'version' does not exist on type 'Psbt'.

We can fix TypeScript by first creating a property and setting a random value of the correct type so it can understand what's going on, then overwriting that definition in the constructor to what we actually want with Object.defineProperty:

version: number = 0;

constructor()
  Object.defineProperty(this, 'version', {
    enumerable: true,
    get: (): number => this.__CACHE.__TX.version,
    set: (version: number): Psbt => this.setVersion(version),
  });
}

It feels pretty hacky and we need to do that for each property. We're just tricking TypeScript into thinking it knows what's going on, we could be returning any other type from the getter and it wouldn't realise.

It's also causing errors in the Should clone a psbt exactly with no reference test. I can have a look and fix it but just wanna make sure you actually want them enumerable if it involves this code.

[junderw]

Re: enumerable

Yeah, forget about that part.