💬 maflcko commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2108287018)
> GHA

I don't know about GHA, but the Cirrus tasks require manual setup, see https://github.com/bitcoin/bitcoin/blob/638a4c0bd8b53766faeb437244b2aae4eed28dcf/.cirrus.yml#L8.

(Closing conversation for now)

🤔 furszy reviewed a pull request: "wallet, rpc, gui: List legacy wallets with a message about migration"
(https://github.com/bitcoin/bitcoin/pull/32619#pullrequestreview-2869966132)
Code review ACK f3a444c45fb4bf4e51d53ebf1cf4c2631ded481c

📝 maflcko opened a pull request: "test: Add missing ipc subtree to lint"
(https://github.com/bitcoin/bitcoin/pull/32623)
The subtree docs and lint checks list the subtrees in three places, making it hard to follow and update and easy to miss one.

Fix all issues by including the missing one and removing the list in one place.

💬 willcl-ark commented on pull request "build: add a depends dependency provider":
(https://github.com/bitcoin/bitcoin/pull/32595#issuecomment-2911607200)
Thanks for the feedback @purpleKarrot.

I like the sound of your approach here myself and will be happy to adapt to it. I think i'll start off by targetting 3.:

> A depends provider: This will ignore system packages and always use vendored depends. This approach will be used for reproducible builds.

...without the optionl "auto-build depends" functionality. This is close to what I have here already (and solves my immediate personal itch regarding building using depends on NixOS).

I wo
...

💬 maflcko commented on pull request "wallet: Remove watchonly behavior and isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2108569954)
> Since this RPC allowed all other types to be aliases for watchonly stuff

I don't think it did. It should have only allowed bool:

```cpp
if (options.type() == UniValue::VBOOL) {
// backward compatibility bool only fallback
```



> this way it doesn't throw for any callers written in that way.

You removed it from the options object as well, which has strict=true checking enabled. So I think it would be more consistent to remove it here as well, or keep it for both
...

🤔 polespinasa reviewed a pull request: "rpc: Round verificationprogress to 1 for a recent tip"
(https://github.com/bitcoin/bitcoin/pull/32528#pullrequestreview-2870288028)
ACK

The test added faf6304bdfdf228354b4072b72f4c0ef90fdaade lgtm and makes much sense.

(non-blocking) My only point is that a warning message at least would be good for regtest as being at the tip and having the verification progress < 1 when no new blocks are getting mined (in regtest) may confuse.

<details>
<summary>see proposal
</summary>

```diff
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 880e35ee7f..b694171583 100644
--- a/src/rpc/blockchain.cpp
+++
...

👋 fanquake's pull request is ready for review: "guix: accomodate migration to codeberg"
(https://github.com/bitcoin/bitcoin/pull/32439)

🤔 Kixunil reviewed a pull request: "contrib: add xor-blocks tool to obfuscate blocks directory"
(https://github.com/bitcoin/bitcoin/pull/32451#pullrequestreview-2870286845)
Sadly, this new implementation is quite broken in multiple ways.

💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2108652853)
I see you're using this to display the messages from `main`. However that will lead to horrible messages containing `Error { msg: "..." }`. I suggest you implement `Debug` manually instead just displaying the string.

💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2108657220)
This loses error sources. The proper way to do this is to create a mutable string and then append the sources in a loop with `": "` as a separator. (You could also do cargo-style `"\n\tcaused by: "`).

💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2108685983)
This kind of c-style handling is not idiomatic Rust. I'd do something like:

```rust
let mut args = std::env::args_os();
args.next().ok_or("Missing program name")?;
let data_dir_path = if let Some(first_arg) = args.next() {
let data_dir = if first_arg == "-datadir" || first_arg == "--datadir" {
args.next().ok_or("the datadir argument is missing a value")?
} else if let Some(data_dir) = first_arg.strip_prefix("-datadir=") {
data_dir.to_owned()
} else if let S
...

💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2108718168)
A comment explaining that this doesn't need to be cryptographic would be nice.

💬 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?