Skip to content

tests: add regression tests for withdraw returning unsigned tx#8942

Open
vincenzopalazzo wants to merge 2 commits intoElementsProject:masterfrom
vincenzopalazzo:claude/epic-black
Open

tests: add regression tests for withdraw returning unsigned tx#8942
vincenzopalazzo wants to merge 2 commits intoElementsProject:masterfrom
vincenzopalazzo:claude/epic-black

Conversation

@vincenzopalazzo
Copy link
Collaborator

Summary

  • Adds two regression tests for mandatory-script-verify-flag-failed v25.09.3-modded #8701 where withdraw returns an unsigned raw transaction in the tx response field
  • test_withdraw_returns_signed_tx: verifies witness data exists for all inputs in the returned tx
  • test_withdraw_close_output_signed: reproduces the exact scenario from the issue — withdrawing funds that include channel close outputs (anchor/P2WSH with CSV=1)

Root Cause Analysis

The withdraw command flow:

  1. finish_txpreparesignpsbtsignpsbt_donesendpsbtsendpsbt_done
  2. In signpsbt_done, psbt_txid() extracts utx->tx using WALLY_PSBT_EXTRACT_NON_FINAL, which strips all signatures/witnesses
  3. The broadcast succeeds because sendpsbt internally finalizes the PSBT via psbt_final_tx()
  4. But sendpsbt_done returns utx->tx (the unsigned version) in the response

The tx field in the withdraw response is always unsigned. The psbt field is correct.

Test plan

  • test_withdraw_returns_signed_tx should fail on current master (confirming the bug)
  • test_withdraw_close_output_signed should fail on current master with anchor close outputs
  • Both tests should pass after the fix is applied to signpsbt_done in plugins/txprepare.c

Fixes: #8701

🤖 Generated with Claude Code

Adds two tests to reproduce issue ElementsProject#8701 where the withdraw command
returns an unsigned raw transaction in the 'tx' response field:

1. test_withdraw_returns_signed_tx: verifies that withdraw's 'tx' field
   contains witness data for all inputs (basic wallet UTXOs).

2. test_withdraw_close_output_signed: verifies signing works when
   withdrawing funds that include channel close outputs (anchor/P2WSH
   with CSV locks), which was the exact scenario in the reported issue.

The root cause is that psbt_txid() uses WALLY_PSBT_EXTRACT_NON_FINAL
which strips signatures/witnesses, and the withdraw response returns
this unsigned tx instead of the finalized one.

Changelog-None
Signed-off-by: Vincenzo Palazzo <vincenzopalazzo@member.fsf.org>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vincenzopalazzo
Copy link
Collaborator Author

Root Cause Analysis

This is a regression introduced in commit 908f834 ("Update libwally to 0.8.8, support PSBTv2") from 2023-02-01.

Before PSBTv2 (pre-908f834d6)

psbt_txid cloned the PSBT's global psbt->tx and manually copied final_scriptsig / redeem_script into the extracted tx's inputs:

wally_tx_clone_alloc(psbt->tx, 0, &tx);
for (size_t i = 0; i < tx->num_inputs; i++) {
    if (psbt->inputs[i].final_scriptsig)
        wally_tx_set_input_script(tx, i, ...);
    else if (psbt->inputs[i].redeem_script)
        wally_tx_set_input_script(tx, i, ...);
}

The returned wtx had scriptSigs populated from the signed PSBT, so signpsbt_done storing utx->tx from this call was (mostly) fine — the tx field in the withdraw response was at least partially valid.

After PSBTv2 (908f834d6)

PSBTv2 removed the global psbt->tx field. The function was rewritten to:

wally_psbt_extract(psbt, WALLY_PSBT_EXTRACT_NON_FINAL, &tx);

WALLY_PSBT_EXTRACT_NON_FINAL extracts the transaction without any signatures or witness data — by design, it's meant purely for txid computation. But signpsbt_done in plugins/txprepare.c was still storing the wtx output in utx->tx and returning it as the tx field in the withdraw response.

The fix

Instead of relying on psbt_txid for both the txid check and the tx extraction, we now:

  1. Use psbt_txid(NULL, ..., &txid, NULL) — only for the txid verification (pass NULL for wtx)
  2. Call psbt_finalize() + psbt_final_tx() — to extract the fully signed transaction with witness data

These are the same functions that json_sendpsbt already uses internally for the actual broadcast, so the approach is proven.

The withdraw command was returning an unsigned raw transaction in
its 'tx' response field. This happened because signpsbt_done() used
psbt_txid() to extract utx->tx, which internally calls
wally_psbt_extract() with WALLY_PSBT_EXTRACT_NON_FINAL — stripping
all signature and witness data.

The broadcast itself succeeded because sendpsbt internally finalizes
the PSBT via psbt_final_tx(), but the 'tx' field returned to the
user had empty scriptSigs and no witness data.

This is a regression from 908f834 ("Update libwally to 0.8.8,
support PSBTv2") which rewrote psbt_txid() from manually copying
final_scriptsig/redeem_script into the cloned tx, to using
wally_psbt_extract(WALLY_PSBT_EXTRACT_NON_FINAL) which strips all
signing data by design.

Fix by finalizing the signed PSBT in signpsbt_done() and extracting
the fully signed transaction via psbt_final_tx(). The txid
verification still uses psbt_txid() (which is correct for txid
computation since txids exclude witness data).

Fixes: ElementsProject#8701
Changelog-Fixed: withdraw now returns a fully signed transaction in the `tx` response field.
Signed-off-by: Vincenzo Palazzo <vincenzopalazzo@member.fsf.org>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sangbida sangbida added this to the 26.06 milestone Mar 16, 2026
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.

mandatory-script-verify-flag-failed v25.09.3-modded

2 participants