✅ 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
...

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#issuecomment-1546173220)
> This PR makes that 3.

Makes it 3 parallel block downloads, first can be of any kind, the 2nd and 3rd must be compact block announcements from high badwidth peers.

> I am confused by the "it will not help" phrase

This PR does not take advantage of low-bandwidth compact block peers message patterns, in other words.

💬 theuni commented on pull request "build: Fix shared lib linking for darwin with lld":
(https://github.com/bitcoin/bitcoin/pull/27628#issuecomment-1546173685)
Updated the original commit to quote `"yes"` to match the others.

Also fixed up the comments as @hebasto suggested.

💬 Sjors commented on issue "Parallel compact block download":
(https://github.com/bitcoin/bitcoin/issues/25258#issuecomment-1546184277)
If we wait a little bit before the second attempt, and if we track which peers have since announced the header, we could pick from a larger set. Randomly and maybe with a preference for outbound. We can rely on high bandwidth mode compact block peer(s) for speed, but then for reliability use a bit more patience. Maybe we could even do one attempt at a getting compact block and then a few seconds later ask for a regular block.

💬 sr-gi commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#discussion_r1192731365)
I think those are rate limited so they don't overflow in less than two minutes, after which they are not relevant anymore.

For outbound connections, INVs are sent out every 2s (`OUTBOUND_INV_BROADCAST_INTERVAL`), while for inbound, they are sent every 5s (`OUTBOUND_INV_BROADCAST_INTERVAL`). In a two-minute timespan, that results in 2100 and 840 INVs at max¹, respectively. The `m_recently_announced_invs` rolling filter is only checked in `FindTxForGetData` after making sure the trac
...

💬 instagibbs commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1546196121)
re:topology restrictions beyond generalized ancestor packages, is the thought to restrict the typologies at a higher level, vs AcceptMultipleTransactions? Noticed this branch is restricting at RPC layer. I presume it would also require tree structure in https://github.com/bitcoin/bitcoin/pull/26711 ?

💬 jamesob commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1546204526)
> I became convinced that this is unsafe

I'm curious, what's the worst-case failure here? Is it that a participating miner's mempool gets screwed up? or that replacement may not happen properly? or that pinning becomes an issue?

In other words, I'm wondering if the failure is tolerable in the sense that the damage would be contained to the user trying to submit a dumb (or "not topoolgically sensical") package?

I was enthusiastic about a small change that would open up this functionality
...