💬 maflcko commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1820658956)
However, as the file is taken from upstream, I wonder how much it should be modified at all.

💬 l0rinc commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1820669760)
hmmm, I'll check it out, I wasn't aware that's how this works (fuzzing is currently completely broken for me locally), but this sounds like a huge blind-spot for fuzzing - like you said, we should investigate, false confidence in this area is dangerous.

💬 fanquake commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#issuecomment-2444030316)
I wonder if a better approach here would be to further integrate the manpage generation into the build system; given that the two issues of "which binaries exist" and "where do they exist" are already solved in that case. This should also remove the dep on `git`.

👍 stickies-v approved a pull request: "Remove mempoolfullrbf"
(https://github.com/bitcoin/bitcoin/pull/30592#pullrequestreview-2401632616)
re-ACK 806ecd6958e4c0fe2a51bee04e20e0dca515bd40

Documentation changes to address review comments, and further functional test cleanups.

💬 stickies-v commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#discussion_r1820661404)
Ah, I see. We don't mention BIP125 until further down the document, so I don't think (most) readers are expecting that symmetry here, so I still think it'd least awkward to just remove the item entirely (and renumbering the following items).

We're in bikeshed territory, so I'll stop commenting on it further.

💬 stickies-v commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#discussion_r1820658233)
This list enumerates "BIPs that are implemented by Bitcoin Core". Adding one that isn't implemented seems confusing. I'd just remove the line entirely, we already reference BIP125 in https://github.com/bitcoin/bitcoin/blob/da10e0bab4a3e98868dd663af02c43b1dc8b7f4a/doc/policy/mempool-replacements.md?plain=1#L62

If you're worried about discoverability, adding the hyperlink there could be an alternative:

<details>
<summary>git diff on 806ecd6958</summary>

```diff
diff --git a/doc/policy/m
...

💬 marcofleon commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2444038494)
Fuzzed the two targets for about 250 cpu hours each. Coverage looks good.
[txdownloadman](https://marcofleon.github.io/coverage/txdownloadman/)
[txdownloadman_impl](https://marcofleon.github.io/coverage/txdownloadmanimpl/)

Finally, the children hanging out in the recent reject filter are quiet 😌

I'll look to run the one honest peer test as well once it's ready.

📝 hebasto opened a pull request: "[POC] ci: Test cross-built Windows executables on Windows natively"
(https://github.com/bitcoin/bitcoin/pull/31176)
Resolves https://github.com/bitcoin/bitcoin/issues/31071.

Things left to do:
- introduce caching
- build and test GUI
- run functional tests

💬 1440000bytes commented on pull request "build: increase minimum supported Windows to 10.0":
(https://github.com/bitcoin/bitcoin/pull/31172#issuecomment-2444057243)
Concept ACK

https://gs.statcounter.com/os-version-market-share/windows/desktop/worldwide

💬 hebasto commented on pull request "[POC] ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2444060142)
@achow101 @fanquake

This PR needs the `actions/download-artifact@*` and `actions/upload-artifact@*` actions to be allowed in the repository Settings - General - Actions - General.

📝 polespinasa opened a pull request: "rpc, cli: return "verificationprogress" of 1 when up to date"
(https://github.com/bitcoin/bitcoin/pull/31177)
```getblockchaininfo``` never reaches 1.0 as reported in issue https://github.com/bitcoin/bitcoin/issues/31127.
This PR is based on the reviews given on https://github.com/bitcoin/bitcoin/pull/31135.

Some calls to the function ```GuessVerificationProgress``` remain unchanged pending comments on this proposal.

💬 polespinasa commented on pull request "rpc, cli: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31135#issuecomment-2444063183)
Hi, I tried to go with a proposal on this based on the comments given here https://github.com/bitcoin/bitcoin/pull/31177

📝 polespinasa converted_to_draft a pull request: "rpc, cli: return "verificationprogress" of 1 when up to date"
(https://github.com/bitcoin/bitcoin/pull/31177)
```getblockchaininfo``` never reaches 1.0 as reported in issue https://github.com/bitcoin/bitcoin/issues/31127.
This PR is based on the reviews given on https://github.com/bitcoin/bitcoin/pull/31135.

Some calls to the function ```GuessVerificationProgress``` remain unchanged pending comments on this proposal.

👍 tdb3 approved a pull request: "bench: add support for custom data directory"
(https://github.com/bitcoin/bitcoin/pull/31000#pullrequestreview-2401711134)
ACK ebdace5aaf74626da83eeee1f966acaad3ed0c35

Light code review and tested.

```
$ build/src/bench/bench_bitcoin -testdatadir=/mnt/31000/
...
$ ls /mnt/31000/test_common\ bitcoin/
AssembleBlock LoadExternalBlockFile ReadBlockFromDiskTest WalletCreateEncrypted
BlockAssemblerAddPackageTxns LogWithDebug ReadRawBlockFromDiskTest WalletCreatePlain
BlockFilterIndexSync LogWithThreadNames RpcMempool WalletCreateTxUseOnlyPresetInputs
...

💬 tdb3 commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1820702143)
nit: for awareness, may want to mention in OP that this changes directory names from 256-bit randoms to 32-bit randoms. I'm not seeing an issue with it (virtually non-existent likelihood of collision).

💬 hebasto commented on pull request "build: increase minimum supported Windows to 10.0":
(https://github.com/bitcoin/bitcoin/pull/31172#issuecomment-2444103245)
> > > The cross-compiled on Ubuntu 24.10 bitcoind.exe fails to run on Windows 11 Pro 23H2.
> >
> >
> > Can you provide some actionable information? The CI and cross-compiled unit tests have run & passed.
>
> The only difference from CI is that I run `bitcoind.exe` on Windows, not under Wine.

> This seems like even more reason to do #31071.

This branch @ 7dd0ee89a092c6ec4e305fbdb0cf3afa9e41cab6 rebased on top of the https://github.com/bitcoin/bitcoin/pull/31176 fails to run `bitcoin
...

💬 l0rinc commented on pull request "doc: Extend developer-notes with file-name-only debugging fix":
(https://github.com/bitcoin/bitcoin/pull/30670#discussion_r1820749380)
Done, thanks

💬 ryanofsky commented on issue "ci: intermittent issue in rpc_misc.py node0 stderr terminate called after throwing an instance of 'kj::ExceptionImpl' [15:12:14.943] what(): mp/proxy.cpp:242: disconnected: write(m_post_fd, &buffer, 1): Broken pipe [15:12:14.943] stack: 657ed083 657ed86e f7ac7d20 f7713156 f77a7e07 ":
(https://github.com/bitcoin/bitcoin/issues/31151#issuecomment-2444170080)
Assuming this is a new error, it is possible this is caused by #31105 which was merged a week ago and changed some parts of IPC shutdown sequence.

It seems this failure here is happening because some code is trying to use the IPC event loop after it is closed. Specifically some code is calling the `mp::EventLoop::removeClient` which fails writing to the event loop's local pipe at https://github.com/chaincodelabs/libmultiprocess/blob/abe254b9734f2e2b220d1456de195532d6e6ac1e/src/mp/proxy.cpp#L2
...

👍 laanwj approved a pull request: "doc: Extend developer-notes with file-name-only debugging fix"
(https://github.com/bitcoin/bitcoin/pull/30670#pullrequestreview-2401819547)
ACK 1b0b9b4c7873ff0c6323de0c7f439466eed06049

👍 hebasto approved a pull request: "Fix unsigned integer overflows in interpreter"
(https://github.com/bitcoin/bitcoin/pull/24214#pullrequestreview-2401888182)
ACK bbbbaa0d9ac9ae9c9b8109503aa30213eed543b9, I have reviewed the code and it looks OK.

A decent alternative to https://github.com/bitcoin/bitcoin/pull/29543.