💬 instagibbs commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1828105140)
It also causes double-validation for single transaction package that is missing inputs.

Could special case this but not sure I love making things even harder to understand:
```
diff --git a/src/validation.cpp b/src/validation.cpp
index 3e48335255..c89050e895 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1790,10 +1790,16 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
// some of them may still be valid.

...

💬 laanwj commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1828133259)
Done

💬 l0rinc commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1828135621)
note: with clang I measured ~24: https://tc-imba.github.io/posts/cpp-sso

🤔 l0rinc reviewed a pull request: "net: Use actual memory size in receive buffer accounting"
(https://github.com/bitcoin/bitcoin/pull/31164#pullrequestreview-2413727212)
Concept ACK

💬 l0rinc commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1828153937)
Could you please add a test in this commit that demonstrates the intended behavior? Or would that tie the implementation down too much?

💬 l0rinc commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1828152759)
Since I think these are related pointers (unlike https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1827884684), can we use simple pointer comparisons here? Or do I misunderstand how this works?
```C++
if (s.data() >= s_ptr && s.data() + s.size() <= s_ptr + sizeof(s)) {
```

💬 laanwj commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1828173154)
interesting!
that also suggests that on 32-bit platforms, the size would still be 15 bytes for g++ (because it's the minimum bound), but 10 (3*4 minus 2 for size byte and zero byte) for clang
so it would trigger an allocation for message types of 11/12 chars there 😄

💬 laanwj commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1828177808)
as i understand it, in the case that sso is not used, these are not related pointers

💬 sipa commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1828177895)
In case the small string optimization is in effect, then `s.data()` and `s_ptr` are possibly related (the real question is whether they point into the same allocated array), but if sso is not in effect then they are definitely unrelated.

💬 l0rinc commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1828180248)
Thanks for clarifying.

💬 laanwj commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1828184257)
i guess we could do some basic generalizable test like "the memory usage increases or stays equal when the string becomes longer"
but i'm not sure how useful that is, and there are currently no memusage tests to add it to

💬 laanwj commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1828195177)
fwiw the underlying reason for this pointer absurdity is to support platforms with segmented memory (such as GPUs), where pointers to different types of allocations might refer to different address spaces and won't be comparable
probably, `std::greater` adds some address space prefix to give them a defined order
this is a super edge case and unlikely to come up for any platform people compile bitcoind for, but better to be safe i guess...

💬 instagibbs commented on pull request "fuzz: Fix difficulty target generation in `p2p_headers_presync`":
(https://github.com/bitcoin/bitcoin/pull/31213#discussion_r1828204231)
are there any easy ways to verify these lower and higher target values?

💬 instagibbs commented on pull request "fuzz: Fix difficulty target generation in `p2p_headers_presync`":
(https://github.com/bitcoin/bitcoin/pull/31213#discussion_r1828200914)
some pedantic assertions to catch hypothetical issues with the function earlier
```suggestion
assert(min + result >= min);
assert(min + result <= max);
return min + result;
```

💬 darosior commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#issuecomment-2455474172)
Concept ACK

💬 darosior commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1828281626)
Done.

💬 laanwj commented on pull request "net: Use actual memory size in receive buffer accounting":
(https://github.com/bitcoin/bitcoin/pull/31164#discussion_r1828284824)
i've tested it manually (gcc, x86_64) with the following patch:
<details>
<summary>patch</summary>

```patch
diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp
index 8a4c1746161c30c04dee1f41a811740d0038df0b..f8c688dd3617123548af13611519adba59eef775 100644
--- a/src/bitcoin-cli.cpp
+++ b/src/bitcoin-cli.cpp
@@ -23,6 +23,7 @@
#include <util/strencodings.h>
#include <util/time.h>
#include <util/translation.h>
+#include <memusage.h>

#include <algorithm>
#include <chrono>
...

✅ Abdulkbk closed a pull request: "Improve lockunspent validation for vout"
(https://github.com/bitcoin/bitcoin/pull/31205)

📝 achow101 opened a pull request: "Update secp256k1 subtree to v0.6.0"
(https://github.com/bitcoin/bitcoin/pull/31216)
v0.6.0 was just released, main change is that it has the musig module which #29675 needs.

🤔 mzumsande reviewed a pull request: "consensus: fix `OP_1NEGATE` handling in `CScriptOp`"
(https://github.com/bitcoin/bitcoin/pull/29589#pullrequestreview-2413945142)
I didn’t see earlier versions of this PR, but It’s not clear to me what the goal of this PR is from the OP / Title. Should the title be updated, is this still supposed to fix a bug?

As mentioned above, `DecodeOP_N` / `EncodeOP_N` currently don't need to handle `OP_1NEGATE` in any of the current use cases - so I think that there should be some explanation why they should be changed to support it.

I noticed that `EncodeOP_N` is currently called from `PushAll` in `sign.cpp` with an `unsigned
...