💬 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

👍 TheCharlatan approved a pull request: "contrib: simplify `test-security-check`"
(https://github.com/bitcoin/bitcoin/pull/30423#pullrequestreview-2194838525)
ACK 1bc9f64bee919bc46eb061ef8c66f936eb6a8918

💬 hebasto commented on pull request "depends: bump libmultiprocess for CMake fixes":
(https://github.com/bitcoin/bitcoin/pull/30490#issuecomment-2246089575)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/277.

💬 hodlinator commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1688610458)
Agree the diff would have been simpler, in this instance I was attempting to revert back to what already had been discussed and ACK:ed before you joined the review. Thanks for helping improve this PR!

💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1688616750)
Let me try a different angle: if a method is inline, I would expect to be able to copy-paste its content to the call site directly (at least conceptually, to understand its effect).
If the method is pure in a functional sense (including that it doesn't even mutate its parameters), it's a lot easier to reason about its overall behavior.
Does this resonate more?

⚠️ maflcko opened an issue: "implicit-integer-sign-change in ActivateSnapshot"
(https://github.com/bitcoin/bitcoin/issues/30514)
```
$ echo 'dXR4b/8BAPq/tdph' | base64 --decode > ./crash

$ UBSAN_OPTIONS="suppressions=$PWD/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" FUZZ=utxo_snapshot ./src/test/fuzz/fuzz ./crash
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 2946336184
INFO: Loaded 1 modules (625551 inline 8-bit counters): 625551 [0x572ffe28af78, 0x572ffe323b07),
INFO: Loaded 1 PC tables (625551 PCs): 625551 [0x572ffe323b08,0x572ffecaf3f8),
./
...