π¬ sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2435683806)
Done in 6881d21c57ce122f0c2f5900e6caab7ba806d279.
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2435683806)
Done in 6881d21c57ce122f0c2f5900e6caab7ba806d279.
π¬ alexanderwiederin commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3410666329)
re-ACK https://github.com/bitcoin/bitcoin/pull/30595/commits/a755e00a13541b3b5a707cf385f1cbec0449c6a9
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3410666329)
re-ACK https://github.com/bitcoin/bitcoin/pull/30595/commits/a755e00a13541b3b5a707cf385f1cbec0449c6a9
π¬ hebasto commented on pull request "Fix Wayland visual glitches":
(https://github.com/bitcoin-core/gui/pull/904#discussion_r2435752838)
Is it necessary now to call `w->show();` here?
Would it be more clear to simply revert https://github.com/bitcoin-core/gui/commit/15aa7d023688700a47997b92108de95f2d864f5a?
(https://github.com/bitcoin-core/gui/pull/904#discussion_r2435752838)
Is it necessary now to call `w->show();` here?
Would it be more clear to simply revert https://github.com/bitcoin-core/gui/commit/15aa7d023688700a47997b92108de95f2d864f5a?
π¬ diegoviola commented on pull request "Fix Wayland visual glitches":
(https://github.com/bitcoin-core/gui/pull/904#issuecomment-3410727752)
Just FYI: the Wayland developers have been working on the [ext-zones](https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/264) protocol for a while now, which I think is the sort of functionality that provides "always on top" and similar:
> Professional apps like DAWs rely a lot on having windows "always on top" floating above the other UI elements. On Wayland, this is not possible and the usability of these apps suffers a lot.
Until this isn't ready, I think we shoul
...
(https://github.com/bitcoin-core/gui/pull/904#issuecomment-3410727752)
Just FYI: the Wayland developers have been working on the [ext-zones](https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/264) protocol for a while now, which I think is the sort of functionality that provides "always on top" and similar:
> Professional apps like DAWs rely a lot on having windows "always on top" floating above the other UI elements. On Wayland, this is not possible and the usability of these apps suffers a lot.
Until this isn't ready, I think we shoul
...
β οΈ fanquake opened an issue: "ci: short read: expected xxxxxxxxx bytes but got xxxxxxxxx: unexpected EOF"
(https://github.com/bitcoin/bitcoin/issues/33640)
This issue is happening semi-frequently. i.e https://github.com/bitcoin/bitcoin/actions/runs/18560885434/job/52909339714?pr=33591#step:9:231:
```bash
#10 [5/5] RUN ["bash", "-c", "cd /ci_container_base/ && set -o errexit && source ./ci/test/00_setup_env.sh && ./ci/test/01_base_install.sh"]
#10 sha256:9ff7145d1456945488a8d8d2466983d85b23a449070f110460965a1547aec2c0 122.68MB / 660.19MB 5.7s
#10 sha256:3d058fa1c6c9853659e7c57f9938cc3e7bc8b256a1221e2fa5dfc955b8055a50 3.86kB / 3.86kB 0.3s done
#10 s
...
(https://github.com/bitcoin/bitcoin/issues/33640)
This issue is happening semi-frequently. i.e https://github.com/bitcoin/bitcoin/actions/runs/18560885434/job/52909339714?pr=33591#step:9:231:
```bash
#10 [5/5] RUN ["bash", "-c", "cd /ci_container_base/ && set -o errexit && source ./ci/test/00_setup_env.sh && ./ci/test/01_base_install.sh"]
#10 sha256:9ff7145d1456945488a8d8d2466983d85b23a449070f110460965a1547aec2c0 122.68MB / 660.19MB 5.7s
#10 sha256:3d058fa1c6c9853659e7c57f9938cc3e7bc8b256a1221e2fa5dfc955b8055a50 3.86kB / 3.86kB 0.3s done
#10 s
...
π¬ diegoviola commented on pull request "Fix Wayland visual glitches":
(https://github.com/bitcoin-core/gui/pull/904#discussion_r2435784579)
@hebasto Yeah, I think a revert would also do the trick.
(https://github.com/bitcoin-core/gui/pull/904#discussion_r2435784579)
@hebasto Yeah, I think a revert would also do the trick.
π€ janb84 reviewed a pull request: "test: [move-only] binary utils to utils.py"
(https://github.com/bitcoin/bitcoin/pull/33633#pullrequestreview-3344696191)
ACK fa75ef4328f638221bcf85fcbefa885122084622
Pr moves Helper class for information about bitcoin binaries to util.py.
To move the a helper class outside `Test framework` makes sense to me, this cleans up the `test_framework.py` code
Not completely convinced that these functions in the helper class will be used externally due to the tight coupling with these config settings `config["environment"]["BUILDDIR"], bin",` (this could be a good follow-up, if needed by external testers)
(https://github.com/bitcoin/bitcoin/pull/33633#pullrequestreview-3344696191)
ACK fa75ef4328f638221bcf85fcbefa885122084622
Pr moves Helper class for information about bitcoin binaries to util.py.
To move the a helper class outside `Test framework` makes sense to me, this cleans up the `test_framework.py` code
Not completely convinced that these functions in the helper class will be used externally due to the tight coupling with these config settings `config["environment"]["BUILDDIR"], bin",` (this could be a good follow-up, if needed by external testers)
π fanquake opened a pull request: "Update leveldb subtree to latest master"
(https://github.com/bitcoin/bitcoin/pull/33641)
Rather than continue to close PRs/"Send these upstream" i.e: #33638, #33148, #22664, #13781; just fix the typos.
Includes https://github.com/bitcoin-core/leveldb-subtree/pull/57.
(https://github.com/bitcoin/bitcoin/pull/33641)
Rather than continue to close PRs/"Send these upstream" i.e: #33638, #33148, #22664, #13781; just fix the typos.
Includes https://github.com/bitcoin-core/leveldb-subtree/pull/57.
π¬ diegoviola commented on pull request "Fix Wayland visual glitches":
(https://github.com/bitcoin-core/gui/pull/904#discussion_r2435815330)
@hebasto compiling 30.0 with https://github.com/bitcoin-core/gui/commit/15aa7d023688700a47997b92108de95f2d864f5a reverted, will confirm soon.
(https://github.com/bitcoin-core/gui/pull/904#discussion_r2435815330)
@hebasto compiling 30.0 with https://github.com/bitcoin-core/gui/commit/15aa7d023688700a47997b92108de95f2d864f5a reverted, will confirm soon.
π¬ vasild commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2435833376)
```cpp
// pindexLastCommonBlock is required to be an ancestor of pindexBestKnownBlock
...
state->pindexBestKnownBlock->GetAncestor(state->pindexLastCommonBlock->nHeight)
```
If `Last` is an ancestor of `Best` and we get `Best`'s ancestor at height `Last.height`, isn't this always going to be `Last`?
(https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2435833376)
```cpp
// pindexLastCommonBlock is required to be an ancestor of pindexBestKnownBlock
...
state->pindexBestKnownBlock->GetAncestor(state->pindexLastCommonBlock->nHeight)
```
If `Last` is an ancestor of `Best` and we get `Best`'s ancestor at height `Last.height`, isn't this always going to be `Last`?
π¬ instagibbs commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2435843010)
sounds reasonable, leave as is
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2435843010)
sounds reasonable, leave as is
π¬ instagibbs commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2435845248)
oops you're right
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2435845248)
oops you're right
π¬ cedwies commented on pull request "miner: fix empty mempool case for waitNext()":
(https://github.com/bitcoin/bitcoin/pull/33566#discussion_r2435849449)
I ran a small external python simulation to get a rough sense of how often the early-exit fee loop would actually stop much earlier than a full sum under realistic mempool conditions.
Using a model with **3000 txs** (fees = feerate Γ vsize, realistic feerate ranges, small per-tick mempool drift, and occasional high-fee arrivals), the results were:
β `threshold=0`: **median touch fraction β 0.97**, meaning in a typical run the loop still scans about 97% of transactions before returning; about 43
...
(https://github.com/bitcoin/bitcoin/pull/33566#discussion_r2435849449)
I ran a small external python simulation to get a rough sense of how often the early-exit fee loop would actually stop much earlier than a full sum under realistic mempool conditions.
Using a model with **3000 txs** (fees = feerate Γ vsize, realistic feerate ranges, small per-tick mempool drift, and occasional high-fee arrivals), the results were:
β `threshold=0`: **median touch fraction β 0.97**, meaning in a typical run the loop still scans about 97% of transactions before returning; about 43
...
π€ yuvicc reviewed a pull request: "test: [move-only] binary utils to utils.py"
(https://github.com/bitcoin/bitcoin/pull/33633#pullrequestreview-3344760908)
Code review ACK fa75ef4328f638221bcf85fcbefa885122084622
The `Binaries` class and `get_binary_paths()` are logically separate from TestFramework orchestration, so moving them to util.py makes sense for modularity.
As mentioned by @janb84, the tight coupling with `config["environment"]["BUILDDIR"]` and related settings means external usage still requires test framework. Not blocking for this PR, but worth considering for future refactoring if there's the requirement.
(https://github.com/bitcoin/bitcoin/pull/33633#pullrequestreview-3344760908)
Code review ACK fa75ef4328f638221bcf85fcbefa885122084622
The `Binaries` class and `get_binary_paths()` are logically separate from TestFramework orchestration, so moving them to util.py makes sense for modularity.
As mentioned by @janb84, the tight coupling with `config["environment"]["BUILDDIR"]` and related settings means external usage still requires test framework. Not blocking for this PR, but worth considering for future refactoring if there's the requirement.
π willcl-ark approved a pull request: "[28.x] Backport & finalise 28.3"
(https://github.com/bitcoin/bitcoin/pull/33613#pullrequestreview-3344772395)
ACK 2dfb3a06902e9a98e00a422705afa002a3744545
(https://github.com/bitcoin/bitcoin/pull/33613#pullrequestreview-3344772395)
ACK 2dfb3a06902e9a98e00a422705afa002a3744545
π¬ maflcko commented on pull request "test: [move-only] binary utils to utils.py":
(https://github.com/bitcoin/bitcoin/pull/33633#issuecomment-3410831632)
The config is written by the build system, so I don't think it is coupled with the `TestFramework` class itself.
(https://github.com/bitcoin/bitcoin/pull/33633#issuecomment-3410831632)
The config is written by the build system, so I don't think it is coupled with the `TestFramework` class itself.
π¬ diegoviola commented on pull request "Fix Wayland visual glitches":
(https://github.com/bitcoin-core/gui/pull/904#discussion_r2435876164)
@hebasto Yeah, I can confirm that reverting https://github.com/bitcoin-core/gui/commit/15aa7d023688700a47997b92108de95f2d864f5a solves the problem for me (on sway).
(https://github.com/bitcoin-core/gui/pull/904#discussion_r2435876164)
@hebasto Yeah, I can confirm that reverting https://github.com/bitcoin-core/gui/commit/15aa7d023688700a47997b92108de95f2d864f5a solves the problem for me (on sway).
π¬ danielabrozzoni commented on pull request "p2p: Mitigate GETADDR fingerprinting by setting address timestamps to a fixed value":
(https://github.com/bitcoin/bitcoin/pull/33498#issuecomment-3410869340)
Thank you Martin, I agree that the current solution has this issue.
Our other approach would have run into the same problem. We had considered setting all timestamps to zero, but we didnβt implement it because weβre not sure if itβs compatible with btcd, as mentioned in the PR description. This approach has the same problem as the current one:
- Suppose we have an address in our addrman with a timestamp of 29 days ago, and the corresponding node has already left the network
- We send the ad
...
(https://github.com/bitcoin/bitcoin/pull/33498#issuecomment-3410869340)
Thank you Martin, I agree that the current solution has this issue.
Our other approach would have run into the same problem. We had considered setting all timestamps to zero, but we didnβt implement it because weβre not sure if itβs compatible with btcd, as mentioned in the PR description. This approach has the same problem as the current one:
- Suppose we have an address in our addrman with a timestamp of 29 days ago, and the corresponding node has already left the network
- We send the ad
...
π¬ furszy commented on issue "Slow unit tests delay functional tests and leave CPU unused":
(https://github.com/bitcoin/bitcoin/issues/32770#issuecomment-3410924471)
On the libsecp notes, since https://github.com/bitcoin-core/secp256k1/pull/1734 we can parallelize tests. Tagging @hebasto as he had a patch to do it.
(https://github.com/bitcoin/bitcoin/issues/32770#issuecomment-3410924471)
On the libsecp notes, since https://github.com/bitcoin-core/secp256k1/pull/1734 we can parallelize tests. Tagging @hebasto as he had a patch to do it.
β
willcl-ark closed an issue: "Add an EffectiveSan (Effective Type Sanitizer) CI job to detect sub-object overflows and type errors"
(https://github.com/bitcoin/bitcoin/issues/23393)
(https://github.com/bitcoin/bitcoin/issues/23393)