📝 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.
💬 plebhash commented on issue "Block template memory management (for IPC clients)":
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3570541428)
that's good to know!
but do we have to explicitly call `destroy` or is it sufficient to drop the reference from memory on the client side?
from my understanding of capnp, I believe it should be sufficient to drop it from memory, but on the other hand there must be a reason for `destroy` to exist?
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3570541428)
that's good to know!
but do we have to explicitly call `destroy` or is it sufficient to drop the reference from memory on the client side?
from my understanding of capnp, I believe it should be sufficient to drop it from memory, but on the other hand there must be a reason for `destroy` to exist?
💬 maflcko commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556116259)
I think it is fine to keep the libfuzzer-nosan command, but no strong opinion. Seems like it works without the extra flags for most people, and any extra flags can be added in the future, if there is need to.
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2556116259)
I think it is fine to keep the libfuzzer-nosan command, but no strong opinion. Seems like it works without the extra flags for most people, and any extra flags can be added in the future, if there is need to.