tests: add regression tests for withdraw returning unsigned tx

[vincenzopalazzo]

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_txprepare → signpsbt → signpsbt_done → sendpsbt → sendpsbt_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

[vincenzopalazzo]

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.

[madelinevibes]

@vincenzopalazzo can you resolve the conflicts and clear CI by the end of this week so we can add this to 26.06? Release candidate planned for Monday 11 May.

[sangbida]

The tests pass locally for me after the change has been added. The tests are failing on liquid because liquid doesn't support p2tr addresses. You could either change them to bech32 addresses, skip if the network isn't regtest or use the good_addrtype() function.

@vincenzopalazzo could you please add this fix? I should be able to merge this after CI is cleared.

[sangbida]

Could we perhaps reword some of this comment? This comment is a bit verbose, also we don't really want github links through source/test code.

[sangbida]

If you could remove the github link from this comment as well, that would be great.

[rustyrussell]

Modified to explicitly mark the tests xfail in the first commit (and I checked they do fail!).