💬 LarryRuane commented on pull request "util: generalize accounting of system-allocated memory in pool resource":
(https://github.com/bitcoin/bitcoin/pull/27748#discussion_r1209457660)
I copied this function from `memusage.h` ([here](https://github.com/bitcoin/bitcoin/blob/6cf47a8f44f96099a84e5c6e628e3499045e024d/src/memusage.h#L52)). I'm hoping not to duplicate this function, even though it's pretty small. `pool.h` can't include `memusage.h` because that creates a circular dependency. I'll try to figure out a way to improve this and also of course I welcome suggestions.

Anyway, if we need to duplicate this function, I'll remove this comment, I agree it's not useful.

💬 joostjager commented on issue "Allow accepting non-standard transactions on mainnet via local rpc":
(https://github.com/bitcoin/bitcoin/issues/27768#issuecomment-1567353059)
Standardising the annex would be great, but how likely is it that that will be done in policy only without a soft fork? I haven't followed L1 closely, but it seems that soft forks aren't an easy thing to pull off.

💬 MarcoFalke commented on pull request "Fix `#include`s in `src/wallet`":
(https://github.com/bitcoin/bitcoin/pull/27759#issuecomment-1567357663)
May be good to report the crash upstream if you can reduce the reproducer

🤔 hebasto reviewed a pull request: "lint: stop ignoring LIEF imports"
(https://github.com/bitcoin/bitcoin/pull/27507#pullrequestreview-1449741321)
Post-merge ACK 015cc5e588fa25f65f6ea2ed03701def8dfd5444.

💬 pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1567380170)
Updates:
- Working on `fuzzer` failure.

💬 brunoerg commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1209501764)
We want to protect a peer if it's the only one for its network. However, we can run a node supporting just one network. In this case, I suppose that calling `GetFullOutboundAndManualCount` might be unnecessary, we would iterating the whole `m_nodes` with no need.

In this case, I think we could have a function like `SupportsMultipleNetworks` to check whether we support more than one network, e.g.:
```cpp
bool CConnman::SupportsMultipleNetworks() const
{
int count = 0;
for (int n =
...

💬 brunoerg commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1567445222)
Just a suggestion, but I think we could cover `GetFullOutboundAndManualCount` in fuzz.

```diff
diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp
index f81658b83..f5867f8b4 100644
--- a/src/test/fuzz/connman.cpp
+++ b/src/test/fuzz/connman.cpp
@@ -128,4 +128,6 @@ FUZZ_TARGET_INIT(connman, initialize_connman)
(void)connman.GetTotalBytesSent();
(void)connman.GetTryNewOutboundPeer();
(void)connman.GetUseAddrmanOutgoing();
+ const Network net{fuzzed_data
...

📝 xeroxparc opened a pull request: "BITCOIN EMERGENCY MODE - Lets talk about ethics and integrity"
(https://github.com/bitcoin/bitcoin/pull/27776)
Im not saying my idea is the solution. I wrote it so that the internal team can have a real honest conversation about the ethic and integrity of the project.

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch a
...

✅ fanquake closed a pull request: "BITCOIN EMERGENCY MODE - Lets talk about ethics and integrity"
(https://github.com/bitcoin/bitcoin/pull/27776)

📝 fanquake locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/27776)
Im not saying my idea is the solution. I wrote it so that the internal team can have a real honest conversation about the ethic and integrity of the project.

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch a
...

💬 brunoerg commented on pull request "p2p, rpc, test: `getpeerinfo` improv + add test coverage for invalid command":
(https://github.com/bitcoin/bitcoin/pull/26366#issuecomment-1567622316)
CI failure seems unrelated to these changes.

💬 brunoerg commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1209611483)
In `377e81e2e409470d00e69247d0d9e980773ab73e`, I suppose this verification could be avoided. In `Select`,
`new_only` is false by default and `preferred_net` is optional and it will only have a value when `MaybePickPreferredNetwork`returns true. So, if `preferred_net` doesn't have any value, I suppose that `addrman.Select(false, preferred_net)` and `addrman.Select()` will have the same effect.

```diff
@@ -1901,15 +1915,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string>
...

📝 MarcoFalke opened a pull request: "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN"
(https://github.com/bitcoin/bitcoin/pull/27777)
This should prevent the persistent workers from running out of disk space. Containers are already removed, but not images. This is required since CI images are built and cached.

💬 MarcoFalke commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#issuecomment-1567867741)
This should fix the out-of-space error, see https://cirrus-ci.com/task/6118746146734080?logs=ci#L4903

💬 MarcoFalke commented on pull request "p2p, rpc, test: `getpeerinfo` improv + add test coverage for invalid command":
(https://github.com/bitcoin/bitcoin/pull/26366#issuecomment-1567868668)
> CI failure seems unrelated to these changes.

Thanks, fixed in https://github.com/bitcoin/bitcoin/pull/27777

💬 MarcoFalke commented on pull request "Fix rpc_getblockfrompeer intermittency by introducing 'getblockfileinfo' RPC command":
(https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-1567870784)
What about a one-line fix to clear the test issue and then a follow-up with the RPC, which may take longer to review than a trivial one-line patch?

💬 MarcoFalke commented on issue "index: ThreadSanitizer: data race on vptr ":
(https://github.com/bitcoin/bitcoin/issues/27355#issuecomment-1567890421)
A quick hack in the meantime could be:

```
sed -i 's/constexpr auto timeout{10s};/constexpr auto timeout{300s};/g' ./src/test/blockfilter_index_tests.cpp ./src/test/txindex_tests.cpp
```

💬 martinus commented on pull request "util: generalize accounting of system-allocated memory in pool resource":
(https://github.com/bitcoin/bitcoin/pull/27748#discussion_r1209810958)
About that function, I do not think that it is accurate any more. I wrote a different version of that estimation that makes use of `malloc_usable_size`, and that way I get different values, see here: https://godbolt.org/z/3a6E57dPx

It might be worth having a look if using that estimation is more accurate

💬 MarcoFalke commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#issuecomment-1567905527)
Looks like it is working, but not claiming a lot of space:


https://cirrus-ci.com/task/6656469207089152?logs=ci#L408

```
Total reclaimed space: 949.4MB

💬 MarcoFalke commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#issuecomment-1567995893)
Not sure what is going on. Seems there are unreachable zombie containers in the overlay2 directory. Let's try `podman` instead.