💬 fanquake commented on pull request "test: fix race condition in encrypted wallet rescan tests":
(https://github.com/bitcoin/bitcoin/pull/27199#issuecomment-1455216689)
Worked for me under a recent Valgrind run.

💬 fanquake commented on issue "Intermittent issue in p2p_ibd_stalling.py self.wait_until(lambda: self.total_bytes_recv_for_blocks() == 172761)":
(https://github.com/bitcoin/bitcoin/issues/27112#issuecomment-1455217136)
I've also just had `p2p_ibd_stalling.py` fail under Valgrind (40c6c85c05812ee8bf824b639307b1ac17a001c4):
```bash
test 2023-03-05T21:26:43.074000Z TestFramework.node0 (DEBUG): Connecting to 127.0.0.1:12173 outbound-full-relay
node0 2023-03-05T21:26:43.265731Z [msghand] [net_processing.cpp:5807] [SendMessages] [net] Requesting block 752405439cea869d584044084502582bc209e4ef97e4bf3b8c2ba3958acaf606 (21) peer=0
node0 2023-03-05T21:26:43.267295Z [msghand] [net.cpp:2816] [PushMessage] [net] s
...

⚠️ Victorcorreiaaraujo opened an issue: "Vahhh"
(https://github.com/bitcoin/bitcoin/issues/27204)
<!-- Needs the label "good first issue" assigned manually before or after opening -->

<!-- A good first issue is an uncontroversial issue, that has a relatively unique and obvious solution -->

<!-- Motivate the issue and explain the solution briefly -->

#### Useful skills:

<!-- (For example, “std::thread”, “Qt5 GUI and async GUI design” or “basic understanding of Bitcoin mining and the Bitcoin Core RPC interface”.) -->

#### Want to work on this issue?

For guidance on contributing, please r
...

📝 TheCharlatan opened a pull request: "doc: Show how less noisy clang-tidy output can be achieved"
(https://github.com/bitcoin/bitcoin/pull/27205)
Adds a paragraph to the clang-tidy section explaining how to de-noise its output. By default clang-tidy will print errors arrising from included headers in leveldb and other dependencies. By passing `--enable-suppress-external-warnings` flag to configure, errors arising from external dependencies are suppressed. Additional errors arrising from internal dependencies such as leveldb are suppressed by passing the `src/.bear-tidy-config` configuration file to bear. This file includes exclusionary ru
...

⚠️ Victorcorreiaaraujo opened an issue: "Hjik"
(https://github.com/bitcoin/bitcoin/issues/27206)
0x1dd6EAB101CAE27413440901B4A3854B4C797136

💬 ishaanam commented on pull request "test: fix race condition in encrypted wallet rescan tests":
(https://github.com/bitcoin/bitcoin/pull/27199#issuecomment-1455354602)
> Is this a draft because you haven't tested under Valgrind yet?

Yes, it was.
> Worked for me under a recent Valgrind run.

Great, I will mark it as ready for review.

👋 ishaanam's pull request is ready for review: "test: fix race condition in encrypted wallet rescan tests"
(https://github.com/bitcoin/bitcoin/pull/27199)

⚠️ YANGHONGEUN opened an issue: "79f0a3f1f42e0421e60cf9a57f6c70c6be9221a1"
(https://github.com/bitcoin/bitcoin/issues/27207)
<!-- Needs the label "good first issue" assigned manually before or after opening -->

<!-- A good first issue is an uncontroversial issue, that has a relatively unique and obvious solution -->

<!-- Motivate the issue and explain the solution briefly -->

#### Useful skills:

<!-- (For example, “std::thread”, “Qt5 GUI and async GUI design” or “basic understanding of Bitcoin mining and the Bitcoin Core RPC interface”.) -->

#### Want to work on this issue?

For guidance on contributing, please r
...

✅ fanquake closed an issue: "79f0a3f1f42e0421e60cf9a57f6c70c6be9221a1"
(https://github.com/bitcoin/bitcoin/issues/27207)

:lock: fanquake locked an issue: "79f0a3f1f42e0421e60cf9a57f6c70c6be9221a1"
(https://github.com/bitcoin/bitcoin/issues/27207)

✅ fanquake closed an issue: "Hjik"
(https://github.com/bitcoin/bitcoin/issues/27206)

:lock: fanquake locked an issue: "Hjik"
(https://github.com/bitcoin/bitcoin/issues/27206)

✅ fanquake closed an issue: "Vahhh"
(https://github.com/bitcoin/bitcoin/issues/27204)

:lock: fanquake locked an issue: "Vahhh"
(https://github.com/bitcoin/bitcoin/issues/27204)

💬 S3RK commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1126052875)
> add (a TODO for) method to the wallet tool to re-run the helper functions; this would be for advanced users who somehow ended up with a wallet not matching this pattern, but have repented, added the right descriptors, and now want the global HD key field set


Is this really needed if we add support for `sethdseed` later?

💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#issuecomment-1455748229)
utACK 1cfed6ce6cd8c9f9f1e93662ff8f92cd5a5b91c4 (also reviewed the tests)

💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1126088720)
this is slightly confusing because I'd expect the number to go one up from the previous case (from 3 => 4).

The test is passing, because you flipped `positive_only` and now excluded negative group is balanced by the group for `dest7`.

I'd suggest keeping `positive_only=false` and bumping the numbers

💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1126099320)
Commit message is now inconsistent with the code. Either drop the details of test cases or update them.

💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1126122594)
The reason why I suggested this change is because it's hard to explain why we need a `insert_mixed` flag. Why don't we just always add groups to `mixed` vector? I understand that's because of how aps works, but just looking at this method it's mysterious.

💬 MarcoFalke commented on pull request "test: fix race condition in encrypted wallet rescan tests":
(https://github.com/bitcoin/bitcoin/pull/27199#discussion_r1126125555)
Any reason to use the cli, when the normal rpc can be used directly?

Also, I wonder if this introduces another race where the actual rescan in `t.start()` is scheduled after this call. Avoiding this race might be ugly, but I think it could be possible by using assert_debug_log with a timeout value to sync on the rescan actually starting?