💬 sipa commented on pull request "[IBD] precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2395021434)
I was wrong; it's initialized in `CBlockHeaderAndShortTxIDs::FillShortTxIDSelector`.

If we want runtime checks, I'd rather have an `Assert()` or `Assume()` than relying on the throwing behavior of `std::optional<T>::value()`, but that's just a nit.

💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2395072031)
We can match `JSON` and `JSON_OR_STRING` because both are added to `cmds` list and follow the same rule, and `STRING` is added to string_params list. Recent changes simplify this.

💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2395073330)
Agreed. Now fixed.

💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2395075138)
Thanks. Done.

💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2395078470)
Since the definition of both the `FromPosition` and `FromName` functions tells that they're returning the pointer to the CRPCConvertParam class, thus the parameter names here represent the true meaning in their names according to me. And here `p.rpcMethod == method` if we change `methodName` to `rpcMethod` we need to change the existing name of the member of the class.

💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2395100057)
As we are not obtaining some value from the iterator the current implementation seems simpler.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3357077663)
`22fadecc14...b01d6df464`: resolve silent merge conflict / fix CI

💬 luke-jr commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2395186828)
You're not doing that with this.

💬 luke-jr commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2395187920)
Right, not since the reject message was removed, oh well. :(

💬 l0rinc commented on pull request "[IBD] precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#discussion_r2395190917)
You mean
```C++
return (*Assert(m_hasher))(wtxid.ToUint256()) & 0xffffffffffffL;
```
would be more explicit than
```C++
return m_hasher.value()(wtxid.ToUint256()) & 0xffffffffffffL;
```
?

💬 tyuxar commented on issue "Higher **reported** memory usage of Bitcoin Core after version 29":
(https://github.com/bitcoin/bitcoin/issues/33351#issuecomment-3357203898)
It is not just a reporting issue, there is a problem with the memory use of v29.

I've been running Core in a FreeBSD jail for quite a long time (it's at 14.3p4 right now) and has been using the binary pkg.

v28 used at most approximately 1-1.5 GB, but usually it was hovering around 6-800 MB RSS.

v29 went up to slightly over 3GB shortly after startup, and remained there for 24+ hours. This is RSS! The virtual size was 12+ GB (I assume b/c of the mentioned LevelDB memory mapping).

As I'm runnin
...

💬 cedwies commented on pull request "Avoid file overwriting in fallback `AllocateFileRange` implementation":
(https://github.com/bitcoin/bitcoin/pull/33164#discussion_r2395335416)
I would be a bit hesitant about mixing `lseek` with `FILE*`, since `stdio` has its own buffering. From what I read, a safer option might be to stick with the `stdio` layer (`fseeko`/`ftello`) or call `fstat` on the `fd` after flushing. Even though I expect mixing the two layers to work most of the time, it can be risky because stdio may have buffered data or be tracking the file position separately from the kernel. I think it's safer to either stick to _stdio_ **or** _fd-only.

💬 polespinasa commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2395336697)
how so? Care to explain? :)

💬 ryanofsky commented on pull request "init: Signal m_tip_block_cv on Ctrl-C":
(https://github.com/bitcoin/bitcoin/pull/33511#issuecomment-3357409495)
<!-- begin push-2 -->
Updated 4ee8e2248e54a33f0a3a2e808d46a7b0b8ea7bc3 -> 68cad90dace40f7a015ca4ff81b878fc8fdc1dd5 ([`pr/sigwait.1`](https://github.com/ryanofsky/bitcoin/commits/pr/sigwait.1) -> [`pr/sigwait.2`](https://github.com/ryanofsky/bitcoin/commits/pr/sigwait.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/sigwait.1..pr/sigwait.2))<!-- end --> to fix CI failures, also added more documentation and explanation in commit description

🚀 achow101 merged a pull request: "[28.x] backports + 28.3rc1"
(https://github.com/bitcoin/bitcoin/pull/33476)

👍 hodlinator approved a pull request: "net: improve the interface around FindNode() and avoid a recursive mutex lock"
(https://github.com/bitcoin/bitcoin/pull/32326#pullrequestreview-3290340107)
re-ACK 87e7f37918d42c28033e9f684db52f94eeed617b

Changes since previous ACK:
* Avoided function overloading and documented that hostnames can be used in more places, not just raw IP addresses and the like.
* `--auto`

👍 theStack approved a pull request: "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys"
(https://github.com/bitcoin/bitcoin/pull/29675#pullrequestreview-3290195368)
Code-review ACK ac599c4a9cb3b2d424932d3fd91f9eed17426827 :old_key:

Happy to review potential follow-ups.

💬 theStack commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2395372134)
nit: seems that this shouldn't be in upper-case, if it's not really a constant

💬 theStack commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2395283294)
nit: could move the MUSIG_CHAINCODE constant from the header to musig.cpp, as its only used there now

📝 sipa opened a pull request: "Improve LastCommonAncestor performance + add tests"
(https://github.com/bitcoin/bitcoin/pull/33515)
In theory, the `LastCommonAncestor` function in chain.cpp can take $\mathcal{O}(n)$ time, walking over the entire chain, if the forking point is very early, which could take ~milliseconds. I expect this to be very rare in normal occurrences, but it seems nontrivial to reason about worst cases as it's accessible from several places in net_processing.

This PR modifies the algorithm to make use of the `CBlockIndex::pskip` skip pointers to find the forking point in sublinear time (a simulation sh
...