Bitcoin Core Github
43 subscribers
123K links
Download Telegram
πŸ’¬ 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
πŸ€” 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.
πŸ“ 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
πŸ’¬ 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
...
πŸ€” 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.
πŸ’¬ 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:
```
βœ… maflcko closed a pull request: "CI: migrate workflows to checkout v6"
(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
πŸ’¬ 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.
πŸ‘ 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
πŸ“ 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
πŸ’¬ 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.
πŸ’¬ 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
...
πŸ’¬ 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.
πŸ’¬ dergoegge commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(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.
πŸ’¬ 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.
πŸ’¬ 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
πŸ’¬ 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.
πŸ’¬ 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.