💬 hebasto commented on issue "Fuzzing related configuration/build options can be improved":
(https://github.com/bitcoin/bitcoin/issues/30318#issuecomment-2231190195)
> > * `--enable-fuzz-binary=yes|no|exclusive`, but renaming it would break every fuzzing script. Maybe the transition to CMake is a good time to improve this since scripts would have to be adjusted anyway.
>
> Seems fine to do, if `-DBUILD_FUZZ_BINARY=ON/OFF/EXCL...` is easy to implement.

It's not a problem at all.

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2231192536)
Recompiling BlindBit had no effect. Pushed some fixes for various CI failures, but that also doesn't impact the above mismatch.

@theStack that would be useful. Our index doesn't store it. We could add an optional `list_txid` option to the RPC that reprocesses the block. The RPC result would then be a map of txid -> tweak.

💬 maflcko commented on pull request "build: Introduce CMake-based buid system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2231205319)
> Please refer to the build options parity table.

Missing link/reference?

💬 hebasto commented on pull request "build: Introduce CMake-based buid system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2231220385)
> > Please refer to the build options parity table.
>
> Missing link/reference?

Thanks! Added.

✅ brunoerg closed a pull request: "wallet, rpc: add BIP44 `account` in `createwallet`"
(https://github.com/bitcoin/bitcoin/pull/29129)

💬 brunoerg commented on pull request "wallet, rpc: add BIP44 `account` in `createwallet`":
(https://github.com/bitcoin/bitcoin/pull/29129#issuecomment-2231224581)
Closing for now.

💬 achow101 commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1679633885)
Added a help text.

🤔 ismaelsadeeq reviewed a pull request: "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending"
(https://github.com/bitcoin/bitcoin/pull/30352#pullrequestreview-2180579805)
Concept ACK

💬 ismaelsadeeq commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1679609544)
whats the `0x02`?
P2Anchor is `OP_TRUE <0x4e73>`

💬 ismaelsadeeq commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1679615452)
```suggestion
ANCHOR, //!< anyone can spend script
```

💬 ismaelsadeeq commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1679638523)
A test for this will be nice!

💬 ismaelsadeeq commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1679627045)
In 7013da222720fde5ea1c76bba99b1c91ae18558f "policy: Add OP_TRUE <0x4e73> as a standard output type"

Add `ANCHOR` txout script type description in the `CTxDestination` docstring.

💬 theStack commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1679647788)
AFAICT this assert would lead to a crash if a tx is scanned with a taproot output where the x-only-pubkey is not on the curve (note that such outputs adhere to standardness rules, i.e. it's trivial to create such a tx, and probably there are already a good amount of them on mainnet). Proposed fix, not tested yet:
```suggestion
if (!secp256k1_xonly_pubkey_parse(secp256k1_context_static, &tx_output_obj, txoutputs[i].data())) {
continue;
}
```
For consistency reaso
...

👍 maflcko approved a pull request: "fix: Make TxidFromString() respect string_view length"
(https://github.com/bitcoin/bitcoin/pull/30436#pullrequestreview-2180659057)
ACK c3a9c71c7077324bf0aa40f834f7537a14619340 🐞

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK c3a9c71c7077324bf0aa40f834
...

💬 instagibbs commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1679665692)
It's the instruction to push next two bytes on the stack

💬 instagibbs commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1679666497)
should have coverage, please check it fails :)

💬 sipa commented on pull request "Fix SSE4.1-related issues":
(https://github.com/bitcoin/bitcoin/pull/28893#issuecomment-2231284275)
utACK d440f13db02c82c842000abe4fe4d0c721a4ad3b

💬 theuni commented on pull request "build: Introduce CMake-based buid system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1679674217)
In commit ff368465a66298487a88d8da575f5251d094d687

Could you remind me what's going on here, please? I assume these aren't needed because we're meant to be adding compile definitions to specific libs. But from what I can tell, we're not actually adding them to `sha256.cpp`? So... is this currently using an un-optimized sha2?

Generally I'd feel more comfortable turning these into opt-outs, I think, to avoid accidentally building without optims.

For the sake of review/sanity, I think for
...

💬 fjahr commented on pull request "index: Check all necessary block data is available before starting to sync":
(https://github.com/bitcoin/bitcoin/pull/29770#issuecomment-2231294972)
> maflcko's suggestion wouldn't remove the dependency. Unless you place the new enum somewhere else?. Which I don't think it worth it.

Yes, I was thinking of putting it in a file for types which we have been doing in the code base in several places. And @ryanofsky suggests introducing a `kernel/types.h` [here](https://github.com/bitcoin/bitcoin/pull/30214/commits/97cb62492903b188074de5c27b3dcba2c9b768ff) already. Moving code isn't hard to review and `chain.h` is big enough to be broken up a b
...

👍 BrandonOdiwuor approved a pull request: "init: change shutdown order of load block thread and scheduler"
(https://github.com/bitcoin/bitcoin/pull/30435#pullrequestreview-2180709184)
Code Review ACK 5fd48360198d2ac49e43b24cc1469557b03567b8