💬 theuni commented on pull request "wallet: don't include bdb files from our headers":
(https://github.com/bitcoin/bitcoin/pull/28039#discussion_r1255896389)
Nice catch. Fixed.

👍 hebasto approved a pull request: "wallet: don't include bdb files from our headers"
(https://github.com/bitcoin/bitcoin/pull/28039#pullrequestreview-1519034588)
ACK 8b5397c00e821d7eaab22f512e9d71a1a0392ebf

💬 MarcoFalke commented on pull request "rfc: Nuke getblocks message":
(https://github.com/bitcoin/bitcoin/pull/28045#issuecomment-1625488424)
It may be good to add basic tests for this feature, otherwise the risk is that it will break eventually and no one noticing for a long time, since there are only few users and existing nodes will continue to reply to messages for the time they are still running?

💬 fanquake commented on pull request "wallet: address book migration bug fixes":
(https://github.com/bitcoin/bitcoin/pull/28038#issuecomment-1625494895)
@achow101 do you want these backported for 25.1?

📝 hebasto opened a pull request: "Rebased cmake-staging branch (post PR#15)"
(https://github.com/bitcoin/bitcoin/pull/28046)
This is the [cmake-staging](https://github.com/hebasto/bitcoin/tree/cmake-staging) branch rebased on the recent [bitcoin/master](https://github.com/bitcoin/bitcoin/commit/d908877c4774c2456eed09167a5f382758e4a8a6) one with squashed "FIXUP" commits.

Rebase phase: [`cmake-staging/pr16-init`](https://github.com/hebasto/bitcoin/commits/cmake-staging/pr16-init) --> [`cmake-staging/pr16-rebased`](https://github.com/hebasto/bitcoin/commits/cmake-staging/pr16-rebased).

Squash phase: [`cmake-staging
...

💬 pinheadmz commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#issuecomment-1625497339)
> 🐙 This pull request conflicts with the target branch and [needs rebase](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#rebasing-changes).

Thanks handsome bot! 🍦

💬 theuni commented on pull request "wallet: don't include bdb files from our headers":
(https://github.com/bitcoin/bitcoin/pull/28039#issuecomment-1625499989)
@hebasto I'm ~0 on doing either/none of those changes. Happy to make a change if you or @achow101 or @MarcoFalke have a strong preference, otherwise I'll leave it as-is.

✅ hebasto closed a pull request: "Rebased cmake-staging branch (post PR#15)"
(https://github.com/bitcoin/bitcoin/pull/28046)

💬 jonatack commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1255997178)
There's also the RPC, so maybe (or something equivalent)

`// In-memory counter used for P2P tx relay, RPC, and ZMQ mempool tracking purposes.`

💬 pinheadmz commented on pull request "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py":
(https://github.com/bitcoin/bitcoin/pull/27452#discussion_r1256004369)
update update: manually calling `onion_proxy.close()` in the test now with a comment

💬 pinheadmz commented on pull request "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py":
(https://github.com/bitcoin/bitcoin/pull/27452#issuecomment-1625581603)
@jonatack @vasild thanks for reviewing guys. I put back the `keep_alive` option to ensure the onion peer is logged, expecting successful CI on this push 🤞

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1256035908)
Reverting this change fails the test because we are divorcing HTTP status from RPC error, so you can have an HTTP OK with RPC error and some test cases now also ensure both are OK.

```
2023-07-07T15:44:27.147000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/Users/matthewzipkin/Desktop/work/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main
self.run_test()
File "/Users/matthewzipkin/Desktop/work/bitcoin/test/functional
...

💬 achow101 commented on pull request "wallet: address book migration bug fixes":
(https://github.com/bitcoin/bitcoin/pull/28038#issuecomment-1625650369)
> @achow101 do you want these backported for 25.1?

Yes

💬 achow101 commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1256091925)
Hmm, the tests still seemed to work when I had run it..

💬 jonatack commented on pull request "test: move remaining rand code from util/setup_common to util/random":
(https://github.com/bitcoin/bitcoin/pull/27425#discussion_r1256094854)
Good points. I'm a little hesitant to invalidate the reviews (thank you both!) but tentatively updated the name of the new helper and removed the `static` annotations as they are trivial to re-ACK (deferring discussion of the last point for a possible follow-up, if that's ok).

🚀 fanquake merged a pull request: "wallet: address book migration bug fixes"
(https://github.com/bitcoin/bitcoin/pull/28038)

📝 fanquake opened a pull request: "[25.x] Further backports for 25.1"
(https://github.com/bitcoin/bitcoin/pull/28047)
Currently backports:
* #28038

💬 fanquake commented on pull request "wallet: address book migration bug fixes":
(https://github.com/bitcoin/bitcoin/pull/28038#issuecomment-1625667338)
Added to #28047 for backporting.

📝 ryanofsky opened a pull request: "kernel: Remove StartShutdown calls from validation code"
(https://github.com/bitcoin/bitcoin/pull/28048)
This change drops the last kernel dependency on shutdown.cpp. It also adds new hooks for libbitcoinkernel applications to be able to stop kernel operations after blocks are imported and each time the chain tip changes.

💬 hebasto commented on pull request "build: Add CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/25797#issuecomment-1625710276)
Closing this PR as its usefulness seems marginal.

Although, I'll keep maintaining this branch.

Reminding that the active reviewing process still happening in https://github.com/hebasto/bitcoin/tree/cmake-staging and related PRs.

💬 ryanofsky commented on pull request "kernel: Remove shutdown globals from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1256159653)
re: https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1214568318

> I think it would be great to get rid of the 3 StartShutdown calls in kernel code, but probably better to do in a separate PR that was independent of this one, since there's lots of approaches that could be used and none of them seem to depend on the other changes here.

I implemented this idea in #28048, getting rid of the `stop_after_block_import` and `stop_at_height` kernel options and replacing them with more flex
...