💬 Retropex commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1680470224)
> Using this line of thinking inscriptions should also be removed from isStandard becuase of the statements made at [Bitcoin 2023 "The Great Ordinal Debate"](https://youtu.be/AdYsQ2DWt5M?t=191) "There is no value here ... it's like an objective crap" while referring to inscriptions.
>
>>

Precisely I made the request via [Bitcoin-dev](https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2023-July/021805.html) and via an PR #27589 (I know the PR are not intended for that)

Unfortunately
...

🚀 fanquake merged a pull request: "ci: Fix macOS-cross SDK rsync"
(https://github.com/bitcoin/bitcoin/pull/28273)

💬 MarcoFalke commented on pull request "test: rpc: add last block announcement time to getpeerinfo result":
(https://github.com/bitcoin/bitcoin/pull/27052#discussion_r1295821775)
This is fragile, because when the type of `oldest_block_announcement` changes, the log output will change, too, without a compile error. It would be better to use `Ticks` or `TicksSinceEpoch`.

💬 MarcoFalke commented on pull request "test: rpc: add last block announcement time to getpeerinfo result":
(https://github.com/bitcoin/bitcoin/pull/27052#discussion_r1295822613)
It is possible to write a clang-tidy check for this, but I am not sure if people want this. cc @theuni


```diff
diff --git a/contrib/devtools/bitcoin-tidy/CMakeLists.txt b/contrib/devtools/bitcoin-tidy/CMakeLists.txt
index 35e60d1d87..b523692b12 100644
--- a/contrib/devtools/bitcoin-tidy/CMakeLists.txt
+++ b/contrib/devtools/bitcoin-tidy/CMakeLists.txt
@@ -14,7 +14,10 @@ find_program(CLANG_TIDY_EXE NAMES "clang-tidy-${LLVM_VERSION_MAJOR}" "clang-tidy
message(STATUS "Found LLVM ${LLVM
...

💬 kristapsk commented on pull request "test: rpc: add last block announcement time to getpeerinfo result":
(https://github.com/bitcoin/bitcoin/pull/27052#issuecomment-1680531024)
Concept ACK

📝 MarcoFalke opened a pull request: "ci: Refactor: Remove CI_USE_APT_INSTALL"
(https://github.com/bitcoin/bitcoin/pull/28278)
Also, for github CI:
* restore MAKEJOBS to the same value as in cirrus.yml.
* remove cirrus-only PACKAGE_MANAGER_INSTALL.

💬 MarcoFalke commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1295837730)
Fixed in https://github.com/bitcoin/bitcoin/pull/28278

💬 MarcoFalke commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1295837848)
Fixed in https://github.com/bitcoin/bitcoin/pull/28278

💬 MarcoFalke commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1295837941)
Fixed in https://github.com/bitcoin/bitcoin/pull/28278

💬 MarcoFalke commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1295854996)
Fixed in https://github.com/bitcoin/bitcoin/pull/28237

🤔 Sjors reviewed a pull request: "net: transport abstraction"
(https://github.com/bitcoin/bitcoin/pull/28165#pullrequestreview-1580205899)
The new approach in 384c798cc3836558463e88ec7f563b236f50bf22 is a nice improvement.

I'll continue reviewing from 09972b518bed5809d37c79fb0ddef034dce27ba0 onwards after you've had a chance to comment on my `more` comment.

💬 Sjors commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1295642780)
384c798cc3836558463e88ec7f563b236f50bf22

The `have_next_message` argument is confusing and seems contradictory to the goal of separating the bytes stream from messages.

> It is set by the caller when they know they have
> * another message ready to send.

In this commit it's always `false`, so if we need it at all, maybe delay its introduction to 09972b518bed5809d37c79fb0ddef034dce27ba0? That's a better place to explain how it relates to setting `Sock::SEND` (if it can't be in its o
...

💬 Sjors commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1295649173)
384c798cc3836558463e88ec7f563b236f50bf22 could we annotate the tuple?

```cpp
std::tuple<Span<const uint8_t> /*to_send*/, bool /*more*/, const std::string& /* m_type */>
```

💬 brunoerg commented on pull request "fuzz: call `LookupSubNet` before calling `Ban` with a subnet":
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1680589999)
Rebased.

💬 vasild commented on pull request "Make all networking code mockable":
(https://github.com/bitcoin/bitcoin/pull/21878#issuecomment-1680606505)
`b497200c7a...80d9dfd0f0`: rebase due to conflicts

👍 hebasto approved a pull request: "ci: Refactor: Remove CI_USE_APT_INSTALL"
(https://github.com/bitcoin/bitcoin/pull/28278#pullrequestreview-1580616607)
ACK fa04623bbb284b35b27b2575b217341ab2e81e48.

Thank you!

💬 vasild commented on pull request "Make it possible to disable Tor binds and abort startup on bind failure":
(https://github.com/bitcoin/bitcoin/pull/22729#issuecomment-1680635537)
`7180859e7e...6a51eaf4a9`: rebase, did not lose any relevance through time

An alternative to this is described in:
https://github.com/bitcoin/bitcoin/pull/22729#issuecomment-1313383147
that would be more complicated, but maybe it would be more appealing to reviewers?

> only concern is if there's a way around https://github.com/bitcoin/bitcoin/pull/22729#issuecomment-900506737 - the need for =onion to accept inbound connections wouldn't be intuitive to users and might break existing confi
...

💬 theuni commented on pull request "refactor: Enforce C-str fmt strings in WalletLogPrintf()":
(https://github.com/bitcoin/bitcoin/pull/28237#discussion_r1295934037)
Huh. For whatever reason, my clang-tidy-16 acts as if `IgnoreUnlessSpelledInSource` is set. That's weird.

It came from here: https://github.com/llvm/llvm-project/releases/download/llvmorg-16.0.0/clang+llvm-16.0.0-x86_64-linux-gnu-ubuntu-18.04.tar.xz

I can't reproduce with clang-tidy-15. clang-query-16 only matches if traversal is set to `IgnoreUnlessSpelledInSource`.

> $ /opt/clang+llvm-15.0.2-x86_64-unknown-linux-gnu/bin/clang-query wallet/wallet.cpp -- -DHAVE_BUILD_INFO -I. -I./minisk
...

💬 theuni commented on pull request "refactor: Enforce C-str fmt strings in WalletLogPrintf()":
(https://github.com/bitcoin/bitcoin/pull/28237#issuecomment-1680641624)
ACK fa60fa3b0cba4a30726af8e0e9d1e84e14849eda

💬 MarcoFalke commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1295951009)
This is still up-for-grabs, if someone wants to take it :)