💬 maflcko commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1820543443)
Nit in 1d16d6e6bac994fed7c695f530b9984edcd290bd: I don't really see why this is useful. Maybe I am missing something, but everything compiles by just moving the two members to the derived struct:


```diff
diff --git a/src/tinyformat.h b/src/tinyformat.h
index 56c25d8f7f..6227cdeca0 100644
--- a/src/tinyformat.h
+++ b/src/tinyformat.h
@@ -179,21 +179,16 @@ namespace tfm = tinyformat;

namespace tinyformat {

-// Added for Bitcoin Core. Wrapper type for format strings to allow comp
...

💬 laanwj commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1820550576)
By adding this if here, this no longer accounts for the situation where port mapping is turned off. i think i slightly prefer the old API.

💬 laanwj commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1820574292)
Was this verbose code that is replaced here a workaround for lack of `try_emplace`?

📝 TheCharlatan opened a pull request: "rpc: Remove submitblock pre-checks"
(https://github.com/bitcoin/bitcoin/pull/31175)
With the introduction of a mining ipc interface and the potential future introduction of a kernel library API it becomes increasingly important to offer common behaviour between them. An example of this is ProcessNewBlock, which is used by ipc, rpc, net_processing and (potentially) the kernel library. Having divergent behaviour on suggested pre-checks and checks for these functions is confusing to both developers and users and is a maintenance burden.

The rpc interface for ProcessNewBlock (su
...

✅ hodlinator closed a pull request: "Windows bitcoind stall debugging [NOMERGE, DRAFT]"
(https://github.com/bitcoin/bitcoin/pull/30956)

💬 hodlinator commented on pull request "Windows bitcoind stall debugging [NOMERGE, DRAFT]":
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2443920476)
Alright, our time is finite and hopefully `CryptGenRandom` should be enough for now. Closing.

👍 rkrux approved a pull request: "test: Don't enforce BIP94 on regtest unless specified by arg"
(https://github.com/bitcoin/bitcoin/pull/31156#pullrequestreview-2401473616)
tACK e60cecc8115d3b28be076792baa5e4ea26d353a6

Successful make and functional tests. Agree with the approach to enable BIP94 on regtest through the test argument.

💬 rkrux commented on pull request "test: Don't enforce BIP94 on regtest unless specified by arg":
(https://github.com/bitcoin/bitcoin/pull/31156#discussion_r1820557540)
Worth creating a directory `doc/release-notes-prs` and putting it there?
I see there's a `release-notes` directory that contains notes for the releases.

💬 naumenkogs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1820609170)
Yeah i agree, was confused initially. Please resolve.

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