✅ maflcko closed a pull request: "rpc: getdescriptorinfo also returns normalized descriptor"
(https://github.com/bitcoin/bitcoin/pull/29396)

💬 maflcko commented on pull request "rpc: getdescriptorinfo also returns normalized descriptor":
(https://github.com/bitcoin/bitcoin/pull/29396#issuecomment-2586850491)
Closing for now. The issue remains open (https://github.com/bitcoin/bitcoin/issues/29320) and discussion can continue there. This can also be picked up again, a fresh pull can be created, or it can be re-opened.

💬 hebasto commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2586865081)
Reworked per recent [feedback](https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2586587671).

`libexec` is an installation path for targets whose output path is set to `libexec`.

💬 maflcko commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2586911979)
Checked locally that the following fails for me as well: `time env -i HOME="$HOME" PATH="$PATH" USER="$USER" MAKEJOBS="-j$( nproc )" FILE_ENV="./ci/test/00_setup_env_native_asan.sh" DEBUGINFOD_URLS='https://debuginfod.fedoraproject.org/' ./ci/test_run_all.sh`

In theory it is not recommended to run the tests without a clean `env`, according to the `ci/README.md`. So I am not sure if this can be left as-is, or a workaround for DEBUGINFOD_URLS is convenient .

💬 TheCharlatan commented on pull request "kernel: Flush in ChainstateManager destructor":
(https://github.com/bitcoin/bitcoin/pull/31382#discussion_r1913097418)
I'll do that next time I have to push.

📝 DevRatnu opened a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/31647)
Pow Similar from 2009-2010 SAme from Satoshi history

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significan
...

✅ fanquake closed a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/31647)

📝 fanquake locked a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/31647)
Pow Similar from 2009-2010 SAme from Satoshi history

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significan
...

🤔 brunoerg reviewed a pull request: "test: avoid internet traffic"
(https://github.com/bitcoin/bitcoin/pull/31646#pullrequestreview-2546528618)
Concept ACK

What about having an option to set this in `BitcoinTestFramework` and avoid the duplicated code?

💬 laanwj commented on pull request "net: Use GetAdaptersAddresses to get local addresses on Windows":
(https://github.com/bitcoin/bitcoin/pull/31014#issuecomment-2587052680)
Can anyone review here please?

Mind that the first commit "Add optional length checking to CService::SetSockAddr" is also part of #31022.

💬 Sjors commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2587121964)
re-ACK b448b014947093cd217dbde47c8fb9e6c2bc8ba3

💬 laanwj commented on pull request "doc: Archive 28.1 release notes":
(https://github.com/bitcoin/bitcoin/pull/31630#issuecomment-2587164948)
ACK, but probably want to fix https://github.com/bitcoin-core/bitcoincore.org/pull/1100#discussion_r1910510272 first.

🤔 stratospher reviewed a pull request: "validation: stricter internal handling of invalid blocks"
(https://github.com/bitcoin/bitcoin/pull/31405#pullrequestreview-2546740188)
ACK b2136da9

- Went through the logs in `rpc_invalidateblock.py` and checked how the children were marked invalid/`m_best_header` was set on master vs in this PR
- Went through logs of some functional tests like `feature_block.py` and verified that BLOCK_FAILED_CHILD wasn't being set in [`m_failed_blocks` loop](https://github.com/bitcoin/bitcoin/blob/35bf426e02210c1bbb04926f4ca2e0285fbfcd11/src/validation.cpp#L4395)
- Ran `invalidateblock` RPC on a full node.

💬 Sjors commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r1913267051)
e6ccd17275e06e6278473587257f80c2d9bd8834: maybe move the legacy wallet migration code to a separate file? And mark it read-only :-)

👍 rkrux approved a pull request: "doc: Archive 28.1 release notes"
(https://github.com/bitcoin/bitcoin/pull/31630#pullrequestreview-2546789777)
crACK 42cbf1fcb057374eec1e5b6f89724e1f192142bd

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2587261731)
`b8b042626e...f71f1a346c`: rebase due to conflicts

💬 TheCharlatan commented on pull request "versionbits refactoring":
(https://github.com/bitcoin/bitcoin/pull/29039#issuecomment-2587302557)
> and probably makes more work for merging BIP8.

Is there a PR open for such logic?

💬 stickies-v commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1913317790)
in 0e2887262e1a3a4a5ab0382f9e8713b135408a77

nit: this should now also be a `size_t`

```suggestion
size_t max_cache{std::min(total_cache / 8, MAX_FILTER_INDEX_CACHE)};
```

💬 stickies-v commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1913290613)
in 0e2887262e1a3a4a5ab0382f9e8713b135408a77

nit: optional header no longer necessary

💬 stickies-v commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1913008528)
in e0a541702def3a4c6cce8e44b6ebe8d608edad4e:

Right-shifting by more bits than the width of Output is UB, so we should probably add an extra check for that. I've also added some test cases, but interestingly they don't seem to trigger UBSan (without the `util/overflow.h` patch) on my machine:

<details>
<summary>git diff on e0a541702d</summary>

```diff
diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
index 09ba278d3a..ae292d7761 100644
--- a/src/test/util_tests.cpp
+++
...

💬 stickies-v commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1913095820)
in ccf8caa4430f57b8f148e96ac2a241221977bcab

nit: I've raised this before, but since we now have a `kernel::CacheSizes` and `node::CacheSizes`, I think `using` a `kernel::CacheSizes` in a `node` file is unnecessarily confusing, so I would prefer making this explicit (as in the `chainstate.h` file):

<details>
<summary>git diff on ccf8caa443</summary>

```diff
diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
index 660d2c3289..d4d8828f73 100644
--- a/src/node/chainstate.cpp
...