💬 Sjors commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2079004334)
(I looked for `multi-op-return` rather than `datacarrier` because it's a more unique string)

💬 Sjors commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2079028306)
TIL we have a `-daemon` option. Indeed we'll have to pick a different term here.

💬 Sjors commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2861994398)
> We never change block files, so that is not a problem. I'm also not sure how real this problem actually is. A bunch of databases just maintain one big file and have good performance doing so.

But we prune blocks, and they may not all be at the start of the big file.

💬 davidgumberg commented on pull request "crypto: disable ASan for sha256_sse4 with Clang":
(https://github.com/bitcoin/bitcoin/pull/32437#issuecomment-2862049101)
Tested ACK https://github.com/bitcoin/bitcoin/pull/32437/commits/4e8ab5e00fa72016a7ec0e0505ca025d4e59e4d8

Fixes #31913, I built `fuzzamoto` against this branch with fcf-protection enabled https://github.com/fanquake/fuzzamoto/commit/036fdedd36399c0c04294e7a63d356fb92e3d50f

💬 TheCharlatan commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2079086002)
I think it is fine as is to be honest. `bitcoind -daemon` already means the same thing (albeit abbreviated).

💬 Sjors commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2079093221)
I think `-daemon` detaches, so you can't ctrl + c it.

✅ fanquake closed an issue: "build: x86 afl++ ASan build broken "error: inline assembly requires more registers than available""
(https://github.com/bitcoin/bitcoin/issues/31913)

🚀 fanquake merged a pull request: "crypto: disable ASan for sha256_sse4 with Clang"
(https://github.com/bitcoin/bitcoin/pull/32437)

📝 fanquake opened a pull request: "doc: swap "Docker image" for "container image""
(https://github.com/bitcoin/bitcoin/pull/32444)
I haven't used Docker for some time (now Podman), and the images are generic, so just use "container image". I'll be pushing some changes to https://github.com/fanquake/core-review/tree/master/guix, to reflect this.

🤔 hodlinator reviewed a pull request: "[IBD] multi-byte block obfuscation"
(https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-2807054428)
Code review bba64732ffb6f9463bb1eced7953493935950cec.

Thanks for improving the dbwrapper test case and minimizing `_hex_v` usage in the end state of the PR!

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2068663871)
What is the point of changing this?

1. Renaming `OBFUSCATE_KEY_KEY` -> `OBFUSCATION_KEY` makes the first word nicer, but it is arguably the database key (1) for looking up the key (2) used to obfuscate (0) the data.
2. Making it `inline` might be shifting some work from the linker to the compiler (better for parallelism?). Having it all in the header is slightly nicer for humans, but seems like it would duplicate the constant where it is used. Could it just be declared as a file-local (`stat
...

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2079132170)
nit in 366bffd1252a768e1161c7b632ef8c4816bb504e:
Could drop rename here from `Xor` -> `XorObfuscationBench` since next commit renames it again to `ObfuscationBench`.

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2079140446)
nit in 9712481ae727bb0d8ad392c5fc2980aacbd9091b: prefer verb for function, as you did for `DataStream::Obfuscate()`:
```diff
- inline void Obfuscation(std::span<std::byte> write, std::span<const std::byte> key, size_t key_offset = 0)
+ inline void Obfuscate(std::span<std::byte> write, std::span<const std::byte> key, size_t key_offset = 0)
```

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2079094504)
nit in b9c54ccd8c4248e060c3b4186af0deb3b577d34f:
Could drop string conversions:
```suggestion
BOOST_CHECK_EQUAL(read_value, expected_value);
```

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2079059244)
In b9c54ccd8c4248e060c3b4186af0deb3b577d34f:
nit, avoid repeated `dbwrapper_private::GetObfuscateKey` call:
```diff
- BOOST_CHECK(obfuscate != is_null_key(dbwrapper_private::GetObfuscateKey(dbw)));
- obfuscation_key = dbwrapper_private::GetObfuscateKey(dbw);
+ obfuscation_key = dbwrapper_private::GetObfuscateKey(dbw);
+ BOOST_CHECK_NE(obfuscate, is_null_key(obfuscation_key));
```
similarly on lines 68-69:
```diff
- BOOST_CHECK
...

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2079245753)
The size of these vectors are treating `Obfuscation` with kid gloves, only to let it detonate in the wild. See https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2053773631

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2079237359)
Seems like this function is no longer referenced after this PR and could be dropped in the final commit.

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2079184969)
in 13cc039f20eae787ddbc00140dbc89a9a0b1ea05:
`CreateObfuscation` remains here despite the commit message.

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2079218910)
In the `!key_missing` case, we've just read the `vector` from a file on disk. It could have been corrupted and got a wildly unexpected length. If it is too short we will read out of bounds here, if it is too long we will continue when we probably should be erroring out. See https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2053773631

💬 laanwj commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#issuecomment-2862367684)
Some more context: https://issues.guix.gnu.org/76503