What changed, and why it matters
This commit is a small internal cleanup in Electrum's transaction batching code. It changes how sweep inputs are tracked—from using string-based keys to using proper transaction-outpoint objects and lists. The change appears to prevent a potential mismatch or bug where the wrong input could be removed from a batch, but it is not a clear, exploitable security vulnerability on its own. It is more likely a correctness/robustness fix.
Treat as a routine correctness fix. Reviewers should verify that all callers of _to_sweep_after and _create_batch_tx handle the new types correctly, and confirm no remaining string-keyed lookups on batch_inputs elsewhere. No urgent security response is indicated by the diff alone.
Security signals we found
Type annotation change from Dict[str, SweepInfo] to Dict[TxOutpoint, SweepInfo]
Added assert prevout == v.txin.prevout to enforce key/value consistency
Changed to_sweep_now from dict to list, removing string-keyed intermediate
Existing comment indicates prior pop-by-key could trigger assert error
No explicit security disclosure, CVE, or researcher attribution in commit
Evidence from the diff
In electrum/txbatcher.py, _to_sweep_after previously iterated over self.batch_inputs items using string keys k and derived prevout from v.txin.prevout, then returned a dict keyed by those strings. create_next_transaction then built to_sweep_now as a dict. The patch changes the return type to Dict[TxOutpoint, SweepInfo], uses prevout directly as the key, asserts prevout == v.txin.prevout, and changes to_sweep_now to a list of SweepInfo. _create_batch_tx now accepts Sequence[SweepInfo]. The existing comment warns that popping by the wrong key ‘if the input is already in a batch tx … will trigger assert error.’ The change reduces the chance of key mismatch and removes reliance on string serialization for internal lookups.
Changed components
electrum/txbatcher.pyTxBatch._to_sweep_afterTxBatch.create_next_transactionTxBatch._create_batch_txInspect captured patch +11 / −11
diff --git a/electrum/txbatcher.py b/electrum/txbatcher.py
index 7171eb8..a94813e 100644
--- a/electrum/txbatcher.py
+++ b/electrum/txbatcher.py
@@ -315,19 +315,19 @@ class TxBatch(Logger):
return to_pay
@locked
- def _to_sweep_after(self, tx: Optional[PartialTransaction]) -> Dict[str, SweepInfo]:
+ def _to_sweep_after(self, tx: Optional[PartialTransaction]) -> Dict[TxOutpoint, SweepInfo]:
tx_prevouts = set(txin.prevout for txin in tx.inputs()) if tx else set()
result = []
- for k, v in list(self.batch_inputs.items()):
- prevout = v.txin.prevout
+ for prevout, v in list(self.batch_inputs.items()):
+ assert prevout == v.txin.prevout
prev_txid, index = prevout.to_str().split(':')
if not self.wallet.adb.db.get_transaction(prev_txid):
continue
if v.is_anchor():
prev_tx_mined_status = self.wallet.adb.get_tx_height(prev_txid)
if prev_tx_mined_status.conf > 0:
- self.logger.info(f"anchor not needed {k}")
- self.batch_inputs.pop(k) # note: if the input is already in a batch tx, this will trigger assert error
+ self.logger.info(f"anchor not needed {prevout}")
+ self.batch_inputs.pop(prevout) # note: if the input is already in a batch tx, this will trigger assert error
continue
if spender_txid := self.wallet.adb.db.get_spent_outpoint(prev_txid, int(index)):
tx_mined_status = self.wallet.adb.get_tx_height(spender_txid)
@@ -335,7 +335,7 @@ class TxBatch(Logger):
continue
if prevout in tx_prevouts:
continue
- result.append((k,v))
+ result.append((prevout, v))
return dict(result)
def _should_bump_fee(self, base_tx: Optional[PartialTransaction]) -> bool:
@@ -462,11 +462,11 @@ class TxBatch(Logger):
def create_next_transaction(self, base_tx: Optional[PartialTransaction]) -> Optional[PartialTransaction]:
to_pay = self._to_pay_after(base_tx)
to_sweep = self._to_sweep_after(base_tx)
- to_sweep_now = {}
+ to_sweep_now = []
for k, v in to_sweep.items():
can_broadcast, wanted_height = self._can_broadcast(v, base_tx)
if can_broadcast:
- to_sweep_now[k] = v
+ to_sweep_now.append(v)
else:
self.wallet.add_future_tx(v, wanted_height)
while True:
@@ -505,16 +505,16 @@ class TxBatch(Logger):
self,
*,
base_tx: Optional[PartialTransaction],
- to_sweep: Mapping[str, SweepInfo],
+ to_sweep: Sequence[SweepInfo],
to_pay: Sequence[PartialTxOutput],
) -> PartialTransaction:
- self.logger.info(f'to_sweep: {list(to_sweep.keys())}')
+ self.logger.info(f'to_sweep: {[x.txin.prevout.to_str() for x in to_sweep]}')
self.logger.info(f'to_pay: {to_pay}')
inputs = [] # type: List[PartialTxInput]
outputs = [] # type: List[PartialTxOutput]
locktime = base_tx.locktime if base_tx else None
# sort inputs so that txin-txout pairs are first
- for sweep_info in sorted(to_sweep.values(), key=lambda x: not bool(x.txout)):
+ for sweep_info in sorted(to_sweep, key=lambda x: not bool(x.txout)):
if sweep_info.cltv_abs is not None:
if locktime is None or locktime < sweep_info.cltv_abs:
# nLockTime must be greater than or equal to the stack operand.
Why this scored 23/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.