💬 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)
💬 willcl-ark commented on issue "Add an EffectiveSan (Effective Type Sanitizer) CI job to detect sub-object overflows and type errors":
(https://github.com/bitcoin/bitcoin/issues/23393#issuecomment-3410926221)
The linked project, whilst potentially looking interesting, has not had any updates in over 4 years, and has only a 0.1-alpha release which appears to point to the 4.0 release of llvm.
Along with this issue itself having no activity in 5 years, I think we can close this for now due to lack of interest.
Let me know if you think it should be re-opened, or feel free to open a new issue if this is still important.
(https://github.com/bitcoin/bitcoin/issues/23393#issuecomment-3410926221)
The linked project, whilst potentially looking interesting, has not had any updates in over 4 years, and has only a 0.1-alpha release which appears to point to the 4.0 release of llvm.
Along with this issue itself having no activity in 5 years, I think we can close this for now due to lack of interest.
Let me know if you think it should be re-opened, or feel free to open a new issue if this is still important.
💬 cedwies commented on pull request "miner: fix empty mempool case for waitNext()":
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3410987155)
tACK 8f76732
Reviewed and tested. The empty mempool case (`fee_threshold==0`) now behaves as expected and is covered by tests.
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3410987155)
tACK 8f76732
Reviewed and tested. The empty mempool case (`fee_threshold==0`) now behaves as expected and is covered by tests.
💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3411048449)
I can't reproduce locally. An attempt to resolve `x9.dummySeed.invalid.` during unit tests looks like one of those issues that this PR aims to uncover.
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3411048449)
I can't reproduce locally. An attempt to resolve `x9.dummySeed.invalid.` during unit tests looks like one of those issues that this PR aims to uncover.
💬 cedwies commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2436060116)
Thanks for the correction, that makes sense. I was focusing on the mechanism rather than the permission. I appreciate the clarification.
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2436060116)
Thanks for the correction, that makes sense. I was focusing on the mechanism rather than the permission. I appreciate the clarification.
💬 zaidmstrr commented on pull request "miner: fix empty mempool case for waitNext()":
(https://github.com/bitcoin/bitcoin/pull/33566#discussion_r2436063318)
Thanks for the benchmarking. But I think to simulate the actual mempool behaviour is complicated. The order of transactions inside mempool is not very deterministic based on fee. So we can't predict that our threshold always will be hit at the last stages of search, even though it searches till the last in most of the cases, but that's not for all. The function also uses polling, so if you increase the number of executions, the time will increase a bit. But as Sjors said, this calculation will
...
(https://github.com/bitcoin/bitcoin/pull/33566#discussion_r2436063318)
Thanks for the benchmarking. But I think to simulate the actual mempool behaviour is complicated. The order of transactions inside mempool is not very deterministic based on fee. So we can't predict that our threshold always will be hit at the last stages of search, even though it searches till the last in most of the cases, but that's not for all. The function also uses polling, so if you increase the number of executions, the time will increase a bit. But as Sjors said, this calculation will
...
✅ willcl-ark closed an issue: "Auto detect IPv6 connectivity"
(https://github.com/bitcoin/bitcoin/issues/28061)
(https://github.com/bitcoin/bitcoin/issues/28061)
💬 willcl-ark commented on issue "Auto detect IPv6 connectivity":
(https://github.com/bitcoin/bitcoin/issues/28061#issuecomment-3411176533)
This issue may have merit, but it hasn’t attracted much interest from other contributors.
Given that, it doesn’t seem important enough to keep open indefinitely. I’m closing it for now due to lack of activity — but pull requests or renewed discussion are always welcome.
(https://github.com/bitcoin/bitcoin/issues/28061#issuecomment-3411176533)
This issue may have merit, but it hasn’t attracted much interest from other contributors.
Given that, it doesn’t seem important enough to keep open indefinitely. I’m closing it for now due to lack of activity — but pull requests or renewed discussion are always welcome.
💬 diegoviola commented on pull request "Fix Wayland visual glitches":
(https://github.com/bitcoin-core/gui/pull/904#issuecomment-3411194348)
Reverted as per the previous discussion.
(https://github.com/bitcoin-core/gui/pull/904#issuecomment-3411194348)
Reverted as per the previous discussion.
💬 diegoviola commented on pull request "Fix Wayland visual glitches":
(https://github.com/bitcoin-core/gui/pull/904#issuecomment-3411199249)
Choose the revert approach as per the previous discussion.
(https://github.com/bitcoin-core/gui/pull/904#issuecomment-3411199249)
Choose the revert approach as per the previous discussion.
💬 hebasto commented on issue "Slow unit tests delay functional tests and leave CPU unused":
(https://github.com/bitcoin/bitcoin/issues/32770#issuecomment-3411200053)
> On the libsecp notes, since [bitcoin-core/secp256k1#1734](https://github.com/bitcoin-core/secp256k1/pull/1734) we can parallelize tests. Tagging [@hebasto](https://github.com/hebasto) as he had a patch to do it.
See https://github.com/bitcoin-core/secp256k1/pull/1760.
(https://github.com/bitcoin/bitcoin/issues/32770#issuecomment-3411200053)
> On the libsecp notes, since [bitcoin-core/secp256k1#1734](https://github.com/bitcoin-core/secp256k1/pull/1734) we can parallelize tests. Tagging [@hebasto](https://github.com/hebasto) as he had a patch to do it.
See https://github.com/bitcoin-core/secp256k1/pull/1760.
✅ willcl-ark closed an issue: "Creating a Wallet Feature Guidelines Document"
(https://github.com/bitcoin/bitcoin/issues/28062)
(https://github.com/bitcoin/bitcoin/issues/28062)