π¬ Sjors commented on issue "Block template memory management (for IPC clients)":
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3569959628)
I'm planning to make similar plots for #33922 (without CPU and ideally with marks to indicate where blocks were found).
If you're measuring the process memory instead of only the template memory (which #33922 enables), you'll want to hold the mempool itself constant. E.g. by picking some value for `-maxmempool` and waiting for it to fill before starting the measurement. You also want to set `-dbcache` to its minimum, because that's also accuring
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3569959628)
I'm planning to make similar plots for #33922 (without CPU and ideally with marks to indicate where blocks were found).
If you're measuring the process memory instead of only the template memory (which #33922 enables), you'll want to hold the mempool itself constant. E.g. by picking some value for `-maxmempool` and waiting for it to fill before starting the measurement. You also want to set `-dbcache` to its minimum, because that's also accuring
π€ rkrux reviewed a pull request: "doc: clarify and cleanup macOS fuzzing notes"
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3499650048)
ACK 6e37b3b4861733ca97ec5d27d0bf52d187b6a2c9
I do remember facing issues on Mac some time back that prompted me to create issue #33667.
Like mentioned in earlier couple comments, it'd be helpful to mention that Linux is recommended in the beginning "Quickstart guide" section.
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3499650048)
ACK 6e37b3b4861733ca97ec5d27d0bf52d187b6a2c9
I do remember facing issues on Mac some time back that prompted me to create issue #33667.
Like mentioned in earlier couple comments, it'd be helpful to mention that Linux is recommended in the beginning "Quickstart guide" section.
π rejected-l opened a pull request: "CI: migrate workflows to checkout v6"
(https://github.com/bitcoin/bitcoin/pull/33933)
Updates `actions/checkout` to v6 in CI workflows. No code changes.
https://github.com/actions/checkout/releases/tag/v6.0.0
(https://github.com/bitcoin/bitcoin/pull/33933)
Updates `actions/checkout` to v6 in CI workflows. No code changes.
https://github.com/actions/checkout/releases/tag/v6.0.0
π¬ Sjors commented on pull request "mining: add getMemoryLoad() and track template non-mempool memory footprint":
(https://github.com/bitcoin/bitcoin/pull/33922#issuecomment-3570105870)
> Delegating template eviction responsibility to the client can put us in a situation where they handle it poorly and cause us to OOM
Note that it's _already_ the clients responsibility, that's inherent to how multiprocess works.
In the scenario where they handle it poorly, we can use FIFO deletion. All `getMemoryLoad()` does is give clients an opportunity to handle it better. If they're fine with FIFO, then they never have to call this method.
> treat clients differently from our own c
...
(https://github.com/bitcoin/bitcoin/pull/33922#issuecomment-3570105870)
> Delegating template eviction responsibility to the client can put us in a situation where they handle it poorly and cause us to OOM
Note that it's _already_ the clients responsibility, that's inherent to how multiprocess works.
In the scenario where they handle it poorly, we can use FIFO deletion. All `getMemoryLoad()` does is give clients an opportunity to handle it better. If they're fine with FIFO, then they never have to call this method.
> treat clients differently from our own c
...
π€ rkrux reviewed a pull request: "ci: Make the max number of commits tested explicit"
(https://github.com/bitcoin/bitcoin/pull/33909#pullrequestreview-3499758912)
ACK d14d1b52b51c4e8e2388af7e54c854e8a133e088
I vaguely recall noticing this limit some time back. Makes sense to me that it is highlighted in the job name.
(https://github.com/bitcoin/bitcoin/pull/33909#pullrequestreview-3499758912)
ACK d14d1b52b51c4e8e2388af7e54c854e8a133e088
I vaguely recall noticing this limit some time back. Makes sense to me that it is highlighted in the job name.
π¬ rkrux commented on pull request "ci: Make the max number of commits tested explicit":
(https://github.com/bitcoin/bitcoin/pull/33909#discussion_r2555796819)
This or something similar?
```diff
- test-each-commit:
+ test-last-few-ancestor-commits:
```
(https://github.com/bitcoin/bitcoin/pull/33909#discussion_r2555796819)
This or something similar?
```diff
- test-each-commit:
+ test-last-few-ancestor-commits:
```
β
maflcko closed a pull request: "CI: migrate workflows to checkout v6"
(https://github.com/bitcoin/bitcoin/pull/33933)
(https://github.com/bitcoin/bitcoin/pull/33933)
π¬ maflcko commented on pull request "CI: migrate workflows to checkout v6":
(https://github.com/bitcoin/bitcoin/pull/33933#issuecomment-3570216444)
thx, but subtrees need to be modified upstream, not here
(https://github.com/bitcoin/bitcoin/pull/33933#issuecomment-3570216444)
thx, but subtrees need to be modified upstream, not here
π¬ maflcko commented on pull request "ci: Run GUI unit tests in cross-Windows task":
(https://github.com/bitcoin/bitcoin/pull/33919#issuecomment-3570225361)
Looks like the output is missing, but this works: https://github.com/bitcoin/bitcoin/actions/runs/19630865894/job/56211087066#step:8:206:
```
Run ./bin/test_bitcoin-qt.exe
Error: Process completed with exit code 1.
(https://github.com/bitcoin/bitcoin/pull/33919#issuecomment-3570225361)
Looks like the output is missing, but this works: https://github.com/bitcoin/bitcoin/actions/runs/19630865894/job/56211087066#step:8:206:
```
Run ./bin/test_bitcoin-qt.exe
Error: Process completed with exit code 1.
π vasild approved a pull request: "ci: Use latest Xcode that the minimum macOS version allows"
(https://github.com/bitcoin/bitcoin/pull/33932#pullrequestreview-3500009912)
ACK fa9537cde10120b12c96061cbc3f79a7680f9d64
(https://github.com/bitcoin/bitcoin/pull/33932#pullrequestreview-3500009912)
ACK fa9537cde10120b12c96061cbc3f79a7680f9d64
π wedjob0X opened a pull request: "Update GitHub Actions "
(https://github.com/bitcoin/bitcoin/pull/33934)
Updated checkout action to v6 for Node.js 24 support and enhanced credential security.
v6 - https://github.com/actions/checkout/releases/tag/v6.0.0
(https://github.com/bitcoin/bitcoin/pull/33934)
Updated checkout action to v6 for Node.js 24 support and enhanced credential security.
v6 - https://github.com/actions/checkout/releases/tag/v6.0.0
π¬ vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2556032057)
Here the failure was not about the `<=>` operator itself, but because `NodeClock::time_point` did not have such an operator provided by the standard library. That was only for macos-cross CI jobs, which use Clang 19.1.7. The native macos jobs succeeded. They use XCode 16.0 and Clang 16.0.0.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2556032057)
Here the failure was not about the `<=>` operator itself, but because `NodeClock::time_point` did not have such an operator provided by the standard library. That was only for macos-cross CI jobs, which use Clang 19.1.7. The native macos jobs succeeded. They use XCode 16.0 and Clang 16.0.0.
π¬ stratospher commented on pull request "cli: rework -addrinfo cli to use addresses which arenβt filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-3570474430)
I pushed an [update](https://github.com/bitcoin/bitcoin/compare/858f49bcb3590b211f014e8d290008bb13f5b17f..5b05a9959f1633bfee78d9edb180c672b0640ab5). I also don't think we should implement a fallback to support a rare usecase for few individuals. I've run into a few occasions where I used older bitcoind and newer cli but it's when I misconfigure something accidentally and wasn't the behaviour I intended. I guess this is what most normal users would run into.
This is why I think we shouldn't im
...
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-3570474430)
I pushed an [update](https://github.com/bitcoin/bitcoin/compare/858f49bcb3590b211f014e8d290008bb13f5b17f..5b05a9959f1633bfee78d9edb180c672b0640ab5). I also don't think we should implement a fallback to support a rare usecase for few individuals. I've run into a few occasions where I used older bitcoind and newer cli but it's when I misconfigure something accidentally and wasn't the behaviour I intended. I guess this is what most normal users would run into.
This is why I think we shouldn't im
...
π¬ dergoegge commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556065929)
It's not related to mac, so I put it back in. Although I'm not sure how useful it is to document this here, as it is documented more accurately in the afl++ docs already.
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556065929)
It's not related to mac, so I put it back in. Although I'm not sure how useful it is to document this here, as it is documented more accurately in the afl++ docs already.
π¬ dergoegge commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556066319)
Done
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556066319)
Done
π¬ dergoegge commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556067401)
This comment led me to remove the steps entirely. They'll just invite bike-shedding over what works for different people, which is partly what we're trying to avoid with this effort in the first place.
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556067401)
This comment led me to remove the steps entirely. They'll just invite bike-shedding over what works for different people, which is partly what we're trying to avoid with this effort in the first place.
π¬ dergoegge commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556068835)
Kept something similar in the quickstart section at the top.
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556068835)
Kept something similar in the quickstart section at the top.
π¬ dergoegge commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#issuecomment-3570508575)
* removed the macos steps (https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556067401)
* added a reference to the macos notes section in the quickstart section
(https://github.com/bitcoin/bitcoin/pull/33921#issuecomment-3570508575)
* removed the macos steps (https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556067401)
* added a reference to the macos notes section in the quickstart section
π¬ Sjors commented on issue "Block template memory management (for IPC clients)":
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3570523079)
@plebhash it looks like you found a real bug. Because `BlockTemplate::createNewBlock` doesn't have a context param, it looks like its destroy method is not invoked until sv2-tp disconnects. So the node keeps holding on to templates even though the Template Provider already pruned them.
I'll open a PR to fix that.
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3570523079)
@plebhash it looks like you found a real bug. Because `BlockTemplate::createNewBlock` doesn't have a context param, it looks like its destroy method is not invoked until sv2-tp disconnects. So the node keeps holding on to templates even though the Template Provider already pruned them.
I'll open a PR to fix that.
π¬ l0rinc commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556093636)
> This comment led me to remove the steps entirely
Hmmm, so what happened to "best effort"?
> which is partly what we're trying to avoid with this effort in the first place
I thought we were aiming to provide Mac users a way to have at least basic fuzzing support.
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556093636)
> This comment led me to remove the steps entirely
Hmmm, so what happened to "best effort"?
> which is partly what we're trying to avoid with this effort in the first place
I thought we were aiming to provide Mac users a way to have at least basic fuzzing support.