💬 josibake commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3069964449)



> > am not sure how I am to interpret your NACK here.
>
> Just as "I think this PR should not be merged in its current form." I definitely do agree with the approach of adding a C API.

FWIW, I read this as "Concept ACK, Approach NACK" (per https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#conceptual-review), which I think is a helpful distinction.

💬 maflcko commented on pull request "refactor: cleanup index logging":
(https://github.com/bitcoin/bitcoin/pull/32948#issuecomment-3069982232)
review ACK c18bf0bd9be632e54591493a5309e9ed4020f2e5 🚣

<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: review ACK c18bf0bd9be6
...

💬 purpleKarrot commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3069994277)
Yes, Concept ACK, Approach NACK

Thanks, @josibake.

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3070013951)
Updated 267a7b3f321304f75e8c47e380da49ba9c64bc84 -> 1ffc1c9d94b16cdbfb92a26d0f0e75451efad4fe ([kernelApi_44](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_44) -> [kernelApi_45](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_45), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_44..kernelApi_45))

* Enforce better function names in the API, which should make future discussions on their desired end format a bit easier.
* Dropped the macro check for gcc 4.

🚀 fanquake merged a pull request: "refactor: cleanup index logging"
(https://github.com/bitcoin/bitcoin/pull/32948)

🤔 mzumsande reviewed a pull request: "test: fix intermittent failure in rpc_invalidateblock.py"
(https://github.com/bitcoin/bitcoin/pull/32968#pullrequestreview-3016812338)
Code Review ACK 28416f367a5d720d89214aa8ef94013caa1d1a40

💬 m3dwards commented on pull request "test: use notarized v28.2 binaries and fix macOS detection":
(https://github.com/bitcoin/bitcoin/pull/32922#issuecomment-3070026124)
ACK 4bb4c865999bfa0b0cb5aa806cf62dbf982fcfe9

Tested that the correct binaries were self signed on ARM Mac. Also checked the SHA256 sums for 28.2.

📝 crypomen9 opened a pull request: "chore: rename misspelling variable"
(https://github.com/bitcoin/bitcoin/pull/32971)
### Description

This pull request refactors the variable name from `shuffled_indeces` to `shuffled_indices` for improved code readability and consistency with standard naming conventions.

### Impact

- No changes to logic or functionality.
- No new tests are required, as this is a non-functional refactor.
- This change improves the developer experience by making the codebase easier to read and maintain.

💬 anhilde commented on issue "Bitcoin Core v29.0 incorrectly enters IBD mode when only ~600 blocks behind, preventing normal sync":
(https://github.com/bitcoin/bitcoin/issues/32955#issuecomment-3070086012)
ok, I will look out for that the next time. I am pretty sure the node was up a few hours before I noticed that the block number had not changed. What is the normal time until a node must have resumed syncing again? Is there a timeframe where I can be sure something is wrong?

💬 sipa commented on issue "Bitcoin Core v29.0 incorrectly enters IBD mode when only ~600 blocks behind, preventing normal sync":
(https://github.com/bitcoin/bitcoin/issues/32955#issuecomment-3070117257)
It should literally *always* be syncing. If it doesn't pick up an announced block immediately (within seconds, at the very longest), there is something wrong.

💬 1440000bytes commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3070190437)
Better to admit that USD is still the currency used for payments and there isn't any difference from 1 USD in 2015 and 2025.

Maybe Bitcoin(protocol) is dependent on USD(US government)?

🤔 Sjors reviewed a pull request: "descriptors: MuSig2"
(https://github.com/bitcoin/bitcoin/pull/31244#pullrequestreview-3016814938)
Reviewed from 4af0dca096ca497a6b4e5314c9edea683efe620e to a53924bee321f9d01d053cf562ee3d9493e00529.

In a53924bee321f9d01d053cf562ee3d9493e00529 _descriptor: Parse musig() key expressions_: it might be slightly easier to follow if you extract `& has_hardened`, the `ParsePubkey` signature change and `FromString` changes into their own commit(s).

💬 Sjors commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2205328232)
In a53924bee321f9d01d053cf562ee3d9493e00529 _descriptor: Parse musig() key expressions_: I'm confused by how this variable starts as `true`.

```diff
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -1843,20 +1843,20 @@ std::vector<std::unique_ptr<PubkeyProvider>> ParsePubkey(uint32_t& key_exp_index
bool any_ranged = false;
bool all_bip32 = true;
std::vector<std::vector<std::unique_ptr<PubkeyProvider>>> providers;
- bool any_key_parsed
...

💬 Sjors commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2205223219)
And if there accidentally was / becomes a way, hopefully a fuzzer catches it with the `Assume` above.

(in 4af0dca096ca497a6b4e5314c9edea683efe620e _descriptor: Add MuSigPubkeyProvider_)

💬 Sjors commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2205299119)
In a53924bee321f9d01d053cf562ee3d9493e00529 _descriptor: Parse musig() key expressions_: well, it only ratchets `hardened` from `false` to `true`, never the other way around. This should be documented, since it's not obvious.

💬 Sjors commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2205263157)
Agree this behavior makes sense. TIL about #32471.

(in 4af0dca096ca497a6b4e5314c9edea683efe620e _descriptor: Add MuSigPubkeyProvider_)

👍 instagibbs approved a pull request: "cluster mempool: add TxGraph work controls"
(https://github.com/bitcoin/bitcoin/pull/32263#pullrequestreview-3017015616)
ACK 62ed1f92efff42bc79c50935e6dbd9da4e072020

📝 waketraindev opened a pull request: "rpc,net: Add getpeerbyid and listpeersbyids RPCs"
(https://github.com/bitcoin/bitcoin/pull/32972)
Add fast peer lookup by NodeId using `m_node_id_map` in CConnman.

Adds `GetNodeStatsById` and `GetNodeStatsByIds` in `net.{cpp,h}` for efficient internal access.

Introduces two new RPCs:
- `getpeerbyid(id)`: returns info for a single connected peer
- `listpeersbyids([ids], strict=false)`: batch query with optional strict mode

Includes functional tests covering expected behavior and edge cases.

Useful for tools that track peers by ID without scanning the full list, or for quickly lo
...

💬 ryanofsky commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-3070299589)
re: https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-3063715290

>I think the utility of the change is more one of drawing a border for ourselves

It is true that defining the CLI through `bin/`is only "drawing a border," not adding an enforcement mechanism that prevents internal binaries from being used directly. This seems appropriate to me, and analogous to python's convention of using `_` prefixes to draw a border between internal and external APIs without stopping them from be
...

💬 Sjors commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#issuecomment-3070380062)
Sorry, #32948 probably caused some conflicts.