💬 ishaanam commented on pull request "package validation: relax the package-not-child-with-unconfirmed-parents rule":
(https://github.com/bitcoin/bitcoin/pull/31385#discussion_r2223506244)
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.

💬 ishaanam commented on pull request "package validation: relax the package-not-child-with-unconfirmed-parents rule":
(https://github.com/bitcoin/bitcoin/pull/31385#discussion_r2223378883)
In bc9538823f52c7274d11ae28fa2b2f7236daa69c " relax child-with-unconfirmed-parents rule "
nit for precision:
```suggestion
// Since a submitted package must be child-with-parents (all of the transactions are a parent
```

💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#issuecomment-3104340209)
Yes @josibake, it's a slightly tricky problem. Rather than rush to a solution, I'd like to discuss it next meeting.

💬 maflcko commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2223539466)
> > Write may throw, but Read does not
>
> Did that change in this PR?

Yes, you are adding a call to `Read()` in this line, which does not check the return code. In the previous master, there was just a simple `m_obfuscation = new_key;` (Stuff like this is why I prefer to keep the code as-is, when possible, unless there is a reason to change and no downsides :)

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2223587480)
in the follow-up we don't have another `Read` anymore and `Write` has a constant `return true` (wanted to get rid of this in one of my PRs before)

👋 l0rinc's pull request is ready for review: "refactor,test: follow-ups to multi-byte block obfuscation"
(https://github.com/bitcoin/bitcoin/pull/33039)

💬 pablomartin4btc commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#issuecomment-3104464694)
<ins>_Updates_</ins>:
- Addressed [feedback](https://github.com/bitcoin/bitcoin/pull/32845#pullrequestreview-3024249604) from @stickies-v:
- [Updated](https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2204543796) `getdescriptoractivity` test for required params;
- [Corrected](https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2204560989) variables naming within RPC `getdescriptoractivity`;
- [Updated](https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2208503121)
...

🤔 ishaanam reviewed a pull request: "package validation: relax the package-not-child-with-unconfirmed-parents rule"
(https://github.com/bitcoin/bitcoin/pull/31385#pullrequestreview-3044403477)
utACK 613e66dfec67ca747a8c75cc70a60a3343346d83

This looks like a good improvement as it allows for package relay to be used in more situations, without updating a lot of code. This PR also also adds good testing. I only have some minor nits, please feel free to ignore.

💬 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?