💬 1440000bytes commented on pull request "rest: Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2433416053)
What is the use of REST interface that doesn't have enough endpoints?

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1813527740)
Yes this matches the old behavior, I believe.

💬 darosior commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#discussion_r1813555299)
Done.

💬 darosior commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2433449973)
Kept the option as a hidden arg which will return an error explaining UPnP support was dropped and encouraging users to set `-natpmp` instead.

Added a release note.

🤔 theStack reviewed a pull request: "refactor: TxDownloadManager + fuzzing"
(https://github.com/bitcoin/bitcoin/pull/30110#pullrequestreview-2390493996)
Re-reviewed up to 190d87f912737c46668072ea9a460c520f778c92, lgtm. The introduced minor behaviour changes to address https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2391859626 and https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1801661898 seem reasonable and harmless. Planning to finish reviewing unit test and fuzzer tomorrow (not opposed to defer the one_honest_peer fuzz test for a future PR at all :)).

💬 theStack commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1813507907)
```suggestion
* downloaded, whether and how to validate them. It is also responsible for
```

💬 achow101 commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2433458355)
In 221410bc14616eaf7017e5097cd6848fcb4f8e1b "bench: specialize working directory name" , the commit message says

> After this commit, benchmarks are contained within its own directory:
> /<OS_tmp_dir>/test_common bitcoin/<benchmark_name>/<random_uint256>/

However I am not observing this behavior. It omits the benchmark name.

The benchmark name is included when `-testdatadir` is used.

💬 sipa commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#discussion_r1813583243)
This is perhaps a bit confusing, given that from the user's perspective these two are the same thing (`-natpmp` controls both).

💬 achow101 commented on pull request "test: add test for specifying custom pidfile via `-pid`":
(https://github.com/bitcoin/bitcoin/pull/30724#issuecomment-2433468377)
ACK 04e4d52420a0e6bf40d4bd6fe1f31f66db9eab0a

✅ danielabrozzoni closed a pull request: "rest: Support transaction broadcast in REST interface"
(https://github.com/bitcoin/bitcoin/pull/31065)

💬 danielabrozzoni commented on pull request "rest: Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2433474617)
Closing as it doesn't seem this is the way to go, thanks everyone for the feedback! :)

📝 achow101 unlocked a pull request: "validation: fix coins disappearing mid-package evaluation"
(https://github.com/bitcoin/bitcoin/pull/28251)
While we are evaluating a package, we split it into "subpackages" for evaluation (currently subpackages all have size 1 except the last one). If a subpackage has size 1, we may add a tx to mempool and call `LimitMempoolSize()`, which evicts transactions if the mempool gets full. We handle the case where the just-submitted transaction is evicted immediately, but we don't handle the case in which a transaction from a previous subpackage (either just submitted or already in mempool) is evicted. Mai
...

💬 sdaftuar commented on pull request "validation: fix coins disappearing mid-package evaluation":
(https://github.com/bitcoin/bitcoin/pull/28251#issuecomment-2433501732)
I'm reviewing this PR more carefully, after seeing @instagibbs' comment here: https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1813272637

As I understand it, I think there are two key behaviors introduced in this PR:
1) Clearing state between subpackage evaluations (specifically, clearing the coins in `m_viewmempool`)
2) Deferring mempool limiting until the end of package validation

However, the specific issue that @glozow described above (https://github.com/bitcoin/bitcoin/pull
...

💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2433506200)
> In [221410b](https://github.com/bitcoin/bitcoin/commit/221410bc14616eaf7017e5097cd6848fcb4f8e1b) "bench: specialize working directory name" , the commit message says
>
> > After this commit, benchmarks are contained within its own directory:
> > /<OS_tmp_dir>/test_common bitcoin/<benchmark_name>/<random_uint256>/
>
> However I am not observing this behavior. It omits the benchmark name.
>
> The benchmark name is included when `-testdatadir` is used.

Yeah, that's because the data d
...

🚀 achow101 merged a pull request: "test: add test for specifying custom pidfile via `-pid`"
(https://github.com/bitcoin/bitcoin/pull/30724)

💬 achow101 commented on pull request "init: Some small chainstate load improvements":
(https://github.com/bitcoin/bitcoin/pull/31046#issuecomment-2433519694)
ACK 31cc5006c3de4dd6a1f7a238684163956604df45

🤔 furszy reviewed a pull request: "bench: add support for custom data directory"
(https://github.com/bitcoin/bitcoin/pull/31000#pullrequestreview-2390711656)
Updated per feedback. Thanks achow.

💬 achow101 commented on pull request "doc: Extend developer-notes with file-name-only debugging fix":
(https://github.com/bitcoin/bitcoin/pull/30670#issuecomment-2433523846)
ACK d1546b3c4f24d3ff6abe729a4288e827786cf6e9

👍 theStack approved a pull request: "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`"
(https://github.com/bitcoin/bitcoin/pull/29954#pullrequestreview-2390749484)
ACK modulo nits

As others have expressed before, I'd also slightly prefer to name the options exactly like their command line option (i.e. "datacarriersize" without the "max" prefix), but no blocker.

💬 theStack commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#discussion_r1813770793)
nit, a little shorter:
```suggestion
ret.pushKV("maxdatacarriersize", pool.m_opts.max_datacarrier_bytes.value_or(0));
```