📝 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),
./
...

💬 maflcko commented on issue "implicit-integer-sign-change in ActivateSnapshot":
(https://github.com/bitcoin/bitcoin/issues/30514#issuecomment-2246139045)

Generally, I am not sure if the metadata is the right place to add the block height. It is just another field that may be corrupt (intentionally, or accidentally) and may lead to issues down the line, when used in a context that hasn't fully checked it.

My recommendation would be to just remove it from the metadata. The block hash should be sufficient to get the height (for logging and other purposes). And if the block headers are insufficient to get the height, `ActivateSnapshot` will erro
...

💬 maflcko commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#issuecomment-2246141298)
Found https://github.com/bitcoin/bitcoin/issues/30514

🤔 stickies-v reviewed a pull request: "fix: Make TxidFromString() respect string_view length"
(https://github.com/bitcoin/bitcoin/pull/30436#pullrequestreview-2194917738)
post-merge ACK 09ce3501fa2ea2885a857e380eddb74605f7038c

💬 stickies-v commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1688621638)
Is the `ArithToUint256` conversion necessary?

💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2246153002)
Updated 4e67086e4c0babb97be30bd9fe3c3b96e258a502 -> 79cfc0d198faecec598f923e6bf83ccf8dc09bbb ([`pr/mine-types.1`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.1) -> [`pr/mine-types.2`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine-types.1..pr/mine-types.2)) to fix link error in https://github.com/bitcoin/bitcoin/pull/30510/checks?check_run_id=27810447228