💬 S3RK commented on pull request "RPC/Wallet: Add "use_txids" to output of getaddressinfo":
(https://github.com/bitcoin/bitcoin/pull/22693#discussion_r1560921100)
This would conflate user entered data with programmatically generated data. Address book today is managed by the user. If we want to store a list of all addresses we sent to we should pick a new storage for that.

📝 hebasto opened a pull request: "Fix typos in `subprocess.hpp`"
(https://github.com/bitcoin/bitcoin/pull/29849)
Resolves one item in the https://github.com/bitcoin/bitcoin/pull/28981#pullrequestreview-1991272752:
> - Remove linter exclusions and fix all issues.

Based on upstream https://github.com/arun11299/cpp-subprocess/pull/101.

💬 S3RK commented on pull request "RPC/Wallet: Add "use_txids" to output of getaddressinfo":
(https://github.com/bitcoin/bitcoin/pull/22693#issuecomment-2049553912)
I'm still not convinced about the approach of storing programmatically generated data in the address book, which is supposed to store user entered data.

For that reason I'm leaning Approach NACK.

💬 hebasto commented on pull request "Fix typos in `subprocess.hpp`":
(https://github.com/bitcoin/bitcoin/pull/29849#issuecomment-2049556946)
cc @theStack

👍 fanquake approved a pull request: "Fix typos in `subprocess.hpp`"
(https://github.com/bitcoin/bitcoin/pull/29849#pullrequestreview-1994047870)
ACK 13f5391bbb45cd8aebc6ae70cad08aff632ebd55

💬 laanwj commented on issue "guix: re-enable exported symbol checking for RISC-V":
(https://github.com/bitcoin/bitcoin/issues/28095#issuecomment-2049584868)
i'll try to figure out why it exports those three symbols, and if it is indication of a problem, and if not, add them to the exceptions

demangled, these are:
```
std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string<std::allocator<char> >(char const*, std::allocator<char> const&)
std::__cxx11::basic_stringbuf<char, std::char_traits<char>, std::allocator<char> >::~basic_stringbuf()
std::ctype<char>::do_widen(char) const
```

💬 BrandonOdiwuor commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#discussion_r1560951419)
updated the exit error message

💬 theuni commented on pull request "ci: use Clang 16 for Valgrind":
(https://github.com/bitcoin/bitcoin/pull/29848#issuecomment-2049594993)
Concept ACK

💬 theuni commented on pull request "guix: replace GCC unaligned VMOV patch with binutils patch":
(https://github.com/bitcoin/bitcoin/pull/29846#issuecomment-2049597893)
Concept ACK, looks like a strict improvement.

💬 BrandonOdiwuor commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#issuecomment-2049602284)
Could you please take another look at this after incorporating the suggested changes

@achow101 @maflcko @kevkevinpal @davidgumberg @epiccurious @tdb3 @itornaza

📝 laanwj opened a pull request: "net: Decrease nMaxIPs when learning from DNS seeds"
(https://github.com/bitcoin/bitcoin/pull/29850)
Limit number of IPs learned from a single DNS seed to 32, to prevent the results from one DNS seed from dominating AddrMan. Note that the number of results from a UDP DNS query is bounded to 33 already, but it is possible for it to use TCP where a larger number of results can be returned.

Closes #16070.

💬 brunoerg commented on pull request "i2p: fix and improve logs":
(https://github.com/bitcoin/bitcoin/pull/29833#discussion_r1560968649)
Sounds good, I will change it.

🚀 glozow merged a pull request: "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure"
(https://github.com/bitcoin/bitcoin/pull/29735)

⚠️ dergoegge opened an issue: "fuzz: Crash in `rpc` "CHECK_NONFATAL(last - first == 32)" "
(https://github.com/bitcoin/bitcoin/issues/29851)
```
$ echo "ZmluYWxpemVwc2J0XABwc2J0/wEAo6ujYwMDAwMDmAAAANUABQAAY2xlYXJiDAAAAAAAAABgAPX///8A6AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAACTAAAAAAAAAAAAAABgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAIAAAAAAAAAAAAAAAAAAAAAAAAAwoDAwMDAwMDAwMDAwAK8wEDAHFkAAAAAAAAAAAAZQAAAAAAAAAAANzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3NzcAAAAAD8/Pz8/Pz8/Pz8/Pz8/P0Y/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz89Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8/Pz8
...

💬 maflcko commented on pull request "test: Refactor fee calculation to remove satoshi_round function":
(https://github.com/bitcoin/bitcoin/pull/29566#discussion_r1560977607)
```suggestion
rand_fee = satoshi_round(fee_increment * (1.1892 ** random.randint(0, 28)), rounding=ROUND_DOWN)
```

Why the decimal?

💬 maflcko commented on pull request "test: Refactor fee calculation to remove satoshi_round function":
(https://github.com/bitcoin/bitcoin/pull/29566#discussion_r1560978096)
```suggestion
def satoshi_round(amount, *, rounding) -> Decimal:
```

why the default, if the goal is to not use a default?

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1560983729)
added something similar

💬 achow101 commented on issue "fuzz: Crash in `rpc` "CHECK_NONFATAL(last - first == 32)" ":
(https://github.com/bitcoin/bitcoin/issues/29851#issuecomment-2049639006)
The `CHECK_NONFATAL` is in `FromPKBytes`: https://github.com/bitcoin/bitcoin/blob/bdb33ec51986570ea17406c83bad2c955ae23186/src/script/sign.cpp#L298 which is called when getting the miniscript for a script.

This particular script is `173d36c8c9c9c9ffffffffffff0200000000021e1e37373721361818181818181e1e1e1e19000000000000000000b19292929292926b006c9b9b9292`

💬 glozow commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#issuecomment-2049639073)
Rebased for #29735 and fixed failure.

💬 achow101 commented on issue "fuzz: Crash in `rpc` "CHECK_NONFATAL(last - first == 32)" ":
(https://github.com/bitcoin/bitcoin/issues/29851#issuecomment-2049640339)
cc @darosior