💬 itornaza commented on issue "depends: llvm-ranlib (etc): "No such file or directory" on Intel macOS 15.0":
(https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2393255981)
Same error messages as @Sjors on a clean build as well.
```
ioannis@ergot_a depends % make
/bin/sh: command -v llvm-ranlib: No such file or directory
/bin/sh: command -v llvm-strip: No such file or directory
/bin/sh: command -v llvm-nm: No such file or directory
/bin/sh: command -v llvm-objdump: No such file or directory
/bin/sh: command -v dsymutil: No such file or directory
Fetching boost_1_81_0.tar.gz from https://archives.boost.io/release/1.81.0/source/
% Total % Received % X
...
  (https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2393255981)
Same error messages as @Sjors on a clean build as well.
```
ioannis@ergot_a depends % make
/bin/sh: command -v llvm-ranlib: No such file or directory
/bin/sh: command -v llvm-strip: No such file or directory
/bin/sh: command -v llvm-nm: No such file or directory
/bin/sh: command -v llvm-objdump: No such file or directory
/bin/sh: command -v dsymutil: No such file or directory
Fetching boost_1_81_0.tar.gz from https://archives.boost.io/release/1.81.0/source/
% Total % Received % X
...
💬 Sjors commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1787471881)
> The code should just be able to call `waitTipChanged()` with the zero value, which will return as soon as node is started and it knows what the tip is.
That indeed seems like a better approach, will change that for the TP.
  (https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1787471881)
> The code should just be able to call `waitTipChanged()` with the zero value, which will return as soon as node is started and it knows what the tip is.
That indeed seems like a better approach, will change that for the TP.
💬 Sjors commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#issuecomment-2393327733)
ACK fa22e5c430acaef9713d9a4b4b97bb3f4876f816
Manually tested `-stopatheight`, a fatal indexer error (by breaking some code), calling `bitcoin-cli stop`, manually quitting qt, calling `stop` form the GUI console.
  (https://github.com/bitcoin/bitcoin/pull/30967#issuecomment-2393327733)
ACK fa22e5c430acaef9713d9a4b4b97bb3f4876f816
Manually tested `-stopatheight`, a fatal indexer error (by breaking some code), calling `bitcoin-cli stop`, manually quitting qt, calling `stop` form the GUI console.
💬 hebasto commented on issue "Unable to cross compile on linux for macos (28.x branch)":
(https://github.com/bitcoin/bitcoin/issues/31027#issuecomment-2393328681)
> debian unstable (sid) runs on clang-20 doesn't need to downgrade it
Debian Sid's [default clang](https://packages.debian.org/sid/clang) is clang-16, which is enough to build every dependency except for the `qt` package.
Here is a full log of the commands on a fresh Debian Sid image:
```
# history
1 apt update
2 apt install git bison cmake curl make pkg-config xz-utils wget
3 apt install clang llvm lld
4 git clone https://github.com/bitcoin/bitcoin.git
5 w
...
  (https://github.com/bitcoin/bitcoin/issues/31027#issuecomment-2393328681)
> debian unstable (sid) runs on clang-20 doesn't need to downgrade it
Debian Sid's [default clang](https://packages.debian.org/sid/clang) is clang-16, which is enough to build every dependency except for the `qt` package.
Here is a full log of the commands on a fresh Debian Sid image:
```
# history
1 apt update
2 apt install git bison cmake curl make pkg-config xz-utils wget
3 apt install clang llvm lld
4 git clone https://github.com/bitcoin/bitcoin.git
5 w
...
👍 Sjors approved a pull request: "refactor: Replace g_genesis_wait_cv with m_tip_block_cv"
(https://github.com/bitcoin/bitcoin/pull/30967#pullrequestreview-2347659011)
ACK fa22e5c430acaef9713d9a4b4b97bb3f4876f816
Manually tested `-stopatheight`, a fatal indexer error (by breaking some code), calling `bitcoin-cli stop`, manually quitting qt, calling `stop` form the GUI console.
  (https://github.com/bitcoin/bitcoin/pull/30967#pullrequestreview-2347659011)
ACK fa22e5c430acaef9713d9a4b4b97bb3f4876f816
Manually tested `-stopatheight`, a fatal indexer error (by breaking some code), calling `bitcoin-cli stop`, manually quitting qt, calling `stop` form the GUI console.
🤔 hebasto reviewed a pull request: "refactor: move util/pcp and util/netif to common/"
(https://github.com/bitcoin/bitcoin/pull/31011#pullrequestreview-2347681620)
Post-merge ACK fd38711217cafbd62e8abd22d2b43f85fede8cde.
  (https://github.com/bitcoin/bitcoin/pull/31011#pullrequestreview-2347681620)
Post-merge ACK fd38711217cafbd62e8abd22d2b43f85fede8cde.
🚀 hebasto merged a pull request: "qt6: Fix linking when configured with `-DENABLE_WALLET=OFF`"
(https://github.com/bitcoin-core/gui/pull/837)
  (https://github.com/bitcoin-core/gui/pull/837)
💬 promag commented on pull request "cmake: Avoid hardcoding Qt's major version in Find module / variable names":
(https://github.com/bitcoin/bitcoin/pull/31010#issuecomment-2393380465)
Code review ACK deacf3c7cd68dd4ee973526740e45c7015b6433a.
  (https://github.com/bitcoin/bitcoin/pull/31010#issuecomment-2393380465)
Code review ACK deacf3c7cd68dd4ee973526740e45c7015b6433a.
💬 marcofleon commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2393409008)
@instagibbs thanks, that makes sense. I did see the `fRejectedParents` is true case with an input that crashed on 3c1c9e766ad4fc6defdc9b4814c1e184f6603003 (before the last push).
Option 1 in your list seems most robust to me as well.
  (https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2393409008)
@instagibbs thanks, that makes sense. I did see the `fRejectedParents` is true case with an input that crashed on 3c1c9e766ad4fc6defdc9b4814c1e184f6603003 (before the last push).
Option 1 in your list seems most robust to me as well.
🤔 ismaelsadeeq reviewed a pull request: "Cluster linearization: separate tests from tests-of-tests"
(https://github.com/bitcoin/bitcoin/pull/30605#pullrequestreview-2347653035)
I reviewed the latest changes in the last push
Code review 8eb70e3ca467ac66bc1f43e3e0b3338d63806ee5
Changes since my last review were
1. Addressed my previous review comments.
2. An optimization was introduced in the `clusterlin_simple_linearize` fuzz test that skips to the last non-topological permutation.
For example, given transactions `[A, B, C, D, E]`.
if A-B-C are a valid topological ancestor set, and the first permutation results in [A, E, B, C, D], which is not topologica
...
  (https://github.com/bitcoin/bitcoin/pull/30605#pullrequestreview-2347653035)
I reviewed the latest changes in the last push
Code review 8eb70e3ca467ac66bc1f43e3e0b3338d63806ee5
Changes since my last review were
1. Addressed my previous review comments.
2. An optimization was introduced in the `clusterlin_simple_linearize` fuzz test that skips to the last non-topological permutation.
For example, given transactions `[A, B, C, D, E]`.
if A-B-C are a valid topological ancestor set, and the first permutation results in [A, E, B, C, D], which is not topologica
...
💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1787549994)
We can cache the subset and then reuse it here and when finding topological subset to be deleted.
  (https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1787549994)
We can cache the subset and then reuse it here and when finding topological subset to be deleted.
💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1787476002)
Is this not $O(N * 2{^N})$
  (https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1787476002)
Is this not $O(N * 2{^N})$
💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1787508540)
I might be missing something here, so I would appreciate some clarification.
1. Why don't we directly remove the candidate set transactions found by the search?
2. It seems like there could be multiple valid topological sets at a given time. How does it know which set was actually removed?
  (https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1787508540)
I might be missing something here, so I would appreciate some clarification.
1. Why don't we directly remove the candidate set transactions found by the search?
2. It seems like there could be multiple valid topological sets at a given time. How does it know which set was actually removed?
💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1787551272)
Same comment applies here as well.
  (https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1787551272)
Same comment applies here as well.
💬 laanwj commented on pull request "test: Add mockable steady clock, tests for PCP and NATPMP implementations":
(https://github.com/bitcoin/bitcoin/pull/31022#discussion_r1787577470)
Should be unambigious now, i've renamed it and the methods to set/clear the mock time are included on the clock itself.
```c++
/**
* Version of SteadyClock that is mockable in the context of tests (set the
* current value with SetMockSteadyTime), otherwise the system steady clock.
*/
struct MockableSteadyClock : public std::chrono::steady_clock {
using time_point = std::chrono::time_point<MockableSteadyClock>;
/** Return current system time or mocked time, if set */
stati
...
  (https://github.com/bitcoin/bitcoin/pull/31022#discussion_r1787577470)
Should be unambigious now, i've renamed it and the methods to set/clear the mock time are included on the clock itself.
```c++
/**
* Version of SteadyClock that is mockable in the context of tests (set the
* current value with SetMockSteadyTime), otherwise the system steady clock.
*/
struct MockableSteadyClock : public std::chrono::steady_clock {
using time_point = std::chrono::time_point<MockableSteadyClock>;
/** Return current system time or mocked time, if set */
stati
...
💬 sipa commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1787606089)
Yeah, it's $\mathcal{O}(2^N)$ *iterations*, but overall runtime is indeed $\mathcal{O}(N \cdot 2^N)$ (or, arguably, $\mathcal{O}(N^2 \cdot 2^N)$ even if you take into account that for larger $N$, the bitsets involved need to grow too.
  (https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1787606089)
Yeah, it's $\mathcal{O}(2^N)$ *iterations*, but overall runtime is indeed $\mathcal{O}(N \cdot 2^N)$ (or, arguably, $\mathcal{O}(N^2 \cdot 2^N)$ even if you take into account that for larger $N$, the bitsets involved need to grow too.
💬 sipa commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1787610478)
The goal here is simply continuing the test, so this isn't just exercising the first candidate found for a linearization, but can hit arbitrary remainders in later iterations too. To that end, we read a topological subset from the fuzzer, which can be anything, but since `ReadTopologicalSet` may return an empty set, we need to specially deal with that case.
One possibility would be adding to `ReadTopologicalSet` a way to force it to return a non-empty set. Instead, the code here uses the foun
...
  (https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1787610478)
The goal here is simply continuing the test, so this isn't just exercising the first candidate found for a linearization, but can hit arbitrary remainders in later iterations too. To that end, we read a topological subset from the fuzzer, which can be anything, but since `ReadTopologicalSet` may return an empty set, we need to specially deal with that case.
One possibility would be adding to `ReadTopologicalSet` a way to force it to return a non-empty set. Instead, the code here uses the foun
...
📝 hebasto opened a pull request: "qt6: Handle different signatures of `QANEF::nativeEventFilter`"
(https://github.com/bitcoin-core/gui/pull/840)
Split from https://github.com/bitcoin/bitcoin/pull/30997.
This PR ensures compatibility across all supported Qt versions.
For more details, please refer to https://github.com/qt/qtbase/commit/3b38c73c7ffa71c00c172cf0e05742835a304300.
No behaviour change.
  (https://github.com/bitcoin-core/gui/pull/840)
Split from https://github.com/bitcoin/bitcoin/pull/30997.
This PR ensures compatibility across all supported Qt versions.
For more details, please refer to https://github.com/qt/qtbase/commit/3b38c73c7ffa71c00c172cf0e05742835a304300.
No behaviour change.
💬 sipa commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1787611090)
Sure we could, but I don't think there is reason to assume that's necessarily the best choice.
  (https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1787611090)
Sure we could, but I don't think there is reason to assume that's necessarily the best choice.
💬 fanquake commented on pull request "qt6: Handle different signatures of `QANEF::nativeEventFilter`":
(https://github.com/bitcoin-core/gui/pull/840#issuecomment-2393573834)
If Qt 6.7.0 is going to be required for Windows after the switchover; why change this code now just to immediately drop it after migrating to Qt 6?
  (https://github.com/bitcoin-core/gui/pull/840#issuecomment-2393573834)
If Qt 6.7.0 is going to be required for Windows after the switchover; why change this code now just to immediately drop it after migrating to Qt 6?