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

💬 maflcko commented on 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#issuecomment-2854218017)
Is this still an issue with a recent version of Bitcoin Core? If yes, what are the steps to reproduce?

As mentioned above, running in `rr` could be the most useful.

✅ maflcko closed an issue: "Intermittent failure in rpc_bind.py: in get_socket_inodes FileNotFoundError: [Errno 2] No such file or directory: '/proc/34007/fd/23'"
(https://github.com/bitcoin/bitcoin/issues/29643)

💬 maflcko commented on issue "Intermittent failure in rpc_bind.py: in get_socket_inodes FileNotFoundError: [Errno 2] No such file or directory: '/proc/34007/fd/23'":
(https://github.com/bitcoin/bitcoin/issues/29643#issuecomment-2854221586)
b2e9fdc00f5c40c241a37739f7b73b74c2181e39

💬 Sjors commented on pull request "rpc, rest: Improve block rpc error handling, check header before attempting to read block data.":
(https://github.com/bitcoin/bitcoin/pull/30410#issuecomment-2854226908)
It would be nice to have unique (new) RPC error codes for this, because e.g. with ForkMonitor I was parsing the string that comes with `RPC_MISC_ERROR`, which this PR slightly changes.

https://github.com/jonathanbier/forkmonitor/commit/2d0e9f48cbd95b5ce52416ef497d1691ea3e24f0

💬 stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2075270458)
> I think that's why sometimes one or the other is used.

Right, that makes sense.

I think in the two other cases where `CBlockIndexWorkComparator` is used in this PR, comparing directly on `nChainwork` would be more readable and more correct?
- https://github.com/bitcoin/bitcoin/pull/31405/files#diff-97c3a52bc5fad452d82670a7fd291800bae20c7bc35bb82686c2c0a4ea7b5b98R3704: "that have at least as much work as where we expect the new tip to end up."
- https://github.com/bitcoin/bitcoin/pull/3
...

💬 murrayn commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2854249655)
Concept NACK

I speak as a core contributor. I find this whole situation terrifying. There is clearly enough resistance to this PR that it should either be dismissed outright, or its rationale suitably justified to the whole community, whose judgement has been proven correct several times in the past. Yes, this may mean utilising X or Youtube instead of the mailing list or moderated github comments, as uncomfortable as that may be for some, but if there is a solid rationale for this change it
...

💬 0xmatt11 commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2854263883)
Removing the OP_RETURN limit is a sensible step. The 83-byte cap no longer serves its original purpose and only encourages inefficient workarounds. Larger, provably unspendable outputs don’t burden the UTXO set and open the door for legitimate use cases like sidechains and timestamping. Bitcoin should enable innovation, not restrict it arbitrarily.

💬 laanwj commented on pull request "rpc: Undeprecate rpcuser/rpcpassword, store all credentials hashed in memory":
(https://github.com/bitcoin/bitcoin/pull/32423#discussion_r2075287790)
i intentionally kept this as-is (eg converting to use `Split` slightly changes semantics), but thinking of it, calling `user_pass.find(':')`three times in a row is maybe a bit much.

💬 maflcko commented on issue "wallet: Data race in GetOrCreateLegacyScriptPubKeyMan vs IsMine":
(https://github.com/bitcoin/bitcoin/issues/27354#issuecomment-2854265066)
For reference, BDB removal https://github.com/bitcoin/bitcoin/pull/31250 was merged (for 30.0)

However, this issue may still exist, as `m_spk_managers` still exists.