Skip to content

Conversation

@fjahr
Copy link
Contributor

@fjahr fjahr commented Jan 9, 2026

This tries to fix #34205

I stared at the test code quite a bit and initially suspected some MiniWallet internals to be the issue but I think that was the wrong direction and there is simply a very small chance that the loop in test_sequence_lock_confirmed_inputs runs out of available utxos: We are starting out with 200-250 utxos and run the loop 400 times. If a transaction is accepted it could have up to 10 inputs but it always has only one output, so the pool is depleting in this case. And it's actually even worse because the output produced is not recognized as spendable by the MiniWallet because it is not using the correct output script. However, only a small fraction of transactions are actually accepted, which is why this issue almost never occurs. I did some extra printing and usually we end up with >100 utxos still available by the end of the test. But there is a small chance that too many transactions are accepted and then we can run out of utxos.

I considered two fixes: The first was a break at the beginning of the loop if available_utxos == 0: break, this would work fine but I went with the second option: Simply creating the output with the correct output script so that MiniWallet recognizes it as spendable. This minimal replentishment of available utxos ensures that at worst we should get a few 1 input, 1 ouput transactions by the end but we should never run out of available utxos. I didn't look back in history but I suspect that this is how it was intended before MiniWallet introduced.

Also moves the random import in the same function to the top of the file.

@DrahtBot DrahtBot added the Tests label Jan 9, 2026
@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 9, 2026

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34244.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK maflcko, bensig, darosior

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

@maflcko
Copy link
Member

maflcko commented Jan 10, 2026

lgtm ACK ab41492

@bensig
Copy link
Contributor

bensig commented Jan 12, 2026

ACK ab41492

Copy link
Member

@darosior darosior left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK ab41492

@glozow glozow merged commit d200378 into bitcoin:master Jan 13, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

6 participants