💬 maflcko commented on pull request "wip: Split fuzz binary":
(https://github.com/bitcoin/bitcoin/pull/29010#issuecomment-1844917023)
I guess the "one fuzz target per file" makes the "one fuzz binary per message/rpc type" idea a bit harder to implement: https://github.com/bitcoin/bitcoin/pull/28015#issuecomment-1621453405 ?

💬 naumenkogs commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1418616411)
I won't mind, but this should be commented somewhere above — perhaps where 0 returned is handled.

💬 naumenkogs commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1418629765)
6c65f61b625212e128e5db52cbd80a3b57b1450b

nit: your code assumes that N is less than the capacity of `size_t`, otherwise this addition can overflow. You might consider adding a static assert for that too, just in case someone in the future decides to use `TimeOffsets<bazillion>`

💬 naumenkogs commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1418647036)
After your changes, we won't consider `BLOCK_RELAY`, `FEELER`, `ADDR_FETCH` anymore, right?

🤔 MarnixCroes reviewed a pull request: "Fix: Disable both "Mask Values" and "Transaction View" if no wallet is selected"
(https://github.com/bitcoin-core/gui/pull/780#pullrequestreview-1769626322)
nice find

I'm not sure if this is the right approach: user might want to enable `Mask values` before opening his wallet?

💬 Sjors commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1844981799)
This does not meet the bar for "good". If only Ocean pool uses this and it remains a relatively small pool, it will have no effect. If it is widely deployed, it's trivial to circumvent, which will cause this code to grow in complexity. And every time that happens there'll be a lot of pressure on maintainers to push it through quickly "to stop the spam".

For example you're currently looking for this pattern, used for inscriptions:

```
OP_FALSE OP_IF spam OP_ENDIF
```

This pattern fits
...

📝 TheCharlatan opened a pull request: "mempool: Don't sort in entryAll"
(https://github.com/bitcoin/bitcoin/pull/29019)
Current call sites of entryAll do not require the entries to be sorted, so iterate through mapTx directly to retrieve the entries instead of using SortedDepthAndScore.

This is a follow up to #28391 and was noted here https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1418261403.

💬 naumenkogs commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1418654682)
Why don't you implement going out of this warning?

💬 TheCharlatan commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1418656096)
This function is supposed to mimic `infoAll`, which was originally used in this patchset to retrieve mempool information instead of directly accessing `mapTx`. Looking at its usage you are right, the way I read this too there were no order promises made prior to this patch. I opened https://github.com/bitcoin/bitcoin/pull/29019 to instead just iterate through `mapTx`.

💬 naumenkogs commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#discussion_r1418656693)
>stops sampling entirely after 200 samples

perhaps record this bug in the commit message, might be useful in the future?

💬 maflcko commented on pull request "mempool: Don't sort in entryAll":
(https://github.com/bitcoin/bitcoin/pull/29019#issuecomment-1845000268)
lgtm ACK 87fa444a04854a5d3a9ff283a8b6c34587e8430f

Though, if the function will be removed either way, I wonder if this is worth it. Maybe wait for an OK by @sdaftuar to avoid a merge conflict with https://github.com/bitcoin/bitcoin/pull/28676 ?

✅ fanquake closed a pull request: "build: Require C++20 compiler"
(https://github.com/bitcoin/bitcoin/pull/28349)

🚀 fanquake merged a pull request: "build: use macOS 14 SDK (Xcode 15.0)"
(https://github.com/bitcoin/bitcoin/pull/28622)

📝 maflcko reopened a pull request: "build: Require C++20 compiler"
(https://github.com/bitcoin/bitcoin/pull/28349)
C++20 allows to write safer code, because it allows to enforce more stuff at compile time (`constinit`, `conteval`, `constexpr`, `std::span`, ...).

Also, it allows to write less verbose and easier to understand code (C++ 20 Concepts).

See https://github.com/bitcoin/bitcoin/issues/23363 and https://en.cppreference.com/w/cpp/compiler_support#cpp20

With g++-10 (https://github.com/bitcoin/bitcoin/pull/28348) and clang-13 (https://github.com/bitcoin/bitcoin/pull/28210), there is broad suppor
...

👋 maflcko's pull request is ready for review: "build: Require C++20 compiler"
(https://github.com/bitcoin/bitcoin/pull/28349)

💬 maflcko commented on pull request "build: Require C++20 compiler":
(https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1845050631)
Rebased.

Once and if this is merged, I'll follow-up with a `fs.h` cleanup.

💬 fanquake commented on pull request "guix: update time-machine":
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1845087846)
Does Guix work normally?
Does it work with any other build, or is it only the arm64-apple-darwin that is not working?

⚠️ maflcko opened an issue: "./contrib/guix/guix-build does not work on riscv64"
(https://github.com/bitcoin/bitcoin/issues/29020)
Steps to reproduce:

```
# dpkg --print-architecture && HOSTS="arm64-apple-darwin" V=1 VERBOSE=1 MAX_JOBS=$(nproc) ./contrib/guix/guix-build
riscv64
Found macOS SDK at '/bitcoin-core/depends/SDKs/Xcode-12.2-12B45b-extracted-SDK-with-libcxx-headers', using...
Checking that we can connect to the guix-daemon...

Hint: If this hangs, you may want to try turning your guix-daemon off and on
again.

accepted connection from pid 2870003, user root
make: Entering directory '/bitcoin-co
...

💬 maflcko commented on issue "./contrib/guix/guix-build does not work on riscv64":
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845096426)
guix itself seems to be working:

```
# type rustc && rustc --version
rustc is hashed (/var/guix/profiles/per-user/root/guix-profile/bin/rustc)
rustc 1.60.0

💬 maflcko commented on pull request "guix: update time-machine":
(https://github.com/bitcoin/bitcoin/pull/28580#issuecomment-1845097342)
Yeah, it works. Let's move the discussion to https://github.com/bitcoin/bitcoin/issues/29020