💬 willcl-ark commented on issue "[FEATURE REQUEST] Enable new Tor PoW feature for automatic creation of Bitcoin Core onion hidden service":
(https://github.com/bitcoin/bitcoin/issues/28499#issuecomment-2199888419)
@twofaktor thanks for the feature request, it does seem like it would be a nice-to-have.

However we are here 9 months later and nothing has changed upstream, at least insomuch as it's still not possible to configure `HiddenServicePoWDefensesEnabled` via the control port, as @recursive-rat4 correctly identified. It also does not appear that there are any current plans to add such functionality.

There seems little point to me in keeping this issue open indefinitely in _this_ repo, for someth
...

✅ willcl-ark closed an issue: "importing a wallet containing an hdseed overwrites target wallet hdseed"
(https://github.com/bitcoin/bitcoin/issues/28927)

💬 willcl-ark commented on issue "importing a wallet containing an hdseed overwrites target wallet hdseed":
(https://github.com/bitcoin/bitcoin/issues/28927#issuecomment-2199906283)
This issue hasn't had activity in a while and appears to have gone stale so I'm going to close it for now.

Feel free to open a new issue or comment here if you are still experiencing this problem so we can investigate further.

💬 maflcko commented on issue "fuzz: Fix stability, determinism issues":
(https://github.com/bitcoin/bitcoin/issues/29018#issuecomment-2199910861)
Reported the oss-fuzz bug on https://www.github.com/google/oss-fuzz/issues/12142

💬 sipa commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1660943290)
Yes and no. Compilers do decide for themselves what to inline, but the `inline` keyword does increase compilers' eagerness to inline.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1660978352)
I thought about this further and removed the requirement to set `-walletbroadcast=0` when `-privatebroadcast=1`. The description of `-privatebroadcast` begins with

> Broadcast transactions submitted via sendrawtransaction RPC using short lived connections...

so it should be clear that this applies to the `sendrawtransaction` RPC. In addition I extended the text with

> Transactions submitted through the wallet are not affected by this option

🤔 glozow reviewed a pull request: "test: Add Compact Block Encoding test `ReceiveWithExtraTransactions` covering non-empty `extra_txn`"
(https://github.com/bitcoin/bitcoin/pull/30237#pullrequestreview-2151265702)
ACK 55eea003af24169c883e1761beb997e151845225

💬 glozow commented on pull request "test: Add Compact Block Encoding test `ReceiveWithExtraTransactions` covering non-empty `extra_txn`":
(https://github.com/bitcoin/bitcoin/pull/30237#discussion_r1660980865)
Another PR could change these calls to use `PeerManagerImpl::m_rng` though it would require taking off the `g_msgproc_mutex` guard.

💬 theStack commented on issue "ci: failure in p2p_handshake.py":
(https://github.com/bitcoin/bitcoin/issues/30368#issuecomment-2200034205)
Seems like it's possible that the VERSION message for a new outbound connection is sent out, received locally and processed _before_ the corresponding `CNode` instance is added to the `m_nodes` list, so the self-connection can't be detected yet. I can reproduce the failure locally with the following patch:
```diff
diff --git a/src/net.cpp b/src/net.cpp
index 990c58ee3d..64b5995dbe 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -2921,6 +2921,7 @@ void CConnman::OpenNetworkConnection(const C
...

💬 Sjors commented on issue "ci: failure in p2p_handshake.py":
(https://github.com/bitcoin/bitcoin/issues/30368#issuecomment-2200035584)
Noticed as well for #30356 which is _not_ based on https://github.com/bitcoin/bitcoin/pull/30362, but I guess CI runs on a merge commit? https://github.com/bitcoin/bitcoin/actions/runs/9742619062/job/26884283814?pr=30356

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1660993146)
Fixed and added `@param[in]` comments.

What is enforced by clang (not by doxygen) via `-Wdocumentation` is a mismatch in the parameter names - e.g. if there is `@param[in] throughput ...` and the actual parameter is `int thrughput`, then it will give a warning (or error if `-Werror` is used).

💬 maflcko commented on issue "fuzz: Fix stability, determinism issues":
(https://github.com/bitcoin/bitcoin/issues/29018#issuecomment-2200045414)
The latest build log from https://oss-fuzz-build-logs.storage.googleapis.com/index.html#bitcoin-core says:

```
Step #4 - "build-check-afl-address-x86_64": Retrying failed fuzz targets sequentially 4
Step #4 - "build-check-afl-address-x86_64": INFO: performing bad build checks for /tmp/not-out/tmpjtp3xr64/process_message
Step #4 - "build-check-afl-address-x86_64": INFO: performing bad build checks for /tmp/not-out/tmpjtp3xr64/process_messages
Step #4 - "build-check-afl-address-x86_64": INF
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1660998055)
Could be either way. If there is more than one caller of this function, then having it here would avoid duplicating the check in every caller, but there is one caller so far.

💬 maflcko commented on pull request "optimization: Speed up Base58 encoding/decoding by 400%/200% via preliminary byte packing":
(https://github.com/bitcoin/bitcoin/pull/29473#issuecomment-2200056770)


I don't think this is used in a critical path, to warrant a speedup?

💬 paplorinc commented on pull request "optimization: Speed up Base58 encoding/decoding by 400%/200% via preliminary byte packing":
(https://github.com/bitcoin/bitcoin/pull/29473#issuecomment-2200078041)
> I don't think this is used in a critical path, to warrant a speedup?

It's literally the example from the [benchmarking docs](https://github.com/bitcoin/bitcoin/blob/master/doc/benchmarking.md):
<img src="https://github.com/bitcoin/bitcoin/assets/1841944/822e347b-0064-4343-8292-bf753530a7e9">

which was specifically optimized in various other commits before: https://github.com/bitcoin/bitcoin/commit/bcab47bc1b2bfdd29f8b89f8a211755299938aea, https://github.com/bitcoin/bitcoin/commit/159ed
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1661016997)
Because we want to ignore incoming messages after a successful handshake. Ignoring the `VERACK` would disturb the handshake.

💬 maflcko commented on pull request "optimization: Speed up Base58 encoding/decoding by 400%/200% via preliminary byte packing":
(https://github.com/bitcoin/bitcoin/pull/29473#issuecomment-2200092884)
What you link is supporting my point, is it not? https://github.com/bitcoin/bitcoin/pull/12704#issuecomment-374835228

> Stop wasting time on discussing the performance. This does not matter. Decoding an address could take 50 us and I don't think anyone would notice.

> If the resulting code looks better, go for it. Otherwise, don't.

> -0

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1661030282)
Note that this would only be reached if `m_by_priority` contains txid that does not have an entry in `m_by_txid` which is a gross bug in the `PrivateBroadast` class. IIRC this was an assert before but somebody suggested to use `Assume()` instead.

💬 paplorinc commented on pull request "optimization: Speed up Base58 encoding/decoding by 400%/200% via preliminary byte packing":
(https://github.com/bitcoin/bitcoin/pull/29473#issuecomment-2200101421)
What's the point of the benchmarks in that case exactly?
Anyway, I can make the code slightly simpler and similarly performant, would that be welcome?

💬 maflcko commented on pull request "optimization: Speed up Base58 encoding/decoding by 400%/200% via preliminary byte packing":
(https://github.com/bitcoin/bitcoin/pull/29473#issuecomment-2200102096)
Ok, the reason for https://github.com/bitcoin/bitcoin/pull/7656#issue-139438690 was an improvement in `listunspent`. Seems fine, if this is still the case. But this will need to be checked first.