💬 stickies-v commented on pull request "refactor: Revert additional univalue check in ParseSighashString":
(https://github.com/bitcoin/bitcoin/pull/28162#discussion_r1276034368)
Ooh actually, would this be better? Equally clear (I think), but we still feed it unexpected types.

```diff
diff --git a/src/test/fuzz/parse_univalue.cpp b/src/test/fuzz/parse_univalue.cpp
index a3d6ab6375..df3410eab8 100644
--- a/src/test/fuzz/parse_univalue.cpp
+++ b/src/test/fuzz/parse_univalue.cpp
@@ -67,8 +67,10 @@ FUZZ_TARGET(parse_univalue, .init = initialize_parse_univalue)
} catch (const std::runtime_error&) {
}
try {
- if (univalue.isNull() || univalue
...

📝 willcl-ark opened a pull request: "init: Add option for rpccookie permissions (replace 26088)"
(https://github.com/bitcoin/bitcoin/pull/28167)
This PR picks up #26088 by @aureleoules and adds a bitcoind launch option `-rpccookieperms` to configure the file permissions of the cookie generated by bitcoin core.

Example usage: `./src/bitcoind -rpccookieperms=0640`.

I added a length check to `StringToOctal()` to address @luke-jr's review comment [here](https://github.com/bitcoin/bitcoin/pull/26088#discussion_r1275362802) and swapped the ordering of the permission setting and data writing in `GenerateAuthCookie()` as per his other [sug
...

💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions":
(https://github.com/bitcoin/bitcoin/pull/26088#discussion_r1276039698)
Thanks for the reminder on this luke, as I said I'd take it up previously. I took these changes in opening #28167

💬 fanquake commented on pull request "guix: use GCC 12.3.0 to build releases":
(https://github.com/bitcoin/bitcoin/pull/27897#issuecomment-1653318660)
Pushed an update that incorporates a number of changes.
* Updated to latest time-machine, which includes a number of updates for packages we've upstreamed.
* Reworked our manifest to a state that I think is an improvement over what it used to be, and which avoids Guix build failures, i.e https://lists.gnu.org/archive/html/bug-guix/2023-07/msg00009.html.
* WIP commits are now split per platform.

Still todo:
* macOS libtapi failure.
* Re-enable / follow up with `powerpc64-linux-gnu` build
...

📝 MarcoFalke opened a pull request: "refactor: Remove unused raw-pointer read helper from univalue"
(https://github.com/bitcoin/bitcoin/pull/28168)
The helpers are unused outside of tests and redundant with the existing `bool read(std::string_view raw);`.

Fix both issues by removing them.

Also, simplify the tests code by removing a `std::string` constructor where possible.

🚀 fanquake merged a pull request: "ci: document that -Wreturn-type has been fixed upstream (mingw-w64)"
(https://github.com/bitcoin/bitcoin/pull/28092)

💬 naumenkogs commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1276078272)
nit: may be worth explaining that this randomization helps to get to NET_I2P option without traversing the entire NET_ONION set of potentially rubbish or buggy stuff :)

💬 naumenkogs commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1276083657)
Do we have any risk of TOR and I2P peers bouncing each other forever?
Say, you have 7 ip4 peers and 1 Tor peer. I2P bounces Tor. In the next step, Tor (maybe another node, or maybe the same) bounces I2P node back.

📝 ChrisCho-H opened a pull request: "script: check op_verif and op_vernotif"
(https://github.com/bitcoin/bitcoin/pull/28169)
I suggest adding `SCRIPT_ERR_OP_VERIF_VERNOTIF` error to check whether it's either `OP_VERIF` or `OP_VERNOTIF` since,
1. As opcodes rejected even in an unexecuted branch, are checked and throw error with an appropriate message(like disabled and codeseperator in non-segwit) to identify, op_verif and op_vernotif(which are rejected even in an unexecuted branch as well) should be treated in same way which is clear to debug
2. It still makes script invalid now as both opcodes are in between op_if a
...

💬 MarcoFalke commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1276097526)
You'll need to pin this to a tag/commit/branch that changes when the features change. Otherwise the CI will be non-deterministic

💬 fanquake commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1276101720)
Yea. I'm not sure exactly what we'll do yet, but I think we could just have a "stable" branch in the tidy-plugin repo.

💬 MarcoFalke commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1276102904)
That wouldn't work, the id of the tag/commit/branch needs to be different for each added or removed feature.

💬 MarcoFalke commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1653367400)
Also, CI (tidy) fails

💬 fanquake commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1653367770)
> Also, CI (tidy) fails

Obviously. Given there is a commit here to make it fail?

💬 fanquake commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1276105268)
I don't understand what you mean.

💬 MarcoFalke commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1276110597)
The CI system caches built images. The cache-key is (among other stuff) this file. If the file does not change, the cache-key does not change either. However, if the `bitcoin-tidy-plugin` changes, everyone may have a different version of `bitcoin-tidy-plugin` in the cache, without the cache being invalidated. Thus, everyone may get different results for the same commit-id in this repo. (This is known as non-deterministic).

It can be fixed by pinning to a tag/commit/branch/...

💬 MarcoFalke commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1653380016)
Sure, no worries. Maybe keep it a draft for as long as CI is red or fix the errors? Otherwise, it can't be merged/ack'd anyway.

💬 dergoegge commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1653381135)
> Obviously. Given there is a commit here to make it fail?

Yes but it's also failing on a different LogPrint statement, so no need for that commit i guess

💬 fanquake commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1653386005)
> Yes but it's also failing on a different LogPrint statement, so no need for that commit i guess

Yea, assuming it's an actual issue, and not a false positive. If it is, then it means our current linter doesn't work anyways.

💬 MarcoFalke commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#discussion_r1276120896)
For example, you can add `git checkout 1806ef33ff8b14256d766eb9f616a3528aad4464` in the next line.