📝 maflcko opened a pull request: "net: Log accepted connection after m_nodes.push_back; Fix intermittent test issue"
(https://github.com/bitcoin/bitcoin/pull/30512)
Fix the two issues reported in https://github.com/bitcoin/bitcoin/pull/30468/files#r1688444784:

* Delay a debug log line for consistency.
* Fix an intermittent test issue.

They are completely separate fixes, but both `net` related.

💬 maflcko commented on pull request "test: bump mocktime only after node has received and sent bytes":
(https://github.com/bitcoin/bitcoin/pull/30468#discussion_r1688462553)
Fixed in https://github.com/bitcoin/bitcoin/pull/30512

💬 maflcko commented on pull request "net: Log accepted connection after m_nodes.push_back; Fix intermittent test issue":
(https://github.com/bitcoin/bitcoin/pull/30512#issuecomment-2245863400)
Can be tested by adding a sleep before the LOCK:

```diff
diff --git a/src/net.cpp b/src/net.cpp
index ad43b316b3..a60aa5f31c 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1813,2 +1813,3 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
{
+ UninterruptibleSleep(100ms);
LOCK(m_nodes_mutex);

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1688468495)
It's similar, but not actually the same, because it isn't testing the exact result of `LinearizationChunking::Intersect`, but testing the *properties* it has we care about for LIMO/Merging to work (feerate of output is as high as feerate of input, and no intersection with a chunk prefix is better. Specifically, if you replace `LinearizationChunking::Intersect` with something that returns the highest-feerate intersection (rather than the first intersection) which has as high feerate as the input,
...

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1688471698)
It should work, but wouldn't be testing the exact same thing (because the remaining part of a `LinearizationChunking` isn't required to be topological, and in practice it'll generally be the opposite: the full cluster with a topological subset removed).

🚀 achow101 merged a pull request: "rpc: doc: use "output script" terminology consistently in "asm"/"hex" results"
(https://github.com/bitcoin/bitcoin/pull/30408)

💬 morehouse commented on pull request "fuzz: Version handshake":
(https://github.com/bitcoin/bitcoin/pull/30413#discussion_r1688272087)
If time isn't attacker controlled, how would it go backwards? And how realistic is the range [-10m, +24h]? Naively, 24h seems like a long time between messages in the handshake.

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1688479676)
Will do if I retouch.

🤔 mzumsande reviewed a pull request: "index: Fix coinstats overflow and introduce index versioning"
(https://github.com/bitcoin/bitcoin/pull/30469#pullrequestreview-2194705197)
Concept ACK

Haven't tested anything yet, but did you check what happens in case of a downgrade? (an index generated with this PR is loaded with master)

💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2245938009)
> Is the idea to incorporate the interface changes proposed in #30440 and #30409 after they're merged? And in the mean time keep them in #30437?

I could add them here. Just let me know what you prefer. I am planning to rebase #30437 on top of this, too.

> Maybe also link to #30437 in the description as it's a more simple demo than [Sjors#48](https://github.com/Sjors/bitcoin/pull/48).

Thanks, added

🚀 ryanofsky merged a pull request: "fix: Make TxidFromString() respect string_view length"
(https://github.com/bitcoin/bitcoin/pull/30436)

📝 hebasto opened a pull request: "depends: Bump `libmultiprocess` for CMake fixes"
(https://github.com/bitcoin/bitcoin/pull/30513)
This PR amends https://github.com/bitcoin/bitcoin/pull/30490 and bumps the upstream branch, which now includes a required CMake [fix](https://github.com/chaincodelabs/libmultiprocess/pull/103).

Addresses https://github.com/bitcoin/bitcoin/pull/30490#issuecomment-2241153244.

The CI logs are available in https://github.com/bitcoin/bitcoin/pull/29790 where the recent [push](https://github.com/hebasto/bitcoin/tree/pr29790-0723.2.mp) uses this PR implementation.

💬 hebasto commented on pull request "depends: Bump `libmultiprocess` for CMake fixes":
(https://github.com/bitcoin/bitcoin/pull/30513#issuecomment-2245989301)
cc @ryanofsky @theuni

💬 fanquake commented on pull request "depends: Bump `libmultiprocess` for CMake fixes":
(https://github.com/bitcoin/bitcoin/pull/30513#issuecomment-2245998944)
Any reason to not include the most recent merge?

💬 maflcko commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1688273553)
style nit in f11f816800ac520064a1e96871d0b4cc9601ced7: Seems confusing to use the left size to calculate the right end. Seems easier to just use `std::end(right)`, if it compiles?

💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1688553678)
I originally did that for the +32 case, but wasn't working for the +20

💬 ryanofsky commented on pull request "depends: Bump `libmultiprocess` for CMake fixes":
(https://github.com/bitcoin/bitcoin/pull/30513#issuecomment-2246038452)
> Any reason to not include the most recent merge?

It would be nice to include https://github.com/chaincodelabs/libmultiprocess/pull/104 not just https://github.com/chaincodelabs/libmultiprocess/pull/103, though only 103 should be necessary to fix problems with CMake.

104 is just nice because it lets earlier commits of #30510 compile, since they include empty structs.

💬 fanquake commented on pull request "depends: Bump `libmultiprocess` for CMake fixes":
(https://github.com/bitcoin/bitcoin/pull/30513#issuecomment-2246046844)
Seems good to pull it in then, and unblock other work, otherwise we are just going to have to bump again.

👍 ryanofsky approved a pull request: "depends: Bump `libmultiprocess` for CMake fixes"
(https://github.com/bitcoin/bitcoin/pull/30513#pullrequestreview-2194825297)
Code review ACK a9bfa3abbe5c168230b20b756034c3801244d717

(checked version hash locally and ran `make MULTIPROCESS=1 native_libmultiprocess_extracted` to check the package hash)

💬 hebasto commented on pull request "depends: Bump `libmultiprocess` for CMake fixes":
(https://github.com/bitcoin/bitcoin/pull/30513#issuecomment-2246063183)
> It would be nice to include [chaincodelabs/libmultiprocess#104](https://github.com/chaincodelabs/libmultiprocess/pull/104) not just [chaincodelabs/libmultiprocess#103](https://github.com/chaincodelabs/libmultiprocess/pull/103), though only 103 should be necessary to fix problems with CMake.
>
> 104 is just nice because it lets earlier commits of #30510 compile, since they include empty structs.

https://github.com/chaincodelabs/libmultiprocess/pull/104 has been included.

👍 ryanofsky approved a pull request: "depends: Bump `libmultiprocess` for CMake fixes"
(https://github.com/bitcoin/bitcoin/pull/30513#pullrequestreview-2194835643)
Code review ACK ec0e805d11d6a73c542032fc49a58a1d05b62d24