💬 laanwj commented on pull request "netinfo: add peer services column and outbound-only option":
(https://github.com/bitcoin/bitcoin/pull/30930#discussion_r1807719827)
> I did maintain the "high details levels = maximum" behavior preferred by laanwj in https://github.com/bitcoin/bitcoin/pull/21192 for values above 5.

Thank you 😄

💬 laanwj commented on pull request "doc: Add `nproc` support for Mac through `coreutils`":
(https://github.com/bitcoin/bitcoin/pull/30936#discussion_r1807727134)
Could also use `-G ninja`, the ninja build system uses all available CPU threads by default 😎
(not sure we'd want to recommend it as default tho)

👍 laanwj approved a pull request: "init: Correct coins db cache size setting"
(https://github.com/bitcoin/bitcoin/pull/31064#pullrequestreview-2380260832)
Code review ACK 3a4a788ee0db83d20607f14801dbed2ee932943c
Both changes look sensible to me, but have not done any specific testing.

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

💬 laanwj commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1807753176)
i'm very confused about this one; `begin()` returns a `unsigned char*`, and unsigned values are definitely allowed to overflow within defined behavior. Also, `m_data` is an array so by definition [0] will not be out of bounds.

👍 rkrux approved a pull request: "rpc: Add support to populate PSBT input utxos via rpc"
(https://github.com/bitcoin/bitcoin/pull/30886#pullrequestreview-2380312703)
tACK https://github.com/bitcoin/bitcoin/commit/abc835282a66495f65f342cfc113e45aac4ebb48

Successful make and functional tests.
I manually tested the new argument in `utxoupdatepsbt` using my regtest environment by creating parent transaction and a subsequent child transaction. Processing the child PSBT without passing in the raw parent transaction hex led to incomplete processing and was successful later when the parent was passed.
Also tested for duplicate parent transaction error.

💬 rkrux commented on pull request "rpc: Add support to populate PSBT input utxos via rpc":
(https://github.com/bitcoin/bitcoin/pull/30886#discussion_r1807750310)
```suggestion
{"", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "A serialized previous transaction in hex"},
```

💬 rkrux commented on pull request "rpc: Add support to populate PSBT input utxos via rpc":
(https://github.com/bitcoin/bitcoin/pull/30886#discussion_r1807751783)
Outside the scope of this PR: While testing I realised that the output does not display the reason why it failed, would it be a good idea to display the error as well?
```
➜ src git:(2024-09-updateutxo_psbt) ✗ bitcoincli walletprocesspsbt cHNidP8BAFICAAAAAfDLyQnCSPG31wZz1rHABK8QVnQLIubUuakp3tA1b688AAAAAAD9////AQBlzR0AAAAAFgAU0NrK8DrQVPpN/ZzmgNtNu55HmNgAAAAAAAAA
{
"psbt": "cHNidP8BAFICAAAAAfDLyQnCSPG31wZz1rHABK8QVnQLIubUuakp3tA1b688AAAAAAD9////AQBlzR0AAAAAFgAU0NrK8DrQVPpN/ZzmgNtNu55HmNgAAA
...

💬 rkrux commented on pull request "rpc: Add support to populate PSBT input utxos via rpc":
(https://github.com/bitcoin/bitcoin/pull/30886#discussion_r1807750224)
```suggestion
{"", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "A serialized previous transaction in hex"},
```
Assuming it's supposed to say `previous`.

💬 rkrux commented on pull request "rpc: Add support to populate PSBT input utxos via rpc":
(https://github.com/bitcoin/bitcoin/pull/30886#discussion_r1807751232)
Does it make sense to deduplicate this code piece by accepting `prev_txs, request.params[i]` as inputs to a function?

💬 laanwj commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1807789736)
a workaround may be to do `val.data()[0] = (val.data()[0] + 1) & 0xff;` to make the wrap-around explicit, the C++ compiler will almost certainly just optimize it away

💬 sipa commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1807792537)
This is not UB, but ubsan is explicitly told to warn for certain error-prone but implementation-defined or well-defined behaviors.

If the effect is deliberate, it is possible to add the function name to the ubsan suppressions.

💬 l0rinc commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1807792637)
Would you prefer I '& 0xFF' the current implementation?

💬 laanwj commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1807812963)
dunno, i would prefer `&0xff` to adding a supression, it more explicitly defines what is expected

💬 EthanHeilman commented on pull request "CAT in Tapscript (BIP-347)":
(https://github.com/bitcoin/bitcoin/pull/29247#issuecomment-2425013338)
@achow101 Thanks, that's helpful for understanding the context. If consensus forms around OP_CAT should I or @0xBEEFCAF3 mark this as no longer as draft? I can't speak for @0xBEEFCAF3 but I would feel uncomfortable making that decision as it would open me up to criticism that I was attempting to manufacture consensus, even if there was consensus. What is the process here?

💬 achow101 commented on pull request "CAT in Tapscript (BIP-347)":
(https://github.com/bitcoin/bitcoin/pull/29247#issuecomment-2425036043)
> If consensus forms around OP_CAT should I or @0xBEEFCAF3 mark this as no longer as draft?

Yes. A maintainer may also do so as well.

📝 fanquake opened a pull request: "guix: Enable CET for `glibc` package"
(https://github.com/bitcoin/bitcoin/pull/31121)
Pulled from #30685. This doesn't need to wait for anything.

💬 laanwj commented on pull request "guix: Enable CET for `glibc` package":
(https://github.com/bitcoin/bitcoin/pull/31121#issuecomment-2425148227)
Concept ACK

📝 sdaftuar opened a pull request: "cluster mempool: Implement changeset interface for mempool"
(https://github.com/bitcoin/bitcoin/pull/31122)
It became clear while working on cluster mempool that it would be helpful for transaction validation if we could consider a full set of proposed changes to the mempool -- consisting of a set of transactions to add, and a set of transactions (ie conflicts) to simultaneously remove -- and perform calculations on what the mempool would look like if the proposed changes were to be applied. Two specific examples of where we'd like to do this:

- Determining if ancestor/descendant/TRUC limits would
...

💬 mzumsande commented on issue "Listen on random port by default (not 8333)":
(https://github.com/bitcoin/bitcoin/issues/31036#issuecomment-2425156768)
> Downsides:

Another downside would be that finding an initial batch of peers would take longer. GETADDR answers will include many addresses that are not reachable, whereas DNS seeders only resolve to addresses of nodes that the crawler could reach very recently. I would expect that there would be quite a noticeable effect.