Skip to content

fix(psbt): route addOutput unknownKeyVals to output map - #21

Draft
bitgo-ai-agent-dev[bot] wants to merge 1 commit into
masterfrom
fix/WCN-1934-addOutput-unknownKeyVals-routing-pt1
Draft

fix(psbt): route addOutput unknownKeyVals to output map#21
bitgo-ai-agent-dev[bot] wants to merge 1 commit into
masterfrom
fix/WCN-1934-addOutput-unknownKeyVals-routing-pt1

Conversation

@bitgo-ai-agent-dev

@bitgo-ai-agent-dev bitgo-ai-agent-dev Bot commented Aug 7, 2026

Copy link
Copy Markdown

What

  • Fix `Psbt.addOutput()` in `ts_src/lib/psbt.ts:159`: change
    `this.addUnknownKeyValToInput(outputIndex, keyVal)` →
    `this.addUnknownKeyValToOutput(outputIndex, keyVal)`.
  • Add two regression tests in `ts_src/tests/addInputOutput.ts`:
    • Verifies unknown key-values land on `outputs[N].unknownKeyVals`
      (not `inputs[N].unknownKeyVals`) and survive a `toHex()`/`fromHex()` round-trip.
    • Verifies the crash `"No input #N"` no longer occurs when the number
      of outputs exceeds the number of inputs (e.g. 1-input / 2-output transactions).

Why

`addOutput()` was calling the input variant of `addUnknownKeyVal` instead of
the output variant, causing two confirmed failure modes (WCN-1934 / FND-001):

  1. Silent corruption when `outputIndex < inputs.length`: the keyval was
    attached to `inputs[outputIndex].unknownKeyVals` and survived
    serialize/parse round-trips in the wrong map. The PSBT proprietary-key
    support added for musig2/MPC coordination data (commit 8a33996) was
    completely broken: co-signers never found the required output metadata,
    causing signing ceremonies to fail with funds stuck.

  2. Crash (`"No input #N"`) when `outputIndex >= inputs.length` — the
    common 1-input / 2-output case — making PSBT construction impossible
    whenever `unknownKeyVals` accompany outputs beyond the first.

Additionally, the old code ran the duplicate/type guard against `InputTypes`
(16 entries) instead of `OutputTypes` (6 entries), so output unknown keys with
type byte 6–15 were additionally misrejected. This is addressed in pt2.

Test plan

  • `npm run unit` — 387 tests pass (2 new regression tests added)
  • `npm run format:ci` — no formatting issues

Stack

This PR is part 1 of 2 in a stack. Review and merge in order:

  1. fix(psbt): route addOutput unknownKeyVals to output map #21 — primary routing fix + regression tests (base: `master`) ← you are here
  2. fix(utils): use max-enum-value+1 as checkHasKey boundary; reject empty keys #22 — `checkHasKey`/`getEnumLength` correctness fixes (base: `pt1`)

Ticket: WCN-1934

addOutput() was calling addUnknownKeyValToInput(outputIndex, keyVal)
instead of addUnknownKeyValToOutput(outputIndex, keyVal).  This caused
two confirmed failure modes:

1. Silent corruption when outputIndex < inputs.length: the keyval was
   attached to inputs[outputIndex].unknownKeyVals and survived
   serialize/parse round-trips in the wrong (input) map.  Because the
   duplicate-check also ran against InputTypes (16 entries) instead of
   OutputTypes (6 entries), output unknown-key type bytes 6–15 were
   additionally misrejected.

2. Crash ("No input #N") when outputIndex >= inputs.length — the
   common 1-input/2-output case — making PSBT construction impossible
   whenever unknownKeyVals accompany the second output.

Both modes affect musig2/MPC coordination data carried as output
proprietary fields: the signing ceremony crashes or corrupts the PSBT,
leaving funds stuck in the shared wallet.

Fix: change line 159 to call addUnknownKeyValToOutput, matching the
upstream bip174 v2.1.1 fix.  Add regression tests covering correct
placement, round-trip survival, and the outputs>inputs case.

Ticket: WCN-1934
Session-Id: 9abd2e08-b701-4f8d-9355-06124e17bf0c
Task-Id: edef13cc-a93b-4c12-85ac-d51bcc1dfaaa
@linear-code

linear-code Bot commented Aug 7, 2026

Copy link
Copy Markdown

WCN-1934

@ralph-bitgo
ralph-bitgo Bot force-pushed the fix/WCN-1934-addOutput-unknownKeyVals-routing-pt1 branch from 7f1bff3 to b9b3f18 Compare August 7, 2026 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant