👍 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.

💬 darosior commented on pull request "fuzz: Test headers pre-sync through p2p interface":
(https://github.com/bitcoin/bitcoin/pull/28043#issuecomment-1819363878)
Concept ACK. Planning on reviewing soon.

💬 martinus commented on pull request "coins: make sure PoolAllocator uses the correct alignment":
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1819368254)
I've updated the test in the PR, this time I think it should have caught the issue. It should be reproducible by adding an `alignof(void*)` back as the `PoolAllocator`'s last template argument

👍 hebasto approved a pull request: "depends: bump libmultiprocess to fix capnproto deprecation warnings"
(https://github.com/bitcoin/bitcoin/pull/28907#pullrequestreview-1740227113)
ACK 21bfee0720ba72935d4f9fc4c2a2887ae5b54092, I have reviewed the code and it looks OK. I've also skimmed through the related changes in the https://github.com/chaincodelabs/libmultiprocess repository.

💬 hebasto commented on pull request "coins: make sure PoolAllocator uses the correct alignment":
(https://github.com/bitcoin/bitcoin/pull/28913#issuecomment-1819422908)
A failure in the "Win64 native, VS 2022" CI job is unrelated. See: https://github.com/bitcoin/bitcoin/pull/28905.

✅ dergoegge closed a pull request: "fuzz: Delete wallet_notifications"
(https://github.com/bitcoin/bitcoin/pull/28882)

💬 dergoegge commented on pull request "fuzz: Delete wallet_notifications":
(https://github.com/bitcoin/bitcoin/pull/28882#issuecomment-1819438212)
🤷

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#issuecomment-1819452855)
CI failure is unrelated see https://github.com/bitcoin/bitcoin/pull/28905

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-1819490214)
> I don't think a detailed line-by-line code review at this stage is a good use of your time.

That's not what I'm trying to do. I strategically / haphazerdly picked a dozen lines to ask questions about to get a better understanding of the whole thing. Based on IRC chat today, I'll wait now for the upcoming update.