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

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