🤔 maflcko reviewed a pull request: "rpc: Undeprecate rpcuser/rpcpassword, store all credentials hashed in memory"
(https://github.com/bitcoin/bitcoin/pull/32423#pullrequestreview-2817605176)
concept ack

💬 maflcko commented on pull request "rpc: Undeprecate rpcuser/rpcpassword, store all credentials hashed in memory":
(https://github.com/bitcoin/bitcoin/pull/32423#discussion_r2075115534)
nit in 1b28314259952b70071eeeae11867e921729c4c7: Can drop the trailing newline, if you retouch.

💬 maflcko commented on pull request "rpc: Undeprecate rpcuser/rpcpassword, store all credentials hashed in memory":
(https://github.com/bitcoin/bitcoin/pull/32423#discussion_r2075124023)
nit in 2392b4324777e002dc5363bac7f85cc6f8586818: For new code, `UCharCast` may be better than a "raw" cast, but only if you re-touch.

💬 stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2075137416)
Thanks, the new comment "Children of failed blocks should be marked as BLOCK_FAILED_CHILD instead." works for me, can be marked as resolved.

💬 stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2075145379)
> So I have remove the commit that adds it for now.

Actually, it looks like 79bef1bc52afab860782d6d9a6016b93f94c4503 is still here (with HEAD 9d001dc1b1627f7a973e21227955bba1f473d6aa)

💬 laanwj commented on pull request "rpc: Undeprecate rpcuser/rpcpassword, store all credentials hashed in memory":
(https://github.com/bitcoin/bitcoin/pull/32423#discussion_r2075159204)
self-nit: This could be `std::string_view` as well.

💬 maflcko commented on pull request "miniscript refactor: Remove unique_ptr-indirection (#30866 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/31713#issuecomment-2854028630)
If there is a clang-tidy bug, it could make sense to minimize the reproducer and report/fix it upstream

✅ maflcko closed an issue: "ci: failures in cross-built Windows CI"
(https://github.com/bitcoin/bitcoin/issues/32197)

💬 maflcko commented on issue "ci: failures in cross-built Windows CI":
(https://github.com/bitcoin/bitcoin/issues/32197#issuecomment-2854038524)
4eee328a9820fe4f9b70129f25b65a2be1833596

💬 hodlinator commented on pull request "miniscript refactor: Remove unique_ptr-indirection (#30866 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/31713#issuecomment-2854049412)
> If there is a clang-tidy bug, it could make sense to minimize the reproducer and report/fix it upstream

Currently trying to just recreate the CI failure locally on NixOS instead of spamming subscribers of the PR (a bit involved due to clang-tidy depending on generated LLVM headers).

https://reviews.llvm.org/D72362 which added the recursion check does mention not supporting destructors.

💬 laanwj commented on pull request "rpc: Undeprecate rpcuser/rpcpassword, store all credentials hashed in memory":
(https://github.com/bitcoin/bitcoin/pull/32423#issuecomment-2854061161)
[2392b4324777e002dc5363bac7f85cc6f8586818 → 3b0366d1775d6c00844ff0c90542814bf641bb18](https://github.com/bitcoin/bitcoin/compare/2392b4324777e002dc5363bac7f85cc6f8586818..3b0366d1775d6c00844ff0c90542814bf641bb18)
Use `UCharCast`, no newline on log message, pass in `std::string_view`, some `&` code formatting

✅ maflcko closed an issue: "intermittent issue in wallet_upgradewallet.py AssertionError: [node 2] Node returned unexpected exit code (1) vs (0) when stopping"
(https://github.com/bitcoin/bitcoin/issues/30798)

💬 maflcko commented on issue "intermittent issue in wallet_upgradewallet.py AssertionError: [node 2] Node returned unexpected exit code (1) vs (0) when stopping":
(https://github.com/bitcoin/bitcoin/issues/30798#issuecomment-2854063740)
For reference, BDB removal https://github.com/bitcoin/bitcoin/pull/31250 was merged (for 30.0)

💬 pinheadmz commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2854116062)
> a block could be filled almost entirely with OP_RETURN data

@metasmile this PR does not change this. A mining pool recently filled a block with transactions that encoded an image of an American politician. I wish there were a way to filter that but "filters" and "censorship resistance" are hard to reconcile together.

💬 pinheadmz commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2854119246)
> As a node runner I should be allowed to configure my mempool and not have it filled with what I consider spam.

@i5hi what is the use case for running a mempool at all then?

💬 stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2075208994)
Oh right, I didn't consider that `pindex` is already set to `BLOCK_FAILED_INVALID` a few lines up. I agree that this change does not introduce any additional risks from unexpected `BLOCK_TIME_FUTURE` etc `BlockValidationResult`. Thanks for the insight, can be marked as resolved.

(I still think adding the `Assume` could be helpful to catch future bugs, but it's orthogonal to the PR)

✅ maflcko closed an issue: "intermittent issue in wallet_keypool.py: assert_equal(nodes[0].getwalletinfo()["unlocked_until"], 0) AssertionError: not(1725366101 == 0)"
(https://github.com/bitcoin/bitcoin/issues/30797)

💬 maflcko commented on issue "intermittent issue in wallet_keypool.py: assert_equal(nodes[0].getwalletinfo()["unlocked_until"], 0) AssertionError: not(1725366101 == 0)":
(https://github.com/bitcoin/bitcoin/issues/30797#issuecomment-2854123548)
I guess this is expected rarely, as it is using real time

💬 maflcko commented on issue "TSAN/MSAN fails with vm.mmap_rnd_bits=32 even with llvm 18.1.3":
(https://github.com/bitcoin/bitcoin/issues/30674#issuecomment-2854174907)
Not sure how to fix this. In theory one could print an early warning (and the workaround) if the value is too large. However, this may need additional `CI_CONTAINER_CAP="--cap-add ...` for reading it directly.

An alternative would be to just close it and let everyone figure out the workaround by themselves.

✅ maflcko closed an issue: "Intermittent issue in test/ipc_tests.cpp Fatal glibc error: pthread_mutex_lock.c:450 (__pthread_mutex_lock_full): assertion failed: e != ESRCH || !robust"
(https://github.com/bitcoin/bitcoin/issues/29889)