💬 mzumsande commented on pull request "p2p: Increase inbound capacity for block-relay only connections":
(https://github.com/bitcoin/bitcoin/pull/28463#issuecomment-1812811783)
> I'd argue that this is a significant change.

Hmm yes, in that comment I was thinking about the theoretical limit of all inbound slots being filled with tx-relaying peers. You are right, in practice that amount will be lower because some block-relay-only connections are made to us. In that sense, the introduction of block-relay-only peers likely came with a reduction in bandwidth for popular nodes that were already at capacity before.

One way to accommodate this would be to reduce the per
...

💬 TheCharlatan commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-1812821242)
Rebased 40e151697d3d1ed594364498d9e6220219d9b545 -> de6d79847537cd7d0dfc3e2dd7a57b39b4281b5a ([kernelInternalLib_5](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_5) -> [kernelInternalLib_6](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_6), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInternalLib_5..kernelInternalLib_6))
* Fixed merge conflict.

💬 MatthewLM commented on pull request "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes":
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-1812829602)
Could we use `UNKNOWN_n` where n denotes the op code hex/decimal such as `UNKOWN_FE` for `0xfe`?

💬 mzumsande commented on pull request "p2p: peer connection bug fixes":
(https://github.com/bitcoin/bitcoin/pull/28248#issuecomment-1812859356)
> Empirically, the regression is that we're actively adding addnode peers as outbound ones, and keeping them there.

Oh, I thought you were talking about a race at startup - since the extra outbound should only kick in after 5 minutes, I think that should be enough to make connections to the addnode peers.

I think what might be happening later is that an addnoded peer disconnects us, and if it was the only one from its network then there could be a race between picking another network-rela
...

💬 theuni commented on pull request "Remove version field from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/28878#issuecomment-1812867807)
Concept ACK.

Is this next after #28438? Or something else first?

💬 Riahiamirreza commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1812892050)
I'm working on adding tests, but I'm not sure where is the correct place for putting tests. Should I put tests in `RawTransactionsTest.getrawtransaction_verbositry_tests` in the `tests/functional/rpc_rawtransaction.py`? I need to have a transaction of type P2SH in the chain and check whether the result has the correct `redeemScript` field? Is this approach correct?

👍 fanquake approved a pull request: "bench: Update nanobench to 4.3.11"
(https://github.com/bitcoin/bitcoin/pull/28877#pullrequestreview-1732483272)
ACK fe434a469534766f18d7560d968deed37193835f - have not tested.

📝 fanquake opened a pull request: "doc: remove x86_64 build assumption from depends doc"
(https://github.com/bitcoin/bitcoin/pull/28884)
This dates from the introduction of depends, and has not been the case for some time now.

💬 sr-gi commented on pull request "net: Attempts to connect to all resolved addresses on `addnode`":
(https://github.com/bitcoin/bitcoin/pull/28834#issuecomment-1812964820)
There was an issue with the interaction of the change and using a proxy. It should be fixed now

💬 jonatack commented on pull request "p2p: peer connection bug fixes":
(https://github.com/bitcoin/bitcoin/pull/28248#issuecomment-1812965773)
@mzumsande I think so, yes, along with the other races mentioned in the commit message https://github.com/bitcoin/bitcoin/pull/28248/commits/21f8426c823fd83c40fd81f08ff2bf39ec6a4575. I'll add the case you describe to it (thanks!)

💬 fanquake commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1812967822)
Removed `LDFLAGS=-Wno-error=unused-command-line-argument` from the CI in the depends bump commit. Also added a commit to remove the no-longer used `pyhton3-setuptools` dep. Also rebsaed on recent merges. Still have to investigate the non-determinism.

💬 jonatack commented on pull request "p2p: peer connection bug fixes":
(https://github.com/bitcoin/bitcoin/pull/28248#issuecomment-1812970597)
I'd be interested to hear anyone's thoughts on this, from that commit message:

*Finally, there does not seem to be a reason to make block-relay or short-lived feeler connections to addnode peers, as the addnode logic will ensure we connect to them if they are up, within the addnode connection limit.*

💬 maflcko commented on pull request "doc: remove x86_64 build assumption from depends doc":
(https://github.com/bitcoin/bitcoin/pull/28884#issuecomment-1812987723)
lgtm ACK 821a8a11256ccf26fe8b0255cea4ec04dddd8f18

💬 jonatack commented on pull request "net: Attempts to connect to all resolved addresses on `addnode`":
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1394550037)
CI linter needs appeasing here
```suggestion
```

💬 sr-gi commented on pull request "net: Attempts to connect to all resolved addresses on `addnode`":
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1394555196)
I even left some debug comments there. My bad

💬 hebasto commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1813004612)
My SDK hash:
```
$ sha256sum Xcode-15.0-15A240d-extracted-SDK-with-libcxx-headers.tar.gz
6afb7c24461729167c5f5976ce56e774657a97b052f44f1618e9717da67fddb7 Xcode-15.0-15A240d-extracted-SDK-with-libcxx-headers.tar.gz
```
and Guix builds:
```
x86_64
fa1db57217fe8646c529f312ba5af6fee9f3085768d2033359b2f1ec9f1c4624 guix-build-cf8353a9cf9c/output/arm64-apple-darwin/SHA256SUMS.part
e64ee8bf9901e5a37cbb67cb5ed23daaa7d0086b256b64870b26a94d5ac3ee4e guix-build-cf8353a9cf9c/output/arm64-apple-da
...

👍 hebasto approved a pull request: "doc: remove x86_64 build assumption from depends doc"
(https://github.com/bitcoin/bitcoin/pull/28884#pullrequestreview-1732628851)
ACK 821a8a11256ccf26fe8b0255cea4ec04dddd8f18.

💬 theuni commented on pull request "Remove version field from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/28878#issuecomment-1813031718)
Playing around locally, I was also able to drop the include in:
```bash
$ git diff --stat
src/bitcoin-util.cpp | 1 -
src/coins.cpp | 1 -
src/core_read.cpp | 1 -
src/kernel/coinstats.cpp | 1 -
src/rest.cpp | 1 -
src/script/bitcoinconsensus.cpp | 1 -
src/zmq/zmqpublishnotifier.cpp | 1 -
```

(I didn't mess with the tests, maybe could drop some there too).

Also worth pointing out: libbitcoinkernel now only r
...

💬 brunoerg commented on pull request "fuzz: Delete wallet_notifications":
(https://github.com/bitcoin/bitcoin/pull/28882#issuecomment-1813037774)
> Can you add steps to reproduce?

The way it creates the wallet, calls `SetupDescriptorScriptPubKeyMans` without passing a key, it means that the function will create it internally.

```cpp
// Make a seed
CKey seed_key;
seed_key.MakeNewKey(true);
CPubKey seed = seed_key.GetPubKey();
assert(seed_key.VerifyPubKey(seed));

// Get the extended key
CExtKey master_key;
master_key.SetSeed(seed_key);

SetupDescriptorScriptPubKeyMans(master_key);
```

💬 TheCharlatan commented on pull request "doc: remove mention of missing bdb being a configure error":
(https://github.com/bitcoin/bitcoin/pull/28881#issuecomment-1813066650)
ACK 30bd4b1e4aee00edbe77da7c20bf80e28f0561cc