💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#issuecomment-1818210163)
> But it seems confusing to fail with a stack trace / uncaught exception in this case ("Fatal error: protocol.data_received() call failed.") when this is actually expected to happen.


<details>
<summary> is this the test log you observed? </summary>
<br>

```
2023-11-18T13:52:32.976000Z TestFramework (INFO): PRNG seed is: 8699047777059301467
2023-11-18T13:52:32.977000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_70zlmcgr
2023-11-18T13:52:33.535000Z TestFra
...

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1398651836)
you're right! done.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1398651878)
done.

💬 ajtowns commented on pull request "refactor: P2P transport without serialize version and type":
(https://github.com/bitcoin/bitcoin/pull/28892#discussion_r1398688765)
Calling `NetMsg::Make` within `PushMessage` seems unnecessary; just add an extra `PeerManagerImpl` helper:

```c++
template <typename... Args>
void PushMessage(CNode& node, std::string msg_type, Args&&... args) const
{
PushMessage(node, NetMsg::Make(std::move(msg_type), args...));
}
```

cf https://github.com/ajtowns/bitcoin/commit/0fe129b1b437d39221cef843f969f1adc31b543e

✅ fujicoin closed an issue: "Signet mining is not possible when using descriptor wallet"
(https://github.com/bitcoin/bitcoin/issues/28911)

💬 martinus commented on issue "RAM usage regression in 26.x and master on ARM 32-bit":
(https://github.com/bitcoin/bitcoin/issues/28906#issuecomment-1818343244)
> [...] try to make sure fallback allocations get counted too in master.

Related PRs: #27748 tries to do exactly this, and #28531 wants to improve MallocUsage estimation.

💬 martinus commented on pull request "util: generalize accounting of system-allocated memory in pool resource":
(https://github.com/bitcoin/bitcoin/pull/27748#issuecomment-1818347616)
@LarryRuane are you still working on this? I think this shouldn't be blocked by #28531.

📝 TaiseiLuette opened a pull request: "Create staging tree"
(https://github.com/bitcoin/bitcoin/pull/28914)
<!--
*** 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 and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

✅ TaiseiLuette closed a pull request: "Create staging tree"
(https://github.com/bitcoin/bitcoin/pull/28914)

💬 maflcko commented on pull request "refactor: P2P transport without serialize version and type":
(https://github.com/bitcoin/bitcoin/pull/28892#discussion_r1398744244)
Correct, but I wasn't sure if reviewers preferred the slightly more verbose way to clarify that serialization is involved on every call.

💬 martinus commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#issuecomment-1818440265)
I can reproduce the same results with glibc 2.31 and 2.38, in 32bit and 64bit. I don't think it needs to be perfect on all platforms, I think your new calculation is fine.

I played a bit with a reproducer, here is mine: https://godbolt.org/z/zqhzxoobx I refactored your `MallocUsage` formula a bit, but it's basically the same

💬 maflcko commented on pull request "lint: Report all lint errors instead of early exit":
(https://github.com/bitcoin/bitcoin/pull/28862#discussion_r1398875960)
Are you sure? `all()` short circuits (https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.all), but the goal of this pull is to report all lint errors, not only the first one.

💬 maflcko commented on pull request "lint: Report all lint errors instead of early exit":
(https://github.com/bitcoin/bitcoin/pull/28862#discussion_r1398877832)
Also, style-wise, some people prefer `for` over functional programming when the predicate has side effects. Happy to push what you prefer, but I wanted to mention that this was written on purpose in this way.

💬 maflcko commented on pull request "lint: Report all lint errors instead of early exit":
(https://github.com/bitcoin/bitcoin/pull/28862#discussion_r1398881891)
I think this is already printed. For me:

```
...
^---- Failure generated from subtree check!
...

👍 hebasto approved a pull request: "coins: make sure PoolAllocator uses the correct alignment"
(https://github.com/bitcoin/bitcoin/pull/28913#pullrequestreview-1739334189)
ACK 486c2916a18b56d6011117077d906c1f8a81623f, tested on ARM 32-bit (ODROID-HC1 + Armbian 23.8.3 jammy) both native and Guix binaries.

The first commit should not introduce any behaviour change in any other [release](https://bitcoincore.org/en/download/) platforms as they are 64-bit and it is holds that `alignof(T) == alignof(void*) == 8` for all of them.

I agree with the second commit diff. It indeed handles the case where "int64_t is aligned to 8 bytes but void* is aligned to 4 bytes". Bu
...

💬 TheCharlatan commented on pull request "lint: Report all lint errors instead of early exit":
(https://github.com/bitcoin/bitcoin/pull/28862#discussion_r1398896203)
Yeah, you're right. This is good as is.

💬 Retropex commented on pull request "util: Replace std::filesystem with util/fs.h":
(https://github.com/bitcoin/bitcoin/pull/28076#issuecomment-1818736154)
The integration of Rust into Core, further increases the technical debt. Each person's ability to run tests locally is essential, and relying on CIs to manage Rust code is not a viable solution. In addition, introducing Rust can encourage the addition of other languages, risking fragmenting and further complicating the code base.

📝 hethm999 opened a pull request: "Update SECURITY.md"
(https://github.com/bitcoin/bitcoin/pull/28915)
ايلون ماسك رموزات مميزة بملايين البيتكوين ولم تتجاوب معي

<!--
*** 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 and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
signif
...

✅ fanquake closed a pull request: "Update SECURITY.md"
(https://github.com/bitcoin/bitcoin/pull/28915)

📝 fanquake locked a pull request: "Update SECURITY.md"
(https://github.com/bitcoin/bitcoin/pull/28915)
ايلون ماسك رموزات مميزة بملايين البيتكوين ولم تتجاوب معي

<!--
*** 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 and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
signif
...