Psbtv2 operator role validation (#65)

* feat: psbtv2 operator role validation

* chore: run changeset

* docs: fix typo for psbtv2 api

* feat: implement PsbtV2.isReadyForTransactionExtractor

* test: isReadyForTransactionExtractor

* style: explicit -> implicit types and cleaner conditional check

.changeset/cool-cows-learn.md

@@ -0,0 +1,5 @@
+---
+"@caravan/psbt": minor
+---
+
+PsbtV2 operator role validation getters are added to provide a way for validating role readiness. These getters are used in some Constructor and Signer methods. For example, an error will be thrown if `addPartialSig` is called when the PsbtV2 is not ready for a Signer.

packages/caravan-psbt/README.md

@@ -19,6 +19,12 @@ A set of utilities for working with PSBTs.
 - [PSBTv2](#psbtv2)
   - [Exports](#psbtv2-exports)
     - [`class PsbtV2`](#class-psbtv2)
+      - [`get isReadyForConstructor`](#get-isreadyforconstructor)
+      - [`get isReadyForUpdater`](#get-isreadyforupdater)
+      - [`get isReadyForSigner`](#get-isreadyforsigner)
+      - [`get isReadyForCombiner`](#get-isreadyforcombiner)
+      - [`get isReadyForInputFinalizer`](#get-isreadyforinputfinalizer)
+      - [`get isReadyForTransactionExtractor`](#get-isreadyfortransactionextractor)
       - [`get nLockTime`](#get-nlocktime)
       - [`public dangerouslySetGlobalTxVersion1`](#public-dangerouslysetglobaltxversion1)
       - [`public addGlobalXpub`](#public-addglobalxpub)
@@ -30,8 +36,6 @@ A set of utilities for working with PSBTs.
       - [`public removePartialSig`](#public-removepartialsig)
       - [`static PsbtV2.FromV0`](#static-psbtv2fromv0)
     - [`function getPsbtVersionNumber`](#function-getpsbtversionnumber)
-    - [`abstract class PsbtV2Maps`](#abstract-class-psbtv2maps)
-      - [`public copy`](#public-copy)
 - [Concepts](#concepts)
   - [The operator role saga](#the-operator-role-saga)
 - [TODO](#todo)
@@ -104,6 +108,56 @@ An object class representing a PSBTv2 and its current state. While not yet compl
 
 Getters and setters are provided for the keytypes defined in [BIP 174](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki) and [BIP 370](https://github.com/bitcoin/bips/blob/master/bip-0370.mediawiki). These getters/setters are named after the keytype in ALL_CAPS_SNAKE like `PsbtV2.PSBT_GLOBAL_VERSION`. Additional getters/setters may be listed in the following API documentation.
 
+##### `get isReadyForConstructor`
+
+A getter to check readiness for an operator role. Operator roles are defined in [BIP 174](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki#user-content-Roles) and [BIP 370](https://github.com/bitcoin/bips/blob/master/bip-0370.mediawiki#user-content-Roles).
+
+Returns `true` if the PsbtV2 is ready for an operator taking the Constructor role.
+
+This check assumes that the Creator used this class's constructor method to initialize the PsbtV2 without passing a psbt (constructor defaults were set).
+
+##### `get isReadyForUpdater`
+
+A getter to check readiness for an operator role. Operator roles are defined in [BIP 174](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki#user-content-Roles) and [BIP 370](https://github.com/bitcoin/bips/blob/master/bip-0370.mediawiki#user-content-Roles).
+
+Returns `true` if the PsbtV2 is ready for an operator taking the Updater role.
+
+Before signatures are added, but after an input is added, a PsbtV2 is likely to be ready for Constructor, ready for Updater, and ready for Signer simultaneously.
+
+According to BIP370, the Updater can modify the sequence number, but it is unclear if the Updater retains permissions provided in psbtv0 (BIP174). It is likely not the case that the Updater has the same permissions as previously because it seems to now be the realm of the Constructor to add inputs and outputs.
+
+##### `get isReadyForSigner`
+
+A getter to check readiness for an operator role. Operator roles are defined in [BIP 174](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki#user-content-Roles) and [BIP 370](https://github.com/bitcoin/bips/blob/master/bip-0370.mediawiki#user-content-Roles).
+
+Returns `true` if the PsbtV2 is ready for an operator taking the Signer role.
+
+Right now, this method only checks for two things: There is an input for signing and `this.isReadyForTransactionExtractor === false`. The point of the latter is to check that the PsbtV2 has not been finalized.
+
+A future improvement to this method might be to more thoroughly check inputs to determine if the PsbtV2 does or does not need to collect more signatures.
+
+##### `get isReadyForCombiner`
+
+A getter to check readiness for an operator role. Operator roles are defined in [BIP 174](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki#user-content-Roles) and [BIP 370](https://github.com/bitcoin/bips/blob/master/bip-0370.mediawiki#user-content-Roles).
+
+Returns `true` if the PsbtV2 is ready for an operator taking the Combiner role.
+
+Since a Combiner can potentially provide everything needed to a mostly blank PsbtV2, instances of a PsbtV2 are likely to return true as long as inputs have not been finalized.
+
+##### `get isReadyForInputFinalizer`
+
+A getter to check readiness for an operator role. Operator roles are defined in [BIP 174](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki#user-content-Roles) and [BIP 370](https://github.com/bitcoin/bips/blob/master/bip-0370.mediawiki#user-content-Roles).
+
+Callable, but unimplemented. Returns `undefined`.
+
+##### `get isReadyForTransactionExtractor`
+
+A getter to check readiness for an operator role. Operator roles are defined in [BIP 174](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki#user-content-Roles) and [BIP 370](https://github.com/bitcoin/bips/blob/master/bip-0370.mediawiki#user-content-Roles).
+
+Returns `true` if the PsbtV2 is ready for an operator taking the Transaction Extractor role.
+
+If all the inputs have been finalized, then the psbt is ready for the Transaction Extractor. According to BIP 174, it's the responsibility of the Input Finalizer to add scriptSigs or scriptWitnesses and then remove other details besides the UTXO. This getter checks that the Input Finalizer has finished its job.
+
 ##### `get nLockTime`
 
 Returns the `nLockTime` field for the psbt as if it were a bitcoin transaction.
@@ -114,7 +168,7 @@ From BIP 370:
 
 ##### `public dangerouslySetGlobalTxVersion1`
 
-A helper method for compatibility, but seems to no-longer be required.
+A helper method for compatibility. Some devices require a psbtV2 configured with a transaction version of 1. BIP370 leaves room for this if the Creator is also the Constructor.
 
 ##### `public addGlobalXpub`
 
@@ -171,27 +225,28 @@ Attempts to return a `PsbtV2` by converting from a PSBTv0 string or Buffer
 
 Attempts to extract the version number as uint32LE from raw psbt regardless of psbt validity.
 
-#### `abstract class PsbtV2Maps`
+## Concepts
 
-This is provided for utility for compatibility and to allow for mapping, map copying, and serialization operations for psbts. This does almost no validation, so do not rely on it for ensuring a valid psbt.
+### The operator role saga
 
-##### `public copy`
+The PSBT is a resource which may be passed between several operators or services. It's best to look at the operator roles as stages of a saga. The next valid operator role(s) can be determined by the state of the PSBT. The actions allowed for a PSBT are determined by which operator role the PSBT can be now and which role it could be next. See the following blog article at Unchained for a more detailed illustration: [Operator roles: Life stages in the saga of a PSBT](https://unchained.com/blog/operator-roles-life-stages-in-the-saga-of-a-psbt/)
 
-Copies the maps in this PsbtV2 object to another PsbtV2 object.
+### TODO
 
-NOTE: This copy method is made available to achieve parity with the PSBT api required by `ledger-bitcoin` for creating merklized PSBTs. HOWEVER, it is not recommended to use this when avoidable as copying maps bypasses the validation defined in the constructor, so it could create a psbtv2 in an invalid psbt state. PsbtV2.serialize is preferable whenever possible.
+#### PsbtV2
 
-## Concepts
+##### Operator role validation
 
-### The operator role saga
+Work remains for determining readiness for operator roles Input Finalizer and Transaction Extractor. The getters responsible for these checks are `isReadyForInputFinalizer` and `isReadyForTransactionExtractor`. Work also remains to expand the PsbtV2 method functionality beyond the Signer role. A huge benefit might be gained from building methods aimed at the Combiner role.
 
-The PSBT is a resource which may be passed between several operators or services. It's best to look at the operator roles as stages of a saga. The next valid operator role(s) can be determined by the state of the PSBT. The actions allowed for a PSBT are determined by which operator role the PSBT can be now and which role it could be next. See the following blog article at Unchained for a more detailed illustration: [Operator roles: Life stages in the saga of a PSBT](https://unchained.com/blog/operator-roles-life-stages-in-the-saga-of-a-psbt/)
+##### Class constructor
 
-### TODO
+The constructor must be able to handle values which the Creator role is responsible for. Currently, the constructor can only accept an optional psbt which it parses to configure itself. It would be ideal if a fresh PsbtV2 instance could be initialized with minimal arguments for which the Creator role is responsible. See `private create()`.
+
+##### Add input timelocks
 
-Work remains on PSBTv2 for determining the next valid operator role(s) and restricting actions based on PSBT state. In other words, if the state of the PSBT suggests the PSBT might be in role A and is ready for role B, it should only allow actions within the context of role A or B.
+The `public addInput` must be able to properly handle input locktimes which interact with the global value.
 
-The following list is a non-comprehensive list of validation checks which must be performed:
+##### Add input sighash_single
 
-- Check ready for Updater role on `addInput`, `addOutput`, `deleteInput`, and `deleteOuput`.
-- Check ready for Signer role on `addPartialSig`.
+The `public addInput` must be able to properly handle new inputs when the psbt has a `SIGHASH_SINGLE` flag on `PSBT_GLOBAL_TX_MODIFIABLE`.

packages/caravan-psbt/src/psbtv2/psbtv2.test.ts

@@ -1,5 +1,6 @@
 import { PsbtV2, getPsbtVersionNumber } from "./";
 import { test } from "@jest/globals";
+import { KeyType, PsbtGlobalTxModifiableBits } from "./types";
 
 const BIP_370_VECTORS_INVALID_PSBT = [
   // Case: PSBTv0 but with PSBT_GLOBAL_VERSION set to 2.
@@ -818,6 +819,149 @@ describe("PsbtV2", () => {
   });
 });
 
+describe("PsbtV2.isReadyForConstructor", () => {
+  let psbt: PsbtV2;
+
+  beforeEach(() => {
+    psbt = new PsbtV2();
+  });
+
+  it("Returns not ready for Constructor when PSBT_GLOBAL_FALLBACK_LOCKTIME is not set", () => {
+    psbt.PSBT_GLOBAL_FALLBACK_LOCKTIME = null;
+    expect(psbt.isReadyForConstructor).toBe(false);
+  });
+
+  it("Returns not ready for Constructor when neither inputs or outputs are modifiable", () => {
+    psbt.PSBT_GLOBAL_TX_MODIFIABLE = [];
+    expect(psbt.isReadyForConstructor).toBe(false);
+  });
+
+  it("Returns ready for Constructor when created with the object class constructor method", () => {
+    expect(psbt.isReadyForConstructor).toBe(true);
+  });
+
+  it("Returns ready for Constructor when a custom PSBT_GLOBAL_FALLBACK_LOCKTIME has been set", () => {
+    psbt.PSBT_GLOBAL_FALLBACK_LOCKTIME = 500000000;
+    expect(psbt.isReadyForConstructor).toBe(true);
+  });
+
+  it("Returns ready for Constructor when at least inputs or outputs are modifiable", () => {
+    psbt.PSBT_GLOBAL_TX_MODIFIABLE = [PsbtGlobalTxModifiableBits.INPUTS];
+    expect(psbt.isReadyForConstructor).toBe(true);
+    psbt.PSBT_GLOBAL_TX_MODIFIABLE = [PsbtGlobalTxModifiableBits.OUTPUTS];
+    expect(psbt.isReadyForConstructor).toBe(true);
+  });
+});
+
+describe("PsbtV2.isReadyForUpdater", () => {
+  let psbt: PsbtV2;
+
+  beforeEach(() => {
+    psbt = new PsbtV2();
+    psbt.addInput({ previousTxId: Buffer.from([0x00]), outputIndex: 0 });
+    psbt.PSBT_GLOBAL_TX_MODIFIABLE = [PsbtGlobalTxModifiableBits.INPUTS];
+  });
+
+  it("Returns not ready for Updater when there are no inputs to update", () => {
+    psbt.deleteInput(0);
+    expect(psbt.isReadyForUpdater).toBe(false);
+  });
+
+  it("Returns not ready for Updater when there are no modifiable inputs", () => {
+    psbt.PSBT_GLOBAL_TX_MODIFIABLE = [];
+    expect(psbt.isReadyForUpdater).toBe(false);
+  });
+
+  it("Returns ready for Updater when it has at least one input and inputs are modifiable", () => {
+    expect(psbt.isReadyForUpdater).toBe(true);
+  });
+});
+
+describe("PsbtV2.isReadyForSigner", () => {
+  let psbt: PsbtV2;
+
+  beforeEach(() => {
+    psbt = new PsbtV2();
+    psbt.addInput({ previousTxId: Buffer.from([0x00]), outputIndex: 0 });
+  });
+
+  it("Returns not ready for Signer when there are no inputs to sign", () => {
+    psbt.deleteInput(0);
+    expect(psbt.isReadyForSigner).toBe(false);
+  });
+
+  it("Returns not ready for Signer when the psbt has already finalized inputs", () => {
+    jest
+      .spyOn(psbt, "isReadyForTransactionExtractor", "get")
+      .mockReturnValue(true);
+    expect(psbt.isReadyForSigner).toBe(false);
+  });
+
+  it("Returns ready for Signer when the psbt has an input for signing", () => {
+    expect(psbt.isReadyForSigner).toBe(true);
+  });
+});
+
+describe("PsbtV2.isReadyForTransactionExtractor", () => {
+  let psbt: PsbtV2;
+
+  beforeEach(() => {
+    psbt = new PsbtV2();
+    // Create finalized non-witness input
+    psbt.addInput({
+      previousTxId: Buffer.from([0x00]),
+      outputIndex: 0,
+    });
+    (psbt as any).inputMaps[0].set(
+      KeyType.PSBT_IN_NON_WITNESS_UTXO,
+      Buffer.from([0x01]),
+    );
+    (psbt as any).inputMaps[0].set(
+      KeyType.PSBT_IN_FINAL_SCRIPTSIG,
+      Buffer.from([0x00]),
+    );
+    // Create finalized witness input
+    psbt.addInput({
+      previousTxId: Buffer.from([0x00]),
+      outputIndex: 1,
+    });
+    (psbt as any).inputMaps[1].set(
+      KeyType.PSBT_IN_WITNESS_UTXO,
+      Buffer.from([0x01]),
+    );
+    (psbt as any).inputMaps[1].set(
+      KeyType.PSBT_IN_FINAL_SCRIPTWITNESS,
+      Buffer.from([0x01]),
+    );
+  });
+
+  it("Returns not ready for Transaction Extractor when there are no finalized scripts", () => {
+    // Unset script from second input
+    (psbt as any).inputMaps[1].delete(KeyType.PSBT_IN_FINAL_SCRIPTWITNESS);
+    expect(psbt.isReadyForTransactionExtractor).toBe(false);
+  });
+
+  it("Returns not ready for Transaction Extractor when there are missing UTXOs", () => {
+    (psbt as any).inputMaps[1].delete(
+      KeyType.PSBT_IN_WITNESS_UTXO,
+      Buffer.from([0x00]),
+    );
+    expect(psbt.isReadyForTransactionExtractor).toBe(false);
+  });
+
+  it("Returns not ready for Transaction Extractor when extra fields have not been removed", () => {
+    (psbt as any).inputMaps[1].set(
+      KeyType.PSBT_IN_TAP_BIP32_DERIVATION,
+      Buffer.from([0x01]),
+    );
+    expect(psbt.isReadyForTransactionExtractor).toBe(false);
+  });
+
+  it("Returns ready for Transaction Extractor when the Input Finalizer's job has been completed", () => {
+    expect(psbt.isReadyForTransactionExtractor).toBe(true);
+  });
+});
+
 describe("PsbtV2.nLockTime", () => {
   it("Returns 0 when No locktimes specified", () => {
     const vect = BIP_370_VECTORS_VALID_PSBT[14];
@@ -1024,9 +1168,14 @@ describe("PsbtV2.addPartialSig", () => {
   it("Throws on validation failures", () => {
     const addSig = (index: number, pub?: any, sig?: any) =>
       psbt.addPartialSig(index, pub, sig);
-    expect(() => addSig(0)).toThrow("PsbtV2 has no input at 0");
+
+    // No inputs, so it's not ready for Signer
+    expect(() => addSig(0)).toThrow(
+      "The PsbtV2 is not ready for a Signer. Partial sigs cannot be added.",
+    );
 
     psbt.addInput({ previousTxId: Buffer.from([0x00]), outputIndex: 0 });
+    expect(() => addSig(1)).toThrow("PsbtV2 has no input at 1");
     expect(() => addSig(0)).toThrow(
       "PsbtV2.addPartialSig() missing argument pubkey",
     );