👍 tdb3 approved a pull request: "netbase: extend CreateSock() to support creating arbitrary sockets"
(https://github.com/bitcoin/bitcoin/pull/30202#pullrequestreview-2114480092)
re-ACK 5f549c35d9a75c7019fe8a96963b988df5498eed

💬 tdb3 commented on pull request "test: cover more errors for `signrawtransactionwithkey` RPC":
(https://github.com/bitcoin/bitcoin/pull/30278#discussion_r1637369302)
nit: An invalid private key and an invalid transaction could be tested separately rather than in the same function. Was there a rationale for combining them?

👍 tdb3 approved a pull request: "test: cover more errors for `signrawtransactionwithkey` RPC"
(https://github.com/bitcoin/bitcoin/pull/30278#pullrequestreview-2114521185)
ACK e2779ce98b39e14cada08a654928e798436f5a46

Ran `rpc_signrawtransactionwithkey` locally (passed). Intentionally caused test failure by 1) using privkey `cUeKHd5orzT3mz8P9pxyREHfsWtVfgsfDjiZZBcjUBAaGk1BTj7N' and independently by 2) removing the trailing `00` from the tx. Test failed as expected each time.

💬 RichardGantz commented on issue "importaddress failed, Only legacy wallets are supported by this command.":
(https://github.com/bitcoin/bitcoin/issues/29772#issuecomment-2164672729)
Dear sipa,

thank you so much for your quick and crystal clear answer.
The EPS started up successfully after I created the legacy wallet.

Please allow me to add for newbees like me that
1. I had to stop bitcoin-core and start it once with the additional parameter `-deprecatedrpc=create_bdb`.
2. After that started up I was able to create the legacy wallet with the following command in the console window:
```createwallet "LegacyWallet" false false "" false false```.

Shutdown of bitc
...

💬 maflcko commented on pull request "test: cover more errors for `signrawtransactionwithkey` RPC":
(https://github.com/bitcoin/bitcoin/pull/30278#issuecomment-2164734573)
ACK e2779ce98b39e14cada08a654928e798436f5a46

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