qa: `wallet_importdescriptors` failure "TypeError: 'bool' object is not subscriptable"

hebasto commented at 1:36 pm on February 16, 2025: member

From https://github.com/bitcoin/bitcoin/actions/runs/13355177559/job/37296904714:

 0244/317 - wallet_importdescriptors.py --descriptors failed, Duration: 13 s
 1
 2stdout:
 32025-02-16T12:59:56.602000Z TestFramework (INFO): PRNG seed is: 3316204795374696824
 42025-02-16T12:59:56.602000Z TestFramework (INFO): Initializing test directory /Users/runner/work/bitcoin/bitcoin/ci/scratch/test_runner/test_runner_₿_🏃_20250216_125556/wallet_importdescriptors_99
 52025-02-16T12:59:57.273000Z TestFramework (INFO): Setting up wallets
 62025-02-16T12:59:57.317000Z TestFramework (INFO): Mining coins
 72025-02-16T12:59:58.510000Z TestFramework (INFO): Import should fail if a descriptor is not provided
 82025-02-16T12:59:58.519000Z TestFramework (INFO): Should import a p2pkh descriptor
 92025-02-16T12:59:58.537000Z TestFramework (INFO): Test can import same descriptor with public key twice
102025-02-16T12:59:58.543000Z TestFramework (INFO): Test can update descriptor label
112025-02-16T12:59:58.550000Z TestFramework (INFO): Internal addresses cannot have labels
122025-02-16T12:59:58.552000Z TestFramework (INFO): Internal addresses should be detected as such
132025-02-16T12:59:58.564000Z TestFramework (INFO): Should not import a p2sh-p2wpkh descriptor without checksum
142025-02-16T12:59:58.564000Z TestFramework (INFO): Should not import a p2sh-p2wpkh descriptor that has range specified
152025-02-16T12:59:58.565000Z TestFramework (INFO): Should not import a p2sh-p2wpkh descriptor and have it set to active
162025-02-16T12:59:58.566000Z TestFramework (INFO): Should import a (non-active) p2sh-p2wpkh descriptor
172025-02-16T12:59:58.582000Z TestFramework (INFO): Should import a 1-of-2 bare multisig from descriptor
182025-02-16T12:59:58.584000Z TestFramework (INFO): Should not treat individual keys from the imported bare multisig as watchonly
192025-02-16T12:59:58.585000Z TestFramework (INFO): Ranged descriptors cannot have labels
202025-02-16T12:59:58.586000Z TestFramework (INFO): Private keys required for private keys enabled wallet
212025-02-16T12:59:58.586000Z TestFramework (INFO): Ranged descriptor import should warn without a specified range
222025-02-16T12:59:58.590000Z TestFramework (INFO): Should not import a ranged descriptor that includes xpriv into a watch-only wallet
232025-02-16T12:59:58.590000Z TestFramework (INFO): Should not import a descriptor with hardened derivations when private keys are disabled
242025-02-16T12:59:58.595000Z TestFramework (INFO): Verify we can only extend descriptor's range
252025-02-16T12:59:58.619000Z TestFramework (INFO): Check we can change descriptor internal flag
262025-02-16T12:59:58.633000Z TestFramework (INFO): Key ranges should be imported in order
272025-02-16T12:59:58.665000Z TestFramework (INFO): Check we can change next_index
282025-02-16T12:59:58.678000Z TestFramework (INFO): Check imported descriptors are not active by default
292025-02-16T12:59:58.680000Z TestFramework (INFO): Check can activate inactive descriptor
302025-02-16T12:59:58.683000Z TestFramework (INFO): Check can deactivate active descriptor
312025-02-16T12:59:58.687000Z TestFramework (INFO): Verify activation state is persistent
322025-02-16T12:59:58.693000Z TestFramework (INFO): Should import a descriptor with a WIF private key as spendable
332025-02-16T12:59:58.696000Z TestFramework (INFO): Test can import same descriptor with private key twice
342025-02-16T12:59:58.713000Z TestFramework (INFO): Test that multisigs can be imported, signed for, and getnewaddress'd
352025-02-16T13:00:02.944000Z TestFramework (INFO): Multisig with distributed keys
362025-02-16T13:00:03.441000Z TestFramework (INFO): We can create and use a huge multisig under P2WSH
372025-02-16T13:00:04.656000Z TestFramework (INFO): Under P2SH, multisig are standard with up to 15 compressed keys
382025-02-16T13:00:05.714000Z TestFramework (INFO): Amending multisig with new private keys
392025-02-16T13:00:05.897000Z TestFramework (INFO): Combo descriptors cannot be active
402025-02-16T13:00:05.897000Z TestFramework (INFO): Descriptors with no type cannot be active
412025-02-16T13:00:05.900000Z TestFramework (INFO): Test importing a descriptor to an encrypted wallet
422025-02-16T13:00:09.233000Z TestFramework (ERROR): Assertion failed
43Traceback (most recent call last):
44  File "/Users/runner/work/bitcoin/bitcoin/ci/scratch/build-aarch64-apple-darwin23.6.0/test/functional/wallet_importdescriptors.py", line 711, in run_test
45    duration = wallet_info["scanning"]["duration"]
46               ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^
47TypeError: 'bool' object is not subscriptable
48
49During handling of the above exception, another exception occurred:
50
51Traceback (most recent call last):
52  File "/Users/runner/work/bitcoin/bitcoin/test/functional/test_framework/test_framework.py", line 135, in main
53    self.run_test()
54    ~~~~~~~~~~~~~^^
55  File "/Users/runner/work/bitcoin/bitcoin/ci/scratch/build-aarch64-apple-darwin23.6.0/test/functional/wallet_importdescriptors.py", line 714, in run_test
56    assert "scanning" not in wallet_info
57           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
58AssertionError
592025-02-16T13:00:09.381000Z TestFramework (INFO): Not stopping nodes as test failed. The dangling processes will be cleaned up later.
602025-02-16T13:00:09.381000Z TestFramework (WARNING): Not cleaning up dir /Users/runner/work/bitcoin/bitcoin/ci/scratch/test_runner/test_runner_₿_🏃_20250216_125556/wallet_importdescriptors_99
612025-02-16T13:00:09.381000Z TestFramework (ERROR): Test failed. Test logging available at /Users/runner/work/bitcoin/bitcoin/ci/scratch/test_runner/test_runner_₿_🏃_20250216_125556/wallet_importdescriptors_99/test_framework.log

hebasto added the label Wallet on Feb 16, 2025

hebasto added the label Tests on Feb 16, 2025

hebasto commented at 3:50 pm on February 17, 2025: member

Another one: https://github.com/bitcoin/bitcoin/actions/runs/13373718736/job/37348002275

maflcko added the label CI failed on Feb 17, 2025

maflcko added this to the milestone 29.0 on Feb 17, 2025

maflcko commented at 4:03 pm on February 17, 2025: member

I haven’t tried, but #31768 (review) by @hodlinator may fix it

furszy commented at 4:27 pm on February 17, 2025: member

I haven’t tried, but #31768 (comment) by @hodlinator may fix it

It fixes it. Can test it by adding a small sleep(3) before the getwalletinfo() call.

But… this test is not deterministic. If the scanning process runs slower, the “0” duration check will also fail. I’m not sure we should keep it.

hodlinator commented at 9:20 pm on February 17, 2025: contributor

But… this test is not deterministic. If the scanning process runs slower, the “0” duration check will also fail. I’m not sure we should keep it.

Yeah, it seems like an overly harsh check to me. Curious what was the impulse behind #31768 @brunoerg?

brunoerg commented at 9:29 pm on February 17, 2025: contributor

I haven’t tried, but #31768 (review) by @hodlinator may fix it

Yes, it fixes it.

Yeah, it seems like an overly harsh check to me. Curious what was the impulse behind #31768 @brunoerg?

I’ve tried it on different setups and besides the duration being different as expected for every setup/run I tried I didn’t get a case where it’s 0. Anyway, I’m fine on removing it or addressing the @hodlinator’s suggestion.

hodlinator commented at 9:37 pm on February 17, 2025: contributor

Is the added code protecting some other code? Or just documenting the interface of the RPC, perhaps based on surviving mutants (from mutation testing)? If it’s something like the former, I think it should be fixed. If it’s the latter, I would lean towards reverting unless people agree we should start testing every aspect of the RPC interface (gradually or in larger PR(s)).

brunoerg commented at 9:47 pm on February 17, 2025: contributor

I just opened #31893 to remove it. It’s better to avoid any possibility of intermittent issue.

Is the added code protecting some other code? Or just documenting the interface of the RPC, perhaps based on surviving mutants (from mutation testing)? If it’s something like the former, I think it should be fixed. If it’s the latter, I would lean towards reverting unless people agree we should start testing every aspect of the RPC interface (gradually or in larger PR(s)).

It’s just a test coverage addition but not based on surviving mutants since it does not even have test coverage.

hodlinator commented at 9:56 pm on February 17, 2025: contributor

It’s just a test coverage addition but not based on surviving mutants since it does not even have test coverage.

Ah, forgot that mutants are focused on modifying test code, not implementation. Edit: <– this was wrong, see reply below

brunoerg commented at 9:58 pm on February 17, 2025: contributor

Ah, forgot that mutants are focused on modifying test code, not implementation.

No, mutants are focused on mutating the implementation to verify the tests. However, if there is no test coverage for the implementation, it does not make sense to modify it, there is nothing to verify.

fanquake referenced this in commit 63d625f761 on Feb 18, 2025

hebasto commented at 3:24 pm on February 18, 2025: member

Resolved in #31893.

hebasto closed this on Feb 18, 2025

qa: wallet_importdescriptors failure “TypeError: ‘bool’ object is not subscriptable” #31881

qa: `wallet_importdescriptors` failure “TypeError: ‘bool’ object is not subscriptable” #31881