✅ fanquake closed an issue: "Add Bech32.cpp LocateErrors case to check for minimum length."
(https://github.com/bitcoin/bitcoin/issues/28159)

💬 russeree commented on issue "Add Bech32.cpp LocateErrors case to check for minimum length.":
(https://github.com/bitcoin/bitcoin/issues/28159#issuecomment-1651205286)
I apologize for the double posting, I haven't seen others do it in the past. Lesson learned.

💬 MarcoFalke commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1274568557)
Not sure about adding dead code. Before on master this will already throw and never reach this part of the code:

```
test_framework.authproxy.JSONRPCException: Wrong type passed:
{
"Position 3 (sighashtype)": "JSON value of type bool is not of expected type string"
} (-3)
```

💬 MarcoFalke commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1274567555)
Is this true? Before on master this will already throw:

```
test_framework.authproxy.JSONRPCException: Wrong type passed:
{
"Position 3 (sighashtype)": "JSON value of type bool is not of expected type string"
} (-3)
```

🚀 fanquake merged a pull request: "test: Ignore UTF-8 errors in assert_debug_log"
(https://github.com/bitcoin/bitcoin/pull/28035)

💬 MarcoFalke commented on pull request "util: Remove DirIsWritable, GetUniquePath":
(https://github.com/bitcoin/bitcoin/pull/28075#discussion_r1274571873)
cc @TheCharlatan (libbitcoinkernel)

🚀 fanquake merged a pull request: "valgrind: add suppression for bug 472219"
(https://github.com/bitcoin/bitcoin/pull/28145)

🚀 fanquake merged a pull request: "test: fix `feature_addrman.py` on big-endian systems"
(https://github.com/bitcoin/bitcoin/pull/27529)

🚀 fanquake merged a pull request: "suppressions: note that `type:ClassName::MethodName` should be used"
(https://github.com/bitcoin/bitcoin/pull/28147)

💬 carnhofdaki commented on pull request "BIP324 ciphersuite":
(https://github.com/bitcoin/bitcoin/pull/28008#issuecomment-1651257804)
@sipa I think that the disadvantages you mention are much worse than the advantages it would bring. I think that keeping it simple is (at least in this implementation intended for first appearance in the `master` branch) an important feature of BIP-324.

As for me and my poor old Bitcoin node,
please do not add complexity to the code.

🚀 fanquake merged a pull request: "test: Avoid intermittent issues due to async events in validationinterface_tests"
(https://github.com/bitcoin/bitcoin/pull/28150)

📝 MarcoFalke opened a pull request: "ci: Move ASan USDT to persistent_worker"
(https://github.com/bitcoin/bitcoin/pull/28161)
To run the USDT functional tests, the ASan task currently requires the container host to run the Ubuntu Lunar Linux kernel (or later). Cirrus CI is the only provider that allows to spin up full VMs, however they will start to charge for all tasks (See slightly related discussion in https://github.com/bitcoin/bitcoin/issues/28098).

Since it is cheaper and recommended by Cirrus CI to just run a persistent worker, do that.

💬 naumenkogs commented on pull request "net: Add new permission `forceinbound` to evict a random unprotected connection if all slots are otherwise full":
(https://github.com/bitcoin/bitcoin/pull/27600#issuecomment-1651285875)
>the attacker needs to both learn and then spoof the protected IP range

I think a malicious ISP gets that trivially (at least in the pre-BIP324 world) by simply monitoring the traffic.

In theory, specifying "forceinbound=iprange;n_extra_slots" instead of eviction is probably a good idea, but I'm not sure how ugly the code will look like. I remember max_connections stuff is already sophisticated :(

💬 josibake commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1274629530)
Fixed, thanks!

🚀 fanquake merged a pull request: "test: create wallet specific for test_locked_wallet case"
(https://github.com/bitcoin/bitcoin/pull/28139)

💬 naumenkogs commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1651340516)
ACK 1e09c265a9598df3cc39336e3bcccb993dacf3d8

💬 TheCharlatan commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1274662192)
Yeah, this is very wrong :(

Before a `RPC_PARSE_ERROR` was raised if the argument is the wrong type, and a `RPC_MISC_ERROR` if the sighash type could not be parsed.

Now a `RPC_PARSE_ERROR` is still raised if the argument is the wrong type, and a `RPC_INVALID_PARAMETER` if the sighash type could not be parsed.

Will fix.

💬 TheCharlatan commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1274663777)
Do you have a suggestion for how to handle the fuzz test then? Could just remove this line again and re-add the `runtime_error` catch in the test.

💬 MarcoFalke commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1274667131)
If the options is used to speedup tx relay / mempool sync, it should be named appropriately. For example `self.noban_tx_relay`. Also, the option shouldn't be set for tests that previously didn't have it set. Otherwise they are testing something else. (`noban` has other effects than just immediate trickle)

💬 TheCharlatan commented on pull request "util: Remove DirIsWritable, GetUniquePath":
(https://github.com/bitcoin/bitcoin/pull/28075#issuecomment-1651360743)
Nice, Concept ACK.