💬 l0rinc commented on pull request "Update leveldb subtree to latest master":
(https://github.com/bitcoin/bitcoin/pull/33641#issuecomment-3411960774)
ACK 54ffe3de5b1d15f10516ea536a12e13cd7d338f3
(https://github.com/bitcoin/bitcoin/pull/33641#issuecomment-3411960774)
ACK 54ffe3de5b1d15f10516ea536a12e13cd7d338f3
💬 l0rinc commented on pull request "test: [move-only] binary utils to utils.py":
(https://github.com/bitcoin/bitcoin/pull/33633#issuecomment-3411994551)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33633#issuecomment-3411994551)
Concept ACK
👋 sipa's pull request is ready for review: "Replace cluster linearization algorithm with SFL"
(https://github.com/bitcoin/bitcoin/pull/32545)
(https://github.com/bitcoin/bitcoin/pull/32545)
💬 cedwies commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#issuecomment-3412187177)
ACK 84b2ad0
(https://github.com/bitcoin/bitcoin/pull/33565#issuecomment-3412187177)
ACK 84b2ad0
💬 cedwies commented on pull request "Update leveldb subtree to latest master":
(https://github.com/bitcoin/bitcoin/pull/33641#issuecomment-3412213444)
ACK 54ffe3d
(https://github.com/bitcoin/bitcoin/pull/33641#issuecomment-3412213444)
ACK 54ffe3d
🤔 cedwies reviewed a pull request: "net_processing: rename RelayTransaction to better describe what it does"
(https://github.com/bitcoin/bitcoin/pull/33565#pullrequestreview-3346428420)
ACK 84b2ad0
(https://github.com/bitcoin/bitcoin/pull/33565#pullrequestreview-3346428420)
ACK 84b2ad0
💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-3412455674)
Rebased, and made a significant change to the SFL algorithm itself:
* Switched to a different technique for making the initial state topological. The big advantage is that this approach also works when one already has a linearization which is not entirely topological already.
Also the following changes to the `cluster_linearize.h` code in general:
* Using SFL gaining the ability to fix existing linearizations, replaced `FixLinearization` with just making this feature part of `Linearize`.
*
...
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-3412455674)
Rebased, and made a significant change to the SFL algorithm itself:
* Switched to a different technique for making the initial state topological. The big advantage is that this approach also works when one already has a linearization which is not entirely topological already.
Also the following changes to the `cluster_linearize.h` code in general:
* Using SFL gaining the ability to fix existing linearizations, replaced `FixLinearization` with just making this feature part of `Linearize`.
*
...
🚀 fanquake merged a pull request: "Update leveldb subtree to latest master"
(https://github.com/bitcoin/bitcoin/pull/33641)
(https://github.com/bitcoin/bitcoin/pull/33641)
💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-3412487618)
This PR has grown quite a bit in scope (despite still being a net negative in LoC!), so I don't think it's unreasonable to split it up into an SFL-specific one, and one with follow-up changes to `txgraph`, or even further. I'll wait for reviewer comments.
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-3412487618)
This PR has grown quite a bit in scope (despite still being a net negative in LoC!), so I don't think it's unreasonable to split it up into an SFL-specific one, and one with follow-up changes to `txgraph`, or even further. I'll wait for reviewer comments.
💬 murchandamus commented on issue "coin-grinder missing test for TOTAL_TRIES":
(https://github.com/bitcoin/bitcoin/issues/33419#issuecomment-3412624821)
Yeah, it would be good to add this test. If someone wants to work on this, I would like to suggest that they peruse the very similar test for BnB here: https://github.com/bitcoin/bitcoin/blob/e744fd1249bf9577274614eaf3997bf4bbb612ff/src/wallet/test/coinselection_tests.cpp#L166
And to understand what CoinGrinder is actually doing and how it work, I would suggest that prospective implementers peruse the PR that added CoinGrinder starting with this commit: https://github.com/bitcoin/bitcoin/pull/27
...
(https://github.com/bitcoin/bitcoin/issues/33419#issuecomment-3412624821)
Yeah, it would be good to add this test. If someone wants to work on this, I would like to suggest that they peruse the very similar test for BnB here: https://github.com/bitcoin/bitcoin/blob/e744fd1249bf9577274614eaf3997bf4bbb612ff/src/wallet/test/coinselection_tests.cpp#L166
And to understand what CoinGrinder is actually doing and how it work, I would suggest that prospective implementers peruse the PR that added CoinGrinder starting with this commit: https://github.com/bitcoin/bitcoin/pull/27
...
💬 willcl-ark commented on issue "Node stuck with repeated "Cache size exceeds total space" log message":
(https://github.com/bitcoin/bitcoin/issues/27599#issuecomment-3412640491)
There seem to have been a number of recently merged PRs to address issues related to this recently:
#30611
#30039
...along with no further reports in a good while, so it looks like this issue might have been resolved. Is this still an issue with v30.0 or current master? If not I think this could be closed.
(https://github.com/bitcoin/bitcoin/issues/27599#issuecomment-3412640491)
There seem to have been a number of recently merged PRs to address issues related to this recently:
#30611
#30039
...along with no further reports in a good while, so it looks like this issue might have been resolved. Is this still an issue with v30.0 or current master? If not I think this could be closed.
✅ willcl-ark closed an issue: "getaddrmaninfo RPC: add Transport v1/v2 to `tried` for ipv4 & ipv6"
(https://github.com/bitcoin/bitcoin/issues/28807)
(https://github.com/bitcoin/bitcoin/issues/28807)
💬 willcl-ark commented on issue "getaddrmaninfo RPC: add Transport v1/v2 to `tried` for ipv4 & ipv6":
(https://github.com/bitcoin/bitcoin/issues/28807#issuecomment-3412751085)
This issue hasn’t attracted much interest from other contributors in quite some time.
Given that, it doesn’t seem important enough to keep open indefinitely. I’m going to close it for now due to lack of activity, but pull requests or renewed discussion are always welcome.
Comment here if you think this shoudl be re-opened.
(https://github.com/bitcoin/bitcoin/issues/28807#issuecomment-3412751085)
This issue hasn’t attracted much interest from other contributors in quite some time.
Given that, it doesn’t seem important enough to keep open indefinitely. I’m going to close it for now due to lack of activity, but pull requests or renewed discussion are always welcome.
Comment here if you think this shoudl be re-opened.
💬 enirox001 commented on pull request "test: Replace legacy wallet with MiniWallet in rpc_getblockstats.py":
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2437450950)
Mocktime is still needed for deterministic block hashes in CI. Removing it causes test failures due to varying timestamps. The test relies on fixed block hashes for comparison, so mocktime ensures reproducibility across runs.
(https://github.com/bitcoin/bitcoin/pull/33184#discussion_r2437450950)
Mocktime is still needed for deterministic block hashes in CI. Removing it causes test failures due to varying timestamps. The test relies on fixed block hashes for comparison, so mocktime ensures reproducibility across runs.
🤔 willcl-ark reviewed a pull request: "guix: build for Linux HOSTS with `-static-libgcc`"
(https://github.com/bitcoin/bitcoin/pull/33181#pullrequestreview-3347122839)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33181#pullrequestreview-3347122839)
Concept ACK
💬 willcl-ark commented on pull request "guix: build for Linux HOSTS with `-static-libgcc`":
(https://github.com/bitcoin/bitcoin/pull/33181#discussion_r2437549781)
I think this is expected, from reading the docs on [LDFLAGS](https://cmake.org/cmake/help/latest/envvar/LDFLAGS.html), [CMAKE_EXE_LINKER_FLAGS](https://cmake.org/cmake/help/latest/variable/CMAKE_EXE_LINKER_FLAGS_CONFIG.html) and [CMAKE_SHARED_LINKER_FLAGS](https://cmake.org/cmake/help/latest/variable/CMAKE_SHARED_LINKER_FLAGS.html):
>For any configuration run (including the first), the environment variable (LDFLAGS) will be ignored if the equivalent CMAKE_<TYPE>_LINKER_FLAGS_INIT variable is
...
(https://github.com/bitcoin/bitcoin/pull/33181#discussion_r2437549781)
I think this is expected, from reading the docs on [LDFLAGS](https://cmake.org/cmake/help/latest/envvar/LDFLAGS.html), [CMAKE_EXE_LINKER_FLAGS](https://cmake.org/cmake/help/latest/variable/CMAKE_EXE_LINKER_FLAGS_CONFIG.html) and [CMAKE_SHARED_LINKER_FLAGS](https://cmake.org/cmake/help/latest/variable/CMAKE_SHARED_LINKER_FLAGS.html):
>For any configuration run (including the first), the environment variable (LDFLAGS) will be ignored if the equivalent CMAKE_<TYPE>_LINKER_FLAGS_INIT variable is
...
🤔 cedwies reviewed a pull request: "ci: Only write docker build images to Cirrus cache"
(https://github.com/bitcoin/bitcoin/pull/33639#pullrequestreview-3347085731)
Approach ACK
(https://github.com/bitcoin/bitcoin/pull/33639#pullrequestreview-3347085731)
Approach ACK
💬 cedwies commented on pull request "ci: Only write docker build images to Cirrus cache":
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2437581569)
Maybe a short comment is helpful for future maintainers?
`# The Docker build step has moved into 02_run_container.py (see PR #33639)`
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2437581569)
Maybe a short comment is helpful for future maintainers?
`# The Docker build step has moved into 02_run_container.py (see PR #33639)`
💬 cedwies commented on pull request "ci: Only write docker build images to Cirrus cache":
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2437563972)
we compute `M` from `MAKEJOBS` and then always emit a cpuset.
CI normally uses `-jN` with N > 0, but for edge/local setups, if `MAKEJOBS` ends up as `-j0` (or missing/malformed?), `M` can be `0` -> cpus is `[]` → we emit `--cpuset-cpus= (empty)`. Would it be ok to skip the flag when M <= 0 (and optionally guard parsing)?
```py
def maybe_cpuset():
if os.getenv("HAVE_CGROUP_CPUSET"):
P = int(run(['nproc'], stdout=subprocess.PIPE).stdout)
mj = os.getenv("MAKEJOBS", "-j1") # ma
...
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2437563972)
we compute `M` from `MAKEJOBS` and then always emit a cpuset.
CI normally uses `-jN` with N > 0, but for edge/local setups, if `MAKEJOBS` ends up as `-j0` (or missing/malformed?), `M` can be `0` -> cpus is `[]` → we emit `--cpuset-cpus= (empty)`. Would it be ok to skip the flag when M <= 0 (and optionally guard parsing)?
```py
def maybe_cpuset():
if os.getenv("HAVE_CGROUP_CPUSET"):
P = int(run(['nproc'], stdout=subprocess.PIPE).stdout)
mj = os.getenv("MAKEJOBS", "-j1") # ma
...
💬 cedwies commented on pull request "ci: Only write docker build images to Cirrus cache":
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2437521843)
What if we run this outside CI (where env var might not exist)? In that case `os.getenv(...)` returns `None` and `shlex.split(None)` throws `TypeError`.
Would
```py
maybe_cache_arg = shlex.split(os.getenv('DOCKER_BUILD_CACHE_ARG', ''))
```
make sense here? CI behavior is identical, but if it's missing locally we just get an empty list (no cache flags) instead of crashing. Or would you rather keep CI-only usability?
(https://github.com/bitcoin/bitcoin/pull/33639#discussion_r2437521843)
What if we run this outside CI (where env var might not exist)? In that case `os.getenv(...)` returns `None` and `shlex.split(None)` throws `TypeError`.
Would
```py
maybe_cache_arg = shlex.split(os.getenv('DOCKER_BUILD_CACHE_ARG', ''))
```
make sense here? CI behavior is identical, but if it's missing locally we just get an empty list (no cache flags) instead of crashing. Or would you rather keep CI-only usability?