💬 fanquake commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1664291833)
From: https://cirrus-ci.com/task/6097246463197184

```bash
[100%] Building CXX object CMakeFiles/bitcoin-tidy-tests.dir/example_logprintf.cpp.o
/tmp/cirrus-ci-build/contrib/devtools/bitcoin-tidy/example_logprintf.cpp:65:15: warning: Unterminated format string used with LogPrintf [bitcoin-unterminated-logprintf]
LogPrintf("hello world!");
^
\n
/tmp/cirrus-ci-build/contrib/devtools/bitcoin-tidy/example_logprintf.cpp:22:68: note: expanded from ma
...

💬 fanquake commented on pull request "fuzz: addrman, avoid `ConsumeDeserializable` when possible":
(https://github.com/bitcoin/bitcoin/pull/27918#discussion_r1283451002)
It'd be good to avoid making unrelated changes like this (deleting the file ending newline).

🚀 fanquake merged a pull request: "fuzz: addrman, avoid `ConsumeDeserializable` when possible"
(https://github.com/bitcoin/bitcoin/pull/27918)

💬 furszy commented on pull request "net: don't lock cs_main while reading blocks in net processing":
(https://github.com/bitcoin/bitcoin/pull/26326#discussion_r1283446866)
tiny nit: would be good to initialize the variable to nullptr and false.

🤔 furszy reviewed a pull request: "net: don't lock cs_main while reading blocks in net processing"
(https://github.com/bitcoin/bitcoin/pull/26326#pullrequestreview-1561461768)
Code ACK 3e93e6b7

💬 furszy commented on pull request "net: don't lock cs_main while reading blocks in net processing":
(https://github.com/bitcoin/bitcoin/pull/26326#discussion_r1283443148)
tiny nit: would be good to add a jump line here.

💬 stickies-v commented on pull request "kernel: Prune leveldb headers":
(https://github.com/bitcoin/bitcoin/pull/28186#discussion_r1283455714)
I generally like your approach of keeping it as clean a move as possible, and in the other commits am okay with foregoing some slight improvements because of that. Here, though, I feel like we're making both the interface and implementation way too complex because of it, so personally I'd much rather make some more changes.

🚀 fanquake merged a pull request: "doc: Clarify -datacarriersize, add -datacarriersize=2 tests"
(https://github.com/bitcoin/bitcoin/pull/27832)

💬 fanquake commented on pull request "ci: Move ASan USDT to persistent_worker":
(https://github.com/bitcoin/bitcoin/pull/28161#discussion_r1283470705)
> sed

🥲

💬 paplorinc commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1283470743)
won't this pass if no exception is raised?
Shouldn't we add something like either:
```Python
try:
self.do_test_k_of_n(1, 1000)
assert False, "Expected exception was not raised for 1-of-1000 test"
except JSONRPCException as e:
```

or via unittest:
```Python
# Test 1-of-1000, should not succeed
with self.assertRaises(JSONRPCException) as cm:
self.do_test_k_of_n(1, 1000)
self.assertEqual(cm.exception.error["code"], -5)
self.assertIn("Cannot have 1000 keys in multi_a; mus
...

💬 theuni commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1664316581)
> From: https://cirrus-ci.com/task/6097246463197184
> ...

Nice, thanks!

💬 fanquake commented on pull request "build: Bump minimum supported Clang to clang-13":
(https://github.com/bitcoin/bitcoin/pull/28210#issuecomment-1664317915)
> This fails on macOS 11.0,

Yea, this'll at least need to wait for the macOS compiler bump, but the next time that happens (soon) it will be from 11 to 16, if not 17.

💬 fanquake commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1664319251)
Dropped the test commit back out.

💬 eriknylund commented on pull request "test: verify spend from 999-of-999 taproot multisig wallet":
(https://github.com/bitcoin/bitcoin/pull/28212#discussion_r1283479802)
Thanks for pointing this out 🙏 I agree it shouldn't be possible for this test to succeed so an exception should be raised.

💬 brunoerg commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#issuecomment-1664326798)
Concept ACK

💬 brunoerg commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#issuecomment-1664327227)
Concept ACK

🚀 fanquake merged a pull request: "ci: Move ASan USDT to persistent_worker"
(https://github.com/bitcoin/bitcoin/pull/28161)

💬 YellowRoseCx commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1664336561)
What's the root cause that makes a transaction need Replace-by-fee in the first place? Can we fix that?


RBF is a feature for consenting adults. If you don’t want to participate in it, you don’t need to.
Your passion for it isn’t a reason to force others into using it in transactions by default that don’t involve you.

💬 Sjors commented on pull request "BIP324 connection support":
(https://github.com/bitcoin/bitcoin/pull/28196#issuecomment-1664337914)
Concept ACK

Mostly happy with b7a7ed70d06ab3f994ff58e3a0c99105ee88ab6e. Minor inline suggestions and/or making sure I understand what the code is doing.

I also checked that all intermediate commits compile and ran the new fuzzer for a few hours (but didn't study it very carefully).

* 1937a5fcf795149c44b7f4f016c05000ac3adaf9: `-maxsendbuffer` help could be changed to say "Maximum per-connection memory use for the send buffer"
* 68e48a0185751d24eecb194b8efd7028c8b590f3: can you elaborate
...

🤔 Sjors reviewed a pull request: "net: transport abstraction"
(https://github.com/bitcoin/bitcoin/pull/28165#pullrequestreview-1560842698)
Concept ACK

Mostly happy with b7a7ed70d06ab3f994ff58e3a0c99105ee88ab6e. Minor inline suggestions and/or making sure I understand what the code is doing.

I also checked that all intermediate commits compile and ran the new fuzzer for a few hours (but didn't study it very carefully).

* 1937a5fcf795149c44b7f4f016c05000ac3adaf9: `-maxsendbuffer` help could be changed to say "Maximum per-connection memory use for the send buffer"
* 68e48a0185751d24eecb194b8efd7028c8b590f3: can you elaborate
...