📝 Madgregory123 opened a pull request: "25.x"
(https://github.com/bitcoin/bitcoin/pull/27643)


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/27641#issuecomment-1545908173)
Looks like I goofed somehow when editing my branch to remove some whitespace and that resulted in closing this PR, that's what I get for creating the original commit in the github.com text editor. Reopened in #27642.

💬 Madgregory123 commented on pull request "25.x":
(https://github.com/bitcoin/bitcoin/pull/27643#issuecomment-1545908176)
🤔

✅ fanquake closed a pull request: "25.x"
(https://github.com/bitcoin/bitcoin/pull/27643)

📝 fanquake locked a pull request: "25.x"
(https://github.com/bitcoin/bitcoin/pull/27643)


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

📝 fanquake locked a pull request: "init: add MALLOC_ARENA_MAX=1 to systemd"
(https://github.com/bitcoin/bitcoin/pull/27641)
This adds the `MALLOC_ARENA_MAX=1` environment variable as suggested in https://github.com/bitcoin/bitcoin/blob/master/doc/reduce-memory.md#linux-specific to the systemd service file definition.

Without this env var, memory usage can grow significantly especially when `rpcthreads` is increased above its default value.

Closes #24542.

I have tested this change myself with positive results after dealing with memory consumption issues for a long time using systemd on a 8GB RAM raspi4. I fig
...

💬 sangaman commented on issue "Slow memory leak in v22.0?":
(https://github.com/bitcoin/bitcoin/issues/24542#issuecomment-1545911064)

> Seems like it would be better to add it to the documentation if anything...

It is in the documentation already at https://github.com/bitcoin/bitcoin/blob/master/doc/reduce-memory.md
The behavior was introduced to increase CPU locality of allocated memory and performance with concurrent allocation, so this setting could in theory reduce performance. However, in Bitcoin Core very little parallel allocation happens, so the impact is expected to be small or absent.

💬 willcl-ark commented on pull request "init: add MALLOC_ARENA_MAX=1 to systemd":
(https://github.com/bitcoin/bitcoin/pull/27642#issuecomment-1545912961)
I don't believe this affects the majority of users (where the two pools per core default works fine), so I don't see why we'd update the default service file like this.

We have documentation that describes what to do to reduce memory in the event that one wants to do that?

💬 sangaman commented on pull request "init: add MALLOC_ARENA_MAX=1 to systemd":
(https://github.com/bitcoin/bitcoin/pull/27642#issuecomment-1545934901)
> I don't believe this affects the majority of users (where the two pools per core default works fine), so I don't see why we'd update the default service file like this.
>
> We have documentation that describes what to do to reduce memory in the event that one wants to do that?

Maybe it's my fault for not digging into the docs more but I went over a year with inflated, unpredictable RAM usage and even occasional OOM crashes on a machine that I wouldn't consider particularly memory-constra
...

🤔 pinheadmz reviewed a pull request: "Introduce field element and group element classes to test_framework/key.py"
(https://github.com/bitcoin/bitcoin/pull/26222#pullrequestreview-1424673767)
concept ACK

Did as much code review as my tiny brain could handle. I agree that this implementation is easy to read and follow. I checked as much as I could against [NIST params](https://www.secg.org/sec2-v2.pdf) and equations from [Guide to Elliptic Curve Cryptography](https://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.394.3037&rep=rep1&type=pdf)

Comparing run time of `feature_taproot.py` between master and branch, over a few trials with and without RAM disk, the new code only cost
...

💬 pinheadmz commented on pull request "Introduce field element and group element classes to test_framework/key.py":
(https://github.com/bitcoin/bitcoin/pull/26222#discussion_r1192479969)
Not using hex here?

📝 fanquake converted_to_draft a pull request: "Bugfix: Skip tests for tools not being built"
(https://github.com/bitcoin/bitcoin/pull/23027)

💬 pinheadmz commented on issue "nested invalidate block doesn't work like I expect":
(https://github.com/bitcoin/bitcoin/issues/10439#issuecomment-1545976007)
While reviewing https://github.com/bitcoin/bitcoin/pull/26260 I came back here to see if the issue was related at all... I think I agree with Sipa and Sjors that the current behavior is expected and documented and this issue should probably closed as "won't fix"

💬 fanquake commented on pull request "Bugfix: Skip tests for tools not being built":
(https://github.com/bitcoin/bitcoin/pull/23027#issuecomment-1545976127)
Drafting for now, given review comments and questions above.

💬 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#issuecomment-1545980948)
> No, wait! This PR changes both Case1 and Case2, but Case1 is only about things happening in the same second.
>

I meant regarding the time checks in master, the checks were indeed protecting against querying something that was in the mempool before the mempool message was received. Meaning that while this patch fixes the issues around requesting data after a mempool message is received, they are not as severe as initially assumed.

> However, I agree with @ajtowns's concern raised above
...

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