💬 achow101 commented on pull request "Revert "depends: Update URL for `qrencode` package source tarball"":
(https://github.com/bitcoin/bitcoin/pull/33577#issuecomment-3383051033)
Added a revert of 6de80512632afe612a3427463c94ac51f90f5203. as well since we should be preserving the filename to hash invariant, as discussed in https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3382904047
(https://github.com/bitcoin/bitcoin/pull/33577#issuecomment-3383051033)
Added a revert of 6de80512632afe612a3427463c94ac51f90f5203. as well since we should be preserving the filename to hash invariant, as discussed in https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3382904047
⚠️ rock9701 opened an issue: "rng"
(https://github.com/bitcoin/bitcoin/issues/33579)
### Motivation
# Hello, Enthusiasts 👋
I'm building a community dedicated to exploring the process of *private key generation* — specifically, the logic and inner workings of *RNG (Random Number Generators)* and the algorithms that power modern cryptographic systems.
The goal is to understand how different RNG implementations work, how they affect security, and how we can improve the quality of randomness in key generation.
If you're interested in topics like:
- entropy generation and crypto
...
(https://github.com/bitcoin/bitcoin/issues/33579)
### Motivation
# Hello, Enthusiasts 👋
I'm building a community dedicated to exploring the process of *private key generation* — specifically, the logic and inner workings of *RNG (Random Number Generators)* and the algorithms that power modern cryptographic systems.
The goal is to understand how different RNG implementations work, how they affect security, and how we can improve the quality of randomness in key generation.
If you're interested in topics like:
- entropy generation and crypto
...
✅ rock9701 closed an issue: "rng"
(https://github.com/bitcoin/bitcoin/issues/33579)
(https://github.com/bitcoin/bitcoin/issues/33579)
📝 achow101 opened a pull request: "depends: Use $(package)_file_name when downloading from the fallback"
(https://github.com/bitcoin/bitcoin/pull/33580)
The server hosting the fallbacks uses `make download` so the files are only available with their overridden names rather than the original name on the upstream source. We should therefore also use the overridden name when downloading from the fallback.
Fixes https://github.com/bitcoin-core/bitcoincore.org/issues/1168
(https://github.com/bitcoin/bitcoin/pull/33580)
The server hosting the fallbacks uses `make download` so the files are only available with their overridden names rather than the original name on the upstream source. We should therefore also use the overridden name when downloading from the fallback.
Fixes https://github.com/bitcoin-core/bitcoincore.org/issues/1168
💬 theuni commented on pull request "Revert "depends: Update URL for `qrencode` package source tarball"":
(https://github.com/bitcoin/bitcoin/pull/33577#issuecomment-3383131374)
@hebasto Could you explain the need for 46135d90ea9002e273f2a75283444afd080b81b1 as well? Since its justification was supporting behavior that we really don't want to be supported, is it necessary now that we're reverting that behavior?
(https://github.com/bitcoin/bitcoin/pull/33577#issuecomment-3383131374)
@hebasto Could you explain the need for 46135d90ea9002e273f2a75283444afd080b81b1 as well? Since its justification was supporting behavior that we really don't want to be supported, is it necessary now that we're reverting that behavior?
💬 plebhash commented on issue "RFC: Cancelling waitNext calls in the IPC mining interface":
(https://github.com/bitcoin/bitcoin/issues/33575#issuecomment-3383146351)
oh sorry, just saw the remark saying that option 3 could have undesired consequences, I overlooked that on my first reading!
so yeah option 3 is not necessarily my go-to
option 2 would slightly increase complexity on rust side, but it seems perfectly doable
and also after contemplating option 4, it also seems like a low hanging fruit on our side (even though it seems more complex to implement on C++ side)
that's because we already drop the `waitNext` promise once a `CoinbaseOutputConstraints
...
(https://github.com/bitcoin/bitcoin/issues/33575#issuecomment-3383146351)
oh sorry, just saw the remark saying that option 3 could have undesired consequences, I overlooked that on my first reading!
so yeah option 3 is not necessarily my go-to
option 2 would slightly increase complexity on rust side, but it seems perfectly doable
and also after contemplating option 4, it also seems like a low hanging fruit on our side (even though it seems more complex to implement on C++ side)
that's because we already drop the `waitNext` promise once a `CoinbaseOutputConstraints
...
👍 theuni approved a pull request: "depends: Use $(package)_file_name when downloading from the fallback"
(https://github.com/bitcoin/bitcoin/pull/33580#pullrequestreview-3316448485)
utACK 671b774d1b58c491b53f2b2f6ee42fb6b65a0e71. I was going to PR the same change.
For more context: Our sources server runs `make download` and makes those sources available as a backup host.
We can't control upstream filenames, so sometimes we rename them for local storage in depends. For ex "v0.1.tar.gz" may become "libfoo-v0.1.tar.gz".
But when we try to fetch "v0.1.tar.gz" from our backup, it doesn't exist, because it's been renamed to "libfoo-v0.1.tar.gz" there.
So instead of r
...
(https://github.com/bitcoin/bitcoin/pull/33580#pullrequestreview-3316448485)
utACK 671b774d1b58c491b53f2b2f6ee42fb6b65a0e71. I was going to PR the same change.
For more context: Our sources server runs `make download` and makes those sources available as a backup host.
We can't control upstream filenames, so sometimes we rename them for local storage in depends. For ex "v0.1.tar.gz" may become "libfoo-v0.1.tar.gz".
But when we try to fetch "v0.1.tar.gz" from our backup, it doesn't exist, because it's been renamed to "libfoo-v0.1.tar.gz" there.
So instead of r
...
💬 theuni commented on pull request "Revert "depends: Update URL for `qrencode` package source tarball"":
(https://github.com/bitcoin/bitcoin/pull/33577#issuecomment-3383202945)
Additionally, I think we want to go ahead and change the source and filename in order to disambiguate any builders who are currently in limbo. As suggested on the previous PR, I suggest:
```Makefile
$(package)_download_path=https://github.com/fukuchi/libqrencode/archive/refs/tags/
$(package)_download_file=v$($(package)_version).tar.gz
$(package)_file_name=$(package)-$($(package)_version)-v2.tar.gz
$(package)_sha256_hash=5385bc1b8c2f20f3b91d258bf8ccc8cf62023935df2d2676b5b67049f31a049c
```
...
(https://github.com/bitcoin/bitcoin/pull/33577#issuecomment-3383202945)
Additionally, I think we want to go ahead and change the source and filename in order to disambiguate any builders who are currently in limbo. As suggested on the previous PR, I suggest:
```Makefile
$(package)_download_path=https://github.com/fukuchi/libqrencode/archive/refs/tags/
$(package)_download_file=v$($(package)_version).tar.gz
$(package)_file_name=$(package)-$($(package)_version)-v2.tar.gz
$(package)_sha256_hash=5385bc1b8c2f20f3b91d258bf8ccc8cf62023935df2d2676b5b67049f31a049c
```
...
💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2415021453)
A variant has the same overhead as a vtable pointer (by adding a variant tag), but has the additional downside that the allocated memory will always have space for the larger of the member types. So in this case, that would boil down to always allocating as much memory as vtable-bearing `GenericClusterImpl`, even for singletons.
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2415021453)
A variant has the same overhead as a vtable pointer (by adding a variant tag), but has the additional downside that the allocated memory will always have space for the larger of the member types. So in this case, that would boil down to always allocating as much memory as vtable-bearing `GenericClusterImpl`, even for singletons.
💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2415028019)
I think this should be fatal (as it will crash anyway later when the caller tries to access level -1).
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2415028019)
I think this should be fatal (as it will crash anyway later when the caller tries to access level -1).
💬 achow101 commented on pull request "Revert "depends: Update URL for `qrencode` package source tarball"":
(https://github.com/bitcoin/bitcoin/pull/33577#issuecomment-3383227287)
> Additionally, I think we want to go ahead and change the source and filename in order to disambiguate any builders who are currently in limbo.
My understanding is that the revert needs to go in first to allow the CI caches to be fixed for the release branches. Then we can open a followup that does the URL and filename change, and in a way that won't break release branch CI now that we know it is a problem.
(https://github.com/bitcoin/bitcoin/pull/33577#issuecomment-3383227287)
> Additionally, I think we want to go ahead and change the source and filename in order to disambiguate any builders who are currently in limbo.
My understanding is that the revert needs to go in first to allow the CI caches to be fixed for the release branches. Then we can open a followup that does the URL and filename change, and in a way that won't break release branch CI now that we know it is a problem.
💬 hodlinator commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#issuecomment-3383238791)
Concept ACK 04b943b63ca4c27aa857b03e5de82ed8cae7b17d
Good to see progress on this issue!
(https://github.com/bitcoin/bitcoin/pull/33553#issuecomment-3383238791)
Concept ACK 04b943b63ca4c27aa857b03e5de82ed8cae7b17d
Good to see progress on this issue!
💬 hodlinator commented on pull request "validation: Improve warnings in case of chain corruption":
(https://github.com/bitcoin/bitcoin/pull/33553#discussion_r2415036002)
(Inline comment in random place): Heads up that I've started working on a functional test for this at https://github.com/bitcoin/bitcoin/compare/04b943b63ca4c27aa857b03e5de82ed8cae7b17d...hodlinator:bitcoin:pr/33553_suggestions
It's super-slow (partially due to HeadersSyncState only being created when receiving the very hardcoded 2000 headers message), and still of questionable usefulness.
(https://github.com/bitcoin/bitcoin/pull/33553#discussion_r2415036002)
(Inline comment in random place): Heads up that I've started working on a functional test for this at https://github.com/bitcoin/bitcoin/compare/04b943b63ca4c27aa857b03e5de82ed8cae7b17d...hodlinator:bitcoin:pr/33553_suggestions
It's super-slow (partially due to HeadersSyncState only being created when receiving the very hardcoded 2000 headers message), and still of questionable usefulness.
💬 theuni commented on pull request "Revert "depends: Update URL for `qrencode` package source tarball"":
(https://github.com/bitcoin/bitcoin/pull/33577#issuecomment-3383255459)
Ok, makes sense.
But after that, seems to me we should backport the bump (as well as #33580) to the release branches as well.
(https://github.com/bitcoin/bitcoin/pull/33577#issuecomment-3383255459)
Ok, makes sense.
But after that, seems to me we should backport the bump (as well as #33580) to the release branches as well.
💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2415068153)
The code here is new, but the lay-out of `m_locator` predates this PR. Going to leave it for a follow-up if anyone cares.
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2415068153)
The code here is new, but the lay-out of `m_locator` predates this PR. Going to leave it for a follow-up if anyone cares.
💬 ryanofsky commented on issue "RFC: Cancelling waitNext calls in the IPC mining interface":
(https://github.com/bitcoin/bitcoin/issues/33575#issuecomment-3383290115)
Good catch: [createNewBlock](https://github.com/bitcoin/bitcoin/blob/b510893d00760083ac36948747aa6ebd84656192/src/ipc/capnp/mining.capnp#L20C5-L20C19) (and checkBlock below it) really should take an `mp.Context` parameter. Without it, it's impossible to specify a worker thread for the calls to run on, so the calls wiill block the event loop thread while they execute, potentially affecting other calls and clients. This looks like another thing that should be fixed. But fixing it is more-or-less i
...
(https://github.com/bitcoin/bitcoin/issues/33575#issuecomment-3383290115)
Good catch: [createNewBlock](https://github.com/bitcoin/bitcoin/blob/b510893d00760083ac36948747aa6ebd84656192/src/ipc/capnp/mining.capnp#L20C5-L20C19) (and checkBlock below it) really should take an `mp.Context` parameter. Without it, it's impossible to specify a worker thread for the calls to run on, so the calls wiill block the event loop thread while they execute, potentially affecting other calls and clients. This looks like another thing that should be fixed. But fixing it is more-or-less i
...
💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2415069320)
I'm not going to change that now.
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2415069320)
I'm not going to change that now.
💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2415081345)
I don't understand the question. What change would affect the definition?
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2415081345)
I don't understand the question. What change would affect the definition?
💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2415090220)
This entire `if` branch is an optimization which could be commented out without breaking anything. It just avoids creating a new cluster when the old one could be reused.
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2415090220)
This entire `if` branch is an optimization which could be commented out without breaking anything. It just avoids creating a new cluster when the old one could be reused.
💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2415091446)
And extensibility, yes.
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2415091446)
And extensibility, yes.
💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2415091933)
We always treat allocation errors as fatal.
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2415091933)
We always treat allocation errors as fatal.