Skip to content

Commit ad452a1

Browse files
committed
Merge bitcoin/bitcoin#33528: wallet: don't consider unconfirmed TRUC coins with ancestors
dcd42d6 [test] wallet send 3 generation TRUC (glozow) e753fad [wallet] never try to spend from unconfirmed TRUC that already has ancestors (glozow) Pull request description: Addresses bitcoin/bitcoin#33368 (comment) There is not an explicit check that the to-be-created wallet transaction would be within the {TRUC, normal} ancestor limits. This means that the wallet may create a transaction that violates these limits, but fail to broadcast it in `CommitTransaction`. This appears to be expected behavior for the normal ancestor limits (and any other situation in which the wallet creates a tx that was rejected by mempool) and AFAIK the transaction will be rebroadcast at some point after the ancestors confirm. https://github.com/bitcoin/bitcoin/blob/1ed00a0d39d5190d8ad88a0dd705a09b56d987aa/test/functional/wallet_basic.py#L502-L506 It's a bit complex to address this for the normal ancestor limit, and probably unrealistic for the wallet to check all possible mempool policies in coin selection, but it's quite trivial for TRUC: just skip any unconfirmed UTXOs that have any ancestors. I think it would be much more helpful to the user to say there are insufficient funds. ACKs for top commit: achow101: ACK dcd42d6 monlovesmango: ACK dcd42d6 rkrux: lgtm ACK dcd42d6 Tree-SHA512: b4cf9685bf0593c356dc0d6644835d53e3d7089f42b65f647795257dc7f5dac90c5ee493b41ee30a1c1beb880a859db8e049d3c64a43d5ca9b3e6482ff6bddd5
2 parents 9a29b2d + dcd42d6 commit ad452a1

File tree

2 files changed

+50
-0
lines changed

2 files changed

+50
-0
lines changed

src/wallet/spend.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,11 @@ CoinsResult AvailableCoins(const CWallet& wallet,
403403
if (wtx.tx->version != TRUC_VERSION) continue;
404404
// this unconfirmed v3 transaction already has a child
405405
if (wtx.truc_child_in_mempool.has_value()) continue;
406+
407+
// this unconfirmed v3 transaction has a parent: spending would create a third generation
408+
size_t ancestors, descendants;
409+
wallet.chain().getTransactionAncestry(wtx.tx->GetHash(), ancestors, descendants);
410+
if (ancestors > 1) continue;
406411
} else {
407412
if (wtx.tx->version == TRUC_VERSION) continue;
408413
Assume(!wtx.truc_child_in_mempool.has_value());

test/functional/wallet_v3_txs.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ def run_test(self):
119119
self.sendall_truc_child_weight_limit()
120120
self.mix_non_truc_versions()
121121
self.cant_spend_multiple_unconfirmed_truc_outputs()
122+
self.test_spend_third_generation()
122123

123124
@cleanup
124125
def tx_spends_unconfirmed_tx_with_wrong_version(self, version_a, version_b):
@@ -585,5 +586,49 @@ def cant_spend_multiple_unconfirmed_truc_outputs(self):
585586
{'include_unsafe' : True}
586587
)
587588

589+
@cleanup
590+
def test_spend_third_generation(self):
591+
self.log.info("Test that we can't spend an unconfirmed TRUC output that already has an unconfirmed parent")
592+
593+
# Generation 1: Consolidate all UTXOs into one output using sendall
594+
self.charlie.sendall([self.charlie.getnewaddress()], version=3)
595+
outputs1 = self.charlie.listunspent(minconf=0)
596+
assert_equal(len(outputs1), 1)
597+
598+
# Generation 2: to ensure no change address is created, do another sendall
599+
self.charlie.sendall([self.charlie.getnewaddress()], version=3)
600+
outputs2 = self.charlie.listunspent(minconf=0)
601+
assert_equal(len(outputs2), 1)
602+
total_amount = sum([utxo['amount'] for utxo in outputs2])
603+
604+
# Generation 3: try to send half of total amount to Alice
605+
outputs = {self.alice.getnewaddress(): total_amount / 2}
606+
assert_raises_rpc_error(
607+
-4,
608+
"Insufficient funds",
609+
self.charlie.send,
610+
outputs,
611+
version=3
612+
)
613+
614+
# Also doesn't work with fundrawtransaction
615+
raw_tx = self.charlie.createrawtransaction(inputs=[], outputs=outputs, version=3)
616+
assert_raises_rpc_error(
617+
-4,
618+
"Insufficient funds",
619+
self.charlie.fundrawtransaction,
620+
raw_tx,
621+
{'include_unsafe' : True}
622+
)
623+
624+
# Also doesn't work with sendall
625+
assert_raises_rpc_error(
626+
-6,
627+
"Total value of UTXO pool too low to pay for transaction",
628+
self.charlie.sendall,
629+
[self.alice.getnewaddress()],
630+
version=3
631+
)
632+
588633
if __name__ == '__main__':
589634
WalletV3Test(__file__).main()

0 commit comments

Comments
 (0)