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

💬 MarcoFalke commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1274669797)
> Could just remove this line again and re-add the runtime_error catch in the test.

yes

💬 MarcoFalke commented on issue "ci: Future of macOS and Windows MSVC CI tasks":
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1651390108)
I think before working on GitHub Actions CI, it should be clear how `GITHUB_TOKEN` behaves inside GitHub CI and whether it allows code pushes from the CI or from pull requests to the main branch.

📝 TheCharlatan opened a pull request: "refactor: Revert additional univalue check in ParseSighashString"
(https://github.com/bitcoin/bitcoin/pull/28162)
This is a follow up for #28113.

The string type check is already done by the rpc parser / RPCHelpMan. Re-doing it is adding dead code. Instead, throwing an exception when the assumption does not hold is the already correct behavior. Pointed out in this [comment](https://github.com/bitcoin/bitcoin/pull/28113/files#r1274568557).

Also correct the release note for the correct sighashtype exception change. There is no change in the handling of non-string sighashtype arugments. Pointed out in th
...

📝 brunoerg converted_to_draft a pull request: "fuzz: add target for `ScriptPubKeyMan` (legacy)"
(https://github.com/bitcoin/bitcoin/pull/28153)
This PR adds target for `{Legacy}ScriptPubKeyMan`. I'm working on a descriptor one and will do it in a separate file. I tried to focus here on functions that we use directly and we may have in some unit tests.