💬 maflcko commented on pull request "test: cover edge case for lockunspent rpc":
(https://github.com/bitcoin/bitcoin/pull/31209#issuecomment-2459879971)
> Removing the check for negative would technically change RPC behavior for users ([#30212 (comment)](https://github.com/bitcoin/bitcoin/pull/30212#issuecomment-2347371722)), but do we think anyone's downstream use case is actually relying on this odd edge case (negative `vout` arg)?

Matching errors based on the error string is fragile and considered bad practise, because error strings are considered less stable than error codes or error types. Also, error strings may be verbose and (if trunc
...

💬 hebasto commented on pull request "ci: Split out native fuzz jobs for macOS and windows (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31221#discussion_r1831107121)
Instead of saving another cache, we can simply skip the cache saving step for the "fuzz" job type.

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831108819)
leaving as-is

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831114106)
From an implementation perspective it was cleaner to enforce it in `IsStandardTx`, and I'm not sure I see the value in doubling up enforcement of it vs a single location.

If we already had fee information by the time we would call `IsStandardTx` it might be even cleaner, but I expect it would be a very difficult to evaluate diff.

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831116322)
I don't think it would make the logic simpler or significantly more performant, and would rather the `IsStandardTx` check not be so tightly bound.

💬 vasild commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2459907673)
Before I could see the purpose of the full v2only option: 1. to hide the fact that a bitcoin node is running and 2. hide its own transactions. However I think it would achieve neither of those, but I could see how it looked like it would. Now it is changed:

> I've rebased and changed the approach in the PR to have v2 only outbound connections

Does it still have the same motivation: 1. and 2.? I think the idea in https://github.com/bitcoin/bitcoin/issues/29618 is that v1 connections are som
...

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831118616)
yes, `IsStandardTx` check is very early in PreChecks, not requiring access to any utxo information. including fees.

🤔 BrandonOdiwuor reviewed a pull request: "contrib: skip missing binaries in gen-manpages"
(https://github.com/bitcoin/bitcoin/pull/30986#pullrequestreview-2418443870)
Concept ACK
Adding `--skip-missing-binaries` as an optional flag seems reasonable.

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831122231)
Can change to `MAX_EPHEMERAL_DUST_OUTPUTS_PER_TX` if I touch things

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1831124142)
This is as intended. We don't allow priority on anything with dust, no matter how many non-zero dust outputs.

👍 danielabrozzoni approved a pull request: "net: Use actual memory size in receive buffer accounting"
(https://github.com/bitcoin/bitcoin/pull/31164#pullrequestreview-2418473848)
Light ACK d22a234ed270286b483aec2db1e2f716b9756231 - code looks good to me, but I'm not very familiar with C++ memory management specifics

💬 dergoegge commented on pull request "ci: Split out native fuzz jobs for macOS and windows (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31221#discussion_r1831140955)
Got rid of all the duplicate caches

💬 dergoegge commented on pull request "ci: Split out native fuzz jobs for macOS and windows (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31221#discussion_r1831149076)
Doesn't look like this works

💬 dergoegge commented on pull request "ci: Split out native fuzz jobs for macOS and windows (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31221#discussion_r1831154621)
It had to be `job-name` not `job_name`

👍 instagibbs approved a pull request: "TxDownloadManager followups"
(https://github.com/bitcoin/bitcoin/pull/31190#pullrequestreview-2418505428)
ACK 5dc94d13d419b8d5e543cb50edeb872335c090e7

💬 instagibbs commented on pull request "TxDownloadManager followups":
(https://github.com/bitcoin/bitcoin/pull/31190#discussion_r1831160188)
any value in
```suggestion
Assert(txdownload_impl.m_txrequest.CountInFlight(peer) <= node::MAX_PEER_TX_ANNOUNCEMENTS);
Assert(txdownload_impl.m_txrequest.Count(peer) <= node::MAX_PEER_TX_ANNOUNCEMENTS);
```
?

💬 fanquake commented on pull request "cmake: Revamp `FindLibevent` module":
(https://github.com/bitcoin/bitcoin/pull/31181#issuecomment-2459974303)
Guix build:
```bash
c448a9c8d6d436350846ccb998e6a5e2bb66ac519e3539188c4792c6a88f4a1c guix-build-a6a3cab235cf/output/aarch64-linux-gnu/SHA256SUMS.part
613ef63f92fdf0422179fdedad230a7b8c5eb1530c892745bd2d5e76e8fb580c guix-build-a6a3cab235cf/output/aarch64-linux-gnu/bitcoin-a6a3cab235cf-aarch64-linux-gnu-debug.tar.gz
2d77932ce9f8808f9673420320faaf9b20d910595f3d3d65687b5cbc55cb84dd guix-build-a6a3cab235cf/output/aarch64-linux-gnu/bitcoin-a6a3cab235cf-aarch64-linux-gnu.tar.gz
73019d4fb0f4e548
...

🚀 fanquake merged a pull request: "ci: `add second_deadlock_stack=1` to TSAN options"
(https://github.com/bitcoin/bitcoin/pull/31232)

💬 instagibbs commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1831211399)
I've already made the attempt to completel split out the code paths and it was far more work to understand as I have to use the `BroadcastTransaction` interface which is quite different than `testmempoolaccept`'s requirements.

This is the most minimal fix I can devise:
```
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::AcceptPackag
...

💬 mzumsande commented on pull request "net, init: derive default onion port if a user specified a -port":
(https://github.com/bitcoin/bitcoin/pull/31223#discussion_r1831240767)
Thanks, changed to that.