🚀 achow101 merged a pull request: "net: Replace libnatpmp with built-in PCP+NATPMP implementation"
(https://github.com/bitcoin/bitcoin/pull/30043)

💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2384109994)
> Just as a note, it should already be possible to pick the device via an env var, see https://en.cppreference.com/w/cpp/filesystem/temp_directory_path#Notes.

Hmm, thats a good observation.
Even when that may be possible now, I'm not sure we should start recommending the mixture of env vars and binary args at the user interface level. It seems more natural to plug the already existing `-testdatadir` arg to the benchmark framework. Still, if we move towards the env var direction, the next ste
...

💬 achow101 commented on pull request "init: Remove retry for loop":
(https://github.com/bitcoin/bitcoin/pull/30968#issuecomment-2384121576)
ACK e9d60af9889c12b4d427adefa53fd234e417f8f6

💬 achow101 commented on pull request "test: switch MiniWallet padding unit from weight to vsize":
(https://github.com/bitcoin/bitcoin/pull/30718#issuecomment-2384137098)
ACK 940edd6ac24e921f639b1376fe7d35dc14c1887d

🚀 achow101 merged a pull request: "init: Remove retry for loop"
(https://github.com/bitcoin/bitcoin/pull/30968)

💬 furszy commented on pull request "wallet: optimize migration process, batch db transactions":
(https://github.com/bitcoin/bitcoin/pull/28574#issuecomment-2384140877)
PR description updated. Created #31000 so we can all run the benchmark on an external HDD in a more friendly manner.
Have seen a 15% speedup locally on a USB 3.2 pendrive.

🚀 achow101 merged a pull request: "test: switch MiniWallet padding unit from weight to vsize"
(https://github.com/bitcoin/bitcoin/pull/30718)

💬 achow101 commented on pull request "[28.x] backports and finalize":
(https://github.com/bitcoin/bitcoin/pull/30959#issuecomment-2384156600)
Added release notes (from rev https://github.com/bitcoin-core/bitcoin-devwiki/wiki/28.0-Release-Notes-Draft/eab8c9df5996bfe0d0997f747eefbdb745381aa0)

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1781748330)
`i` is not in scope here anymore.

💬 ffrediani commented on issue "Add IPv6 pinhole support using UPnP / NAT-PMP":
(https://github.com/bitcoin/bitcoin/issues/17012#issuecomment-2384167286)
@laanwj @Sjors I know how UPnP has been controversy over the years (and now PCP maybe inherit this), but what is your view at some point to start having this option enabled by default ? This could enhance significantly the P2P data exchange between peers and reduce a lot the need to use slower and more problematic networks as Tor and I2P, mainly because many residential Broadband connections have working IPv6 and routers that understand it so it will enables a fair amount of nodes to contribute
...

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1781761151)
Changed to:

> clusterlin: merge two DepGraph fuzz tests into simulation test
>
> This combines the clusterlin_add_dependency and clusterlin_cluster_serialization fuzz tests into a single clusterlin_depgraph_sim fuzz test. This tests starts from an empty DepGraph and performs a arbitrary number of AddTransaction, AddDependencies, and RemoveTransactions operations on it, and compares the resulting state with a naive reimplementation.

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1781762376)
Changed to:

> /** Read any set of transactions from the provider (including unused positions). */

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1781762443)
Done.

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1781763047)
Done. I have also added an actual counting of the number of transactions in `real` at the end, and an `assert` to compare it with `num_tx_sim`.

💬 theStack commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1781734202)
in commit 3c1c9e766ad4fc6defdc9b4814c1e184f6603003: the `num_outputs` parameter is currently unused, so all txs created with this helper only have one output

💬 theStack commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1781744846)
was this meant to be
```suggestion
NodeId rand_peer = fuzzed_data_provider.ConsumeIntegralInRange<int64_t>(0, NUM_PEERS - 1);
```
instead, considering that the specified range end is inclusive? the peer loops and asserts have also currently different conditions (`< NUM_PEERS` in `CheckInvariants` vs. `<= NUM_PEERS` in the disconnect loops), so at least one of those needs to be adapted.

🤔 jonatack reviewed a pull request: "doc: Minor update to doc/README.md"
(https://github.com/bitcoin/bitcoin/pull/30992#pullrequestreview-2338659503)
ACK 3208df2a100f58569165081cd20c02abed827286

Might be good to update the PR description to reflect the changes (and the commit message as well, if you need to repush).

Suggestion for the PR title: `doc: update IBD requirements in doc/README.md`

💬 sipa commented on pull request "scripted-diff: Modernize nLocalServices naming":
(https://github.com/bitcoin/bitcoin/pull/30885#issuecomment-2384271156)
utACK 33381ea530ad79ac1e04c37f5707e93d3e0509ca

💬 Javadseyedi12 commented on pull request "scripted-diff: Modernize nLocalServices naming":
(https://github.com/bitcoin/bitcoin/pull/30885#issuecomment-2384288477)
Hello, good time, yes, what should I do?

در تاریخ دوشنبه ۳۰ سپتامبر ۲۰۲۴،‏ ۱۸:۱۵ Pieter Wuille <
***@***.***> نوشت:

> utACK 33381ea
> <https://github.com/bitcoin/bitcoin/commit/33381ea530ad79ac1e04c37f5707e93d3e0509ca>
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/pull/30885#issuecomment-2384271156>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/BLV7G4Q3UB43DPF6OJHTUM3ZZHEQ5AVCNFSM6AAAAABODX6MB6VHI2DSMVQWIX3LMV
...

💬 sipa commented on issue "V2 Only Option":
(https://github.com/bitcoin/bitcoin/issues/29618#issuecomment-2384291593)
I see both points of view here, and I have a hard time weighing them:
* There are certainly users for whom it is helpful that their Bitcoin P2P connections aren't as easily observable as V1 connection are, by their ISP or other entities, even if no strong hiding can be guaranteed.
* An option like this will likely result in a false sense of security for others, and I'm not comfortable with (eventually, if this were to be made the default) making it harder for older/other software to connect to
...