💬 naumenkogs commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1827633761)
Won't we redundantly call `AcceptSubPackage` twice for a single transaction, if `individual_results_nonfinal.emplace(wtxid, single_res);` stuff is triggered?

🚀 fanquake merged a pull request: "doc: archive release notes for v27.2"
(https://github.com/bitcoin/bitcoin/pull/31208)

⚠️ fanquake opened an issue: "intermittent issue in wallet_upgradewallet.py: AssertionError: bdb magic does not match bdb btree magic"
(https://github.com/bitcoin/bitcoin/issues/31210)
https://cirrus-ci.com/task/5232872305459200:
```bash
067] node0 2024-11-04T11:58:48.064759Z [httpworker.0] [src/wallet/wallet.h:936] [WalletLogPrintf] [default wallet] m_address_book.size() = 2
[06:58:49.067] test 2024-11-04T11:58:48.065000Z TestFramework (INFO): Can upgrade to HD
[06:58:49.067] test 2024-11-04T11:58:48.066000Z TestFramework (ERROR): Assertion failed
[06:58:49.067] Traceback (most recent call last):
[06:58:49.067]
...

💬 brianacaley commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#issuecomment-2454593535)
7


Get Outlook for Android<https://aka.ms/AAb9ysg>
________________________________
From: Gleb Naumenko ***@***.***>
Sent: Monday, November 4, 2024 4:23:27 AM
To: bitcoin/bitcoin ***@***.***>
Cc: Subscribed ***@***.***>
Subject: Re: [bitcoin/bitcoin] p2p: Fill reconciliation sets (Erlay) attempt 2 (PR #30116)


@naumenkogs commented on this pull request.

________________________________

In src/node/txreconciliation.cpp<https://github.com/bitcoin/bitcoin/pull/30116#discussion_r
...

💬 maflcko commented on issue "intermittent issue in wallet_upgradewallet.py: AssertionError: bdb magic does not match bdb btree magic":
(https://github.com/bitcoin/bitcoin/issues/31210#issuecomment-2454599900)
This only reproduces on a specific machine. I'll try to take a closer look.

💬 Abdulkbk commented on pull request "Improve lockunspent validation for vout":
(https://github.com/bitcoin/bitcoin/pull/31205#issuecomment-2454613258)
> `getInt` is used in many RPCs. What makes this call to `getInt` in this RPC special?

Thank you, @maflcko, for your feedback. I believe that `getInt` in `lockunspent` is special because the vout param specifically represents transaction output indexes, which are always whole numbers. Since this value comes directly from user input, providing a descriptive error message would be very helpful.

I also updated the PR description to add motivation for this change.

💬 l0rinc commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1827415495)
Nit: Given that `MallocUsage` warns that it `is *not* recursive`, would have been great to add a
```c++
requires std::is_trivial_v<X>
```
here, but apparently it *is* used for nested structures - nothing to do here, just documenting my journey.

💬 l0rinc commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1827342209)
Q: `v.capacity()` can be significantly more here than `v.size()`, right?
Is it because we can't size them properly in https://github.com/bitcoin/bitcoin/blob/master/src/streams.h#L82?

💬 l0rinc commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1827424352)
Q: we assume that `sizeof(v)` (the vector's fixed memory) was already included in the calculation before this call, right?

💬 l0rinc commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1827341320)
Nit: Could we find better names than X and Y (e.g. to avoid readers thinking Y is the size), maybe something like `template<typename ElementType, typename AllocatorType>`

💬 l0rinc commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1827573203)
Slightly unrelated, but `MemUsage` could be generalized to explain the alignment and rounding:

<details>
<summary>Details</summary>

```C++
static inline size_t MallocUsage(const size_t alloc)
{
if (alloc == 0) return 0;
constexpr size_t alignment{2 * sizeof(void*)};
constexpr size_t round_up{alignment * 2 - 1};
constexpr size_t mask{~(alignment - 1)};
return alloc + round_up & mask;
}
```
and
```C++
BOOST_AUTO_TEST_CASE(malloc_usage)
{
auto referenceM
...

💬 l0rinc commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1827588117)
I don't actually understand why we're ignoring `m_type`, the comment doesn't help.

Is it because of https://github.com/bitcoin/bitcoin/blob/master/src/netmessagemaker.h#L17, i.e. the string being moved around instead of being owned?

💬 l0rinc commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1827592846)
we still have a few `m_raw_message_size` usages, can you please explain why that's still needed?

💬 l0rinc commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1827589976)
given that we're ignoring `m_type`, is it still fair to say that we're counting `total memory usage`?

💬 laanwj commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1827709219)
The comment in the referred function explains it.
```
// Don't count the dynamic memory used for the m_type string, by assuming it fits in the
// "small string" optimization area (which stores data inside the object itself, up to some
// size; 15 bytes in modern libstdc++).
```
It has to do with the internal implementation of std::string, where it has a fixed-size buffer that it will use for short strings (such as the message types) before allocating memory on the heap.

💬 laanwj commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1827714339)
i do agree that over-counting is probably better than under-counting here, but i just took this over from `CSerializedNetMsg` as i assumed this was the right thing to do. If we change it it needs to be changed in both places.

💬 maflcko commented on pull request "Improve lockunspent validation for vout":
(https://github.com/bitcoin/bitcoin/pull/31205#issuecomment-2454689466)
Every use of `getInt` represents a whole number. This is not special. If you disagree, it would be good to explain and to provide an example. Also, What real-world end-user-facing issue is this trying to solve?

Tend toward NACK for now, unless there is a reason.

💬 laanwj commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1827719079)
Right, it can, when `reserve()` is called with a larger amount, or when the vector is shortened. But it does represent the actually-allocated size which seems what we want here.

💬 l0rinc commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1827719853)
Can we measure whether this is indeed the case? What is the reason for `assuming it fits in the "small string" optimization area`?

💬 laanwj commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1827720254)
Correct, it's computes the dynamic usage only.