💬 MarcoFalke commented on pull request "doc: Add doc/release-notes/release-notes-25.0.md":
(https://github.com/bitcoin/bitcoin/pull/27751#discussion_r1206361499)
```suggestion
- Transactions of non-witness size 65 bytes and above are now allowed by mempool
```

💬 MarcoFalke commented on pull request "doc: Add doc/release-notes/release-notes-25.0.md":
(https://github.com/bitcoin/bitcoin/pull/27751#discussion_r1206363521)
why? There is no list of all pulls in the notes, so the number is without context?

💬 MarcoFalke commented on pull request "doc: Add doc/release-notes/release-notes-25.0.md":
(https://github.com/bitcoin/bitcoin/pull/27751#issuecomment-1563956991)
Fixed the broken link. Not sure about the other nits.

💬 stratospher commented on pull request "p2p: Log addresses of stalling peers":
(https://github.com/bitcoin/bitcoin/pull/27761#issuecomment-1563972944)
tACK fb02a3c.

nice to have this! cross checked locations where we'd like inbound peer/disconnection peeraddr logging.
(you may have to rebase on master for green CI.)

👍 TheCharlatan approved a pull request: "init: Error if ignored bitcoin.conf file is found"
(https://github.com/bitcoin/bitcoin/pull/27302#pullrequestreview-1445624581)
ACK eefe56967b4eb4b5144325cde4f40fc1cbde3e65

💬 1ma commented on pull request "Allow accepting non-standard transactions on mainnet":
(https://github.com/bitcoin/bitcoin/pull/27578#issuecomment-1564059817)
There are no risks in submitting non-standard transactions to your own node's mempool. Its direct peers will simply discard these when your node broadcasts them, and they will eventually be purged from your mempool as any other non-confirmed transaction.

If anything, node runners want to be able to apply more filters/standardness rules (increased sovereignty), not disable them (decreased sovereignty).

👍 TheCharlatan approved a pull request: "test: Throw error when -signetchallenge is non-hex"
(https://github.com/bitcoin/bitcoin/pull/27765#pullrequestreview-1445755490)
Nice, ACK fa6b11a55663e70369bfbbba5fccc55b33f2b310

💬 kevkevinpal commented on pull request "test: Throw error when -signetchallenge is non-hex":
(https://github.com/bitcoin/bitcoin/pull/27765#issuecomment-1564100105)
ACK [fa6b11a](https://github.com/bitcoin/bitcoin/pull/27765/commits/fa6b11a55663e70369bfbbba5fccc55b33f2b310)

💬 TheCharlatan commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564122573)
Still not sure what is going on here:

```
MAKEJOBS="-j12" FILE_ENV="./ci/test/00_setup_env_native_tidy.sh" ./ci/test_run_all.sh
> ...
> fatal: destination path '/home/drgrid/bitcoin/ci/scratch/iwyu/include-what-you-use' already exists and is not an empty directory.
ls home/drgrid/bitcoin/ci/scratch/iwyu/include-what-you-use
> ls: cannot access 'home/drgrid/bitcoin/ci/scratch/iwyu/include-what-you-use': No such file or directory
```

Command runs fine on master, so likely something tha
...

💬 ismaelsadeeq commented on issue "25.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/27736#issuecomment-1564126871)
Thank you @evansmj and @D33r-Gee for all your feedback the guide has been updated.

💬 willcl-ark commented on issue "rpc_getblockfrompeer.py intermittent failure: assert_equal(pruneheight, 248); not(249 == 248)":
(https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1564135165)
FWIW I have seen this error once before running tests locally, but I was making changes to `reconsiderblock` at the time and assumed it was related to that.

👍 hebasto approved a pull request: "refactor: Replace `std::optional<bilingual_str>` with `util::Result`"
(https://github.com/bitcoin/bitcoin/pull/25977#pullrequestreview-1445878314)
ACK 8aa8f73adce72e1ae855b43413c1f65504423cb7, I have reviewed the code and it looks OK.

💬 hebasto commented on pull request "refactor: Replace `std::optional<bilingual_str>` with `util::Result`":
(https://github.com/bitcoin/bitcoin/pull/25977#discussion_r1206529350)
5f49cb1bc8e6ba0671c21bf6292d2d3de43fd001

nit: Add `#include <utility>`?

📝 hebasto converted_to_draft a pull request: "Avoid lock order inversion in `Chainstate::ConnectTip` function"
(https://github.com/bitcoin/bitcoin/pull/27684)
Due to the synchronous call of `CValidationInterface::BlockChecked` a lock order inversion [happens](https://github.com/bitcoin/bitcoin/issues/19303#issuecomment-1514928912):
```
PeerManagerImpl::m_peer_mutex
|
V
Peer::TxRelay::m_tx_inventory_mutex
|
V
CTxMemPool::cs
|
V
PeerManagerImpl::m_peer_mutex
```

This PR breaks the last link.

The other possible solution is to move `CValidationInterface::BlockChecked` to a background thread (see https://github.com/bitcoin/bit
...

💬 MarcoFalke commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564144864)
Can you replace `set -e` with `set -ex` and show the surrounding line?

💬 MarcoFalke commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564146365)
I can't see how this can happen. The error doesn't happen in base_install, from what I can tell, so it seems unrelated to the changes here. However, it passing on master makes me puzzle.

👍 fanquake approved a pull request: "doc: Add doc/release-notes/release-notes-25.0.md"
(https://github.com/bitcoin/bitcoin/pull/27751#pullrequestreview-1445893134)
ACK 034cb5ad4d4b72cf1ba5b153a558fcf6a8afa9aa

🚀 fanquake merged a pull request: "doc: Add doc/release-notes/release-notes-25.0.md"
(https://github.com/bitcoin/bitcoin/pull/27751)

👍 stickies-v approved a pull request: "[24.x] rpc: Fix invalid bech32 handling"
(https://github.com/bitcoin/bitcoin/pull/27755#pullrequestreview-1445895299)
ACK c2e9214effe9abecae6f81cb10158f9661065da3

I verified that this backports eeee55f9288740747b6e8d806ce8177fd92635cf , but additionally includes some necessary changes from 962a0930e699b74b3c4d019427df6e2b3af5c831 to make compilation and tests work.

I don't see any other, unrelated changes.

📝 MarcoFalke opened a pull request: "fuzz: Change LIMIT_TO_MESSAGE_TYPE from a compile-time to a run-time setting"
(https://github.com/bitcoin/bitcoin/pull/27766)
The `process_message_${msg_type}` fuzz targets have many issues:

* In a context where each fuzz target must be a separate binary, this bloats the storage requirements by the number of message types.
* The qa-assets repo for fuzz inputs also bloats, because each input in the type specific folder (`./process_message_${msg_type}`) is accompanied by a similar input in the general folder (`./`process_message`). The size seems to be ~3GB for the sum of all folders vs 0.3GB for the general folder.
...