💬 ishaanam commented on pull request "package validation: relax the package-not-child-with-unconfirmed-parents rule":
(https://github.com/bitcoin/bitcoin/pull/31385#discussion_r2223553232)
In c8d96e23f734e275dfee840df49b1f4cc78cd4b9 "test: add chained 1p1c propagation test "

nit for consistency:
```suggestion
# Submitting them all together doesn't work, as the topology is not child-with-parents
```

💬 ishaanam commented on pull request "package validation: relax the package-not-child-with-unconfirmed-parents rule":
(https://github.com/bitcoin/bitcoin/pull/31385#discussion_r2223522893)
In c8d96e23f734e275dfee840df49b1f4cc78cd4b9 "test: add chained 1p1c propagation test "

nit: it is a bit confusing to reuse the `low_fee_parent` and `high_fee_child` variables here.

💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#issuecomment-3104499007)
> * I think it would good to add a test to cover cases where positional _non-string_ parameters contain '=', i.e. the `bitcoin-cli -named echojson '["fail=yes"]'` case which [previously failed](https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2196208483) (because it treated everything before the '=' character as a parameter name) and now succeeds.

You are right here, I will add a test for non-string params like `echojson`.

> * I think the code change suggested in [df4f391](https:/
...

💬 frankomosh commented on pull request "fuzz: add mempool_dag fuzzer for transaction dependency testing":
(https://github.com/bitcoin/bitcoin/pull/33038#discussion_r2223621284)
You are right about the duplication. The main difference is I wanted to systematically construct transaction dependency chains, which I thought to do in a seperate target because `tx_pool.cpp` tests only individual transaction acceptance(and I did not want to mess with it anyway).

The goal is to target directed acyclic graph specific edge cases in ancestor/descendant limit enforcement that random transaction generation maybe does not hit.

💬 frankomosh commented on pull request "fuzz: add mempool_dag fuzzer for transaction dependency testing":
(https://github.com/bitcoin/bitcoin/pull/33038#discussion_r2223623449)
i'll generate report and only proceed if there is new coverage

📝 frankomosh converted_to_draft a pull request: "fuzz: add mempool_dag fuzzer for transaction dependency testing"
(https://github.com/bitcoin/bitcoin/pull/33038)
adds a new target, `mempool_dag`, to test mempool DAG invariants by constructing transaction chains with controlled parent/child dependencies. main focus areas include:

a. ancestor / descendant count and size limits (`-limitancestorcount,` `-limitancestorsize`, `-limitdescendantcount`, `-limitdescendantsize`)

b. TRUC-policy interaction with non-TRUC transactions

c. fee-rate consistency after partial package eviction

Invariants are asserted after every successful `AcceptToMemoryPool`
...

✅ Zeegaths closed a pull request: "docs: adds correct updated documentation links"
(https://github.com/bitcoin/bitcoin/pull/32699)

💬 darosior commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3104527522)
@RobinLinus do you plan on addressing review here? Reminder that feature freeze is [less than a month](https://github.com/bitcoin/bitcoin/issues/32275) from now.

💬 PeterWrighten commented on pull request "wallet, test: Replace MockableDatabase with in-memory SQLiteDatabase":
(https://github.com/bitcoin/bitcoin/pull/33032#issuecomment-3104700592)
ACK 78897

📝 Zeegaths reopened a pull request: "docs: adds correct updated documentation links"
(https://github.com/bitcoin/bitcoin/pull/32699)
Added correct links to the docs in place of the missing docs' paths.
Fixes #32565

💬 b-l-u-e commented on pull request "docs: adds correct updated documentation links":
(https://github.com/bitcoin/bitcoin/pull/32699#discussion_r2223787842)
There's an unnecessary semicolon on its own line that should be removed.

💬 glozow commented on pull request "p2p: TxOrphanage revamp cleanups":
(https://github.com/bitcoin/bitcoin/pull/32941#discussion_r2223796103)
Yeah, I think it's a good idea to add an error saying "this option doesn't do anything anymore, remove it from your bitcoin.conf" and continue.

Do you think it's worthwhile to still support `-maxorphantxs=0` e.g. for memory-conscious users?

💬 glozow commented on pull request "p2p: TxOrphanage revamp cleanups":
(https://github.com/bitcoin/bitcoin/pull/32941#discussion_r2223800920)
Hm, I didn't think so either, but wonder what could have caused https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2217735632 🤔

💬 sipa commented on pull request "p2p: TxOrphanage revamp cleanups":
(https://github.com/bitcoin/bitcoin/pull/32941#discussion_r2223809881)
Hmm, my best guess is a compiler/sanitizer bug.

Try something like

```c++
auto rng_seed = rng.rand256();
FastRandomContext rand_ctx(rng_seed);
```

?

💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2223829051)
> Hmm I can imagine that this can be blocking when you want to stop instantly; but I guess there is a reason why you did not return here and then empty the work queue in Stop.

Yes. We need to fulfill all promises so there are no dangling futures waiting for the worker to finish executing the task.
In the future, we could avoid this by tracking all the promises and triggering a "shutdown" exception during stop.

💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2223838649)
> Why 0?

Parallel sync is disabled by default. We’re currently not tracking the number of threads spawned by Core, so I chose not to make assumptions here (don't want indexes threads competing with net/validation ones). It’s safer to let users specify the appropriate number for their setup. In the future, we could improve this by adding thread tracking object/mechanism and picking up the best number for their machine.

👍 pablomartin4btc approved a pull request: "test: Do not pass tests on unhandled exceptions"
(https://github.com/bitcoin/bitcoin/pull/33001#pullrequestreview-3044920696)
tACK faa3e684118bffa7a98cf76eeeb59243219df900

Managed to reproduce the issue with the patch provided in the PR description, this branch fixes it. Nice detail handling `KeyboardInterrupt` for debugging purpose. Code-wise refactoring: better reading removing redundant catches.

👍 stickies-v approved a pull request: "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs"
(https://github.com/bitcoin/bitcoin/pull/32845#pullrequestreview-3044915254)
re-ACK af0ad72a0ed778c7e6f83b1a290705cefe23d78a

💬 stickies-v commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2223852456)
nit: what's the purpose of wrapping this in `namespace wallet`?

Relatedly, should probably make `TestWalletName` `static`.

💬 stickies-v commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2223857169)
nit: can keep comma to avoid touching this line