💬 glozow commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1850173724)
I misunderstood this comment initially, thinking it meant that you would call both `addNewTransaction`s, but it actually means that one is a wrapper for the other, with a prepended CMPA. Callers should use one or the other.
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1850173724)
I misunderstood this comment initially, thinking it meant that you would call both `addNewTransaction`s, but it actually means that one is a wrapper for the other, with a prepended CMPA. Callers should use one or the other.
💬 glozow commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1850204863)
b447416fddcb8c8647391502cca3dbfd1552e02e is nice!
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1850204863)
b447416fddcb8c8647391502cca3dbfd1552e02e is nice!
💬 glozow commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1850244902)
Sorry about this comment - just reread what I said and it makes no sense.
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1850244902)
Sorry about this comment - just reread what I said and it makes no sense.
🤔 BrandonOdiwuor reviewed a pull request: "RFC: build: support for pre-compiled headers."
(https://github.com/bitcoin/bitcoin/pull/31053#pullrequestreview-2448510169)
Concept ACK using pre-compiled headers to speed up compilation
(https://github.com/bitcoin/bitcoin/pull/31053#pullrequestreview-2448510169)
Concept ACK using pre-compiled headers to speed up compilation
💬 maflcko commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1850252005)
for reference, there are new CI machines lined up, but they can't be deployed due to https://github.com/bitcoin/bitcoin/issues/31210#issuecomment-2459823076
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1850252005)
for reference, there are new CI machines lined up, but they can't be deployed due to https://github.com/bitcoin/bitcoin/issues/31210#issuecomment-2459823076
🚀 glozow merged a pull request: "cluster mempool: Implement changeset interface for mempool"
(https://github.com/bitcoin/bitcoin/pull/31122)
(https://github.com/bitcoin/bitcoin/pull/31122)
💬 dergoegge commented on pull request "fuzz: set the output argument of FuzzedSock::Accept()":
(https://github.com/bitcoin/bitcoin/pull/31316#issuecomment-2488561557)
Perhaps this makes more sense in combination with a test that uses it? Currently `FuzzedSock::Accept` seems entirely unused: https://maflcko.github.io/b-c-cov/fuzz.coverage/src/test/fuzz/util/net.cpp.gcov.html
(https://github.com/bitcoin/bitcoin/pull/31316#issuecomment-2488561557)
Perhaps this makes more sense in combination with a test that uses it? Currently `FuzzedSock::Accept` seems entirely unused: https://maflcko.github.io/b-c-cov/fuzz.coverage/src/test/fuzz/util/net.cpp.gcov.html
💬 kevkevinpal commented on pull request "test: locking -testdatadir when not specified and then deleting lock and dir at end of test":
(https://github.com/bitcoin/bitcoin/pull/31328#discussion_r1850347673)
maybe we can update the error message to be
```
ExitFailure("Cannot obtain a lock on test data lock directory " + fs::PathToString(m_path_lock) + '\n' + "The directory and .lock file already exist, please ensure the directory .lock file does not exist.");
```
it is fine if the directory exists, we are just don't want `.lock` file to be present?
I can see if I can come up with better wording than what I suggested above
(https://github.com/bitcoin/bitcoin/pull/31328#discussion_r1850347673)
maybe we can update the error message to be
```
ExitFailure("Cannot obtain a lock on test data lock directory " + fs::PathToString(m_path_lock) + '\n' + "The directory and .lock file already exist, please ensure the directory .lock file does not exist.");
```
it is fine if the directory exists, we are just don't want `.lock` file to be present?
I can see if I can come up with better wording than what I suggested above
💬 kevkevinpal commented on pull request "test: locking -testdatadir when not specified and then deleting lock and dir at end of test":
(https://github.com/bitcoin/bitcoin/pull/31328#discussion_r1850374636)
updated in fe6da3744420bcbac9002ae324dfa04902aed490
(https://github.com/bitcoin/bitcoin/pull/31328#discussion_r1850374636)
updated in fe6da3744420bcbac9002ae324dfa04902aed490
💬 ryanofsky commented on pull request "Add destroy to BlockTemplate schema":
(https://github.com/bitcoin/bitcoin/pull/31288#issuecomment-2488688261)
Will go ahead and merge even though it only has 2 acks, since it's basically just adding a line to a schema file. Having this merged should make the related PR easier to maintain and review.
(https://github.com/bitcoin/bitcoin/pull/31288#issuecomment-2488688261)
Will go ahead and merge even though it only has 2 acks, since it's basically just adding a line to a schema file. Having this merged should make the related PR easier to maintain and review.
📝 MarnixCroes opened a pull request: "contrib: fix BUILDDIR in gen-bitcoin-conf script"
(https://github.com/bitcoin/bitcoin/pull/31332)
On master the gen-bitcoin-conf script doesn't work as it cannot find bitcoind.
This is because of the build dir that is now being used since cmake.
This PR fixes it.
**master**
```
./gen-bitcoin-conf.sh
/home/marnix/projects/bitcoin/src/bitcoind not found or not executable.
```
**PR**
```
./gen-bitcoin-conf.sh
Generating example bitcoin.conf file in share/examples/
```
(https://github.com/bitcoin/bitcoin/pull/31332)
On master the gen-bitcoin-conf script doesn't work as it cannot find bitcoind.
This is because of the build dir that is now being used since cmake.
This PR fixes it.
**master**
```
./gen-bitcoin-conf.sh
/home/marnix/projects/bitcoin/src/bitcoind not found or not executable.
```
**PR**
```
./gen-bitcoin-conf.sh
Generating example bitcoin.conf file in share/examples/
```
📝 hodlinator opened a pull request: "fuzz: Implement G_TEST_GET_FULL_NAME"
(https://github.com/bitcoin/bitcoin/pull/31333)
Catching up to bench & unit tests. Makes for more orderly paths for fuzz tests using `BasicTestingSetup`.
### Before
```
/tmp/test_common bitcoin/0748ae43ef8fa80703bc/regtest/blocks/xor.dat
```
### After
```
/tmp/test_common bitcoin/tx_pool_standard/f18b3744625e0600eb0c/regtest/blocks/xor.dat
```
(https://github.com/bitcoin/bitcoin/pull/31333)
Catching up to bench & unit tests. Makes for more orderly paths for fuzz tests using `BasicTestingSetup`.
### Before
```
/tmp/test_common bitcoin/0748ae43ef8fa80703bc/regtest/blocks/xor.dat
```
### After
```
/tmp/test_common bitcoin/tx_pool_standard/f18b3744625e0600eb0c/regtest/blocks/xor.dat
```
💬 hodlinator commented on pull request "test: Fix RANDOM_CTX_SEED use with parallel tests":
(https://github.com/bitcoin/bitcoin/pull/30737#issuecomment-2488709503)
New PR #31333.
(https://github.com/bitcoin/bitcoin/pull/30737#issuecomment-2488709503)
New PR #31333.
⚠️ fanquake opened an issue: "ci: brew link workaround step no-longer working"
(https://github.com/bitcoin/bitcoin/issues/31334)
CI failure in #31333. https://github.com/bitcoin/bitcoin/actions/runs/11935219880/job/33266067166?pr=31333:
```bash
Run # A workaround for "The `brew link` step did not complete successfully" error.
<...>
Error: The `brew link` step did not complete successfully
The formula built, but is not symlinked into /opt/homebrew
Could not symlink bin/pkg-config
Target /opt/homebrew/bin/pkg-config
```
(https://github.com/bitcoin/bitcoin/issues/31334)
CI failure in #31333. https://github.com/bitcoin/bitcoin/actions/runs/11935219880/job/33266067166?pr=31333:
```bash
Run # A workaround for "The `brew link` step did not complete successfully" error.
<...>
Error: The `brew link` step did not complete successfully
The formula built, but is not symlinked into /opt/homebrew
Could not symlink bin/pkg-config
Target /opt/homebrew/bin/pkg-config
```
💬 kevkevinpal commented on pull request "fuzz: Implement G_TEST_GET_FULL_NAME":
(https://github.com/bitcoin/bitcoin/pull/31333#issuecomment-2488754481)
ACK [92d3d69](https://github.com/bitcoin/bitcoin/pull/31333/commits/92d3d691f097ead8e5ae571eb9bf691133a6aa49)
This helps clean up the `/tmp/test_common bitcoin` dir and makes it easier to know which directory is running which test
I tested by running an individual fuzz test with multiple workers and observing the `/tmp` dir
`FUZZ=utxo_total_supply ./build_fuzz/src/test/fuzz/fuzz -jobs=8 -workers=8`
I also tested by running the fuzz `test_runner.py` and observing the `/tmp` dir
` ./bui
...
(https://github.com/bitcoin/bitcoin/pull/31333#issuecomment-2488754481)
ACK [92d3d69](https://github.com/bitcoin/bitcoin/pull/31333/commits/92d3d691f097ead8e5ae571eb9bf691133a6aa49)
This helps clean up the `/tmp/test_common bitcoin` dir and makes it easier to know which directory is running which test
I tested by running an individual fuzz test with multiple workers and observing the `/tmp` dir
`FUZZ=utxo_total_supply ./build_fuzz/src/test/fuzz/fuzz -jobs=8 -workers=8`
I also tested by running the fuzz `test_runner.py` and observing the `/tmp` dir
` ./bui
...
📝 fanquake opened a pull request: "macOS: swap docs & CI from pkg-config to pkgconf"
(https://github.com/bitcoin/bitcoin/pull/31335)
Migrate the macOS build docs and CI from `pkg-config` to `pkgconf`. As the former now just redirects to the later.
Upstream is currently mass-migrating its formula. i.e https://github.com/Homebrew/homebrew-core/pull/198317.
If #31334 persists we can add a similar workaround as we do for Python.
(https://github.com/bitcoin/bitcoin/pull/31335)
Migrate the macOS build docs and CI from `pkg-config` to `pkgconf`. As the former now just redirects to the later.
Upstream is currently mass-migrating its formula. i.e https://github.com/Homebrew/homebrew-core/pull/198317.
If #31334 persists we can add a similar workaround as we do for Python.
💬 hebasto commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#issuecomment-2488931506)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/31335#issuecomment-2488931506)
Concept ACK.
🚀 ryanofsky merged a pull request: "Add destroy to BlockTemplate schema"
(https://github.com/bitcoin/bitcoin/pull/31288)
(https://github.com/bitcoin/bitcoin/pull/31288)
💬 fanquake commented on pull request "macOS: swap docs & CI from pkg-config to pkgconf":
(https://github.com/bitcoin/bitcoin/pull/31335#issuecomment-2488944785)
Given that the CI is now working here, (and #31334 wasn't one-off, it's happening for all new pushes), have updated the commit message and PR description.
(https://github.com/bitcoin/bitcoin/pull/31335#issuecomment-2488944785)
Given that the CI is now working here, (and #31334 wasn't one-off, it's happening for all new pushes), have updated the commit message and PR description.
💬 andremralves commented on issue "Discover() will not run if listening on any address with an explicit bind=0.0.0.0":
(https://github.com/bitcoin/bitcoin/issues/31293#issuecomment-2488958085)
Is `Discover()` working properly? I tried running `test_runner.py --ihave1111and2222 feature_bind_port_discover.py` and the test didn't find the `1.1.1.1` and `2.2.2.2` addresses. In the commit where `feature-bind_port_discover.py` was added, the test works as expected and passes. Actually, I think, now, only ipv6 addresses are being discovered.
(https://github.com/bitcoin/bitcoin/issues/31293#issuecomment-2488958085)
Is `Discover()` working properly? I tried running `test_runner.py --ihave1111and2222 feature_bind_port_discover.py` and the test didn't find the `1.1.1.1` and `2.2.2.2` addresses. In the commit where `feature-bind_port_discover.py` was added, the test works as expected and passes. Actually, I think, now, only ipv6 addresses are being discovered.