🤔 enirox001 reviewed a pull request: "cli: Handle arguments that can be either JSON or string"
(https://github.com/bitcoin/bitcoin/pull/33230#pullrequestreview-3171265739)
Concept ACK

💬 jsarenik commented on issue "signet: disk-space-DoS due to low mining difficulty":
(https://github.com/bitcoin/bitcoin/issues/33266#issuecomment-3239755243)
What about a safety one-liner fix which would not allow new block to have the same blockheader (except nonce) than an already mined block? Such a local-node "soft-fork" would be possible even without telling anyone network-wise.

An attack block could come only _after_ a new unique mined&signed block header was announced.

💬 stwenhao commented on issue "signet: disk-space-DoS due to low mining difficulty":
(https://github.com/bitcoin/bitcoin/issues/33266#issuecomment-3239765781)
> What about a safety one-liner fix which would not allow new block to have the same blockheader (except nonce) than an already mined block?

As long as you know, which block was propagated first, then you can do that. But note that signet block producers can decide to build things on top of any block, including something, which was grinded by some attacker. If you see two or more blocks with different nonces, at the same height N, then you don't know, what block number N+1 will sign (and also,
...

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2312254549)
https://github.com/bitcoin/bitcoin/compare/4441827ef4e9b5fe306c5f0a81a52b5d2b5e0b69...5e1c80a22f3d3c6ecf6d4e2777d67656423dd3f3

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3239789278)
Rebasing to resolve a conflict with `master`.

🤔 optout21 reviewed a pull request: "refactor: unify container presence checks (without PR conflicts)"
(https://github.com/bitcoin/bitcoin/pull/33192#pullrequestreview-3171424512)
ACK f70d2c7faa8f7d724e146e4c409de9c6778b7299

Re-reviewed; this is a subset of #33097, comments from [there](https://github.com/bitcoin/bitcoin/pull/33094#pullrequestreview-3093962887) still apply:

- The changes increase code readability, as 'contains' expresses the code logic / intent more specifically
- It also results in higher performance, due to potential early exit. The improvement is probably negligible though.
- Changes are localized (each to a single line), local impact only

(
...

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2312290776)
https://github.com/bitcoin/bitcoin/pull/32541/commits/94389c28e1068ffcc116614d16ac3047eb3068e3 seems to be passing (after a rebase over `master`) :thinking:

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2312297480)
Thanks - updated the comment.

💬 naiyoma commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2312399866)
I’d suggest removing this instead of commenting it out.

🤔 naiyoma reviewed a pull request: "test: Fixup fill_mempool docstring"
(https://github.com/bitcoin/bitcoin/pull/33269#pullrequestreview-3171569181)
ACK fa3f682032a3292604f363a5ee4557937f3d8950

✅ martinus closed a pull request: "Draft: CCoinMap Experiments"
(https://github.com/bitcoin/bitcoin/pull/32128)

💬 martinus commented on pull request "Draft: CCoinMap Experiments":
(https://github.com/bitcoin/bitcoin/pull/32128#issuecomment-3240102028)
Up for grabs, if anyone is interested

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2312514122)
@hodlinator would it be OK to squash https://github.com/bitcoin/bitcoin/commit/94389c28e1068ffcc116614d16ac3047eb3068e3 into https://github.com/bitcoin/bitcoin/pull/32541/commits/bdd90ab25f5f3b05c03a8dfb7177399264bed377?

🤔 hebasto reviewed a pull request: "guix: update time-machine to 5cb84f2013c5b1e48a7d0e617032266f1e6059e2"
(https://github.com/bitcoin/bitcoin/pull/33185#pullrequestreview-3171703063)
I’ve obtained matching hashes for Guix builds on the `aarch64` and `riscv64` hardware:
```
835b80bca08284974c5300768fb03be2c50143510548f8c1c03c737d6b22e473 guix-build-91560160a011/output/aarch64-linux-gnu/SHA256SUMS.part
8b0d7a7447ec50624528e42d04c318da20d755dd0e61758a77f2ebfcd74c4a0f guix-build-91560160a011/output/aarch64-linux-gnu/bitcoin-91560160a011-aarch64-linux-gnu-debug.tar.gz
95632363960c1db8aafaab8523f35d3d41961455bf075c5e73bf4f7903136563 guix-build-91560160a011/output/aarch64-li
...

👍 hebasto approved a pull request: "guix: update time-machine to 5cb84f2013c5b1e48a7d0e617032266f1e6059e2"
(https://github.com/bitcoin/bitcoin/pull/33185#pullrequestreview-3171710452)
ACK 91560160a011fe3b2f472aa9144d4072e9e369cd, I have reviewed the code and it looks OK.

🤔 furszy reviewed a pull request: "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet"
(https://github.com/bitcoin/bitcoin/pull/33268#pullrequestreview-3171730621)
q: is `IsFromMe()` method still relevant? It seems we could directly call `IsMine()` on the inputs, which is also cached now and should be slightly safer for migration.

💬 achow101 commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3240325938)
> is `IsFromMe()` method still relevant?

Yes, transactions that we make are still treated specially.

💬 furszy commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3240345028)
> > is `IsFromMe()` method still relevant?
>
> Yes, transactions that we make are still treated specially.

Where?. `IsFromMe()` seems to be only ever called in `AddToWalletIfInvolvingMe()` and `ApplyMigrationData()` next to the `IsMine()` call. In both places we effectively treat it as `is_mine = IsMine(outputs) || IsFromMe(inputs)`.

💬 achow101 commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3240361084)
> Where?

Hmm, I forgot that `CachedTxIsFromMe` does not use `IsFromMe`. Might need to change that in this PR as well.

💬 hebasto commented on pull request "guix: update time-machine to 5cb84f2013c5b1e48a7d0e617032266f1e6059e2":
(https://github.com/bitcoin/bitcoin/pull/33185#issuecomment-3240432791)
> Could be used for #32764.

That PR would benefit from being updated a bit further, to https://codeberg.org/guix/guix/commit/2877c75dc5db0dbf664fb6170d5754068e941d91.