💬 achow101 commented on pull request "tests: speed up coins_tests by parallelizing":
(https://github.com/bitcoin/bitcoin/pull/32945#issuecomment-3104201511)
ACK 06ab3a394ade1e0d4fdb1fef636ff0d6bad71948

The test isn't that slow for me, but this is still a significant improvement.

🚀 achow101 merged a pull request: "tests: speed up coins_tests by parallelizing"
(https://github.com/bitcoin/bitcoin/pull/32945)

👍 brunoerg approved a pull request: "p2p: rename GetAddresses -> GetAddressesUnsafe"
(https://github.com/bitcoin/bitcoin/pull/32994#pullrequestreview-3044342437)
ACK 1cb23997033c395d9ecd7bf2f54787b134485f41

`GetAddresses{Unsafe}` sounds good and it's better than simply just changing the documentation.

💬 maflcko commented on pull request "RPC: Add reserve member function to `UniValue` and use it in `blockToJSON` function":
(https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-3104270978)
review ACK 5d82d92aff7c11ce17ee809c060e37f73a8a687a 🐀

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 5d82d92aff7c
...

💬 yancyribbens commented on pull request "FUZZ: Test that BnB finds best solution":
(https://github.com/bitcoin/bitcoin/pull/32894#discussion_r2223486495)
Isn't it a stronger assertion to show the invariant holds for all cases? Otherwise what's the point of any of the fuzz assertions?

💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2223488388)
I think you're right. I will make those more descriptive.

💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2223489610)
Thanks for pointing this out. I will fix this.

💬 ishaanam commented on pull request "package validation: relax the package-not-child-with-unconfirmed-parents rule":
(https://github.com/bitcoin/bitcoin/pull/31385#discussion_r2223343037)
In bc9538823f52c7274d11ae28fa2b2f7236daa69c "relax child-with-unconfirmed-parents rule "

Maybe instead of just deleting the definition of `child-with-unconfirmed-parents`, a definition of `child-with-parents` could be added?

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