💬 fjahr commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-2364675182)
CI failures (timeouts) seem unrelated, should be ready for review as I am hoping https://github.com/bitcoin/bitcoin/pull/30901 will get merged quickly.

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1769352568)
Fixed.

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1769352792)
Indeed, and `DepGraph` itself is responsible for allocating available positions.

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#issuecomment-2364749507)
@instagibbs I deleted by build directory, redid the `cmake` generation, recompiled, and the slowdown was gone. Weird.

💬 kegdeg commented on issue "rpc auth fails 'Error parsing command line arguments: Invalid parameter -rpcpasssword=password":
(https://github.com/bitcoin/bitcoin/issues/30939#issuecomment-2364772586)
This is the bitcoin.conf

I removed the triple S, now getting the following error from the following command:

bitcoin-cli -rpcuser=user -rpcpassword -stdinrpcpass start

RPC password>
error: Authorization failed: Incorrect rpcuser or rpcpassword

(i type password)

💬 ryanofsky commented on issue "cmake: multiprocess guix build broken":
(https://github.com/bitcoin/bitcoin/issues/30931#issuecomment-2364790354)
From the log it looks like the failing line is [`CMakeLists.txt:177`](https://github.com/bitcoin/bitcoin/blob/e821f0a37a026fa0480c7f6f6c938da7c77e0d52/CMakeLists.txt#L177):

```cmake
find_package(Libmultiprocess COMPONENTS Lib)
```

which fails in [`LibmultiprocessConfig.cmake:46`](https://github.com/chaincodelabs/libmultiprocess/blob/6aca5f389bacf2942394b8738bbe15d6c9edfb9b/cmake/Config.cmake.in#L29):

```cmake
include("${CMAKE_CURRENT_LIST_DIR}/${_comp}Targets.cmake")
```

wit
...

💬 hebasto commented on issue "cmake: multiprocess guix build broken":
(https://github.com/bitcoin/bitcoin/issues/30931#issuecomment-2365135973)
I found three flaws in the build system. Fixing any one of them resolves the issue. Currently working on a PR.

📝 hebasto opened a pull request: "depends: Fix build with `MULTIPROCESS=1` in Guix environment"
(https://github.com/bitcoin/bitcoin/pull/30940)
In the Guix environment, `${BASEPREFIX}/${HOST}/native/bin` is added to the `PATH` environment variable:https://github.com/bitcoin/bitcoin/blob/33adc7521cc8bb24b941d959022b084002ba7c60/contrib/guix/libexec/build.sh#L233-L234

This causes CMake to search for package configurations in the `native` subdirectory first.

Explicitly specifying the top-priority search prefixes for the `Libmultiprocess` and `LibmultiprocessNative` packages resolves https://github.com/bitcoin/bitcoin/issues/30931.

...

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2365160404)
Did a rebase for the changes since last master, including CMake and comparing Span<> using `std::equal_range`. No other changes.

> https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1658764974 avoid unreachable nets (not given to -onlynet=)
> https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1658949236 could announce an addr:port where we do not listen (no -bind)
> https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1684368824 could announce the wrong port because it us
...

💬 brunoerg commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1769550928)
```suggestion
// This is because our peer may have added the conflicting transaction
```

💬 pinheadmz commented on issue "rpc auth fails 'Error parsing command line arguments: Invalid parameter -rpcpasssword=password":
(https://github.com/bitcoin/bitcoin/issues/30939#issuecomment-2365175283)
try removing `-rpcpassword` ?

💬 pinheadmz commented on issue "rpc auth fails 'Error parsing command line arguments: Invalid parameter -rpcpasssword=password":
(https://github.com/bitcoin/bitcoin/issues/30939#issuecomment-2365175753)
also what is `start` for?

💬 hebasto commented on pull request "depends: Fix build with `MULTIPROCESS=1` in Guix environment":
(https://github.com/bitcoin/bitcoin/pull/30940#issuecomment-2365201817)
My Guix build:
```
aarch64
753aee81b01ff7a560a6c2c9ad64e2f3d3a535d0f5484364ea6b0f7ac7d8d595 guix-build-953aa067c565/output/aarch64-linux-gnu/SHA256SUMS.part
2eec13b0a0a28d16b9ec2b09908d6785666b49cd2076fd7b677259be0f30d624 guix-build-953aa067c565/output/aarch64-linux-gnu/bitcoin-953aa067c565-aarch64-linux-gnu-debug.tar.gz
732591be3acdc241d8d7a649bad755561dafe97839e00105682d962844acbe54 guix-build-953aa067c565/output/aarch64-linux-gnu/bitcoin-953aa067c565-aarch64-linux-gnu.tar.gz
279a5e97
...

📝 tdb3 opened a pull request: "test: simplify timewarp boundary test"
(https://github.com/bitcoin/bitcoin/pull/30941)
Follow up from PR #30698 (comments https://github.com/bitcoin/bitcoin/pull/30698#discussion_r1729632270).

`mining_basic` checks that a block with wall time is rejected when the previous block is `MAX_FUTURE_BLOCK_TIME` in the future, then checks that a block just beyond the `MAX_TIMEWARP` is also rejected.

This PR removes the first check, since they are essentially checking the same thing twice.
Also incorporates the suggestion in https://github.com/bitcoin/bitcoin/pull/30698#discussion_r
...

💬 Sjors commented on pull request "depends: Fix build with `MULTIPROCESS=1` in Guix environment":
(https://github.com/bitcoin/bitcoin/pull/30940#issuecomment-2365220991)
@hebasto can you rebase your `240921-guix-mp.DEMO` branch so it builds directly on this PR? That makes it easier to compare guix builds.

I'll make a regular guix build for this PR first, without `MULTIPROCESS=1`.

💬 l0rinc commented on pull request "doc: Add `nproc` support for Mac through `coreutils`":
(https://github.com/bitcoin/bitcoin/pull/30936#issuecomment-2365231090)
Updated, as I think this can be useful.
Once the docs transition to Ninja, we can of course remove these manual steps.

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1769603571)
This was moved to the `AddFlags` already - even though it seems to me the code already makes all of that clear:

> Adding a flag also requires a self reference to the pair that contains this entry in the CCoinsCache map

`AddFlags(uint8_t flags, CoinsCachePair& self`

> and a reference to the sentinel of the flagged pair linked list

, `CoinsCachePair& sentinel`

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1769603574)
This was meant to be a temporary transition, since we're removing the rest of the flags in your PR - but I'm fine with Assume as well - changed.

📝 fjahr opened a pull request: "test: Remove dead code from interface_zmq test"
(https://github.com/bitcoin/bitcoin/pull/30942)
The loop removed here appears to be effectively dead code: In case `get_raw_seq` is behind `zmq_mem_seq` the loop runs and tries to get a more recent (higher) number for `get_raw_seq`. However, the exact number of `get_raw_seq` is asserted in the line above: `assert_equal(get_raw_seq, 6)`. If the loop would actually achieve its purpose this assert would need to be racy. This does not seem to be the case and 6 appears to be the final number. `zmq_mem_seq` however does take some time to catch up (
...

💬 hebasto commented on pull request "depends: Fix build with `MULTIPROCESS=1` in Guix environment":
(https://github.com/bitcoin/bitcoin/pull/30940#issuecomment-2365310194)
@Sjors
> can you rebase your `240921-guix-mp.DEMO` branch so it builds directly on this PR? That makes it easier to compare guix builds.

Done. The PR description has been updated accordingly.