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

🤔 glozow reviewed a pull request: "test: Move test_chain_listunspent wallet check from mempool_packages to wallet_basic"
(https://github.com/bitcoin/bitcoin/pull/27735#pullrequestreview-1445910452)
ACK ffffe622e9cbf926326135bb23958380dcf09df1, thanks for changing! Nice to remove wallet from another non-wallet test.

💬 TheCharlatan commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564189924)
Re https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564144864

> Can you replace `set -e` with `set -ex` and show the surrounding line?

```
0 upgraded, 0 newly installed, 0 to remove and 1 not upgraded.
+ '[' -n '' ']'
+ [[ '' == \t\r\u\e ]]
+ [[ true == \t\r\u\e ]]
+ git clone --depth=1 https://github.com/include-what-you-use/include-what-you-use -b clang_16 /home/drgrid/bitcoin/ci/scratch/iwyu/include-what-you-use
fatal: destination path '/home/drgrid/bitcoin/ci/scratch/i
...

💬 MarcoFalke commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564193669)
Can you share the output where it should say "Skip base install"? Because it should never be called twice

💬 MarcoFalke commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564196636)
> `+ [[ true == \t\r\u\e ]]`

Unrelated: Anyone know why it prints "true" as "\t\r\u\e"? Maybe we should remove bash, but I am not sure if there is an alternative.

💬 willcl-ark commented on issue "bitcoind hangs waiting for `g_requests.empty()`":
(https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1564196898)
Hmmm, curiously enough I was not able to reproduce using either method on master @ aa6cc5bec9

With @theStack method:

```log
2023-05-26T10:39:07Z [http] Received a POST request for / from 127.0.0.1:50704
^C2023-05-26T10:39:10Z [http] Interrupting HTTP server
2023-05-26T10:39:10Z tor: Thread interrupt
2023-05-26T10:39:10Z torcontrol thread exit
2023-05-26T10:39:10Z addcon thread exit
2023-05-26T10:39:10Z opencon thread exit
2023-05-26T10:39:10Z Shutdown: In progress...
2023-05-26T10
...