💬 maflcko commented on pull request "test: switch from curl to urllib for HTTP requests":
(https://github.com/bitcoin/bitcoin/pull/29970#discussion_r1637705656)
Right, but at some point I wonder if it is worth it to try to re-implement `curl` in Python. I am not sure, but is curl not bundled in `git`, which is already a dependency of this project? Of all open source software projects `curl` is currently unlikely to go unmaintained, so the motivation still seems weak.

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#issuecomment-2164863762)
I am still looking into the clang error. I couldn't find an issue with your code, but minimizing/reducing the clang bug will take some time, because it appears non-deterministic.

In the meantime, if you want, you can try a wild shot to work around. For example, temporarily pin to clang-16:

```diff
diff --git a/ci/test/00_setup_env_native_asan.sh b/ci/test/00_setup_env_native_asan.sh
index 668e9ecc8a..d5180189d7 100755
--- a/ci/test/00_setup_env_native_asan.sh
+++ b/ci/test/00_setup_env
...

:lock: fanquake locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/30280)

👍 dergoegge approved a pull request: "refactor: Add explicit cast to expected_last_page to silence fuzz ISan"
(https://github.com/bitcoin/bitcoin/pull/30248#pullrequestreview-2115150107)
utACK fa9cb101cf33b57b2c043b29f1f3d55b990ba4c6

💬 dergoegge commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#issuecomment-2165010123)
#30229 was merged if you want to rebase

💬 brunoerg commented on pull request "test: cover more errors for `signrawtransactionwithkey` RPC":
(https://github.com/bitcoin/bitcoin/pull/30278#discussion_r1637893489)
To avoid calling same stuff twice. e.g. `createrawtransaction`.

💬 theStack commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1637812555)
```suggestion
// This takes the place of ReplacementChecks()'s PaysMoreThanConflicts() in the package RBF setting.
```

💬 theStack commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1637815039)
nit: slightly easier to reason for the reader imho

```suggestion
if (package_feerate <= parent_feerate) {
```

💬 theStack commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1637859001)
nit: it might be worth to add a belt-and-suspenders check that txns has size two (either directly or indirectly by asserting `txns.size() == workspaces.size()` above)

🤔 theStack reviewed a pull request: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984#pullrequestreview-2115139302)
Spent most time reviewing the core commit 612847ae1a55e92bb7732905a026be96a436372a so far, which looks correct to me (though I'm not super-familiar with the details of the feerate diagram checks that are used in this PR for the first time in production code). Still planning on looking deeper at the tests and do another full review round. Left some non-blocking nits on the way.

💬 theStack commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1637914037)
```suggestion
single_coin = self.coins.pop()
```

💬 theStack commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1637918078)
could mention "analogous to regular replacement rule 5"?

💬 theStack commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1637862004)
nit: shorter (can also be done for grandchild_key)
```suggestion
CKey child_key = GenerateRandomKey();
```

💬 vasild commented on pull request "fuzz: FuzzedSock::Recv() don't lose bytes from MSG_PEEK read":
(https://github.com/bitcoin/bitcoin/pull/30273#issuecomment-2165212522)
The UB sanitizer did not like the call to `memcpy(..., nullptr, 0);` which happens when the fuzzer input is exhausted. Added an explicit check about that.

💬 stickies-v commented on pull request "Encapsulate warnings in generalized node::Warnings and remove globals":
(https://github.com/bitcoin/bitcoin/pull/30058#issuecomment-2165244787)
Force pushed to address merge conflict from #29015, no changes otherwise.

🤔 marcofleon reviewed a pull request: "i2p: fix and improve logs"
(https://github.com/bitcoin/bitcoin/pull/29833#pullrequestreview-2115402840)
ACK 7d3662fbe35032178c5a5e27e73c592268f6e41b.

🚀 fanquake merged a pull request: "Update minisketch subtree to eb37a9b8e79f9e49d73b96a49bf97a96d9eb676c"
(https://github.com/bitcoin/bitcoin/pull/30270)

👍 ryanofsky approved a pull request: "Encapsulate warnings in generalized node::Warnings and remove globals"
(https://github.com/bitcoin/bitcoin/pull/30058#pullrequestreview-2115559245)
Code review ACK 260f8da71a35232d859d7705861fc1a88bfbbe81. Only change since last review was rebasing

📝 fanquake opened a pull request: "Update leveldb subtree to latest upstream"
(https://github.com/bitcoin/bitcoin/pull/30281)
Includes https://github.com/bitcoin-core/leveldb-subtree/pull/41 which is used in #30234.

💬 benma commented on issue "docs: Wrong/outdated docs for `tr(KEY)` in doc/descriptors.md":
(https://github.com/bitcoin/bitcoin/issues/30279#issuecomment-2165532816)
I for one was confused. Imo being explicit is better than being implicit. "internal key" is ambiguous to me, as it means it is untweaked, but it does not mean it must be tweaked. BIP-341 merely says "should".

Removing the descriptors.md in in favor of BIPs is not optimal, because:

- the BIPs are not the docs for Bitcoin Core - Bitcoin Core should document itself what it supports
- it is hard to get an overview of what is supported in Bitcoin Core by scrolling BIPs

I think the best woul
...