π¬ 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
(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)
(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)
(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.
(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?
(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)
(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)
(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
(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.
(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)
(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.
(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)
(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
(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
(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
...
(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
...
(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.
(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.
(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.
(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.
π¬ l0rinc commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#issuecomment-2854283411)
Finished measuring `reindex-chainstate` performance for max dbcache (best case scenario), where this PR shines most (i.e. to avoid the worst-case scenario of doing everything in-memory and crashing at the end without anything saved).
Measured it on an HDD with i7 processor, so disk writes are expensive here - would likely be better on SSD.
Unfortunately this change results in a 11% slowdown for scenarios that were hoarding memory, since it has to flush intermediary results regularly, so the
...
(https://github.com/bitcoin/bitcoin/pull/32414#issuecomment-2854283411)
Finished measuring `reindex-chainstate` performance for max dbcache (best case scenario), where this PR shines most (i.e. to avoid the worst-case scenario of doing everything in-memory and crashing at the end without anything saved).
Measured it on an HDD with i7 processor, so disk writes are expensive here - would likely be better on SSD.
Unfortunately this change results in a 11% slowdown for scenarios that were hoarding memory, since it has to flush intermediary results regularly, so the
...