Merge pull request #10981 from spesmilo/do_not_sign_txin_with_witness
What the developer wrote
Authored by ThomasV
Merge pull request #10981 from spesmilo/do_not_sign_txin_with_witness
transaction: do not sign txin if if already has a witness
What changed, and why it matters
This change prevents Electrum from re-signing Bitcoin transaction inputs that already contain a witness (the data proving the input was authorized). The patch fixes a bug where Electrum could incorrectly sign a non-SegWit input as if it were SegWit when a PSBT already included a witness for that input. This could lead to producing invalid or unintended signatures during transaction signing, especially when handling partially-signed transactions from external signers.
Review and merge if not already merged. Users and downstream wallets should update to include this fix, especially when signing PSBTs produced by external signers or multi-sig workflows where inputs may already carry witness data. Test signing flows involving mixed SegWit/non-SegWit inputs and pre-signed PSBTs.
Security signals we found
Incorrect signature algorithm selection for non-SegWit inputs when witness data is present
PSBT handling edge case where pre-existing witness data influences signing path
Potential invalid signature production during transaction signing
Defensive skip of already-witnessed inputs to avoid double-signing or wrong sighash
Evidence from the diff
The commit adds a has_witness() helper and modifies Transaction.sign() to skip inputs where txin.has_witness() is true. The code comment explains that serialize_preimage relies on is_segwit(), which returns true if the PSBT contains a witness even for non-segwit inputs. Without this guard, a non-segwit input with a pre-existing witness could be signed using the SegWit preimage algorithm, producing an incorrect signature. The change is defensive and prevents signing inputs that already have witness data attached.
Changed components
electrum/transaction.pyTransaction.sign()TxInput.is_segwit() / has_witness()Inspect captured patch +10 / −2
### electrum/transaction.py
@@ -462,11 +462,14 @@ def witness_elements(self) -> Sequence[bytes]:
n = vds.read_compact_size()
return list(vds.read_bytes(vds.read_compact_size()) for i in range(n))
- def is_segwit(self, *, guess_for_address=False) -> bool:
+ def has_witness(self) -> bool:
if self.witness not in (b'\x00', b'', None):
return True
return False
+ def is_segwit(self, *, guess_for_address=False) -> bool:
+ return self.has_witness()
+
def is_taproot(self) -> Optional[bool]:
if self._is_taproot is None:
if self.address:
@@ -2043,7 +2046,7 @@ def calc_if_p2sh_segwit_now():
def is_segwit(self, *, guess_for_address=False) -> bool:
"""Whether this input is segwit (any witness version)."""
- if super().is_segwit():
+ if self.has_witness():
return True
if self.is_native_segwit() or self.is_p2sh_segwit():
return True
@@ -2485,6 +2488,11 @@ def sign(self, keypairs: Mapping[bytes, bytes]) -> None:
# keypairs: pubkey_bytes -> secret_bytes
sighash_cache = SighashCache()
for i, txin in enumerate(self.inputs()):
+ if txin.has_witness():
+ # note: serialize_preimage relies on is_segwit(), which returns True
+ # if the PSBT contains a witness, even for non-segwit inputs.
+ _logger.info(f"not signing input {i}: it already has a witness")
+ continue
for pubkey in txin.pubkeys:
if txin.is_complete():
breakWhy this scored 68/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.