🤔 Sjors reviewed a pull request: "wallet: birth time update during tx scanning"
(https://github.com/bitcoin/bitcoin/pull/28920#pullrequestreview-1741418506)
I checked that the test fails if you drop everything but 134960c3828abe658d565af52e3c6c5af03019f2.

💬 Sjors commented on pull request "wallet: birth time update during tx scanning":
(https://github.com/bitcoin/bitcoin/pull/28920#discussion_r1400220704)
4bfb243ac41cf1396cf55f8b24de2ba52186c7a4: alternative name: `MaybeUpdateBirthTime( )`

💬 Sjors commented on pull request "wallet: birth time update during tx scanning":
(https://github.com/bitcoin/bitcoin/pull/28920#discussion_r1400226671)
4bfb243ac41cf1396cf55f8b24de2ba52186c7a4: this seems simpler:

```cpp
TryUpdateBirthTime(spkm_man->GetTimeFirstKey());
m_spk_managers[id] = std::move(spkm_man);
```

💬 Sjors commented on pull request "wallet: birth time update during tx scanning":
(https://github.com/bitcoin/bitcoin/pull/28920#discussion_r1400224214)
4bfb243ac41cf1396cf55f8b24de2ba52186c7a4: I'm a bit puzzled why this function ever accepted an argument that it didn't use.

💬 Sjors commented on pull request "wallet: birth time update during tx scanning":
(https://github.com/bitcoin/bitcoin/pull/28920#discussion_r1400248376)
4bfb243ac41cf1396cf55f8b24de2ba52186c7a4: you can use `std::placeholders::_1` here, by dropping `this` from `NotifyFirstKeyTimeChanged` calls and changing:

```diff
--- a/src/wallet/scriptpubkeyman.h
+++ b/src/wallet/scriptpubkeyman.h
@@ -255,11 +255,11 @@ public:

/** Keypool has new keys */
boost::signals2::signal<void ()> NotifyCanGetAddressesChanged;

/** Birth time changed */
- boost::signals2::signal<void (const ScriptPubKeyMan* spkm, int64_t new_birth_time)>
...

💬 Sjors commented on pull request "doc: Add release note for coinstatsindex in pruned mode":
(https://github.com/bitcoin/bitcoin/pull/28909#issuecomment-1820535005)
Changing the historical release notes won't change the release notes that come with the actual v24.* binaries, so this won't work.

The v24 release will also be [end-of-life](https://bitcoincore.org/en/lifecycle/) as soon as we ship v26.0.

✅ maflcko closed a pull request: "doc: Add release note for coinstatsindex in pruned mode"
(https://github.com/bitcoin/bitcoin/pull/28909)

💬 Sjors commented on pull request "Drop CAutoFile":
(https://github.com/bitcoin/bitcoin/pull/28904#issuecomment-1820585297)
Looks like a nice cleanup, but I'm not familiar with the history of `CAutoFile` -> `AutoFile`. I did check that tests pass for all intermediate commits.

💬 maflcko commented on pull request "doc: Add release note for coinstatsindex in pruned mode":
(https://github.com/bitcoin/bitcoin/pull/28909#issuecomment-1820586422)
Closing for now, per previous comment

💬 maflcko commented on pull request "Drop CAutoFile":
(https://github.com/bitcoin/bitcoin/pull/28904#issuecomment-1820609651)
> Looks like a nice cleanup, but I'm not familiar with the history of `CAutoFile` -> `AutoFile`.

serialization flags embedded in the version-integer and type-integer are gone, serialization now uses params-serialization, so the ser-version and ser-type code can be deleted.

👍 Sjors approved a pull request: "wallet: batch all individual spkms setup db writes in a single db txn"
(https://github.com/bitcoin/bitcoin/pull/28894#pullrequestreview-1741582965)
ACK 1901360ca52c6563a40dc78633cc492a822ac7a9

Not sure if b85b75d6470841813c5844b2e9c883360f1ad94a is needed since we're getting rid of the legacy wallet anyway. But it looks correct.

Ran bench on MacBook Pro 2019 (2,3 GHz 8-Core Intel Core i9, SSD):

Before:

```
src/bench/bench_bitcoin -filter="WalletCreate[E|P].*" -min-time=30000
```

| ns/op | op/s | err% | total | benchmark
|--------------------:|-------------------
...

💬 Sjors commented on pull request "wallet: batch all individual spkms setup db writes in a single db txn":
(https://github.com/bitcoin/bitcoin/pull/28894#discussion_r1400325124)
da133a80bae311373337a0d9c474667a5f73908d nit: `TopUpWithDB`

💬 Sjors commented on pull request "wallet: batch all individual spkms setup db writes in a single db txn":
(https://github.com/bitcoin/bitcoin/pull/28894#discussion_r1400342212)
1901360ca52c6563a40dc78633cc492a822ac7a9: nit "is committed to disk"

💬 dergoegge commented on pull request "Improve peformance of CTransaction::HasWitness (28107 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/28766#issuecomment-1820666311)
I think this is rfm. Could a maintainer take a look?

✅ hebasto closed an issue: "ci: failure in Windows MSVC build"
(https://github.com/bitcoin/bitcoin/issues/28901)

🚀 hebasto merged a pull request: "ci: Avoid toolset ambiguity that MSVC can't handle"
(https://github.com/bitcoin/bitcoin/pull/28905)

💬 TheCharlatan commented on pull request "build: Fix regression in "ARMv8 CRC32 intrinsics" test":
(https://github.com/bitcoin/bitcoin/pull/28919#issuecomment-1820742685)
ACK 228d6a2969e4fcee573c9df7aad31550eab9c8d4

I checkd that the bianry now contains both crc and crypto intrinsic opcodes.

Guix build (x86_64 and aarch64):
```
2afd81f540c6d3b36ff305e88bafe935e4272cd3efef3130aa69d49a0522541b guix-build-228d6a2969e4/output/aarch64-linux-gnu/SHA256SUMS.part
6c704d6d30d495adb3fb86befdb500eb389a02c1167163f14ab5c3c3e630e6b3 guix-build-228d6a2969e4/output/aarch64-linux-gnu/bitcoin-228d6a2969e4-aarch64-linux-gnu-debug.tar.gz
e4419963c9c0d99adc4e38538900b648f
...

💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#issuecomment-1820750522)
Rebased now that #28905 is merged.

💬 hebasto commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#issuecomment-1820787528)
Related:
- https://github.com/bitcoin/bitcoin/issues/28387
- https://bitcoin.stackexchange.com/questions/119491/generate-regtest-blocks-with-bitcoin-qt/119492

💬 hebasto commented on issue "Additional Bitcoin-Qt capability to mine regtest blocks":
(https://github.com/bitcoin/bitcoin/issues/28387#issuecomment-1820789212)
Related:
- https://github.com/bitcoin-core/gui/pull/692

📝 dergoegge opened a pull request: "Use Txid in COutpoint"
(https://github.com/bitcoin/bitcoin/pull/28922)
This PR changes the type of the hash of a transaction outpoint from `uint256` to `Txid`.