📝 l0rinc opened a pull request: "CI: Add label to scripted-diffs"
(https://github.com/bitcoin/bitcoin/pull/31089)
testing
(https://github.com/bitcoin/bitcoin/pull/31089)
testing
✅ hebasto closed an issue: "CMake-based build system tracking issue"
(https://github.com/bitcoin/bitcoin/issues/28607)
(https://github.com/bitcoin/bitcoin/issues/28607)
💬 hebasto commented on issue "CMake-based build system tracking issue":
(https://github.com/bitcoin/bitcoin/issues/28607#issuecomment-2413162020)
Closing. See https://github.com/bitcoin/bitcoin/pull/30454.
(https://github.com/bitcoin/bitcoin/issues/28607#issuecomment-2413162020)
Closing. See https://github.com/bitcoin/bitcoin/pull/30454.
📝 l0rinc converted_to_draft a pull request: "CI: Add label to scripted-diffs"
(https://github.com/bitcoin/bitcoin/pull/31089)
Add a `scripted-diff` label for pull requests that contain `scripted-diff:` commits to make sure that CI is running them properly.
(https://github.com/bitcoin/bitcoin/pull/31089)
Add a `scripted-diff` label for pull requests that contain `scripted-diff:` commits to make sure that CI is running them properly.
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1800679447)
This was introduced in b335710782c2545e6eeed67b5e1763c07eab26b0.
Related to https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1867610348.
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1800679447)
This was introduced in b335710782c2545e6eeed67b5e1763c07eab26b0.
Related to https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1867610348.
💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1800702587)
Can you suggest another name? It's a map to get peer info, hence `m_peer_info`.
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1800702587)
Can you suggest another name? It's a map to get peer info, hence `m_peer_info`.
💬 hebasto commented on issue "Run coverage functional tests in parallel":
(https://github.com/bitcoin/bitcoin/issues/26736#issuecomment-2413249267)
> This was probably resolved by `cmake`.
From the "Compiling for test coverage" section of [Developer Notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#compiling-for-test-coverage):
> To enable test parallelism:
>
> cmake -DJOBS=$(nproc) -P build/Coverage.cmake
(https://github.com/bitcoin/bitcoin/issues/26736#issuecomment-2413249267)
> This was probably resolved by `cmake`.
From the "Compiling for test coverage" section of [Developer Notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#compiling-for-test-coverage):
> To enable test parallelism:
>
> cmake -DJOBS=$(nproc) -P build/Coverage.cmake
💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1800708031)
Grep will show `AddTxAnnouncement` called with `p2p_inv=false` in the new orphan handling code. We don't exit because we might already have a low feerate parent in reconsiderable rejects. You can also comment this out to see what fails.
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1800708031)
Grep will show `AddTxAnnouncement` called with `p2p_inv=false` in the new orphan handling code. We don't exit because we might already have a low feerate parent in reconsiderable rejects. You can also comment this out to see what fails.
💬 laanwj commented on pull request "Don't zero-after-free `DataStream`: Faster IBD on some configurations":
(https://github.com/bitcoin/bitcoin/pull/30987#issuecomment-2413262312)
Concept ACK. Using zero-after-free allocators for all CDataStream usage is overkill. i don't think there are any, but if there are places where it is used (wallet) where security against key leaks is important, we could parametrize `CDataStream<AllocatorType>` and pass a `secure_allocator`. Agree that zeroing adding extra security against buffer overflows is a red herring.
i expect this can make a significant differences on systems with relatively slow CPU or memory, like ARM systems (will r
...
(https://github.com/bitcoin/bitcoin/pull/30987#issuecomment-2413262312)
Concept ACK. Using zero-after-free allocators for all CDataStream usage is overkill. i don't think there are any, but if there are places where it is used (wallet) where security against key leaks is important, we could parametrize `CDataStream<AllocatorType>` and pass a `secure_allocator`. Agree that zeroing adding extra security against buffer overflows is a red herring.
i expect this can make a significant differences on systems with relatively slow CPU or memory, like ARM systems (will r
...
💬 Xaspr commented on issue "Unable to sync blockchain on laptop: ERROR: ReadBlockFromDisk: Deserialize or I/O error":
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-2413278619)
Unfortunately limiting CPU and memory usage in Windows itself and in Project Lasso didn't work. Core still hangs while indexing after a few hours & the blocks are corrupted afterwards. It must be a hardware issue, as no one else experiences this issue.
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-2413278619)
Unfortunately limiting CPU and memory usage in Windows itself and in Project Lasso didn't work. Core still hangs while indexing after a few hours & the blocks are corrupted afterwards. It must be a hardware issue, as no one else experiences this issue.
💬 hebasto commented on pull request "build: Bump minimum supported macOS to 12.0":
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2413296884)
> > Running Bitcoin Core on unsupported OSes may expose users to security issues.
> > macOS Big Sur 11 received its last security update ([11.7.10](https://support.apple.com/en-us/106338)) over a year ago.
>
> If this is the reasoning we are going to use, shouldn't this be bumping to `10.13`, given `10.12` is also "unsupported" in terms of security updates?
You mean "13.0" and "12.0", right?
For the reference, macOS 12 was last updated on 2024-07-29. The three most recent Apple updates
...
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2413296884)
> > Running Bitcoin Core on unsupported OSes may expose users to security issues.
> > macOS Big Sur 11 received its last security update ([11.7.10](https://support.apple.com/en-us/106338)) over a year ago.
>
> If this is the reasoning we are going to use, shouldn't this be bumping to `10.13`, given `10.12` is also "unsupported" in terms of security updates?
You mean "13.0" and "12.0", right?
For the reference, macOS 12 was last updated on 2024-07-29. The three most recent Apple updates
...
💬 hebasto commented on pull request "build: Bump minimum supported macOS to 12.0":
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2413322066)
If we raise the minimum supported macOS version to 13.0, it would mean dropping support for hardware like MacBook Pro (15-inch, 2016) and older, which Apple classifies as ["obsolete worldwide"](https://support.apple.com/en-us/102772).
I'll update this PR shortly.
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2413322066)
If we raise the minimum supported macOS version to 13.0, it would mean dropping support for hardware like MacBook Pro (15-inch, 2016) and older, which Apple classifies as ["obsolete worldwide"](https://support.apple.com/en-us/102772).
I'll update this PR shortly.
💬 l0rinc commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1800757127)
I quickly glanced over this PR only, do I understand it correctly that we're treating benchmarks as tests here (I'm not sure I agree with that concept) and adding parameters to the benchmark setup that only apply to a smaller subset of the benchmarks?
Could we configure them via env variables instead and call the benchmark with e.g. `TEST_DATA_DIR=a/b/c bench` instead?
As an example, if a few of our tests require a for example a timestamp for whatever reason, we wouldn't add it to the testing
...
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1800757127)
I quickly glanced over this PR only, do I understand it correctly that we're treating benchmarks as tests here (I'm not sure I agree with that concept) and adding parameters to the benchmark setup that only apply to a smaller subset of the benchmarks?
Could we configure them via env variables instead and call the benchmark with e.g. `TEST_DATA_DIR=a/b/c bench` instead?
As an example, if a few of our tests require a for example a timestamp for whatever reason, we wouldn't add it to the testing
...
💬 maflcko commented on pull request "CI: Add label to scripted-diffs":
(https://github.com/bitcoin/bitcoin/pull/31089#issuecomment-2413340369)
Not sure what the goal here would be? Why add a label, when there is already a commit message prefix, which is identical in the meaning?
It would be better to just fix the bugs like https://github.com/bitcoin/bitcoin/issues/29692#issuecomment-2411656801 to ensure the CI is red when there is an issue.
(https://github.com/bitcoin/bitcoin/pull/31089#issuecomment-2413340369)
Not sure what the goal here would be? Why add a label, when there is already a commit message prefix, which is identical in the meaning?
It would be better to just fix the bugs like https://github.com/bitcoin/bitcoin/issues/29692#issuecomment-2411656801 to ensure the CI is red when there is an issue.
💬 maflcko commented on pull request "miniscript: convert non-critical asserts to Assumes":
(https://github.com/bitcoin/bitcoin/pull/28678#issuecomment-2413349588)
@sipa @darosior It would be good to mark this as up-for-grabs, if you dropped working on it.
(https://github.com/bitcoin/bitcoin/pull/28678#issuecomment-2413349588)
@sipa @darosior It would be good to mark this as up-for-grabs, if you dropped working on it.
💬 hebasto commented on pull request "build: Bump minimum supported macOS to 13.0":
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2413356842)
> > > Running Bitcoin Core on unsupported OSes may expose users to security issues.
> > > macOS Big Sur 11 received its last security update ([11.7.10](https://support.apple.com/en-us/106338)) over a year ago.
> >
> >
> > If this is the reasoning we are going to use, shouldn't this be bumping to `10.13`, given `10.12` is also "unsupported" in terms of security updates?
>
> Yes, good point. I didn't realize 12.0 was eol'd. We shouldn't let qt dictate what we support.
>
> Agree that 13
...
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2413356842)
> > > Running Bitcoin Core on unsupported OSes may expose users to security issues.
> > > macOS Big Sur 11 received its last security update ([11.7.10](https://support.apple.com/en-us/106338)) over a year ago.
> >
> >
> > If this is the reasoning we are going to use, shouldn't this be bumping to `10.13`, given `10.12` is also "unsupported" in terms of security updates?
>
> Yes, good point. I didn't realize 12.0 was eol'd. We shouldn't let qt dictate what we support.
>
> Agree that 13
...
💬 maflcko commented on pull request "build: Bump minimum supported macOS to 13.0":
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2413362340)
lgtm ACK a0e089a71dc449f4cc177d6eb3050400e63f4b3f
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2413362340)
lgtm ACK a0e089a71dc449f4cc177d6eb3050400e63f4b3f
💬 maflcko commented on issue "wallet: rpc: `settxfee` sets the wallet feerate not fee":
(https://github.com/bitcoin/bitcoin/issues/31088#issuecomment-2413394142)
In theory, it would be possible to register RPC argument name aliases, IIRC. They are denoted and split by `|`.
(https://github.com/bitcoin/bitcoin/issues/31088#issuecomment-2413394142)
In theory, it would be possible to register RPC argument name aliases, IIRC. They are denoted and split by `|`.
💬 m3dwards commented on pull request "build: Bump minimum supported macOS to 13.0":
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2413396346)
ACK a0e089a71dc449f4cc177d6eb3050400e63f4b3f
Tested on ARM Mac 14.6
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2413396346)
ACK a0e089a71dc449f4cc177d6eb3050400e63f4b3f
Tested on ARM Mac 14.6
⚠️ maflcko opened an issue: "doc: Fixup windows-cross build notes"
(https://github.com/bitcoin/bitcoin/issues/31090)
It is also missing bison and patch? Might as well remove it and just refer to the location that is correct?
_Originally posted by @maflcko in https://github.com/bitcoin/bitcoin/pull/31084#discussion_r1799629690_
(https://github.com/bitcoin/bitcoin/issues/31090)
It is also missing bison and patch? Might as well remove it and just refer to the location that is correct?
_Originally posted by @maflcko in https://github.com/bitcoin/bitcoin/pull/31084#discussion_r1799629690_