💬 jonatack commented on pull request "p2p: skip netgroup diversity follow-up":
(https://github.com/bitcoin/bitcoin/pull/27467#issuecomment-1545982776)
Per the first line of the diff and the first line of the PR description, a well-named data structure is preferable to one with an out-of-date name that needs a comment to explain what it now does after its role changed. Our p2p code is sufficiently critical for this to be important, and for that reason we often see review comments requesting clearer naming in p2p changes.

I think the canned response in https://github.com/bitcoin/bitcoin/pull/27467#issuecomment-1521500955, and its use, is pro
...

💬 Sjors commented on pull request "p2p: Increase tx relay rate":
(https://github.com/bitcoin/bitcoin/pull/27630#issuecomment-1545997858)
When this limit was introduced in https://github.com/bitcoin/bitcoin/commit/f2d3ba73860e875972738d1da1507124d0971ae5 it was pointed out that the _per-peer_ rate doesn't have to be that high:

Limits the amount of transactions sent in a single INV to
7tx/sec (and twice that for outbound); this limits the
harm of low fee transaction floods, gives faster relay
service to higher fee transactions. The 7 sounds lower
than it really is because received advertisements need
not be sent,
...

💬 RandyMcMillan commented on pull request "p2p: skip netgroup diversity follow-up":
(https://github.com/bitcoin/bitcoin/pull/27467#discussion_r1192600918)
Since we are talking about direction (outbound) - either removing it or changing it to "toward" makes more sense - now that you mention it.

Note: Since the comment mentions "groups" it seems like a typo.

💬 RandyMcMillan commented on issue "guiutil.cpp: formatNiceTimeOffset":
(https://github.com/bitcoin-core/gui/issues/728#issuecomment-1546040052)
Please post a screen shot - for clarity.

👍 theuni approved a pull request: "depends: no-longer nuke libc++abi.so* in native_clang package"
(https://github.com/bitcoin/bitcoin/pull/27493#pullrequestreview-1424884854)
Sure. utACK no-op 9ae854da193f3c4bda38a75e96f9b989b289baab.

💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#issuecomment-1546082245)
> In contrast to previous effort, it will not help if subsequent peers send block headers only

Can you elaborate on this? Are you saying that falling back to full block download is pointless?

💬 instagibbs commented on pull request "Introduce field element and group element classes to test_framework/key.py":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1192652578)
it's printing `FE`s which have defined a hex __str__, seems to work:

```
(Pdb) print(SECP256K1_G)
(79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798,483ada7726a3c4655da4fbfc0e1108a8fd17b448a68554199c47d08ffb10d4b8)
```

💬 instagibbs commented on pull request "Introduce field element and group element classes to test_framework/key.py":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1192653577)
it's defined by `FE` as hex, so should be fine?

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#issuecomment-1546091957)
@Sjors Basically previous efforts would allow the peer to send a header, not a compact block, and we would fire off an additional `getdata` for the compact block. See this block of code: https://github.com/bitcoin/bitcoin/pull/10984/files#diff-6875de769e90cec84d2e8a9c1b962cdbcda44d870d42e4215827e599e11e90e3R1411

IMO it's unlikely to make an impact, as high-bandwidth peers are already demonstrably fast, and allows us to control better who takes up the additional slots.

⚠️ educob opened an issue: "React-native. const { STATUS_CODES } = require('http'); Unable to resolve module http"
(https://github.com/bitcoin/bitcoin/issues/27644)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

After intalling the latest version ^4.1.0 of bitcoin-core in my react-native project i get Unable to resolve module http

It doesn't happen in node projects.

The most weird part is that module http doesn't really exist as it was malicious and removed. ( [](https://www.npmjs.com/package/http) )

Thanks.

### Expected behaviour

import Client from 'bitcoin-core' should work as expecte
...

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