💬 fanquake commented on pull request "ci: Integrate `bitcoin-tidy` clang-tidy plugin":
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1653257372)
> lint-logs.py can probably be deleted?

Added a commit to drop it.

💬 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-1653257672)
@pinheadmz In the example I had above `n_extra_slots` controls how many extra connections you allow so it should be safe.

👍 stickies-v approved a pull request: "refactor: Revert additional univalue check in ParseSighashString"
(https://github.com/bitcoin/bitcoin/pull/28162#pullrequestreview-1549441994)
ACK 06199a995f20c55583f6948cfe99e608679fcdf1

💬 MarcoFalke commented on pull request "refactor: Revert additional univalue check in ParseSighashString":
(https://github.com/bitcoin/bitcoin/pull/28162#issuecomment-1653269199)
lgtm ACK 06199a995f20c55583f6948cfe99e608679fcdf1

💬 naumenkogs commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#discussion_r1274659835)
3d06fa372c963a0957f54fe80a0ff3462cdb3a2d
nit: might want to rename the file too :)

💬 naumenkogs commented on pull request "net: Make AddrFetch connections to fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/26114#discussion_r1276031483)
30b0f57b19eddd47fa137d0c20ae39174b54dbad

Doesn't this substantially change the observations of a user? If they get unlucky with the 10 seeds and gets stuck for 2 minutes, which was not the case before?

Maybe worth expanding a log `this might take up to 2 minutes.` or something. Ideally we won't wait anymore once we realize all those attempts failed already, but that might be difficult to code.

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