🤔 sipa reviewed a pull request: "net/rpc: Report inv information for debugging"
(https://github.com/bitcoin/bitcoin/pull/33448#pullrequestreview-3257761423)
utACK 2738b63e025d240618b3c72c28243c3e4d7d9c79

💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/33445#discussion_r2372167035)
Your analysis seems correct to me. However, I prefer being explicit here. Otherwise, if any user-declared constructor is added in the future, the default constructor would not be generated.

💬 instagibbs commented on pull request "rpc: Always return per-wtxid entries in submitpackage tx-results":
(https://github.com/bitcoin/bitcoin/pull/33427#issuecomment-3323848596)
ACK cad9a7fd7370f6df38370569f9d2de6ac6b7137a

💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#issuecomment-3323854411)
rfm?

💬 Crypt-iQ commented on pull request "log: reduce excessive "rolling forward" messages during block replay":
(https://github.com/bitcoin/bitcoin/pull/33443#discussion_r2372212754)
I think `Error` and `Warning` should be rate limited as they currently are. I am ok with a critical level and macro, but I think it might need more conceptual review and feedback vs. what this PR aims to do?

💬 sipa commented on pull request "rpc: addpeeraddress: throw on invalid IP":
(https://github.com/bitcoin/bitcoin/pull/33430#issuecomment-3323871396)
utACK 316a0c513278d53cb25f42ea502d20691962aad6

💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/33445#discussion_r2372218235)
Thanks! Taken.

💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/33445#discussion_r2372218950)
Thanks! Fixed.

💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/33445#issuecomment-3323881144)
The feedback from @vasild has been addressed.

💬 sipa commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2372244246)
In commit "net: change FindNode() to not return a node and rename it"

I think this name change is confusing, because `CNode::m_addr_name` is not necessarily an address/port string; in the case of manually-connected nodes (`-addnode` and friends), it can also be a hostname.

Checking the codebase to verify this made me realize that the RPC help for `getpeerinfo` is also wrong in this regard.

💬 fanquake commented on pull request "tor: enable PoW defenses for automatically created hidden services":
(https://github.com/bitcoin/bitcoin/pull/33414#issuecomment-3323930161)
@laanwj you might have some thoughts here?

💬 hebasto commented on pull request "CPack":
(https://github.com/bitcoin/bitcoin/pull/33455#issuecomment-3323935593)
> Run `cpack` from the build directory after a successful build to select multiple package generators:
>
> ```shell
> cmake --build .
> cpack -G '7Z;NSIS64'
> ```
>
> Or build the `package` and/or `package_source` targets:
>
> ```shell
> cmake --build . --target package
> cmake --build . --target package_source
> ```

Is the reproducibility of these commands guaranteed, or can it be enforced somehow?

> TODO:
>
> * [ ] Windows start menu entries
>
> * [ ] Option
...

🚀 fanquake merged a pull request: "ci: disable cirrus cache in 32bit arm job"
(https://github.com/bitcoin/bitcoin/pull/33302)

💬 maflcko commented on pull request "log: reduce excessive "rolling forward" messages during block replay":
(https://github.com/bitcoin/bitcoin/pull/33443#discussion_r2372269835)
I also agree that errors and warnings should be rate limited. Errors are fatal, so should only be printed once, before the program aborts. Warnings should be rare, unless there is a coding bug, in which case the rate limiting seems useful.

The only place where side-stepping the rate limiting makes sense is the info level. If `LogEssential` is named too harmless, it could be named `LogInfoWithoutRateLimiting`, so that the slightly verbose name discourages using it in places where it is not app
...

💬 fanquake commented on pull request "ci: disable cirrus cache in 32bit arm job":
(https://github.com/bitcoin/bitcoin/pull/33302#issuecomment-3323947804)
Backported to 30.x in #33424.

👍 instagibbs approved a pull request: "net/rpc: Report inv information for debugging"
(https://github.com/bitcoin/bitcoin/pull/33448#pullrequestreview-3257937613)
ACK 2738b63e025d240618b3c72c28243c3e4d7d9c79

💬 instagibbs commented on pull request "net/rpc: Report inv information for debugging":
(https://github.com/bitcoin/bitcoin/pull/33448#discussion_r2372277388)
nit: double-checked before and after to check effects of test

```
diff --git a/test/functional/p2p_leak_tx.py b/test/functional/p2p_leak_tx.py
index 42e586fca3..d302a68ebf 100755
--- a/test/functional/p2p_leak_tx.py
+++ b/test/functional/p2p_leak_tx.py
@@ -39,4 +39,9 @@ class P2PLeakTxTest(BitcoinTestFramework):
inbound_peer.last_message.pop("inv", None)

+ assert_equal(self.gen_node.getrawmempool(False, True), {'txids': [], 'mempool_sequence': 1})
+ pi = self
...

💬 fanquake commented on pull request "doc: remove unrelated `bitcoin-wallet` binary from `libbitcoin_ipc` description":
(https://github.com/bitcoin/bitcoin/pull/33459#issuecomment-3323956409)
Assuming these docs came from #10102, which introduces a `bitcoin-wallet` process. Removing seems fine for now.

🚀 fanquake merged a pull request: "doc: remove unrelated `bitcoin-wallet` binary from `libbitcoin_ipc` description"
(https://github.com/bitcoin/bitcoin/pull/33459)

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2372295604)
`GraphIndex` is unsigned, so that would literally always be true?