AaronFiore/electrum
1
0
Fork 0
Code Activity

Merge pull request #9638 from f321x/140325-force-close-exception

Browse Source 
Fix anchor output sweeping bug creating invalid sweeping tx
This commit is contained in:
ThomasV
2025-03-14 19:39:32 +01:00
committed by GitHub
parent 2763e14bb3 81d4e90f66
commit 61c5df8407
4 changed files with 67 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
electrum/coinchooser.py
View File

@@ -298,6 +298,9 @@ class CoinChooserBase(Logger):
utxos = [c.prevout.serialize_to_network() for c in coins]
self.p = PRNG(b''.join(sorted(utxos)))
assert len(outputs) > 0 or len(change_addrs) == 1, \
"sweeps with 0 outputs should not use multiple change addresses"
# Copy the outputs so when adding change we don't modify "outputs"
base_tx = PartialTransaction.from_io(inputs[:], outputs[:], BIP69_sort=BIP69_sort)
input_value = base_tx.input_value()
@@ -308,7 +311,10 @@ class CoinChooserBase(Logger):
# marker and flag are excluded, which is compensated in get_tx_weight()
# FIXME calculation will be off by this (2 wu) in case of RBF batching
base_weight = base_tx.estimated_weight()
spent_amount = base_tx.output_value()
# by setting spent_amount = dust_threshold if there are no outputs we ensure that
# enough inputs are added so there is always at least a change output created
# as txs have to have at least 1 output according to consensus rules
spent_amount = base_tx.output_value() if outputs else dust_threshold
def fee_estimator_w(weight):
return fee_estimator_vb(Transaction.virtual_size_from_weight(weight))

6
electrum/wallet.py
View File

@@ -1916,6 +1916,10 @@ class Abstract_Wallet(ABC, Logger, EventListener):
change_addrs = self.get_change_addresses_for_new_transaction(change_addr or old_change_addrs)
if merge_duplicate_outputs:
txo = transaction.merge_duplicate_tx_outputs(txo)
if len(txo) == 0 or (self.lnworker and send_change_to_lightning):
# even if the option use multiple change outputs is enabled there should be only
# one change address if there are 0 txos as this is a sweep tx, or if we want to swap change to ln
change_addrs = change_addrs[0:1]
tx = coin_chooser.make_tx(
coins=coins,
inputs=txi,
@@ -1926,7 +1930,6 @@ class Abstract_Wallet(ABC, Logger, EventListener):
BIP69_sort=BIP69_sort)
if self.lnworker and send_change_to_lightning:
change = tx.get_change_outputs()
# do not use multiple change addresses
if len(change) == 1:
amount = change[0].value
if amount <= self.lnworker.num_sats_can_receive():
@@ -1957,6 +1960,7 @@ class Abstract_Wallet(ABC, Logger, EventListener):
outputs[i].value += (amount - distr_amount)
tx = PartialTransaction.from_io(list(coins), list(outputs))
assert len(tx.outputs()) > 0, "any bitcoin tx must have at least 1 output by consensus"
if locktime is None:
# Timelock tx to current height.
locktime = get_locktime_for_new_transaction(self.network)

54
tests/test_coinchooser.py
View File

@@ -1,5 +1,8 @@
from electrum.coinchooser import CoinChooserPrivacy
from electrum.util import NotEnoughFunds
from electrum.transaction import PartialTxInput, TxOutpoint, Transaction
from electrum.fee_policy import FeePolicy
from functools import partial
from . import ElectrumTestCase
@@ -18,3 +21,54 @@ class TestCoinChooser(ElectrumTestCase):
coin_chooser.bucket_candidates_any([], sufficient_funds)
with self.assertRaises(NotEnoughFunds):
coin_chooser.bucket_candidates_prefer_confirmed([], sufficient_funds)
def test_make_tx_no_outputs_adds_change(self):
coin_chooser = CoinChooserPrivacy(enable_output_value_rounding=False)
fee_estimator = partial(FeePolicy('eta:2').estimate_fee, allow_fallback_to_static_rates=True)
# dummy input with value of 330 sat
prevout_txid = bytes.fromhex("81d0b29f08c6256dcfbaf02ff1f1e756461cb1df550672e049af7429331c643f")
single_txin = PartialTxInput(
prevout=TxOutpoint(txid=prevout_txid, out_idx=0),
)
single_txin.utxo = Transaction("02000000000101956449bdc8059b680a20483e64e139ce63fe64333b92cd7811a1b116d6b967ad0000000000fdffffff024a01000000000000160014a21d1fbcf571153f57b40855e059c134405a89ecd682010000000000160014fd7debf75d6c410bf6ba1c8ba05f90f23ce4646a0247304402207f07ec0c2415b31743527dea2f7bff3868f494dc0a5d45adec5e05031725a0af02202aa0ac7d06dbcad8ac0b9808a829b6bdaa98bc831aef31a5ab4e5d1890f7552101210278a5d9b2796f2743ccf1b36b2bf47695d766d0841c17b00ce83943c8b37dde0ceea60300")
# dummy input to be used as potential additional input of higher value
prevout_txid = bytes.fromhex("b3d9174cb5d3234764a089bb91fdbd1117b7958be4870d1a544136ab017a67dd")
coin = PartialTxInput(
prevout=TxOutpoint(txid=prevout_txid, out_idx=0),
)
coin.utxo = Transaction("02000000000105a5a00ad10e754a17154446bbe1c557b44b86a7cd53308ad9ab813388a9d6d1520000000000fdffffff2c48659d10a752b0c0a3efa4092ebee5210943a2f41ff0607ba0e03a4cdf7bbd0000000000fdffffff93f4feb485581654caffa523c500dc417a98097fb731045040bb162acb3e14e90000000000fdffffffbf4b69292acaabf8a415db412eedd8a202d4dd2ca12e532628a756276fec00f50000000000fdffffffa96e18c45ecc56608d0be3c1b2cd93e52d569a9c3a68ed51aead570beeef29ff0000000000fdffffff017a991300000000001600142a55fbef3e419e1c862632a826ae89ade0b07e3a0247304402204de416135d26711df2cbd5209e3f79f95a1de5ddea5980215606ebfe639747bd0220286f9de38a96a078c818e97fe6fbbbe3637287461cd7407adf9e85ba6d899005012102c329033555adccaadb6c83fb486f540ba00aad3edba7a4ec3347b5cc0935c4050247304402200132c1c4c41b840f05efeacf96cccedab38d68c9021c79f940738572b049c1a302200d59ba4719d4d4e55ced651cde35cbf79838bf7122cbafd6584dd934a67db0ed01210380194ab3704b5524a0c97f78b4458b80efd365faaedaebe90fa7807eeab041700247304402202c966fbea5db4bb3794e843b59240d678a3c8e97be1d10c18475a467c101b97c02203edb0ca11e7605af2437ddffbbe416317bca8788c23c4816c13698042222d30f0121035edcdcad9affcff41302ad49a19ccbd47ae50b153ba7c2abaaf3bb2d22c859120247304402206890a622513bb9c8b8ca83e5e82532f9753ecd3188c27d8da9452e264f5500cc022012dad8fb872478b7f9d0d9d310859fca6534b8e9f44511eeef5450beaf13c690012103f683aa56e036b1307c1ae75e81553b6730aea312560e36afad487ba2bc6cf98f02473044022006807df115d6bce73e384651fbb9e932ee313133218be7769e52809b758529b6022078b2859f98a62211f130e69982f96d2968e1820c58bb817f9ab31645b6b4ceae0121026402c3c0a2b4dd5703686f8c5b5b3dcecbbf4d772ef83e440bfad22b742753e730a60300")
coin.block_height = 100
tx = coin_chooser.make_tx(
coins=[coin],
inputs=[single_txin],
outputs=[],
change_addrs=["bc1q2089yvkkyw7yq7m6a7lxt45n35c587hk4sgj7c"],
fee_estimator_vb=fee_estimator,
dust_threshold=500,
)
# make_tx should add one additional input and a change output
assert len(tx.outputs()) == 1, f"expected 1 output got {len(tx.outputs())}"
assert len(tx.inputs()) == 2, f"expected 2 input got {len(tx.inputs())}"
# dummy input with value of 99030 sat
prevout_txid = bytes.fromhex("81d0b29f08c6256dcfbaf02ff1f1e756461cb1df550672e049af7429331c643f")
single_txin = PartialTxInput(
prevout=TxOutpoint(txid=prevout_txid, out_idx=1),
)
single_txin.utxo = Transaction("02000000000101956449bdc8059b680a20483e64e139ce63fe64333b92cd7811a1b116d6b967ad0000000000fdffffff024a01000000000000160014a21d1fbcf571153f57b40855e059c134405a89ecd682010000000000160014fd7debf75d6c410bf6ba1c8ba05f90f23ce4646a0247304402207f07ec0c2415b31743527dea2f7bff3868f494dc0a5d45adec5e05031725a0af02202aa0ac7d06dbcad8ac0b9808a829b6bdaa98bc831aef31a5ab4e5d1890f7552101210278a5d9b2796f2743ccf1b36b2bf47695d766d0841c17b00ce83943c8b37dde0ceea60300")
tx = coin_chooser.make_tx(
coins=[coin],
inputs=[single_txin],
outputs=[],
change_addrs=["bc1q2089yvkkyw7yq7m6a7lxt45n35c587hk4sgj7c"],
fee_estimator_vb=fee_estimator,
dust_threshold=500,
)
# make_tx should not add an additional input, as single_txin is large enough
assert len(tx.outputs()) == 1, f"expected 1 output got {len(tx.outputs())}"
assert len(tx.inputs()) == 1, f"expected 1 input got {len(tx.inputs())}"

2
tests/test_txbatcher.py
View File

@@ -47,7 +47,7 @@ class MockNetwork(Logger):
return tx.txid()
async def next_tx(self):
await self._tx_event.wait()
await util.wait_for2(self._tx_event.wait(), timeout=10)
return self._tx