💬 MarcoFalke commented on pull request "Bugfix: RPC: Remove quotes from non-string oneline descriptions":
(https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1271252337)
I think you can remove the IsNull check below? Also, does this need release notes?

💬 MarcoFalke commented on issue "Bitcoin Core v25.0 Crashes":
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1646498972)
Can you share the output of `gettxoutsetinfo muhash`, which would allow to compare the result to one on another machine to detect potential leveldb corruption.

💬 hebasto commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#issuecomment-1646499733)
Suggesting an alternative implementation that is based on the following idea:
```diff
--- a/src/core_read.cpp
+++ b/src/core_read.cpp
@@ -242,10 +242,10 @@ bool ParseHashStr(const std::string& strHex, uint256& result)
return true;
}

-int ParseSighashString(const UniValue& sighash)
+int ParseSighashString(const std::optional<std::string>& sighash)
{
int hash_type = SIGHASH_DEFAULT;
- if (!sighash.isNull()) {
+ if (sighash.has_value()) {
static std::map<
...

📝 MarcoFalke opened a pull request: "fuzz: Re-enable symbolize=1 in ASAN_OPTIONS"
(https://github.com/bitcoin/bitcoin/pull/28124)
Looks like this fixed itself somehow and is no longer reproducible?

💬 hebasto commented on pull request "refactor: Make more transaction size variables signed":
(https://github.com/bitcoin/bitcoin/pull/28059#discussion_r1271259740)
Thanks! Updated.

💬 hebasto commented on issue "32-bit Linux: build flags lost with depends & overriden CC(X)":
(https://github.com/bitcoin/bitcoin/issues/28096#issuecomment-1646515246)
This issue seems related to another one with overridden `CC` and `CXX` -- https://github.com/bitcoin/bitcoin/pull/23571.

💬 MarcoFalke commented on pull request "build: pass sanitize flags to instrument libsecp256k1 code":
(https://github.com/bitcoin/bitcoin/pull/27991#issuecomment-1646529778)
Maybe mark as draft for as long as CI is red?

💬 MarcoFalke commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#discussion_r1271271794)
That's a no-op assignment

💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#discussion_r1271273469)
Thank you so much for noticing this. I don't know how I missed this.

💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-1646540950)
Forced pushed fixing the Shadow variable issue.

💬 MarcoFalke commented on pull request "fuzz: use `ConnmanTestMsg` in `connman`":
(https://github.com/bitcoin/bitcoin/pull/28091#discussion_r1271273781)
Any reason for this?

💬 TheCharlatan commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#issuecomment-1646544970)
Updated 05477b5566dc6f9c3d5c9609268002c0fbe1e1aa -> 67e172a7c0d82271a13f77f76ce72799faddd05c ([kernelRmUnivalue_2](https://github.com/TheCharlatan/bitcoin/tree/kernelRmUnivalue_2) -> [kernelRmUnivalue_3](https://github.com/TheCharlatan/bitcoin/tree/kernelRmUnivalue_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelRmUnivalue_2..kernelRmUnivalue_3))

* Got rid of the new `univalue_helpers` file again.
* Refactored the first commit to move the UniValue-specific code to `rpc/u
...

🤔 theStack reviewed a pull request: "test: Drop 22.x node from TxindexCompatibilityTest"
(https://github.com/bitcoin/bitcoin/pull/28070#pullrequestreview-1542075812)
Concept ACK

Small note: regarding using clean chain in feature_txindex_compatibility.py, is there any obvious advantage of re-generating blocks on every test-run rather than simply using the pre-generated one that's shared between tests? Probably both approaches are fine, Just wondering what's our general strategy here.

💬 MarcoFalke commented on pull request "test: Drop 22.x node from TxindexCompatibilityTest":
(https://github.com/bitcoin/bitcoin/pull/28070#issuecomment-1646547135)
Good question. IFF the cached test blockdir is stored in XOR format (https://github.com/bitcoin/bitcoin/pull/28052), it obviously can't be read by previous tests. So I can just drop the change here, and instead defer it to the later pull. Also, it may be good to include a compat test in the later pull?

👍 MarcoFalke approved a pull request: "doc: cleanup release process doc"
(https://github.com/bitcoin/bitcoin/pull/28003#pullrequestreview-1542076972)
lgtm

💬 MarcoFalke commented on pull request "doc: cleanup release process doc":
(https://github.com/bitcoin/bitcoin/pull/28003#discussion_r1271277231)
```suggestion
git commit -m "Add attestations by ${SIGNER} for ${VERSION} non-codesigned on [$(uname --machine)]"
```

Could include the architecture, since it is possible to use non-x86 machines to do the guix build?

💬 MarcoFalke commented on pull request "doc: cleanup release process doc":
(https://github.com/bitcoin/bitcoin/pull/28003#discussion_r1271276924)
any reason to drop this completely?

💬 MarcoFalke commented on pull request "doc: cleanup release process doc":
(https://github.com/bitcoin/bitcoin/pull/28003#discussion_r1271277474)
Can just drop this line (and above)? Alternatively, it should say to open a pull request after the push. Otherwise the push seems pointless.

💬 MarcoFalke commented on pull request "doc: cleanup release process doc":
(https://github.com/bitcoin/bitcoin/pull/28003#discussion_r1271277276)
Same?

💬 fanquake commented on pull request "doc: cleanup release process doc":
(https://github.com/bitcoin/bitcoin/pull/28003#discussion_r1271285187)
It's no-longer relevant given the number no-longer exists.