💬 kristapsk commented on issue "contrib: Automation for Bitcoin Full Node Deployment":
(https://github.com/bitcoin/bitcoin/issues/30638#issuecomment-2284470978)
> 3\. **Monitoring System**: Prometheus/Grafana monitoring system to monitor Bitcoin Node metrics and health.

For monitoring I have made some scripts and templates for Zabbix monitoring system. https://github.com/zkSNACKs/zbx-bitcoin

💬 maflcko commented on issue "contrib: Automation for Bitcoin Full Node Deployment":
(https://github.com/bitcoin/bitcoin/issues/30638#issuecomment-2284497409)
See https://github.com/bitcoin-core/packaging/issues/55 and https://github.com/bitcoin/bitcoin/pull/25681 about the previous docker discussions (there are more than that).

About the Ansible/monitoring: I'd doubt that the majority of users would want that. The ones that want it, will likely go with their in-house version anyway, so it seems unclear what the target audience would be. (The same applies to docker to a lesser extent)

💬 achow101 commented on pull request "Migrate legacy wallets that are not loaded":
(https://github.com/bitcoin-core/gui/pull/824#discussion_r1714124145)
What compiler? I'm not seeing this error, and it seems neither did CI.

📝 maflcko opened a pull request: "ci: Use clang-19 in msan tasks"
(https://github.com/bitcoin/bitcoin/pull/30639)
A new clang version generally comes with bugfixes, new sanitizer features, deprecations, as well as new features.

Upgrade the memory sanitizer tasks to use the new version.

(Ref https://github.com/bitcoin/bitcoin/pull/30634)

💬 achow101 commented on pull request "Migrate legacy wallets that are not loaded":
(https://github.com/bitcoin-core/gui/pull/824#discussion_r1714137844)
Done

💬 achow101 commented on pull request "Migrate legacy wallets that are not loaded":
(https://github.com/bitcoin-core/gui/pull/824#discussion_r1714138004)
Cherry picked.

💬 agroce commented on issue "fuzz: `script`: Assertion `!extract_destination_ret' failed.":
(https://github.com/bitcoin/bitcoin/issues/30615#issuecomment-2284562885)
Each target will spend < 0.15% of the time with a length of 100 or below, so not much low-max_len fuzzing happening I guess.

⚠️ maflcko opened an issue: "Intermittent failure in feature_fee_estimation.py" in sanity_check_rbf_estimates: est_feerate = node.estimatesmartfee(2)["feerate"] (KeyError: 'feerate')"
(https://github.com/bitcoin/bitcoin/issues/30640)
https://cirrus-ci.com/task/5165357323780096?logs=ci#L2840

```
test 2024-08-09T22:10:43.283000Z TestFramework (ERROR): Key error
Traceback (most recent call last):
File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 132, in main
self.run_test()
File "/ci_container_bas
...

💬 0xawaz commented on issue "contrib: Automation for Bitcoin Full Node Deployment":
(https://github.com/bitcoin/bitcoin/issues/30638#issuecomment-2284604375)
Thanks @maflcko for pointing out docker discussions I missed. I took some time to walk through all the discussions. It just got me more confused. I am genuinely curious to understand what's happening. So I am asking few questions:

- **Docker**
- Since we all agree that we need an official docker image for Bitcoin Core, what all PR attempts fail to consider?
- My understanding is Dockerfile maintenance is one of the main concerns, could you elaborate more? Do you need more hands in the t
...

💬 ryanofsky commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2284621389)
This seems to offer a lot of nice features, but can you explain the tradeoffs of wrapping the C++ interface in C instead of using C++ from rust directly? It seems like having a C middle layer introduces a lot of boilerplate, and I'm wondering if it is really necessary. For example it seems like there is a rust cxx crate (https://docs.rs/cxx/latest/cxx/, https://chatgpt.com/share/dd4dde59-66d6-4486-88a6-2f42144be056) that lets you call C++ directly from Rust and avoid the need for C boilerplate.
...

💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714153255)
a8d4adfea4b50f452d80bc1e7ee322945d886c78: @vasild's #30205 is potentially useful here. Though as long as the test is not brittle, there's no need to wait for that PR to be merged.

🤔 Sjors reviewed a pull request: "multiprocess: Add -ipcbind option to bitcoin-node"
(https://github.com/bitcoin/bitcoin/pull/30509#pullrequestreview-2233467423)
ACK 4137419b0170fa1e9ff9daacde57933f7c995b76

I only lightly looked at the new `ProcessImpl` method bodies.

I tested that I'm able to connect the bitcoin-mine demo #30437, but that (as expected) it immediately fails with a "remote exception: Method not implemented".

💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714041612)
ef57db8711c7fafb6c4f09b880fd4263bc6dc9dd: I also tested building this outside depends on macOS 14.6.1

💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714185762)
a8d4adfea4b50f452d80bc1e7ee322945d886c78: maybe test what happens when there's a second connection attempt to the same address.

💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714199889)
52917ad4be1ca574543e854fdc154e88d65ce57f: nit `/*backlog=*/5`

💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714091657)
4137419b0170fa1e9ff9daacde57933f7c995b76: I have a hard hitting this, instead I tend to get a crash (on intel macOS):

For an existing path:

```
-ipcbind=unix://tmp
...
************************
EXCEPTION: NSt3__112system_errorE
Address already in use
bitcoin in AppInit()
```

Non-existing path that I don't have permission for:

```
-ipcbind=unix://root
...
************************
EXCEPTION: NSt3__112system_errorE
Read-only file system
bitcoin i
...

💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714085148)
4137419b0170fa1e9ff9daacde57933f7c995b76: In the mean time this has the nice side-effect of being able to run `src/bitcoin-gui -ipcbind=unix` (instead of `bitcoin-node`) and then connect to it with e.g. the miner demo `-ipcconnect=unix:gui.sock`.

See https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2226847354

💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#discussion_r1714205805)
52917ad4be1ca574543e854fdc154e88d65ce57f: this could use a test.

Or maybe we can introduce something akin to `CNetAddr` for socket addresses? Fortunately this input can only be provided by the user, so we don't have to be overzealous about sanitising.

💬 achow101 commented on pull request "fuzz: improve `scriptpubkeyman` target":
(https://github.com/bitcoin/bitcoin/pull/30563#issuecomment-2284663808)
ACK 401cc4ec70d67ba2aa0e078d2fab214e1c40742c

💬 achow101 commented on pull request "wallet: fix blank legacy detection":
(https://github.com/bitcoin/bitcoin/pull/30621#discussion_r1714223454)
In 30f701317b4a674fea422766ac3ee179c9c6f554 "wallet: fix, detect blank legacy wallets in IsLegacy"

`IsLegacy()` is unnecessary, same as above.

💬 achow101 commented on pull request "wallet: fix blank legacy detection":
(https://github.com/bitcoin/bitcoin/pull/30621#discussion_r1714223083)
In 30f701317b4a674fea422766ac3ee179c9c6f554 "wallet: fix, detect blank legacy wallets in IsLegacy"

`IsLegacy()` is unnecessary here now since `GetLegacyScriptPubKeyMan()` also checks for `WALLET_FLAG_DESCRIPTORS`.