👋 brunoerg's pull request is ready for review: "fuzz: add target for `DescriptorScriptPubKeyMan`"
(https://github.com/bitcoin/bitcoin/pull/28578)

💬 hebasto commented on issue "Sync slow":
(https://github.com/bitcoin/bitcoin/issues/26063#issuecomment-1819241844)
> Please reopen the issue, I will try to give logs with lock conflicts later. Probably related #776

Windows and Fedora, which was mentioned by this issue author, are quite different systems.

The rule of thumb for Bitcoin Core users on Windows is to add the data and block directories into Windows Security exclusions.

✅ maflcko closed an issue: "Sync slow"
(https://github.com/bitcoin/bitcoin/issues/26063)

💬 maflcko commented on issue "Sync slow":
(https://github.com/bitcoin/bitcoin/issues/26063#issuecomment-1819250607)
> Windows and Fedora, which was mentioned by this issue author, are quite different systems.

Ok, closing again. Feel free to file a new issue if this happens again. Make sure to include details, as explained above. You may reference this issue, if you want.

💬 maflcko commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#discussion_r1399346490)
style nit: Please put `std::` includes in a new section

💬 maflcko commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#discussion_r1399351848)
```suggestion
WalletDescriptor w_desc{(std::move(parsed_desc), 0, 0, 1, 1)};
```

nit: no need to mention the type twice for a single symbol

💬 maflcko commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#discussion_r1399350026)
```suggestion
Chainstate& chainstate = node.chainman->ActiveChainstate();
```

any reason to turn a reference into a pointer when you can keep the reference?

💬 pinheadmz commented on pull request "coins: make sure PoolAllocator uses the correct alignment":
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1819261340)
confirmed unit passes on arm32 but also passes without the actual bugfix commit. so effectively this on top of d752349029:


```diff
commit 2c7f49e83a5d05a5ba8b29d52108c14bf4d65af7
Author: Martin Leitner-Ankerl <martin.ankerl@gmail.com>
Date: Sun Nov 19 18:43:15 2023 +0100

pool: change memusage_test to use int64_t

If alignment of the PoolAllocator would be insufficient, then the test would fail. This also catches the issue with ARM 32bit, where int64_t is aligned to 8 b
...

👍 TheCharlatan approved a pull request: "build: Windows SSP roundup"
(https://github.com/bitcoin/bitcoin/pull/28461#pullrequestreview-1740074663)
ACK f95af98128f17002bf137a48441167020f3ef9bb

I get the same hashes as @hebasto

💬 maflcko commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#discussion_r1399366365)
Using clang-tidy named args would be better than just listing `0,0,1,1`, which is hard to understand without meaning.

👍 stickies-v approved a pull request: "Drop CAutoFile"
(https://github.com/bitcoin/bitcoin/pull/28904#pullrequestreview-1740095172)
ACK 4eb2a9ea4b6262bec0bc7c20cb3e684ea75caf42

🤔 maflcko reviewed a pull request: "fuzz: add target for `DescriptorScriptPubKeyMan`"
(https://github.com/bitcoin/bitcoin/pull/28578#pullrequestreview-1740110220)
lgtm ACK 7774df96942ba10aeb5c818b100aedc9cb7d4b4f

💬 maflcko commented on pull request "ci: Avoid toolset ambiguity that MSVC can't handle":
(https://github.com/bitcoin/bitcoin/pull/28905#issuecomment-1819303679)
lgtm ACK 91d5bd8ac9a28725c735f8e6900bc85673bb190a , assuming it fixes the CI failures

🤔 BrandonOdiwuor reviewed a pull request: "script, assumeutxo: Enhance validations in utxo_snapshot.sh"
(https://github.com/bitcoin/bitcoin/pull/28852#pullrequestreview-1740123770)
Concept ACK

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1399395741)
thanks I understand, updated

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1399396760)
I agree with this point, fixed.
Thank you

💬 hebasto commented on pull request "coins: make sure PoolAllocator uses the correct alignment":
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1819330863)
If anyone consider the second commit as a blocker, it might be dropped from this PR, no?

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1399422341)
Can you clarify what txs, orig_txs and cluster are for, and what the general strategy is here?

📝 furszy opened a pull request: "wallet: birth time update during tx scanning"
(https://github.com/bitcoin/bitcoin/pull/28920)
Fixing #28897.

As the user may have imported a descriptor with a timestamp newer
than the actual birth time of the first key (by setting 'timestamp=now'),
the wallet needs to update the birth time when it detects a transaction
older than the oldest descriptor timestamp.

Testing Notes:
Can cherry-pick the test commit on top of master. It will fail there.

Extra Note:
This works fine as is but I want to do few other cleanups.
E.g. `FirstKeyTimeChanged()` first arg is never used and,
...

💬 dergoegge commented on pull request "fuzz: Test headers pre-sync through p2p interface":
(https://github.com/bitcoin/bitcoin/pull/28043#issuecomment-1819360700)
Thanks! rebased.

💬 mzumsande commented on pull request "test: Make existing functional tests compatible with --v2transport":
(https://github.com/bitcoin/bitcoin/pull/28805#discussion_r1399433051)
> One small drawback of the current way of inspecting `extra_args` for the v2transport option is that it doesn't work if the argument is specified in a slightly different format with same meaning.

Good point - I agree that it makes sense to leave this for a possible follow-up. I'm not sur if we want to port all of the argsmanager logic into python, but having something a bit more general so that it could be reused by other args than just `-v2transport` in the future would be nice.