💬 maflcko commented on issue "bitcoind shouldn't fail to progress with synchronization: endless [leveldb] Generated table ... logs":
(https://github.com/bitcoin/bitcoin/issues/31882#issuecomment-2666700079)
How fast is the hardware? What is the speed for contiguous reads/writes that do not go through the filesystem cache? What is the speed for random read/writes that also do not go through the filesystem cache?
(https://github.com/bitcoin/bitcoin/issues/31882#issuecomment-2666700079)
How fast is the hardware? What is the speed for contiguous reads/writes that do not go through the filesystem cache? What is the speed for random read/writes that also do not go through the filesystem cache?
💬 achow101 commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2666737879)
I've notarized the arm64 binaries, does running the downloaded the binaries still result in an error?
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2666737879)
I've notarized the arm64 binaries, does running the downloaded the binaries still result in an error?
💬 GregTonoski commented on issue "bitcoind shouldn't fail to progress with synchronization: endless [leveldb] Generated table ... logs":
(https://github.com/bitcoin/bitcoin/issues/31882#issuecomment-2666758852)
> How fast is the hardware? What is the speed for contiguous reads/writes that do not go through the filesystem cache? What is the speed for random read/writes that also do not go through the filesystem cache?
Tell me what they are expected to be normally and I will measure the actual values, please.
(https://github.com/bitcoin/bitcoin/issues/31882#issuecomment-2666758852)
> How fast is the hardware? What is the speed for contiguous reads/writes that do not go through the filesystem cache? What is the speed for random read/writes that also do not go through the filesystem cache?
Tell me what they are expected to be normally and I will measure the actual values, please.
📝 tnull opened a pull request: "Fix delimeter in `package-msg` field of `submitpackage` RPC"
(https://github.com/bitcoin/bitcoin/pull/31900)
Previously, the `submitpackage` RPC call returned a response key named `package_msg`, which is inconsistent with the other field names that are using `-` as a delimiter. Here, we align the style of `package-msg`.
cc @glozow @instagibbs
(https://github.com/bitcoin/bitcoin/pull/31900)
Previously, the `submitpackage` RPC call returned a response key named `package_msg`, which is inconsistent with the other field names that are using `-` as a delimiter. Here, we align the style of `package-msg`.
cc @glozow @instagibbs
🤔 zaidmstrr reviewed a pull request: "Feature: Use different datadirs for different signets"
(https://github.com/bitcoin/bitcoin/pull/29838#pullrequestreview-2624854068)
Concept ACK
A very much-needed feature when you're working with different signet chains.
(https://github.com/bitcoin/bitcoin/pull/29838#pullrequestreview-2624854068)
Concept ACK
A very much-needed feature when you're working with different signet chains.
💬 jonatack commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1960490229)
I think so, best avoid needing to (remember to) retouch those.
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1960490229)
I think so, best avoid needing to (remember to) retouch those.
🤔 glozow reviewed a pull request: "Fix delimeter in `package-msg` field of `submitpackage` RPC"
(https://github.com/bitcoin/bitcoin/pull/31900#pullrequestreview-2624878185)
lgtm, though I've heard we don't like hyphens, so perhaps go in the other direction and change all the other ones to `_`?
(https://github.com/bitcoin/bitcoin/pull/31900#pullrequestreview-2624878185)
lgtm, though I've heard we don't like hyphens, so perhaps go in the other direction and change all the other ones to `_`?
💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1960516927)
Both options are O(n log n), but a vector + sorting + deduplication has much better constant factors.
In theory, using an `std::unordered_map` could asymptotically faster for large numbers, but I suspect not for the numbers we care about. I haven't tried or benchmarked it, though.
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1960516927)
Both options are O(n log n), but a vector + sorting + deduplication has much better constant factors.
In theory, using an `std::unordered_map` could asymptotically faster for large numbers, but I suspect not for the numbers we care about. I haven't tried or benchmarked it, though.
💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1960520612)
Agreed, I calculated recently.
This is better.
The constant factors for GetDistinctClusters implementation using vector and then sorting is also better 👍🏾
Please resolve.
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1960520612)
Agreed, I calculated recently.
This is better.
The constant factors for GetDistinctClusters implementation using vector and then sorting is also better 👍🏾
Please resolve.
💬 ismaelsadeeq commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1960020302)
It can be called when oversized but for staging `DoWork` it will be no-op when it is oversized.
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1960020302)
It can be called when oversized but for staging `DoWork` it will be no-op when it is oversized.
💬 philmb3487 commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#issuecomment-2666829924)
@sipa Thanks for bringing this up. Apple moved the exFat filesystem to userspace and it had nasty side effects. What a terrible idea, what is Apple even thinking?
To be sure, have other drivers been moved to user-space?
For all we know they might also be broken. It needs to be checked, it seems doubtful they would only move one driver over.
On Tue, Feb 18, 2025 at 21:00, l0rinc ***@***.***(mailto:On Tue, Feb 18, 2025 at 21:00, l0rinc <<a href=)> wrote:
> @l0rinc commented on this pull request.
...
(https://github.com/bitcoin/bitcoin/pull/31453#issuecomment-2666829924)
@sipa Thanks for bringing this up. Apple moved the exFat filesystem to userspace and it had nasty side effects. What a terrible idea, what is Apple even thinking?
To be sure, have other drivers been moved to user-space?
For all we know they might also be broken. It needs to be checked, it seems doubtful they would only move one driver over.
On Tue, Feb 18, 2025 at 21:00, l0rinc ***@***.***(mailto:On Tue, Feb 18, 2025 at 21:00, l0rinc <<a href=)> wrote:
> @l0rinc commented on this pull request.
...
💬 tnull commented on pull request "Fix delimeter in `package-msg` field of `submitpackage` RPC":
(https://github.com/bitcoin/bitcoin/pull/31900#issuecomment-2666838815)
> lgtm, though I've heard we don't like hyphens, so perhaps go in the other direction and change all the other ones to `_`?
Hmm, sure, happy to change if that would be preferable, mostly went this way as I saw other examples using `-` (e.g., `p2sh-segwit` in `decodescript` result, `bit125-replaceable` in the wallet RPCs, etc), but I now realize there are also many (more?) that use `_`. This makes me wonder if we are really positive it's worth breaking the current API, if neither of the option
...
(https://github.com/bitcoin/bitcoin/pull/31900#issuecomment-2666838815)
> lgtm, though I've heard we don't like hyphens, so perhaps go in the other direction and change all the other ones to `_`?
Hmm, sure, happy to change if that would be preferable, mostly went this way as I saw other examples using `-` (e.g., `p2sh-segwit` in `decodescript` result, `bit125-replaceable` in the wallet RPCs, etc), but I now realize there are also many (more?) that use `_`. This makes me wonder if we are really positive it's worth breaking the current API, if neither of the option
...
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1960532733)
thanks, removed `DEFAULT_MAX_ORPHAN_TRANSACTIONS`
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1960532733)
thanks, removed `DEFAULT_MAX_ORPHAN_TRANSACTIONS`
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1960532993)
thanks, fixed
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1960532993)
thanks, fixed
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1960533250)
changed to be asserting exact counts 👍
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r1960533250)
changed to be asserting exact counts 👍
💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2666866811)
@ismaelsadeeq
> It seems to me that this is designed with the flexibility to accommodate more levels, which explains the use of `std::vector` for `ClusterSet`
It's not really written with the intent of accomodating more levels; it's more that I found it more elegant to write it generically, because most operations don't care what level they are operating on. I can see that the increased abstraction may be confusing though; I'd like to hear more reviewer comments on this.
> If yes then
...
(https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2666866811)
@ismaelsadeeq
> It seems to me that this is designed with the flexibility to accommodate more levels, which explains the use of `std::vector` for `ClusterSet`
It's not really written with the intent of accomodating more levels; it's more that I found it more elegant to write it generically, because most operations don't care what level they are operating on. I can see that the increased abstraction may be confusing though; I'd like to hear more reviewer comments on this.
> If yes then
...
📝 maflcko opened a pull request: " contrib: Add deterministic-unittest-coverage "
(https://github.com/bitcoin/bitcoin/pull/31901)
The `contrib/devtools/test_deterministic_coverage.sh` script is problematic:
* It is written in bash. This can lead to issues when running with the ancient bash version shipped by macOS by default, or can lead to other compatibility issues, such as https://github.com/bitcoin/bitcoin/pull/31588#discussion_r1946784827. Also, pipefail isn't set, so IO errors may be silently ignored.
* It is based on gcov. This can lead to issues, such as https://github.com/bitcoin/bitcoin/pull/31588#pullrequest
...
(https://github.com/bitcoin/bitcoin/pull/31901)
The `contrib/devtools/test_deterministic_coverage.sh` script is problematic:
* It is written in bash. This can lead to issues when running with the ancient bash version shipped by macOS by default, or can lead to other compatibility issues, such as https://github.com/bitcoin/bitcoin/pull/31588#discussion_r1946784827. Also, pipefail isn't set, so IO errors may be silently ignored.
* It is based on gcov. This can lead to issues, such as https://github.com/bitcoin/bitcoin/pull/31588#pullrequest
...
💬 glozow commented on pull request "Fix delimeter in `package-msg` field of `submitpackage` RPC":
(https://github.com/bitcoin/bitcoin/pull/31900#issuecomment-2666867794)
I want to say this changed over time... I can't find the thread where I was convinced, but here's chat GPT's argument for why underscores are better than hyphens in JSON:
1. Consistency with JavaScript & JSON standards – JSON keys are commonly accessed in JavaScript, and using hyphens (some-key) requires bracket notation (obj["some-key"]) instead of the simpler dot notation (obj.some_key).
2. Better compatibility – Many programming languages (Python, JavaScript, etc.) use underscores for var
...
(https://github.com/bitcoin/bitcoin/pull/31900#issuecomment-2666867794)
I want to say this changed over time... I can't find the thread where I was convinced, but here's chat GPT's argument for why underscores are better than hyphens in JSON:
1. Consistency with JavaScript & JSON standards – JSON keys are commonly accessed in JavaScript, and using hyphens (some-key) requires bracket notation (obj["some-key"]) instead of the simpler dot notation (obj.some_key).
2. Better compatibility – Many programming languages (Python, JavaScript, etc.) use underscores for var
...
💬 maflcko commented on pull request "contrib: fix `test_deterministic_coverage.sh` script for out-of-tree builds":
(https://github.com/bitcoin/bitcoin/pull/31588#issuecomment-2666868256)
Done in https://github.com/bitcoin/bitcoin/pull/31901
(https://github.com/bitcoin/bitcoin/pull/31588#issuecomment-2666868256)
Done in https://github.com/bitcoin/bitcoin/pull/31901
💬 maflcko commented on pull request "contrib: Add deterministic-fuzz-coverage":
(https://github.com/bitcoin/bitcoin/pull/31836#issuecomment-2666868659)
Done in https://github.com/bitcoin/bitcoin/pull/31901
(https://github.com/bitcoin/bitcoin/pull/31836#issuecomment-2666868659)
Done in https://github.com/bitcoin/bitcoin/pull/31901