💬 laanwj commented on pull request "rpc, cli: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31135#issuecomment-2437200247)
maybe... im not sure it's a super good first issue, implementing it properly isn't super complicated but does require some knowledge of how various parts work and interact

💬 garlonicon commented on pull request "miner: Reorg Testnet4 minimum difficulty blocks":
(https://github.com/bitcoin/bitcoin/pull/31117#issuecomment-2437215836)
> where a network with realistic mining dynamics is important

What about using "getblocktemplate" on mainnet, and mining blocks, which will quickly become stale? If you want to have worthless coins, then they should be stale, in other cases, they will be traded.

Another possibility is to use "Pay to Proof of Work" addresses on signet/regtest. Then, moving a coin will require grinding a DER signature, to get it below N bytes, so it will require some mining power.

> just pointing out that
...

💬 maflcko commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#discussion_r1816298165)
nit: Maybe to avoid the churn here (changing the same line of code twice), and the c_str bloat, a simple alias could be introduced in an early commit, so that only the alias has to be changed internally in later commits?

For example

```cpp
template<typename... Args>
void fuzz_fmt(const std::string &fmt, const Args&... args){(void)tfm::format_raw(fmt.c_str(), args...);}

💬 rkrux commented on pull request "test: added test to assert TX decode rpc error on submitpackage rpc":
(https://github.com/bitcoin/bitcoin/pull/31139#discussion_r1816314261)
Agree, having this assertion would be good because the valid tx would still not make to mempool in this case.

🤔 rkrux reviewed a pull request: "test: added test to assert TX decode rpc error on submitpackage rpc"
(https://github.com/bitcoin/bitcoin/pull/31139#pullrequestreview-2394696857)
LGTM, will ACK again once @instagibbs's suggestion is also added.

https://github.com/bitcoin/bitcoin/pull/31139#discussion_r1815824499

💬 rkrux commented on pull request "test: added test to assert TX decode rpc error on submitpackage rpc":
(https://github.com/bitcoin/bitcoin/pull/31139#discussion_r1816314768)
Are these blank lines intentional?

💬 laanwj commented on pull request "Drop miniupnp dependency":
(https://github.com/bitcoin/bitcoin/pull/31130#issuecomment-2437290310)
i've checked that no references no miniupnp as dependency remain after this change (except in historical release notes ofc).

Also, successful guix build of 2a541e93cf13:
```
d0674bc3aa12577fb8d0319d51b34ab8650b810f56868a701b388f4f91cfc9b2 guix-build-2a541e93cf13/output/aarch64-linux-gnu/SHA256SUMS.part
6be1f0a8fa6596bd4e6e68996b9a53cb438fb727e55755f6bf101656e1517c47 guix-build-2a541e93cf13/output/aarch64-linux-gnu/bitcoin-2a541e93cf13-aarch64-linux-gnu-debug.tar.gz
a9386bc64b0222ce03a3a
...

👍 hodlinator approved a pull request: "bench: add support for custom data directory"
(https://github.com/bitcoin/bitcoin/pull/31000#pullrequestreview-2394616462)
re-ACK 80d35bc20797e59571e82fe6919705b33dcc40cd

* Commit message improvements.
* `test_path` -> `test_name` (improved naming).
* Made `test_name` be included as part of randomized test directory when *not* setting `-testdatadir`. Including it is helpful for locating test data of a specific test when multiple tests fail. :+1:

Verified correct unit test directory behavior using `build/src/test/test_bitcoin -- -testdatadir=/tmp/foo/`, then not passing the `-testdatadir` and simultaneously r
...

💬 hodlinator commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1816263171)
(Since with the latest non-rebase push you are now touching 2/4 lines mentioning `TEST_DIR_PATH_ELEMENT`, iff you were to re-touch again, it would be nice if you also renamed it to `TESTS_DIR_NAME` as per @ryanofsky's [suggestion](https://github.com/bitcoin/bitcoin/pull/30748#pullrequestreview-2283763614)).

✅ fanquake closed a pull request: "depends: bump miniupnpc to 2.2.8"
(https://github.com/bitcoin/bitcoin/pull/30301)

💬 fanquake commented on pull request "depends: bump miniupnpc to 2.2.8":
(https://github.com/bitcoin/bitcoin/pull/30301#issuecomment-2437321903)
I think #31130 is going to go in for `v29.0`. If that doesn't happen, we can reopen here.

💬 rkrux commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#discussion_r1816346116)
Was this https://github.com/bitcoin/bitcoin/pull/31040#discussion_r1815889280 done in 5c299ecafe6f336cffa145d28036b04b87e26712? I could not find it.

👍 rkrux approved a pull request: "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped"
(https://github.com/bitcoin/bitcoin/pull/31040#pullrequestreview-2394740828)
ACK 5c299ecafe6f336cffa145d28036b04b87e26712

Thanks for adding this test, it's easy to follow through.

💬 rkrux commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#discussion_r1816349294)








```suggestion
assert_equal(len(node.getorphantxs()), DEFAULT_MAX_ORPHAN_TRANSACTIONS)
```
Storing in another variable is not necessary only for one time usage.

You don't need to invalidate ACKs only for this though.

💬 laanwj commented on pull request "depends: bump miniupnpc to 2.2.8":
(https://github.com/bitcoin/bitcoin/pull/30301#issuecomment-2437336876)
> I think https://github.com/bitcoin/bitcoin/pull/31130 is going to go in for v29.0. If that doesn't happen, we can reopen here.

yes... i dont think i've seen any PR that popular in my time here 😄

💬 dergoegge commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#issuecomment-2437349340)
Waiting for #31150 to go in

👍 fanquake approved a pull request: "ci: Temporary workaround for old CCACHE_DIR cirrus env"
(https://github.com/bitcoin/bitcoin/pull/31146#pullrequestreview-2394783289)
ACK fa9747a896188f4dd70f275aec2469dba5cd435e - have seen this now.

🚀 fanquake merged a pull request: "ci: Temporary workaround for old CCACHE_DIR cirrus env"
(https://github.com/bitcoin/bitcoin/pull/31146)

💬 maflcko commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2437357680)
Not sure about the CI failure. IIRC it happened after this push: https://github.com/bitcoin/bitcoin/compare/15cfeebb47587af6ce7be72fd52c57a0483d86d2..5757fdf0dc74ec9d6bbefba937d6e23b09652605

The logs don't say anything except: `validation_chainstatemanager_tests ...Exit code 0xc0000409
2024-10-24T23:23:18.2978457Z ***Exception: 9.33 sec` and the debug log is truncated (presumably when the exception happens).

It would be good if ctest/Windows actually printed the exception and error mess
...

👍 brunoerg approved a pull request: "util: Treat Assume as Assert when evaluating at compile-time"
(https://github.com/bitcoin/bitcoin/pull/31150#pullrequestreview-2394823764)
ACK fa69a5f4b76a4e2a02db6c32d9c3311ce5fe29bd