π¬ Sjors commented on pull request "kernel: chainparams & headersync updates for 30.0":
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2314193410)
`magnet:?xt=urn:btih:7341b215b570e3bc69f5fbbe5e817b51b0b9b542&dn=utxo-testnet4-90000.dat&tr=udp%3A%2F%2Ftracker.openbittorrent.com%3A80&tr=udp%3A%2F%2Ftracker.opentrackr.org%3A1337%2Fannounce&tr=udp%3A%2F%2Ftracker.coppersurfer.tk%3A6969%2Fannounce&tr=udp%3A%2F%2Ftracker.leechers-paradise.org%3A6969%2Fannounce&tr=udp%3A%2F%2Fexplodie.org%3A6969%2Fannounce&tr=udp%3A%2F%2Ftracker.torrent.eu.org%3A451%2Fannounce&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2314193410)
`magnet:?xt=urn:btih:7341b215b570e3bc69f5fbbe5e817b51b0b9b542&dn=utxo-testnet4-90000.dat&tr=udp%3A%2F%2Ftracker.openbittorrent.com%3A80&tr=udp%3A%2F%2Ftracker.opentrackr.org%3A1337%2Fannounce&tr=udp%3A%2F%2Ftracker.coppersurfer.tk%3A6969%2Fannounce&tr=udp%3A%2F%2Ftracker.leechers-paradise.org%3A6969%2Fannounce&tr=udp%3A%2F%2Fexplodie.org%3A6969%2Fannounce&tr=udp%3A%2F%2Ftracker.torrent.eu.org%3A451%2Fannounce&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
π hebasto opened a pull request: "Release: 30.0 translations update"
(https://github.com/bitcoin/bitcoin/pull/33275)
This PR follows our [Release Process](https://github.com/bitcoin/bitcoin/blob/53a996f122663e271efa52c45b173613b8ac635e/doc/release-process.md) and concludes the translation-specific efforts for this release cycle. It follows two previous translation-related PRs, https://github.com/bitcoin/bitcoin/pull/33152 and https://github.com/bitcoin/bitcoin/pull/33193.
It is one of the steps required _before_ branch-off, as scheduled in https://github.com/bitcoin/bitcoin/issues/32275.
A previous simil
...
(https://github.com/bitcoin/bitcoin/pull/33275)
This PR follows our [Release Process](https://github.com/bitcoin/bitcoin/blob/53a996f122663e271efa52c45b173613b8ac635e/doc/release-process.md) and concludes the translation-specific efforts for this release cycle. It follows two previous translation-related PRs, https://github.com/bitcoin/bitcoin/pull/33152 and https://github.com/bitcoin/bitcoin/pull/33193.
It is one of the steps required _before_ branch-off, as scheduled in https://github.com/bitcoin/bitcoin/issues/32275.
A previous simil
...
π¬ hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3242712490)
I've manually removed from Transifex, before pulling into this branch, all malicious translations, such as Bitcoin addresses etc. Translation coordinators, both seasoned and newly assigned, did a great job on keeping their translations safe and consistent.
@maflcko
Could you please run your LLM-based checks on this branch?
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3242712490)
I've manually removed from Transifex, before pulling into this branch, all malicious translations, such as Bitcoin addresses etc. Translation coordinators, both seasoned and newly assigned, did a great job on keeping their translations safe and consistent.
@maflcko
Could you please run your LLM-based checks on this branch?
π¬ Sjors commented on pull request "kernel: chainparams & headersync updates for 30.0":
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2314242221)
Ceterum censeo #31974 :-)
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2314242221)
Ceterum censeo #31974 :-)
π¬ fanquake commented on pull request "macdeploy: avoid use of `Bitcoin Core` in Linux cross build":
(https://github.com/bitcoin/bitcoin/pull/33158#issuecomment-3242773819)
> I find it weird that we would produce files named differently for the same target when the hosts are different
> Agreed that different names depending on originating platform might be a bit confusing.
I don't mind much, but I don't think the different names are that weird, given the two outputs are different. One is a release-like zip, with an immediately usable binary, the other is a zip containing a binary that wont run, which is immediately renamed, and fed into the codesigning process.
...
(https://github.com/bitcoin/bitcoin/pull/33158#issuecomment-3242773819)
> I find it weird that we would produce files named differently for the same target when the hosts are different
> Agreed that different names depending on originating platform might be a bit confusing.
I don't mind much, but I don't think the different names are that weird, given the two outputs are different. One is a release-like zip, with an immediately usable binary, the other is a zip containing a binary that wont run, which is immediately renamed, and fed into the codesigning process.
...
π¬ 151henry151 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3242805794)
Is there anything further required here? I understand that review for pull requests can take a long time (especially for non-critical things such as this one) but just wanted to check in and see if there's further action needed from me, or anything about this pull request that I could improve.
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3242805794)
Is there anything further required here? I understand that review for pull requests can take a long time (especially for non-critical things such as this one) but just wanted to check in and see if there's further action needed from me, or anything about this pull request that I could improve.
π¬ fanquake commented on pull request "build: add `-Wleading-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#discussion_r2314275670)
Sure. https://github.com/arun11299/cpp-subprocess/pull/121.
(https://github.com/bitcoin/bitcoin/pull/32482#discussion_r2314275670)
Sure. https://github.com/arun11299/cpp-subprocess/pull/121.
π¬ fanquake commented on pull request "build: add `-Wleading-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#discussion_r2314284822)
https://github.com/bitcoin-core/minisketch/pull/98.
(https://github.com/bitcoin/bitcoin/pull/32482#discussion_r2314284822)
https://github.com/bitcoin-core/minisketch/pull/98.
π¬ fanquake commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#discussion_r2314285687)
If we're going to call the scripts directly, then you should remove the targets from cmake, as there's no point having them, if they aren't used in the Guix build.
(https://github.com/bitcoin/bitcoin/pull/33247#discussion_r2314285687)
If we're going to call the scripts directly, then you should remove the targets from cmake, as there's no point having them, if they aren't used in the Guix build.
β
fanquake closed a pull request: "cmake: Use `AUTHOR_WARNING` for warnings"
(https://github.com/bitcoin/bitcoin/pull/32865)
(https://github.com/bitcoin/bitcoin/pull/32865)
π¬ fanquake commented on pull request "cmake: Use `AUTHOR_WARNING` for warnings":
(https://github.com/bitcoin/bitcoin/pull/32865#issuecomment-3242839579)
Maybe CMake will provide an option to properly do this in future, but not going to work on this further.
(https://github.com/bitcoin/bitcoin/pull/32865#issuecomment-3242839579)
Maybe CMake will provide an option to properly do this in future, but not going to work on this further.
π€ janb84 reviewed a pull request: "ci: Migrate CI to hosted Cirrus Runners"
(https://github.com/bitcoin/bitcoin/pull/32989#pullrequestreview-3174188672)
re ACK 3c5da69a232ba1cfb935012aa53e57002efe0d77
changes since last ACK:
- changed cirrus-runner for 32 bit ARM
(https://github.com/bitcoin/bitcoin/pull/32989#pullrequestreview-3174188672)
re ACK 3c5da69a232ba1cfb935012aa53e57002efe0d77
changes since last ACK:
- changed cirrus-runner for 32 bit ARM
π¬ Sjors commented on pull request "kernel: chainparams & headersync updates for 30.0":
(https://github.com/bitcoin/bitcoin/pull/33274#issuecomment-3242963481)
ACK 01fbbc5b7ea02c98f54255b8dba342a065edb9f7
I didn't check the datadir sizes.
I tested loading the testnet4 snapshot and then syncing to the tip. Still syncing a mainnet node with `-assumevalid=0` (also using a loaded snapshot), will update here in the unlikely event that it fails.
(https://github.com/bitcoin/bitcoin/pull/33274#issuecomment-3242963481)
ACK 01fbbc5b7ea02c98f54255b8dba342a065edb9f7
I didn't check the datadir sizes.
I tested loading the testnet4 snapshot and then syncing to the tip. Still syncing a mainnet node with `-assumevalid=0` (also using a loaded snapshot), will update here in the unlikely event that it fails.
π¬ 151henry151 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#discussion_r2314390992)
> If we're going to call the scripts directly, then you should remove the targets from cmake, as there's no point having them, if they aren't used in the Guix build.
Thanks @fanquake -- I believe I've correctly removed the targets from cmake now in commit ef5dd15309f9.
The Guix build hashes:
b7775f39fab10c11713ca7bf38a8cac48a143a9478999ee64b927ea9c5040245 bitcoin-ef5dd15309f9.tar.gz
e79d2bd8072a459eb75db8ef06f41a5dd9128ad63ca308cdf7c57c39670eb99f bitcoin-ef5dd15309f9-x86_64-linux-gnu.t
...
(https://github.com/bitcoin/bitcoin/pull/33247#discussion_r2314390992)
> If we're going to call the scripts directly, then you should remove the targets from cmake, as there's no point having them, if they aren't used in the Guix build.
Thanks @fanquake -- I believe I've correctly removed the targets from cmake now in commit ef5dd15309f9.
The Guix build hashes:
b7775f39fab10c11713ca7bf38a8cac48a143a9478999ee64b927ea9c5040245 bitcoin-ef5dd15309f9.tar.gz
e79d2bd8072a459eb75db8ef06f41a5dd9128ad63ca308cdf7c57c39670eb99f bitcoin-ef5dd15309f9-x86_64-linux-gnu.t
...
π¬ l0rinc commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2314389493)
Iβm not exactly sure how this works; my IDEs break here. Is my understanding correct that weβre just piggybacking a new value onto a `Future`βs per-instance `__dict__`?
Would it be cleaner to use a Future-to-string dictionary instead?
```python
self.job_name = {}
...
future = self.executor.submit(proc_wait, task)
self.job_name[future] = test
self.jobs.append(future)
...
print("Remaining jobs: [{}]".format(", ".join(self.job_name[f] for f in self.jobs)))
```
(I checked, and it seems di
...
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2314389493)
Iβm not exactly sure how this works; my IDEs break here. Is my understanding correct that weβre just piggybacking a new value onto a `Future`βs per-instance `__dict__`?
Would it be cleaner to use a Future-to-string dictionary instead?
```python
self.job_name = {}
...
future = self.executor.submit(proc_wait, task)
self.job_name[future] = test
self.jobs.append(future)
...
print("Remaining jobs: [{}]".format(", ".join(self.job_name[f] for f in self.jobs)))
```
(I checked, and it seems di
...
π¬ l0rinc commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2314417456)
is it still correct to claim that we're sleeping?
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2314417456)
is it still correct to claim that we're sleeping?
π l0rinc approved a pull request: "test: Remove polling loop from test_runner (take 2)"
(https://github.com/bitcoin/bitcoin/pull/33141#pullrequestreview-3174274620)
tested ACK fa7a3b5fb04465a3fbadf1ca62eac26827c1ef1d
there are still a few things I'd prefer before we merge, but won't block
(https://github.com/bitcoin/bitcoin/pull/33141#pullrequestreview-3174274620)
tested ACK fa7a3b5fb04465a3fbadf1ca62eac26827c1ef1d
there are still a few things I'd prefer before we merge, but won't block
π¬ l0rinc commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2314392659)
nit: since this is new code, maybe we could decompose `done` and `not_done` here already
```python
done, not_done = futures.wait(self.jobs, timeout=.5, return_when=futures.FIRST_COMPLETED)
self.jobs = list(not_done)
ret = []
for fut in done:
```
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2314392659)
nit: since this is new code, maybe we could decompose `done` and `not_done` here already
```python
done, not_done = futures.wait(self.jobs, timeout=.5, return_when=futures.FIRST_COMPLETED)
self.jobs = list(not_done)
ret = []
for fut in done:
```
π¬ l0rinc commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-3243043898)
Of course, paused it since I'm not exactly sure how do handle single-byte entries: they measurably speed up the scenarios, but the resulting code is quite ugly. Do you have a suggestion? Should I attempt that in a separate PR and ignore it here?
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-3243043898)
Of course, paused it since I'm not exactly sure how do handle single-byte entries: they measurably speed up the scenarios, but the resulting code is quite ugly. Do you have a suggestion? Should I attempt that in a separate PR and ignore it here?
π¬ l0rinc commented on pull request "coins,refactor: Reduce `getblockstats` RPC UTXO overhead estimation":
(https://github.com/bitcoin/bitcoin/pull/31449#discussion_r2314439240)
yes, both are over-estimating, but this is closer to the actual functioning, the previous one explicitly adds the bool which isn't separate.
We can also just fix it by not making this a `constexpr` but a function of the actual values, but I'd argue the current versions is still better.
(https://github.com/bitcoin/bitcoin/pull/31449#discussion_r2314439240)
yes, both are over-estimating, but this is closer to the actual functioning, the previous one explicitly adds the bool which isn't separate.
We can also just fix it by not making this a `constexpr` but a function of the actual values, but I'd argue the current versions is still better.