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

✅ hebasto closed a pull request: "build: Add CMake-based build system"
(https://github.com/bitcoin/bitcoin/pull/25797)

💬 achow101 commented on pull request "wallet: don't include bdb files from our headers":
(https://github.com/bitcoin/bitcoin/pull/28039#issuecomment-1625712726)
reACK 8b5397c00e821d7eaab22f512e9d71a1a0392ebf

💬 TheCharlatan commented on pull request "blockstorage: Return on fatal flush errors":
(https://github.com/bitcoin/bitcoin/pull/27866#issuecomment-1625724128)
Rebased 1fe06a811bf8be941bef5336af97688f48885828 -> d1c92b57a72b62ffa202f1d3d357b59befdc9c12 ([kernelReturnOnAbort_0](https://github.com/TheCharlatan/bitcoin/tree/kernelReturnOnAbort_0) -> [kernelReturnOnAbort_1](https://github.com/TheCharlatan/bitcoin/tree/kernelReturnOnAbort_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelReturnOnAbort_0..kernelReturnOnAbort_1))

* Fixed conflict with #27861
* Fixed formatting if formatting

💬 luke-jr commented on pull request "build: Add CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/25797#issuecomment-1625724201)
>we should be able to enumerate some more benefits to doing so.

One more for the list:

* Real out-of-tree builds. Autotools requires autogen to be run in the tree, which can be complicated when you don't want the build to have write access to the source dir.

🚀 achow101 merged a pull request: "wallet: don't include bdb files from our headers"
(https://github.com/bitcoin/bitcoin/pull/28039)

💬 santochibtc commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1625747312)
Thanks @sipa, yes, it is "a histogram of the fee rates paid by transactions in the memory pool, weighted by transaction size", but my point is still valid. If you want to include your tx in the next block you must estimate your fee based on the pending txs that are paying more per byte. I think that the histogram is not always a good picture of the mempool to estimate fees.

💬 ryanofsky commented on pull request "kernel: Remove StartShutdown calls from validation code":
(https://github.com/bitcoin/bitcoin/pull/28048#issuecomment-1625782988)
Updated 828af9ad19a702e711963f9b498f7462bde8aabb -> c2bcc339d7b8def58289d3ff01b2a905dc291ed6 ([`pr/stopafter.1`](https://github.com/ryanofsky/bitcoin/commits/pr/stopafter.1) -> [`pr/stopafter.2`](https://github.com/ryanofsky/bitcoin/commits/pr/stopafter.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/stopafter.1..pr/stopafter.2)) just updating comments and adding release note about `-stopatheight` behavior

🤔 ishaanam reviewed a pull request: "wallet: Deniability API (Unilateral Transaction Meta-Privacy)"
(https://github.com/bitcoin/bitcoin/pull/27792#pullrequestreview-1519449241)
Hey @denavila, welcome to Bitcoin Core and thanks for working on this! While I think that this idea could be useful to some extent if implemented correctly, I have a few concerns about the current implementation:
- Currently there is no precaution against a user spending two outputs of a deniabilization transaction together. IMO I think that this is quite problematic and could lead to these deniabilization transactions being rendered ineffective. Not only that, but then it also becomes obvious
...