✅ fanquake closed an issue: "React-native. const { STATUS_CODES } = require('http'); Unable to resolve module http"
(https://github.com/bitcoin/bitcoin/issues/27644)

💬 Sjors commented on pull request "init: add MALLOC_ARENA_MAX=1 to systemd":
(https://github.com/bitcoin/bitcoin/pull/27642#issuecomment-1546100497)
From the doc:

> However, in Bitcoin Core very little parallel allocation happens, so the impact is expected to be small or absent.

The "expected to be" could use some benchmarks before making it a default. Does the problem only happen with high `rpcthreads` or equally with other (potentially more common) settings like a high `-dbcache` or `-maxmempool`?

📝 Madgregory123 opened a pull request: "W3CAdd files via upload"
(https://github.com/bitcoin/bitcoin/pull/27645)


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 and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improve coverage are always welcome.
* All other changes should have accomp
...

💬 instagibbs commented on pull request "Introduce field element and group element classes to test_framework/key.py":
(https://github.com/bitcoin/bitcoin/pull/26222#issuecomment-1546111610)
Reads fine to me from my not-even-cryptographer-on-tv level of understanding.

Goes from 27 to 31 seconds on my machine with the precomputed table, which is very easy to understand.

🤔 mzumsande reviewed a pull request: "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count"
(https://github.com/bitcoin/bitcoin/pull/27511#pullrequestreview-1424964754)
Code Review ACK c505611f08a850acb97dea9cc03b36aa468929ca, I would use this rpc a lot in my work.

I also think that a `totals` field would be useful.

I don't think that a split-up with respect to "terrible" would be necessary, because in my opinion "terrible" is an internal property used only in address relay / addrman collisions that is not so critical to expose because it isn't used for the main purpose of addrman: selecting peers for outbound connections

💬 jonatack commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/27511#issuecomment-1546119966)
> "terrible" is an internal property

It's exposed in RPC getnodeaddresses and CLI -addrinfo to return only non-IsTerrible peers to users.

💬 sangaman commented on pull request "init: add MALLOC_ARENA_MAX=1 to systemd":
(https://github.com/bitcoin/bitcoin/pull/27642#issuecomment-1546123761)
> Does the problem only happen with high `rpcthreads` or equally with other (potentially more common) settings like a high `-dbcache` or `-maxmempool`?

From my understanding/reading of `MALLOC_ARENA_MAX` it affects memory usage where multiple threads are concerned. `rpcthreads` is the only configuration option afaict that impacts the number of threads running. Increasing `dbcache` or `maxmempool` wouldn't create new threads, only increase a set limit of RAM usage, so I don't think those alone
...

✅ hebasto closed a pull request: "W3CAdd files via upload"
(https://github.com/bitcoin/bitcoin/pull/27645)

📝 hebasto locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/27645)


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 and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improve coverage are always welcome.
* All other changes should have accomp
...

💬 sangaman commented on pull request "init: add MALLOC_ARENA_MAX=1 to systemd":
(https://github.com/bitcoin/bitcoin/pull/27642#issuecomment-1546129601)
More relevant discussion here: https://github.com/bitcoin/bitcoin/issues/6876#issuecomment-151045764

💬 mzumsande commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/27511#issuecomment-1546135453)
> It's used in RPC getnodeaddresses and CLI -addrinfo to return only non-IsTerrible peers to users. (I meant pre and post that filtering in the suggestion above).

in my opinion unfortunately. I think this was mostly coincidental, because #13152 chose to reuse an addrman function used only for GETADDR answers before. Then #21595 used that output without any mentioning of that restriction, and the doc was only adjusted a year later in #24370. I think that ideally we never should have exposed th
...

💬 Sjors commented on pull request "test: avoid sporadic MINIMALDATA failure in feature_taproot.py (fixes #27595)":
(https://github.com/bitcoin/bitcoin/pull/27631#issuecomment-1546138587)
Alternatively you can fix `__coerce_instance`:

```py
elif isinstance(other, (bytes, bytearray)):
if len(other) == 1 and 0 <= other[0] <= 16:
other = bytes([CScriptOp.encode_op_n(other[0])])
else:
other = CScriptOp.encode_op_pushdata(other)
```

This passes your example, and doesn't seem to break any other test.

cc @MarcoFalke

💬 glozow commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1546138724)
> > With the "tree only" topology requirement I feel more comfortable having this branch exist, but still don't think this is something to merge yet / backport.
>
> Do you think this is not ready to merge / backport yet because of the unexpected edge cases that showed up with the previous, less restrictive iteration, leading to the expectation that something might show up for "tree only" too? Or is there already a known issue for "tree only"?
>
> I am looking at this from a Lightning persp
...

💬 Sjors commented on pull request "init: add MALLOC_ARENA_MAX=1 to systemd":
(https://github.com/bitcoin/bitcoin/pull/27642#issuecomment-1546143791)
@sangaman maybe `-par` as well? It defaults to the number of cores - 1 and is capped at 15.

💬 jonatack commented on pull request "Raise on invalid -debug and -loglevel config options":
(https://github.com/bitcoin/bitcoin/pull/27632#discussion_r1192702584)
Thanks @vasild and @ryanofsky. Updated to use `util::Result` instead.

👍 hebasto approved a pull request: "build: Fix shared lib linking for darwin with lld"
(https://github.com/bitcoin/bitcoin/pull/27628#pullrequestreview-1425029885)
ACK d576d69e139ba5724c87d405d6161dc062ddc6f7

Suggesting to move the mingw-related comment "By default, libtool for mingw refuses..." below the `*mingw*)` case.

This commit has been already incorporated into the https://github.com/bitcoin/bitcoin/pull/21778 draft.

💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1192715879)
Would you also prefer passing this `validation_notifications` class to the other users of these notification functions, even if they only use one of the methods?

👍 john-moffett approved a pull request: "util: implement `noexcept` move assignment & move ctor for `prevector`"
(https://github.com/bitcoin/bitcoin/pull/27334#pullrequestreview-1425013056)
Approach ACK bfb9291a8661fe5b26c14ed755cfa89d27c37110

💬 john-moffett commented on pull request "util: implement `noexcept` move assignment & move ctor for `prevector`":
(https://github.com/bitcoin/bitcoin/pull/27334#discussion_r1192715559)
Maybe a good idea to:

```C++
if (!other.is_direct()) {
other._union.indirect_contents.indirect = nullptr;
}
```

in case anyone tries to `free` it?

💬 john-moffett commented on pull request "util: implement `noexcept` move assignment & move ctor for `prevector`":
(https://github.com/bitcoin/bitcoin/pull/27334#discussion_r1192703890)
I'd normally agree, but the documentation says that `prevector` is meant as a `drop-in replacement for std::vector<T>`, so I think it's best to leave it.

💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#issuecomment-1546170005)
So that earlier attempt would solicit a compact block from a (BIP 152) low-bandwidth mode peer, in response to it announcing a header. That's similar to what we already do in `HeadersDirectFetchBlocks`, but allowing more than 1 in transit?

Whereas in this PR we focus on the high-bandwidth mode peers, which are [allowed to send us](https://github.com/bitcoin/bips/blob/master/bip-0152.mediawiki#intended-protocol-flow) a `cmpctblock` message straight away (rather than waiting for us to ask). Rig
...