💬 hodlinator commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2351044620)
Again, `AssumeValid::SKIPPED` meaning script checking is off seems like some weird double-negation to me. Below naming would make more sense to me.
```suggestion
enum class ScriptCheck : uint8_t
{
DISABLED_ASSUME_VALID, //!< skip script verification
ENABLED_NO_ASSUME_VALID, //!< always verify scripts
ENABLED_HASH_NOT_IN_HEADERS, //!< assumevalid hash not found in m_block_index
ENABLED_NOT_UNDER_ASSUMEVALID, //!< pindex is not an ancestor of the assumevalid
...

📝 BrandonOdiwuor opened a pull request: "ci: Remove bash -c from cmake invocation using eval"
(https://github.com/bitcoin/bitcoin/pull/33401)
Follow up to https://github.com/bitcoin/bitcoin/pull/32970


https://github.com/bitcoin/bitcoin/pull/32970#r2213730157

> Does `cmake -S ...` still need to be wrapped in `bash -c "..."`?

https://github.com/bitcoin/bitcoin/pull/32970#r2213741192
> It is not trivial to replace. Maybe the `eval` hack from below can be used:
>
> ```shell
> # parses TEST_RUNNER_EXTRA as an array which allows for multiple arguments such as TEST_RUNNER_EXTRA='--exclude "rpc_bind.py --ipv6"'
>
> eval
...

🤔 hebasto reviewed a pull request: "ci: Remove bash -c from cmake invocation using eval"
(https://github.com/bitcoin/bitcoin/pull/33401#pullrequestreview-3229000702)
Concept ACK on expanding ShellCheck lint coverage.

🚀 fanquake merged a pull request: "depends: systemtap 5.3"
(https://github.com/bitcoin/bitcoin/pull/33373)

💬 pinheadmz commented on pull request "Remove unnecessary casts when calling socket operations":
(https://github.com/bitcoin/bitcoin/pull/33378#discussion_r2351871877)
right thanks!

💬 hodlinator commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2351892330)
Found a way to test 2):
```python
# nodes[4]
self.start_node(4, extra_args=["-assumevalid=" + block102.hash_hex])
second_chain_tip = int(self.nodes[4].getbestblockhash(), 16)
second_chain_time = self.nodes[4].getblock(self.nodes[4].getbestblockhash())['time'] + 1
second_chain_height = self.nodes[4].getblock(self.nodes[4].getbestblockhash())['height'] + 1

second_chain = []
for _ in range(2100):
block = create_block(sec
...

📝 mstampfer opened a pull request: "contrib: Add zsh completion scripts"
(https://github.com/bitcoin/bitcoin/pull/33402)
Adds zsh completion support using the compdef system for:
- bitcoin-cli with RPC command completion and context-sensitive arguments
- bitcoin-tx with option/command completion and file path handling
- bitcoind with configuration option completion

These complement the existing bash completion scripts and provide
the same functionality for zsh users.

💬 mstampfer commented on pull request "contrib: Add zsh completion scripts":
(https://github.com/bitcoin/bitcoin/pull/33402#issuecomment-3297709907)
This PR adds zsh completion scripts for Bitcoin Core tools, converting the existing bash completions to work with zsh's compdef system. This Improves user experience for zsh users by addressing the need for tab completion support for zsh which doesn't rely on the existing bash completion scripts.

The implementation maintains the same functionality as the bash versions while using zsh-native completion functions.

I'm happy to address any concerns or make requested changes.

🤔 marcofleon reviewed a pull request: "test: Add submitblock test in interface_ipc"
(https://github.com/bitcoin/bitcoin/pull/33380#pullrequestreview-3229240434)
code review ACK 0a26731c4cc182e887ce959cdd301227cdc752d7

✅ fanquake closed a pull request: "util/strencodings: guard SAFE_CHARS index and pre-reserve result"
(https://github.com/bitcoin/bitcoin/pull/33396)

💬 fanquake commented on pull request "util/strencodings: guard SAFE_CHARS index and pre-reserve result":
(https://github.com/bitcoin/bitcoin/pull/33396#issuecomment-3297809297)
Thanks, however we can leave this code as-is.

✅ fanquake closed a pull request: "refactor: Fix typo and correct template parameter inconsistency"
(https://github.com/bitcoin/bitcoin/pull/33394)

💬 fanquake commented on pull request "refactor: Fix typo and correct template parameter inconsistency":
(https://github.com/bitcoin/bitcoin/pull/33394#issuecomment-3297824742)
Thanks, however I think this code can be left as-is for now.

💬 TheCharlatan commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2352051546)
Are the enums really only added for logging? That seems a bit heavy. How about moving this `if` block up and making it a lambda that gets called directly from your `if else` branches? afaict that could remove the need for them, which judged by the comments left here so far, seem to confuse people a bit with their communicated intent.

💬 TheCharlatan commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2351814133)
I'm a bit confused what the condition in this test is supposed to trigger if all we are testing are invalid args. What is the rationale here?

👍 hodlinator approved a pull request: "Remove unnecessary casts when calling socket operations"
(https://github.com/bitcoin/bitcoin/pull/33378#pullrequestreview-3229478311)
ACK 67f632b6deb8b4aa190c458b71d2bc8c793626d5

(PR description still ends with "Opening as a draft for now to test CI on supported platforms.")

📝 willcl-ark opened a pull request: "WIP: Backport Cirrus runners to 29.x"
(https://github.com/bitcoin/bitcoin/pull/33403)
Backports #32989 to the 29.x branch

💬 hodlinator commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2352176441)
Yeah, now that you use `InitWarning()` adjacently, it's clearer why we would expect true.

👍 hodlinator approved a pull request: "coins: warn on oversized `-dbcache`"
(https://github.com/bitcoin/bitcoin/pull/33333#pullrequestreview-3229526417)
ACK 13fa0d0a433722f294854001b0561c079db12dbc

Very helpful to warn users against unintentionally making their system start swapping insanely.

💬 fanquake commented on issue "Default ASmap file path is not used unless -asmap is set":
(https://github.com/bitcoin/bitcoin/issues/33386#issuecomment-3298250998)
@fjahr