💬 jnewbery commented on pull request "p2p: Track orphans by who provided them":
(https://github.com/bitcoin/bitcoin/pull/26551#discussion_r1170060171)
> should we move orphan processing outside of ProcessMessages and SendMessages entirely, and just do it separately, independent of both who sent the orphan and who sent the parent?
>
> Unless someone has a good reason not to, I think I might draft that up as an alternative PR so the approaches can be compared more directly.

See #19364

💬 furszy commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1170062170)
Mainly to be consistent with the chain interface, wallet and spkm sources. Timestamps are represented as `int64_t` there. E.g. the `FoundBlock` class, `findFirstBlockWithTimeAndHeight` method, the `CWallet::m_best_block_time` member, first key time, descriptor creation time etc.

I think that is clear that block times are always in seconds, so the use of an arithmetic type shouldn't be much of a worry but we could switch all of them to `NodeSeconds` too (probably in another PR that does it all
...

👍 stickies-v approved a pull request: "[24.x] Additional backports for 24.1"
(https://github.com/bitcoin/bitcoin/pull/27474#pullrequestreview-1390158761)
ACK dc711fbd32653b09e196f72942106114a32353f4

Verified that the backports correspond to their original PR, and I can't see any silent merge conflicts. Release notes match the backported changes.

💬 rebroad commented on issue "add ability to remove (dust) UTXOs":
(https://github.com/bitcoin/bitcoin/issues/23875#issuecomment-1513184940)
I do think there's an unforeseen cost in storing these dust UTXOs, which might result in people willing to pay into a pot to have them cleared up - a bit like people grouping together to clean up the beaches of litter. If people didn't have to pay to pick-up their own dust, then it dust-picking might occur more often.

💬 0xB10C commented on pull request "kernel: chainparams updates for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27482#discussion_r1170066121)
2429000 on testnet might be too recent according to the docs: "Testnet should be set with a height some tens of thousands back from the tip, due to reorgs there.". It's currently 428 blocks old. There are still frequent [reorgs on testnet](https://fork.observer/?network=2), with the youngest being at 2428585. However, most reorgs on testnet I've seen are one-block reorgs and wouldn't affect this.

💬 fanquake commented on pull request "kernel: chainparams updates for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27482#discussion_r1170070814)
It will also be much deeper once 25.x is actually released.

💬 furszy commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1170076066)
Because the other one is a return statement (the flow breaks there) while this one is a setter.
Plus, when the line is too long, like this one, I think that braces help readability.

👍 hebasto approved a pull request: "[24.x] Additional backports for 24.1"
(https://github.com/bitcoin/bitcoin/pull/27474#pullrequestreview-1390181803)
re-ACK dc711fbd32653b09e196f72942106114a32353f4

💬 achow101 commented on pull request "doc: remove outdated version number usage from release-process":
(https://github.com/bitcoin/bitcoin/pull/27484#issuecomment-1513205003)
ACK fde224a6610699a6a28cc27e299ac14cbf7e16ca

🚀 achow101 merged a pull request: "doc: remove outdated version number usage from release-process"
(https://github.com/bitcoin/bitcoin/pull/27484)

💬 furszy commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1170086188)
yes. `BLANK_BIRTH_TIME` is for blank wallets. Blank wallets are wallets with no keys (no spkm exist). So it's ok to skip blocks as the wallet isn't going to find anything on them.

🤔 jonatack reviewed a pull request: "[24.x] Additional backports for 24.1"
(https://github.com/bitcoin/bitcoin/pull/27474#pullrequestreview-1390197262)
ACK dc711fbd32653b09e196f72942106114a32353f4

Pull description is missing some of the changes.

💬 0xB10C commented on pull request "kernel: chainparams updates for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27482#discussion_r1170091153)
Getting 502 GB in `blocks/` and 5.1GB in `chainstate/`. The new values seem fine as they should include enough overhead.

💬 furszy commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1170098672)
The id is the db lookup key for the descriptor, and also a way to detect db corruption.

We could clean it up from here, yeah. And probably, depending on its usage, we should also think about caching it.

💬 furszy commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1170124551)
sure, pushed.

💬 furszy commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1170124375)
Ended up re-adding the id arg. So we don't calculate the spkm id twice.

💬 jonatack commented on pull request "Remove Sock::Get() and Sock::Sock()":
(https://github.com/bitcoin/bitcoin/pull/26312#issuecomment-1513266130)
This is the last part of parent https://github.com/bitcoin/bitcoin/pull/21878 opened 2 years ago, anyone else like to have a look?

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1513251515)
Thank you for the review @mzumsande

Updated 97bf70119e5b8567bcdc553d59b18a09023fd05a -> 09f950e1f3608e3019e97236969b152e9252a220 ([removeBlockstorageArgs_11](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_11) -> [removeBlockstorageArgs_12](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_12), [compare](https://github.com/TheCharlatan/bitcoin/compare/removeBlockstorageArgs_11..removeBlockstorageArgs_12))
* Addressed @mzumsande's [comment](https://githu
...

💬 fanquake commented on pull request "[WIP] ci: standardize custom libc++ usage in MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/27485#issuecomment-1513258618)
We can likely do something similar for our Darwin depends build, once we're using a newer version of LLVM/Clang, might might make the flag usage and configuration in there slightly clearer.

💬 achow101 commented on pull request "kernel: chainparams updates for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27482#issuecomment-1513267552)
ACK a2bef805c11a4ab391f4ea635bfde7dd2ec9ce81