BIP 371 noncompliance: Taproot fields remain after adding PSBT_IN_FINAL_SCRIPTWITNESS in finalize_psbt

[DanGould]

Describe the bug

Finalizers should remove PSBT_IN_TAP_* and PSBT_OUT_TAP_BIP32_DERIVATION after PSBT_IN_FINAL_SCRIPTWITNESS is constructed according to BIP 371 spec

To Reproduce

Fund a psbt with a taproot input. Call wallet.sign on it. See PSBT_IN_TAP_KEY_SIG present on output.

Expected behavior

Fund a psbt with a taproot input. Call wallet.sign on it. See PSBT_IN_TAP_KEY_SIG not present.

Build environment

• BDK-CLI tag/commit: 1.0.0-alpha.1
• Rust/Cargo version: 1.63.0, 1.76.0-nightly
• Rust/Cargo target: wasm32-unknown-unknown

Additional context

I ran into this problem trying to take payjoin BDK-output Original PSBTs and process them at the receiver with walletprocesspsbt on bitcoind, and bitcoind failed to skip the inputs that it encountered having PSBT_IN_TAP_KEY_SIG belonging to the sender. Typically it'd just skip them.

I believe the issue could be fixed by calling clear() on the aforementioned fields here

bdk/crates/bdk/src/wallet/mod.rs

Lines 1883 to 1887 in b13505c

	psbt_input.final_script_sig = Some(tmp_input.script_sig);
	psbt_input.final_script_witness = Some(tmp_input.witness);
	if sign_options.remove_partial_sigs {
	psbt_input.partial_sigs.clear();
	}

[ValuedMammal]

I did verify the faulty behavior as you described - fields in a Psbt taproot input, such as tap_key_sig and others, hanging around after finalization, where the BIP clearly states they should be removed. I'll look at implementing a fix and report on any other findings. Thanks @DanGould for spotting the issue and giving a detailed description of what's expected.

[DanGould]

Thanks for tackling this @evanlinjin

[evanlinjin]

Thanks for tackling this @evanlinjin

It was mostly @ValuedMammal! I only reviewed and merged 😅