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

💬 marcofleon commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205405038)
nit: I believe this should be DoS score > 1.

🤔 marcofleon reviewed a pull request: "p2p: improve TxOrphanage denial of service bounds"
(https://github.com/bitcoin/bitcoin/pull/31829#pullrequestreview-3017110217)
Code review ACK 790f6e7a72d31988c5be9f2d520a04d0c0edfa09

Focused mainly on the orphanage reimplementation d5097d6eef85eab971352cf78fb94a30c6e6f127 and fuzzing. Fuzzed the three txorphan targets for ~1000 cpu hours each. A couple comments I had initially are addressed in the followup.

💬 marcofleon commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205431437)
Thinking out loud, it seems like using `m_outpoint_to_orphan_it` for lookups here instead of going through all orphans could be better performance-wise (if there's a lot of unrelated orphans from this peer). Ultimately, I think this implementation is fine, as it's simpler and automatically returns the children sorted from newest to oldest.

💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2205459748)
Agree, see https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2166251339

💬 marcofleon commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205468802)
Do you mean if we end up constructing txs with more than 9 inputs in the sim target? It seems to me that `txorphan` is more of a stress test than the simulation target, which is a differential test, no?

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2205473289)
> It seems to me that txorphan is more of a stress test than the simulation target, which is a differential test, no?

fair enough

💬 adamjonas commented on issue "bitcoin core crashes when too many rpc calls are made":
(https://github.com/bitcoin/bitcoin/issues/11368#issuecomment-3070432431)
Any chance this was reproduced on a newer version? 26 hit EOL on 2025-04-14.

💬 rkrux commented on pull request "wallet: Allow read-only database access for info and dump commands":
(https://github.com/bitcoin/bitcoin/pull/32685#issuecomment-3070441050)
Re: [Merge branch 'master' into wallet-readonly-access](https://github.com/bitcoin/bitcoin/pull/32685/commits/fa0699eaffee914aded42fce21beffdebf41b1b6)

I don't think merging master in the PR branch is preferred, instead rebasing is done.
https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#rebasing-changes

🤔 Crypt-iQ reviewed a pull request: "fuzz: Add fuzz target for block index tree and related validation events"
(https://github.com/bitcoin/bitcoin/pull/31533#pullrequestreview-3017290197)
I tracked down the `m_best_header` non-determinism I [posted above](https://github.com/bitcoin/bitcoin/pull/31533#pullrequestreview-2950384616) to `RecalculateBestHeader`. It seems that since `m_block_index` is a `std::unordered_map`, the iteration order in `RecalculateBestHeader` is not guaranteed.

I narrowed down my corpus to two files that, when run in separate processes (the first step of the `deterministic-fuzz-coverage` script), are deterministic. When they are run in the same process (
...

💬 Crypt-iQ commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2205513966)
`blocks` is reset each iteration, so can this be removed?