💬 0xB10C commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2328268499)
> Could also look into adding more transaction details (e.g. orphan expiration time, and potentially other tx information similar to what's provided in getrawmempool or getrawtransaction).

> I think it would be helpful for visualizing / trying out different orphan protection methods.

Concept ACK

I agree that this would be interesting. The the orphan pool is mostly a black box to me (and probably others too). To discuss improvements/changes, having a better visual and mental model would
...

💬 fanquake commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1743326932)
Seems like a better reason to not do this is that it doesn't actually work properly with CTest? Did this work when you tested it previously? i.e:
```bash
ctest --test-dir build -R mempool_tests --rerun-failed --output-on-failure
Internal ctest changing into directory: /bitcoin/build
Test project /bitcoin/build
Start 53: mempool_tests
1/1 Test #53: mempool_tests ....................***Failed 0.01 sec
/bitcoin/build/src/test/test_bitcoin.exe: 1: MZ????@???: not found
/bitcoin/build/
...

💬 fanquake commented on pull request "build: Minor build system fixes and amendments":
(https://github.com/bitcoin/bitcoin/pull/30803#issuecomment-2328293922)
Let's also wait for https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1743326932. To clarify if this ever worked (and was later broken?), or was just never expected to work. In either case, it could be worth documenting.

💬 fanquake commented on pull request "fuzz: Rename fuzz_seed_corpus to fuzz_corpora":
(https://github.com/bitcoin/bitcoin/pull/30804#issuecomment-2328307469)
cc @murchandamus @dergoegge

💬 maflcko commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1743353913)
I don't recall when wine was ever working. There was always a a number of intermittent or consistent errors, regardless of cmake, see https://github.com/bitcoin/bitcoin/pull/29881#discussion_r1632851441

Fixing that seems unrelated to the cmake transition.

I think simply removing the accidentally added line is fine and other follow-ups can be done in a follow-up, if they are needed.

💬 Sjors commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#issuecomment-2328325006)
Rebased after #26619.

💬 remyers commented on issue "build: Boost 1.74.0 incompatible with Clang 18":
(https://github.com/bitcoin/bitcoin/issues/30751#issuecomment-2328327755)
Fwiw, I also ran across this problem (Ubuntu 20.04) while trying to run `clang-tidy-diff` on my code. A workaround is to just install and use clang-17.
```
cmake -B build -DCMAKE_C_COMPILER=clang-17 -DCMAKE_CXX_COMPILER=clang++-17 -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
cmake --build build -j $(nproc)
git diff HEAD~2 | ( cd ./src/ && clang-tidy-diff -p2 -path ../build -j $(nproc) )
```

💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block":
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2328330492)
Rebased after trivial conflict with #29553.

👍 fanquake approved a pull request: "doc: Clarify libbitcoin_consensus in design/libraries.md"
(https://github.com/bitcoin/bitcoin/pull/30802#pullrequestreview-2279470892)
ACK fa78ed83be1f6831416a6f6632e2161f12d359e4

🚀 fanquake merged a pull request: "doc: Clarify libbitcoin_consensus in design/libraries.md"
(https://github.com/bitcoin/bitcoin/pull/30802)

📝 maflcko opened a pull request: "lint: Check for release note snippets in the wrong folder"
(https://github.com/bitcoin/bitcoin/pull/30812)
It is a common mistake to place the snippets in the wrong folder, where they could be missed. For example https://github.com/bitcoin/bitcoin/pull/30719#pullrequestreview-2262535007 or commit 84900ac34f6888b7a851d0a6a5885192155f865c.


Fix all issues by adding a simple lint check.

Can be tested by reverting a prior commit that violated the rule and then running the new check:

```
git revert 35ef34eab7b36e3c53ed438d74a9b783cbcaec27
( cd ./test/lint/test_runner/ && RUST_BACKTRACE=1 cargo
...

💬 maflcko commented on issue "build: Boost 1.74.0 incompatible with Clang 18":
(https://github.com/bitcoin/bitcoin/issues/30751#issuecomment-2328378759)
> Fwiw, I also ran across this problem (Ubuntu 20.04)

I guess you also installed a different boost version, because the 1.71 boost version in Ubuntu Focal 20.04 is not supported starting with https://github.com/bitcoin/bitcoin/pull/29066 and shouldn't even pass the `cmake -B` command.

💬 maflcko commented on pull request "Remove unsafe uint256S() and test-only uint160S()":
(https://github.com/bitcoin/bitcoin/pull/30773#discussion_r1743476276)
For reference, the len reporting (and error handling) was added after https://github.com/bitcoin/bitcoin/issues/9040#issuecomment-257161620.

Also, you added len reporting in a similar place recently in https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1717324499, so I fail to see why there it is "helpful" while here it would be not.

I am happy to review and ACK either version, but I think the length can be useful and may be better to not remove from the error string.

💬 maflcko commented on pull request "refactor: Replace ParseHex with consteval ""_hex literals":
(https://github.com/bitcoin/bitcoin/pull/30377#issuecomment-2328519108)
macOS follow-up idea: Someone could check on macOS 13, if compilation with XCode still works. If not, `build-osx.md` could be updated (Ref: https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716956530)

💬 fanquake commented on pull request "build: Minor build system fixes and amendments":
(https://github.com/bitcoin/bitcoin/pull/30803#discussion_r1743507523)
It's disappointing that so many distros don't support CMake in their packaging. We basically can't switch this (and any other packages where we might want to use the "native" CMake config) until *every* distro does, otherwise it'll just be broken for that distro.

Some do, i.e [Alpine](https://pkgs.alpinelinux.org/contents?branch=edge&name=zeromq%2ddev&arch=x86&repo=main), or [Arch](https://archlinux.org/packages/extra/x86_64/zeromq/), or [nix](https://github.com/NixOS/nixpkgs/blob/release-23.
...

💬 GBKS commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-2328544171)
Was just thinking that an alternative approach could be to leave the transactions list as is, but show a message in the send screen that explains the current status and trust assumptions. This is much more explicit than slightly tweaked icons. Just wanted to add it as an idea for an alternative approach, feel free to ignore for the purpose of this PR.

💬 stickies-v commented on pull request "Remove unsafe uint256S() and test-only uint160S()":
(https://github.com/bitcoin/bitcoin/pull/30773#discussion_r1743536308)
> Also, you added len reporting in a similar place recently in https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1717324499,

I don't quite think this is similar. My suggestion in https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1717324499 was to use less code to have more accurate and helpful error reporting, triggered only when that particular requirement is breached. In this PR, we're talking about adding more code to provide context that may not be relevant (and thus make
...

💬 stickies-v commented on pull request "Remove unsafe uint256S() and test-only uint160S()":
(https://github.com/bitcoin/bitcoin/pull/30773#issuecomment-2328592993)
Force-pushed to remove behaviour change in commit "rpc: use uint256::FromHex for ParseHashV", addressing @maflcko's [concerns](https://github.com/bitcoin/bitcoin/pull/30773#discussion_r1738970588) over behaviour change. I don't think this is necessarily an improvement, but I appreciate that's my subjective view and the (slight) behaviour change was orthogonal the goal of the PR anyway so it seems like the most pragmatic approach to make progress. Thanks for your review!

👍 marcofleon approved a pull request: "fuzz: Rename fuzz_seed_corpus to fuzz_corpora"
(https://github.com/bitcoin/bitcoin/pull/30804#pullrequestreview-2279780062)
ACK 8888beea8d477b1d4a2dfd2a0bb5f686de62f3ff

💬 remyers commented on issue "build: Boost 1.74.0 incompatible with Clang 18":
(https://github.com/bitcoin/bitcoin/issues/30751#issuecomment-2328622998)
> > Fwiw, I also ran across this problem (Ubuntu 20.04)
>
> I guess you also installed a different boost version, because the 1.71 boost version in Ubuntu Focal 20.04 is not supported starting with #29066 and shouldn't even pass the `cmake -B` command.

Yup, I have boost 1.74.0.3 installed.
```sh
dpkg -s libboost-dev | grep 'Version'
Version: 1.74.0.3ubuntu7
```