feat(packages): validate depositor-graph psbt inputs against authoritative parent txs

[jonybur]

closes https://github.com/babylonlabs-io/vault-provider-proxy/issues/17

[github-actions]

🔐 Commit Signature Verification

✅ All 1 commit(s) passed verification

Commit	Author	Signature	Key Type	Key Check
8dcd87ed2b89	Jonathan Bursztyn	✅	sk-ssh-ed25519	✅

Summary

• Commits verified: 1
• Signature check: ✅ All passed
• Key type enforcement: ✅ All sk-ssh-ed25519

---

Required key type: sk-ssh-ed25519 (FIDO2 hardware key)

Last verified: 2026-04-17 14:16 UTC

[greptile-apps]

Greptile Summary

This PR hardens the depositor-graph PSBT signing flow by replacing VP-trusted prevout arrays with values derived directly from authoritative parent transactions (peginTxHex from on-chain state, assertTxHex from the VP response). All three PSBT builders (depositorPayout, noPayout, challengeAssert) now validate input outpoints against the real parent tx before embedding prevouts, and signDepositorGraph adds a matching layer of PSBT-level cross-checks.

• P1 — input-1 verification bypass: In signDepositorGraph.ts the Assert:0 check for Payout input 1 is guarded by if (payoutValidated.childTx.ins.length > 1). A VP sending a 1-input payout transaction silently skips this verification rather than throwing, contradicting the strict 2-input invariant enforced in buildDepositorPayoutPsbt and the security goal of this very PR.

Confidence Score: 4/5

Safe to merge after fixing the conditional skip on payout input-1 verification, which can silently bypass the Assert:0 prevout check for VP-supplied single-input payout transactions.

One P1 logic issue: the if (ins.length > 1) guard in collectDepositorGraphPsbts allows a VP to bypass the Assert:0 witnessUtxo verification by providing a 1-input payout transaction. All other validation paths are strict and correct. The remaining findings are P2 style/duplication concerns.

packages/babylon-ts-sdk/src/tbv/core/services/deposit/signDepositorGraph.ts — the conditional Assert:0 check at lines 238-247

Important Files Changed

Filename	Overview
packages/babylon-ts-sdk/src/tbv/core/services/deposit/signDepositorGraph.ts	Core signing orchestrator — adds peginTxHex param and cross-checks every signed input's outpoint + witnessUtxo against authoritative parent txs. One P1 gap: input 1 Assert:0 verification is skipped (not thrown) when payout has fewer than 2 inputs, which a malicious VP could exploit.
packages/babylon-ts-sdk/src/tbv/core/primitives/psbt/depositorPayout.ts	Replaces VP-trusted prevouts with values derived from authoritative peginTx/assertTx; strict 2-input count check added. Clean and correct.
packages/babylon-ts-sdk/src/tbv/core/primitives/psbt/noPayout.ts	Input 0 prevout now derived from authoritative assertTx; additional (VP-supplied) prevouts for fee inputs preserved via optional additionalPrevouts. Logic and validation are sound.
packages/babylon-ts-sdk/src/tbv/core/primitives/psbt/challengeAssert.ts	Replaces flat prevouts array with authoritative assertTx lookup; adds txid and duplicate-output-index checks for all 3 inputs. Correct.
packages/babylon-ts-sdk/src/tbv/core/services/deposit/tests/signDepositorGraph.test.ts	Comprehensive rewrite with proper mock infra for Transaction and PSBT. Covers attacker-txid and wrong-witnessUtxo-value cases; no test for input-1 (Assert:0) verification or wrong-input-count rejection.
packages/babylon-ts-sdk/src/tbv/core/services/deposit/signAndSubmitPayouts.ts	Passes peginTxHex from on-chain signing context through to signDepositorGraph. Minimal, correct change.
services/vault/src/components/deposit/PayoutSignModal/usePayoutSigningState.ts	Passes context.peginTxHex into signDepositorGraph call. Correct plumbing; comment updated to explain the security rationale.

Sequence Diagram

sequenceDiagram
    participant UI as usePayoutSigningState
    participant SAP as signAndSubmitPayouts
    participant SDG as signDepositorGraph
    participant VPRes as VP Response
    participant OnChain as On-chain peginTx

    UI->>SAP: pollAndSignPayouts(signingContext)
    SAP->>OnChain: signingContext.peginTxHex (authoritative)
    SAP->>VPRes: response.depositor_graph
    SAP->>SDG: signDepositorGraph({ depositorGraph, peginTxHex })

    SDG->>SDG: Transaction.fromHex(peginTxHex) → peginTx
    SDG->>VPRes: depositorGraph.assert_tx.tx_hex → graphAssertTx
    SDG->>SDG: validateAndParsePsbt(payout_psbt, payout_tx.tx_hex)
    SDG->>SDG: verifyInputAgainstParent(input0, peginTx, vout=0)
    SDG->>SDG: verifyInputAgainstParent(input1, graphAssertTx, vout=0) [if ins.length > 1]
    SDG->>SDG: verifyInputAgainstParent(noPayoutInput0, graphAssertTx, vout=0)
    SDG->>SDG: sanitizePsbtForScriptPathSigning
    SDG-->>UI: DepositorAsClaimerPresignatures

Loading

_{Reviews (1): Last reviewed commit: "feat(packages): validate depositor-graph..." | Re-trigger Greptile}

[greptile-apps]

Conditional skip silently bypasses Assert:0 verification

If the VP supplies a payout transaction with only 1 input, the if guard skips the Assert:0 prevout check entirely instead of throwing. This contradicts both the protocol requirement (payout MUST have 2 inputs, as enforced in buildDepositorPayoutPsbt) and the stated security goal of this PR: a malicious VP can present a 1-input payout to avoid the graphAssertTx binding check. Should assert exactly 2 inputs first:

Suggested change

	if (payoutValidated.childTx.ins.length > 1) {
	verifyInputAgainstParent(
	payoutValidated.psbt,
	payoutValidated.childTx,
	1,
	graphAssertTx,
	ASSERT_PAYOUT_OUTPUT_INDEX,
	payoutLabel,
	);
	}
	if (payoutValidated.childTx.ins.length !== 2) {
	throw new Error(
	`Depositor Payout transaction must have exactly 2 inputs, got ${payoutValidated.childTx.ins.length}`,
	);
	}
	verifyInputAgainstParent(
	payoutValidated.psbt,
	payoutValidated.childTx,
	1,
	graphAssertTx,
	ASSERT_PAYOUT_OUTPUT_INDEX,
	payoutLabel,
	);

[greptile-apps]

inputTxidHex duplicated across 4 files

The function is defined identically in challengeAssert.ts, depositorPayout.ts, noPayout.ts, and here. Per the project's zero-dead-code / no-duplication policy, this helper belongs in the shared primitives/utils/bitcoin module alongside uint8ArrayToHex (which it already depends on).

[greptile-apps]

Protocol-index constants duplicated across 3 files

PEGIN_VAULT_OUTPUT_INDEX, ASSERT_PAYOUT_OUTPUT_INDEX, and ASSERT_NOPAYOUT_OUTPUT_INDEX are all 0 and defined independently in depositorPayout.ts, noPayout.ts, and here. Because these constants encode a protocol invariant (which output of PegIn/Assert each transaction spends), they should live in one shared location — a drift between them would silently change validation behaviour.