Add some tests and an input duplicate checker

src/psbt.js

@@ -13,9 +13,10 @@ const varuint = require('varuint-bitcoin');
 class Psbt extends bip174_1.Psbt {
   constructor(opts = {}) {
     super();
-    this.__NON_WITNESS_UTXO_CACHE = {
+    this.__CACHE = {
       __NON_WITNESS_UTXO_TX_CACHE: [],
       __NON_WITNESS_UTXO_BUF_CACHE: [],
+      __TX_IN_CACHE: {},
     };
     // set defaults
     this.opts = Object.assign({}, DEFAULT_OPTS, opts);
@@ -48,7 +49,7 @@ class Psbt extends bip174_1.Psbt {
     dpew(this, '__EXTRACTED_TX', false, true);
     dpew(this, '__FEE_RATE', false, true);
     dpew(this, '__TX_BUF_CACHE', false, true);
-    dpew(this, '__NON_WITNESS_UTXO_CACHE', false, true);
+    dpew(this, '__CACHE', false, true);
     dpew(this, 'opts', false, true);
   }
   static fromTransaction(txBuf) {
@@ -56,6 +57,7 @@ class Psbt extends bip174_1.Psbt {
     checkTxEmpty(tx);
     const psbt = new this();
     psbt.__TX = tx;
+    checkTxForDupeIns(tx, psbt.__CACHE);
     let inputCount = tx.ins.length;
     let outputCount = tx.outs.length;
     while (inputCount > 0) {
@@ -84,8 +86,12 @@ class Psbt extends bip174_1.Psbt {
     };
     const psbt = super.fromBuffer(buffer, txCountGetter);
     psbt.__TX = tx;
+    checkTxForDupeIns(tx, psbt.__CACHE);
     return psbt;
   }
+  get inputCount() {
+    return this.inputs.length;
+  }
   setMaximumFeeRate(satoshiPerByte) {
     check32Bit(satoshiPerByte); // 42.9 BTC per byte IS excessive... so throw
     this.opts.maximumFeeRate = satoshiPerByte;
@@ -134,14 +140,17 @@ class Psbt extends bip174_1.Psbt {
       const prevHash = Buffer.isBuffer(_inputData.hash)
         ? _inputData.hash
         : bufferutils_1.reverseBuffer(Buffer.from(_inputData.hash, 'hex'));
-      self.__TX.ins.push({
+      // Check if input already exists in cache.
-        hash: prevHash,
+      const input = { hash: prevHash, index: _inputData.index };
-        index: _inputData.index,
+      checkTxInputCache(self.__CACHE, input);
+      self.__TX.ins.push(
+        Object.assign({}, input, {
           script: Buffer.alloc(0),
           sequence:
             _inputData.sequence || transaction_1.Transaction.DEFAULT_SEQUENCE,
           witness: [],
-      });
+        }),
+      );
       return self.__TX.toBuffer();
     };
     super.addInput(inputData, inputAdder);
@@ -182,7 +191,7 @@ class Psbt extends bip174_1.Psbt {
   addNonWitnessUtxoToInput(inputIndex, nonWitnessUtxo) {
     super.addNonWitnessUtxoToInput(inputIndex, nonWitnessUtxo);
     const input = this.inputs[inputIndex];
-    addNonWitnessTxCache(this.__NON_WITNESS_UTXO_CACHE, input, inputIndex);
+    addNonWitnessTxCache(this.__CACHE, input, inputIndex);
     return this;
   }
   extractTransaction(disableFeeCheck) {
@@ -238,9 +247,9 @@ class Psbt extends bip174_1.Psbt {
       if (input.witnessUtxo) {
         inputAmount += input.witnessUtxo.value;
       } else if (input.nonWitnessUtxo) {
-        const cache = this.__NON_WITNESS_UTXO_CACHE;
+        const cache = this.__CACHE;
         if (!cache.__NON_WITNESS_UTXO_TX_CACHE[idx]) {
-          addNonWitnessTxCache(this.__NON_WITNESS_UTXO_CACHE, input, idx);
+          addNonWitnessTxCache(this.__CACHE, input, idx);
         }
         const vout = this.__TX.ins[idx].index;
         const out = cache.__NON_WITNESS_UTXO_TX_CACHE[idx].outs[vout];
@@ -272,7 +281,7 @@ class Psbt extends bip174_1.Psbt {
       inputIndex,
       input,
       this.__TX,
-      this.__NON_WITNESS_UTXO_CACHE,
+      this.__CACHE,
     );
     if (!script) return false;
     const scriptType = classifyScript(script);
@@ -301,7 +310,7 @@ class Psbt extends bip174_1.Psbt {
       inputIndex,
       keyPair.publicKey,
       this.__TX,
-      this.__NON_WITNESS_UTXO_CACHE,
+      this.__CACHE,
     );
     const partialSig = {
       pubkey: keyPair.publicKey,
@@ -318,7 +327,7 @@ class Psbt extends bip174_1.Psbt {
         inputIndex,
         keyPair.publicKey,
         this.__TX,
-        this.__NON_WITNESS_UTXO_CACHE,
+        this.__CACHE,
       );
       Promise.resolve(keyPair.sign(hash)).then(signature => {
         const partialSig = {
@@ -360,6 +369,19 @@ function addNonWitnessTxCache(cache, input, inputIndex) {
     },
   });
 }
+function checkTxForDupeIns(tx, cache) {
+  tx.ins.forEach(input => {
+    checkTxInputCache(cache, input);
+  });
+}
+function checkTxInputCache(cache, input) {
+  const key =
+    bufferutils_1.reverseBuffer(Buffer.from(input.hash)).toString('hex') +
+    ':' +
+    input.index;
+  if (cache.__TX_IN_CACHE[key]) throw new Error('Duplicate input detected.');
+  cache.__TX_IN_CACHE[key] = 1;
+}
 function isFinalized(input) {
   return !!input.finalScriptSig || !!input.finalScriptWitness;
 }

test/fixtures/psbt.json

@@ -304,6 +304,37 @@
       }
     ]
   },
+  "addInput": {
+    "checks": [
+      {
+        "description": "checks for hash and index",
+        "inputData": {
+          "hash": 42
+        },
+        "exception": "Error adding input."
+      },
+      {
+        "description": "checks for hash and index",
+        "inputData": {
+          "hash": "000102030405060708090a0b0c0d0e0f000102030405060708090a0b0c0d0e0f",
+          "index": 2
+        },
+        "equals": "cHNidP8BADMCAAAAAQABAgMEBQYHCAkKCwwNDg8AAQIDBAUGBwgJCgsMDQ4PAgAAAAD/////AAAAAAAAAAA="
+      }
+    ]
+  },
+  "addOutput": {
+    "checks": [
+      {
+        "description": "checks for hash and index",
+        "outputData": {
+          "address": "1P2NFEBp32V2arRwZNww6tgXEV58FG94mr",
+          "value": "xyz"
+        },
+        "exception": "Error adding output."
+      }
+    ]
+  },
   "signInput": {
     "checks": [
       {

test/psbt.js

@@ -23,6 +23,14 @@ const initBuffers = (attr, data) => {
     data.pubkey = b(data.pubkey)
   } else if (attr === 'witnessUtxo') {
     data.script = b(data.script)
+  } else if (attr === 'hash') {
+    if (
+      typeof data === 'string' &&
+      data.match(/^[0-9a-f]*$/i) &&
+      data.length % 2 === 0
+    ) {
+      data = b(data)
+    }
   }
 
   return data
@@ -140,11 +148,55 @@ describe(`Psbt`, () => {
 
   fixtures.bip174.extractor.forEach(f => {
     it('Extracts the expected transaction from a PSBT', () => {
-      const psbt =  Psbt.fromBase64(f.psbt)
+      const psbt1 =  Psbt.fromBase64(f.psbt)
+      const transaction1 = psbt1.extractTransaction(true).toHex()
 
-      const transaction = psbt.extractTransaction().toHex()
+      const psbt2 =  Psbt.fromBase64(f.psbt)
+      const transaction2 = psbt2.extractTransaction().toHex()
 
-      assert.strictEqual(transaction, f.transaction)
+      assert.strictEqual(transaction1, transaction2)
+      assert.strictEqual(transaction1, f.transaction)
+
+      const psbt3 =  Psbt.fromBase64(f.psbt)
+      delete psbt3.inputs[0].finalScriptSig
+      delete psbt3.inputs[0].finalScriptWitness
+      assert.throws(() => {
+        psbt3.extractTransaction()
+      }, new RegExp('Not finalized'))
+
+      const psbt4 =  Psbt.fromBase64(f.psbt)
+      psbt4.setMaximumFeeRate(1)
+      assert.throws(() => {
+        psbt4.extractTransaction()
+      }, new RegExp('Warning: You are paying around [\\d.]+ in fees'))
+
+      const psbt5 =  Psbt.fromBase64(f.psbt)
+      psbt5.extractTransaction(true)
+      const fr1 = psbt5.getFeeRate()
+      const fr2 = psbt5.getFeeRate()
+      assert.strictEqual(fr1, fr2)
+    })
+  })
+
+  describe('signInputAsync', () => {
+    fixtures.signInput.checks.forEach(f => {
+      it(f.description, async () => {
+        const psbtThatShouldsign = Psbt.fromBase64(f.shouldSign.psbt)
+        assert.doesNotReject(async () => {
+          await psbtThatShouldsign.signInputAsync(
+            f.shouldSign.inputToCheck,
+            ECPair.fromWIF(f.shouldSign.WIF),
+          )
+        })
+
+        const psbtThatShouldThrow = Psbt.fromBase64(f.shouldThrow.psbt)
+        assert.rejects(async () => {
+          await psbtThatShouldThrow.signInputAsync(
+            f.shouldThrow.inputToCheck,
+            ECPair.fromWIF(f.shouldThrow.WIF),
+          )
+        }, {message: f.shouldThrow.errorMessage})
+      })
     })
   })
 
@@ -179,6 +231,54 @@ describe(`Psbt`, () => {
     })
   })
 
+  describe('addInput', () => {
+    fixtures.addInput.checks.forEach(f => {
+      for (const attr of Object.keys(f.inputData)) {
+        f.inputData[attr] = initBuffers(attr, f.inputData[attr])
+      }
+      it(f.description, () => {
+        const psbt = new Psbt()
+
+        if (f.exception) {
+          assert.throws(() => {
+            psbt.addInput(f.inputData)
+          }, new RegExp(f.exception))
+        } else {
+          assert.doesNotThrow(() => {
+            psbt.addInput(f.inputData)
+            if (f.equals) {
+              assert.strictEqual(psbt.toBase64(), f.equals)
+            } else {
+              console.log(psbt.toBase64())
+            }
+          })
+        }
+      })
+    })
+  })
+
+  describe('addOutput', () => {
+    fixtures.addOutput.checks.forEach(f => {
+      for (const attr of Object.keys(f.outputData)) {
+        f.outputData[attr] = initBuffers(attr, f.outputData[attr])
+      }
+      it(f.description, () => {
+        const psbt = new Psbt()
+
+        if (f.exception) {
+          assert.throws(() => {
+            psbt.addOutput(f.outputData)
+          }, new RegExp(f.exception))
+        } else {
+          assert.doesNotThrow(() => {
+            psbt.addOutput(f.outputData)
+            console.log(psbt.toBase64())
+          })
+        }
+      })
+    })
+  })
+
   describe('setVersion', () => {
     it('Sets the version value of the unsigned transaction', () => {
       const psbt = new Psbt()
@@ -225,6 +325,16 @@ describe(`Psbt`, () => {
     })
   })
 
+  describe('setMaximumFeeRate', () => {
+    it('Sets the maximumFeeRate value', () => {
+      const psbt = new Psbt()
+
+      assert.strictEqual(psbt.opts.maximumFeeRate, 5000)
+      psbt.setMaximumFeeRate(6000)
+      assert.strictEqual(psbt.opts.maximumFeeRate, 6000)
+    })
+  })
+
   describe('Method return types', () => {
     it('fromTransaction returns Psbt type (not base class)', () => {
       const psbt = Psbt.fromTransaction(Buffer.from([2,0,0,0,0,0,0,0,0,0]));

ts_src/psbt.ts

@@ -26,6 +26,7 @@ export class Psbt extends PsbtBase {
     checkTxEmpty(tx);
     const psbt = new this() as Psbt;
     psbt.__TX = tx;
+    checkTxForDupeIns(tx, psbt.__CACHE);
     let inputCount = tx.ins.length;
     let outputCount = tx.outs.length;
     while (inputCount > 0) {
@@ -62,11 +63,13 @@ export class Psbt extends PsbtBase {
     };
     const psbt = super.fromBuffer(buffer, txCountGetter) as Psbt;
     psbt.__TX = tx!;
+    checkTxForDupeIns(tx!, psbt.__CACHE);
     return psbt as InstanceType<T>;
   }
-  private __NON_WITNESS_UTXO_CACHE = {
+  private __CACHE = {
     __NON_WITNESS_UTXO_TX_CACHE: [] as Transaction[],
     __NON_WITNESS_UTXO_BUF_CACHE: [] as Buffer[],
+    __TX_IN_CACHE: {} as { [index: string]: number },
   };
   private __TX: Transaction;
   private __TX_BUF_CACHE?: Buffer;
@@ -113,10 +116,14 @@ export class Psbt extends PsbtBase {
     dpew(this, '__EXTRACTED_TX', false, true);
     dpew(this, '__FEE_RATE', false, true);
     dpew(this, '__TX_BUF_CACHE', false, true);
-    dpew(this, '__NON_WITNESS_UTXO_CACHE', false, true);
+    dpew(this, '__CACHE', false, true);
     dpew(this, 'opts', false, true);
   }
 
+  get inputCount(): number {
+    return this.inputs.length;
+  }
+
   setMaximumFeeRate(satoshiPerByte: number): void {
     check32Bit(satoshiPerByte); // 42.9 BTC per byte IS excessive... so throw
     this.opts.maximumFeeRate = satoshiPerByte;
@@ -172,9 +179,13 @@ export class Psbt extends PsbtBase {
       const prevHash = Buffer.isBuffer(_inputData.hash)
         ? _inputData.hash
         : reverseBuffer(Buffer.from(_inputData.hash, 'hex'));
+
+      // Check if input already exists in cache.
+      const input = { hash: prevHash, index: _inputData.index };
+      checkTxInputCache(self.__CACHE, input);
+
       self.__TX.ins.push({
-        hash: prevHash,
+        ...input,
-        index: _inputData.index,
         script: Buffer.alloc(0),
         sequence: _inputData.sequence || Transaction.DEFAULT_SEQUENCE,
         witness: [],
@@ -227,7 +238,7 @@ export class Psbt extends PsbtBase {
   ): this {
     super.addNonWitnessUtxoToInput(inputIndex, nonWitnessUtxo);
     const input = this.inputs[inputIndex];
-    addNonWitnessTxCache(this.__NON_WITNESS_UTXO_CACHE, input, inputIndex);
+    addNonWitnessTxCache(this.__CACHE, input, inputIndex);
     return this;
   }
 
@@ -285,9 +296,9 @@ export class Psbt extends PsbtBase {
       if (input.witnessUtxo) {
         inputAmount += input.witnessUtxo.value;
       } else if (input.nonWitnessUtxo) {
-        const cache = this.__NON_WITNESS_UTXO_CACHE;
+        const cache = this.__CACHE;
         if (!cache.__NON_WITNESS_UTXO_TX_CACHE[idx]) {
-          addNonWitnessTxCache(this.__NON_WITNESS_UTXO_CACHE, input, idx);
+          addNonWitnessTxCache(this.__CACHE, input, idx);
         }
         const vout = this.__TX.ins[idx].index;
         const out = cache.__NON_WITNESS_UTXO_TX_CACHE[idx].outs[vout] as Output;
@@ -327,7 +338,7 @@ export class Psbt extends PsbtBase {
       inputIndex,
       input,
       this.__TX,
-      this.__NON_WITNESS_UTXO_CACHE,
+      this.__CACHE,
     );
     if (!script) return false;
 
@@ -361,7 +372,7 @@ export class Psbt extends PsbtBase {
       inputIndex,
       keyPair.publicKey,
       this.__TX,
-      this.__NON_WITNESS_UTXO_CACHE,
+      this.__CACHE,
     );
 
     const partialSig = {
@@ -382,7 +393,7 @@ export class Psbt extends PsbtBase {
           inputIndex,
           keyPair.publicKey,
           this.__TX,
-          this.__NON_WITNESS_UTXO_CACHE,
+          this.__CACHE,
         );
 
         Promise.resolve(keyPair.sign(hash)).then(signature => {
@@ -409,9 +420,10 @@ export class Psbt extends PsbtBase {
 //
 //
 
-interface NonWitnessUtxoCache {
+interface PsbtCache {
   __NON_WITNESS_UTXO_TX_CACHE: Transaction[];
   __NON_WITNESS_UTXO_BUF_CACHE: Buffer[];
+  __TX_IN_CACHE: { [index: string]: number };
 }
 
 interface PsbtOptsOptional {
@@ -430,7 +442,7 @@ const DEFAULT_OPTS = {
 };
 
 function addNonWitnessTxCache(
-  cache: NonWitnessUtxoCache,
+  cache: PsbtCache,
   input: PsbtInput,
   inputIndex: number,
 ): void {
@@ -460,6 +472,22 @@ function addNonWitnessTxCache(
   });
 }
 
+function checkTxForDupeIns(tx: Transaction, cache: PsbtCache): void {
+  tx.ins.forEach(input => {
+    checkTxInputCache(cache, input);
+  });
+}
+
+function checkTxInputCache(
+  cache: PsbtCache,
+  input: { hash: Buffer; index: number },
+): void {
+  const key =
+    reverseBuffer(Buffer.from(input.hash)).toString('hex') + ':' + input.index;
+  if (cache.__TX_IN_CACHE[key]) throw new Error('Duplicate input detected.');
+  cache.__TX_IN_CACHE[key] = 1;
+}
+
 function isFinalized(input: PsbtInput): boolean {
   return !!input.finalScriptSig || !!input.finalScriptWitness;
 }
@@ -469,7 +497,7 @@ function getHashAndSighashType(
   inputIndex: number,
   pubkey: Buffer,
   unsignedTx: Transaction,
-  cache: NonWitnessUtxoCache,
+  cache: PsbtCache,
 ): {
   hash: Buffer;
   sighashType: number;
@@ -630,7 +658,7 @@ const getHashForSig = (
   inputIndex: number,
   input: PsbtInput,
   unsignedTx: Transaction,
-  cache: NonWitnessUtxoCache,
+  cache: PsbtCache,
 ): HashForSigData => {
   const sighashType = input.sighashType || Transaction.SIGHASH_ALL;
   let hash: Buffer;
@@ -780,7 +808,7 @@ function getScriptFromInput(
   inputIndex: number,
   input: PsbtInput,
   unsignedTx: Transaction,
-  cache: NonWitnessUtxoCache,
+  cache: PsbtCache,
 ): GetScriptReturn {
   const res: GetScriptReturn = {
     script: null,

types/psbt.d.ts

@@ -7,13 +7,14 @@ import { Transaction } from './transaction';
 export declare class Psbt extends PsbtBase {
     static fromTransaction<T extends typeof PsbtBase>(this: T, txBuf: Buffer): InstanceType<T>;
     static fromBuffer<T extends typeof PsbtBase>(this: T, buffer: Buffer): InstanceType<T>;
-    private __NON_WITNESS_UTXO_CACHE;
+    private __CACHE;
     private __TX;
     private __TX_BUF_CACHE?;
     private __FEE_RATE?;
     private __EXTRACTED_TX?;
     private opts;
     constructor(opts?: PsbtOptsOptional);
+    readonly inputCount: number;
     setMaximumFeeRate(satoshiPerByte: number): void;
     setVersion(version: number): this;
     setLocktime(locktime: number): this;