💬 l0rinc commented on pull request "kernel: Flush in ChainstateManager destructor":
(https://github.com/bitcoin/bitcoin/pull/31382#discussion_r1860729040)
nit: this being a console app, it might make sense to return a non-zero error code when `cerr` is used

🤔 l0rinc reviewed a pull request: "kernel: Flush in ChainstateManager destructor"
(https://github.com/bitcoin/bitcoin/pull/31382#pullrequestreview-2465133813)
Concept ACK

Closed https://github.com/bitcoin/bitcoin/pull/31362 in favor of this

🚀 fanquake merged a pull request: "build: Fix coverage builds"
(https://github.com/bitcoin/bitcoin/pull/31337)

💬 TheCharlatan commented on pull request "kernel: Flush in ChainstateManager destructor":
(https://github.com/bitcoin/bitcoin/pull/31382#discussion_r1860756141)
Yeah, done.

💬 maflcko commented on pull request "util/blockstorage: Add `TRACE_RAII`, slightly faster -reindex-chainstate with CBufferedFile":
(https://github.com/bitcoin/bitcoin/pull/28226#issuecomment-2504013215)
> It may be better to modify `CAutoFile` in that case. The file handle in the auto-file should[1] always be unique and pinned to a single thread, so I using `fread_unlocked` seems better, as it improves performance everywhere. (Though, it looks like `std::fread_unlocked` does not exist in C++, so I wonder if it exists on Windows and macOS?)
>
> [1] I may submit a related pull to better enforce this at compile-time.

For reference, `AutoFile::Get` was removed in commit a240e150e837b5a95ed197
...

💬 instagibbs commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2504021426)
reACK 37a5c5d83664c31d83fc649d3c8c858bd5f10f21


blockhash is now an optional return

💬 TheCharlatan commented on pull request "kernel: Flush in ChainstateManager destructor":
(https://github.com/bitcoin/bitcoin/pull/31382#issuecomment-2504098705)
Seems like I am running into this here: https://github.com/bitcoin/bitcoin/pull/25073#issuecomment-1194417234, maybe time to introduce a replace mempool method like Carl suggested there?

✅ yancyribbens closed a pull request: "Add coin-grinder example test"
(https://github.com/bitcoin/bitcoin/pull/31352)

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1860890755)
> The problem is that no one will notice if this isn't run on any machine

I changed it to insist that the tcpdump file was created on the ASAN env. So the ASAN job will be red if this stops working on it.

💬 l0rinc commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1860910578)
I'd like to revive my original concerns here: [`c4eb31b` (#30906)](https://github.com/bitcoin/bitcoin/pull/30906/commits/c4eb31be3e56936479921b7556fcba1747867d02#diff-3d0856e8b7f136c588b229e0cbd3b2e2c309cd097ade0879521daba4e1bb2a33L778-L782) removes this method completely, which I think is a better fix for it than the current workaround.

💬 l0rinc commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1860914367)
I'm not sure I understand the reason for the suppression, please see my original suggestion in https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1846343700

💬 furszy commented on pull request "wallet: fix crash during migration due to invalid multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31378#issuecomment-2504309436)
> Ruling out consensus-invalid scripts makes sense, but discarding non-standard scripts is a much bigger leap. Also it would be a wack-a-mole: does the migration crash on any non-standard script? How much of that do you want to cover?

Ok, I might have made the PR/commit description too broad when the actual changes are very specific.
Migration does not crash on all non-standard scripts, it only crashes on scripts that fail to be parsed by the descriptors' logic. The specific non-standard cra
...

🤔 mzumsande reviewed a pull request: "rpc: Remove submitblock pre-checks"
(https://github.com/bitcoin/bitcoin/pull/31175#pullrequestreview-2465507312)
ACK 73db95c65c1d372822166045ca8b9f173d5fd883
Reviewed the code (didn't look closely at `bitcoin-chainstate.cpp`) and tested it a little bit.

💬 mzumsande commented on pull request "rpc: Remove submitblock pre-checks":
(https://github.com/bitcoin/bitcoin/pull/31175#discussion_r1860960957)
nit: If, after this PR, we re-submit genesis via `submitblock`, a different code path will be taken than for non-genesis duplicate blocks:
We won't return early in `AcceptBlockHeader`, will actually call `AddToBlockIndex()`, and write an incorrect log message
`Saw new header hash=0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206 height=0` for each `submitblock` call.
Not a real problem though because nothing fails and why would you do that in the first place.

💬 ryanofsky commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2504369867)
Updated 1f18eefe1e5bc3a97f5f6c9637a3a193e2c2f22e -> 27072547bb22cdd2080d1014eba9c30bc9d47650 ([`pr/mine.13`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.13) -> [`pr/mine.14`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.14), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine.13..pr/mine.14)) to fix CI errors from `skip_if_no_multiprocess` check being broken and from default testdir path exceeding maximum socket length

💬 achow101 commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2504381025)
ACK 37a5c5d83664c31d83fc649d3c8c858bd5f10f21

💬 rkrux commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#issuecomment-2504393638)
re-ACK 37a5c5d83664c31d83fc649d3c8c858bd5f10f21

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2504400845)
Updated 19ae652376faca65d972c12cb51cfc8af0560c9e -> 63df9f3deb35be79496f7c240e3303e1d96c6832 ([`pr/wrap.3`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.3) -> [`pr/wrap.4`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/wrap.3..pr/wrap.4)) with fixes for windows and fuzz CI errors, and lint and tidy fixes

👍 rkrux approved a pull request: "rpc: add getdescriptoractivity"
(https://github.com/bitcoin/bitcoin/pull/30708#pullrequestreview-2465616436)
re-ACK 37a5c5d83664c31d83fc649d3c8c858bd5f10f21

🚀 achow101 merged a pull request: "rpc: add getdescriptoractivity"
(https://github.com/bitcoin/bitcoin/pull/30708)