💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2108691581)
Reminds me that non-mainnet use paths like `/regtest/blocks`, perhaps we should accept `-chain` argument as well.

💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2108737971)
This seems over-complicated and not needed. You're not saing anything by "avoiding the conversion" - the optimizer would take care of that for you.

💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2108663194)
Using `args` is problematic for paths because it breaks for some values. `args_os` is the appropriate solution.

💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2108721393)
To be atomic, this should create a temporary file first, write it, sync it, and then rename.

💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2108751295)
You need to call `flush()` before `into_inner`.

💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#issuecomment-2911912752)
> Unfortunately file locking is still experimental in Rust (?), so users would have to run nightly to be able to run the program. I think we should wait until it's stabilized instead of pulling in third party deps to do the locking.

You can just define `extern "C" {}` functions to call them the same way bitcoind does. It might even be the more correct approach anyway because there are three (!!!) kinds of locks on Unix and you must not mix them, and if Rust uses the same as bitcoind that's ju
...

👍 hodlinator approved a pull request: "policy: uncap datacarrier by default"
(https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2870311100)
ACK 35bcd8eed38130445aef5ebe217ab42248fa6f18

### Concept

I understand node runners feeling like their agency would be restricted by deprecating and later removing this limit. However, I have not encountered any strong technical or incentive compatible argument in favor of keeping it. Deprecating and giving a little bit more opportunity for such an argument to appear before removal seems like a diplomatically acceptable solution.

I don't feel strongly enough about this to have kicked thi
...

💬 hodlinator commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2108669091)
nit: `MAX_STANDARD_TX_WEIGHT` (400000) is in vbytes AFAIK, but since you divide by `WITNESS_SCALE_FACTOR` the comment should say the unit is bytes (not vbytes), right?

💬 fanquake commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#issuecomment-2911944384)
Guix Build:
```bash
2d907a08701bbfa9a165581f69a997f5be0b3e0c7ee92f1bd5094df729430485 guix-build-ca9bb622af68/output/aarch64-linux-gnu/SHA256SUMS.part
f603a29e4eadeec26a7e82c882837d5369a35d65b3f283e50d72dd2fc7f9ba9d guix-build-ca9bb622af68/output/aarch64-linux-gnu/bitcoin-ca9bb622af68-aarch64-linux-gnu-debug.tar.gz
558bd5cc61134adb2c2a4120629288ffcd53162ffb1f787e95e56762340efce1 guix-build-ca9bb622af68/output/aarch64-linux-gnu/bitcoin-ca9bb622af68-aarch64-linux-gnu.tar.gz
23b3e836a56c8212
...

💬 maflcko commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#issuecomment-2911998654)
If you reviewed the code by a commit hash, and you want to express your review, see [the guideline](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#code-review) for information on the review process. (You'll have to include the commit hash in your review)

I'll leave the regtest stuff to a follow-up, so that it can be evaluated independently.

🤔 maflcko reviewed a pull request: "guix: accomodate migration to codeberg"
(https://github.com/bitcoin/bitcoin/pull/32439#pullrequestreview-2870539601)
review ACK ca9bb622af68f0b89b1e5c8bc5726b60dec823a3

💬 maflcko commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#discussion_r2108829918)
```suggestion
- A commit to skip this test is included since Guix 1.4.0:
```

💬 fanquake commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#discussion_r2108834816)
Thanks, added

💬 maflcko commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#issuecomment-2912031083)
> > I wonder if -reindex could be taught to do this instead.
>
> @maflcko That would be convenient, but a reindex is a much longer task than this tool. So I don't think "instead", but rather "also".

Maybe just from a code-review/maintenance perspective this is preferable, given that the feature implemented here is quite tricky to get right for all edge cases by (re-)implementing the C++ logic.

💬 maflcko commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#issuecomment-2912033007)
lgtm ACK c8d9baae942c94d64ce47ae8f67d3710e6a296bd

👍 TheCharlatan approved a pull request: "test: Add missing ipc subtree to lint"
(https://github.com/bitcoin/bitcoin/pull/32623#pullrequestreview-2870565681)
ACK fa2bf6bdb7e5f8940fe4bce1aaab29bf2e063b49

👍 TheCharlatan approved a pull request: "test: fix pushdata scripts"
(https://github.com/bitcoin/bitcoin/pull/32270#pullrequestreview-2870576815)
Re-ACK f66b14d2ecb0db991d255be49d6ff6ec361569b5

💬 hebasto commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#discussion_r2108867196)
While this is correct, the mailing list [refers](https://lists.gnu.org/archive/html/info-guix/2025-05/msg00002.html) to `https://git.guix.gnu.org/guix.git` as the canonical URL. Should we use that instead?

👍 TheCharlatan approved a pull request: "test: fix and augment block tests of invalid_txs"
(https://github.com/bitcoin/bitcoin/pull/32591#pullrequestreview-2870607751)
ACK 8fcd6845052354fad80ae7e5feda3f6a2e441e12

🚀 fanquake merged a pull request: "test: fix pushdata scripts"
(https://github.com/bitcoin/bitcoin/pull/32270)