💬 ryanofsky commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713997278)
In commit "Replace RPCNotifyBlockChange with waitTipChanged()" (d919a230563bd2efee1fccb83194b43f436781ef)
Passing timeout parameter doesn't seem right, because it could wait too long. For example if timeout is 10 seconds, and tip changed after 8 seconds but did not match expected block hash, the next wait call will wait 10 seconds instead of 2 seconds.
Might be better to replace with:
```c++
while (IsRPCRunning()) {
auto now = timeout ? std::chrono::steady_clock::now() : std::chro
...
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713997278)
In commit "Replace RPCNotifyBlockChange with waitTipChanged()" (d919a230563bd2efee1fccb83194b43f436781ef)
Passing timeout parameter doesn't seem right, because it could wait too long. For example if timeout is 10 seconds, and tip changed after 8 seconds but did not match expected block hash, the next wait call will wait 10 seconds instead of 2 seconds.
Might be better to replace with:
```c++
while (IsRPCRunning()) {
auto now = timeout ? std::chrono::steady_clock::now() : std::chro
...
💬 ryanofsky commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1714002168)
In commit "Replace RPCNotifyBlockChange with waitTipChanged()" (d919a230563bd2efee1fccb83194b43f436781ef)
Seems like this has same problem as waitforblock where waitTipChanged could wait too long because it is called with wrong timeout value. Might be better as:
```c++
while (IsRPCRunning()) {
auto now = timeout ? std::chrono::steady_clock::now() : std::chrono::steady_clock::time_point::min();
if (block.height >= height || now > deadline) break;
block = timeout ? miner.wait
...
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1714002168)
In commit "Replace RPCNotifyBlockChange with waitTipChanged()" (d919a230563bd2efee1fccb83194b43f436781ef)
Seems like this has same problem as waitforblock where waitTipChanged could wait too long because it is called with wrong timeout value. Might be better as:
```c++
while (IsRPCRunning()) {
auto now = timeout ? std::chrono::steady_clock::now() : std::chrono::steady_clock::time_point::min();
if (block.height >= height || now > deadline) break;
block = timeout ? miner.wait
...
💬 hebasto commented on pull request "Drop log category in `SeedStartup`":
(https://github.com/bitcoin/bitcoin/pull/29480#issuecomment-2284374843)
I won’t be working on this PR in the near future. So, closing it and labeling it "Up for grabs".
(https://github.com/bitcoin/bitcoin/pull/29480#issuecomment-2284374843)
I won’t be working on this PR in the near future. So, closing it and labeling it "Up for grabs".
✅ hebasto closed a pull request: "Drop log category in `SeedStartup`"
(https://github.com/bitcoin/bitcoin/pull/29480)
(https://github.com/bitcoin/bitcoin/pull/29480)
💬 ryanofsky commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#issuecomment-2284394393)
> `<datadir>/node.sock` seems fine, although on macOS with the longest permitted username you would go over the limit: `/Users/willem-alexanderclausgeorgeferdinand-koning-der-nederlanden-jonkheer-van-amsberg-67/Library/Application\ Support/Bitcoin/testnet4/bitcoin-node.sock`
Ha! It is pretty easy to go over the socket path limit on linux too since it is only something like 100 characters. I think it might actually be possible to work around this by forking the process, changing the working di
...
(https://github.com/bitcoin/bitcoin/pull/30509#issuecomment-2284394393)
> `<datadir>/node.sock` seems fine, although on macOS with the longest permitted username you would go over the limit: `/Users/willem-alexanderclausgeorgeferdinand-koning-der-nederlanden-jonkheer-van-amsberg-67/Library/Application\ Support/Bitcoin/testnet4/bitcoin-node.sock`
Ha! It is pretty easy to go over the socket path limit on linux too since it is only something like 100 characters. I think it might actually be possible to work around this by forking the process, changing the working di
...
💬 hebasto commented on pull request "FIX:When opening or autoloading wallets there should be clear messages about rescanning in progress and wallets' names.":
(https://github.com/bitcoin-core/gui/pull/786#issuecomment-2284394856)
Closing due to lack of support from the PR author (unaddressed comments, failed CI).
Feel free to re-open.
(https://github.com/bitcoin-core/gui/pull/786#issuecomment-2284394856)
Closing due to lack of support from the PR author (unaddressed comments, failed CI).
Feel free to re-open.
✅ hebasto closed a pull request: "FIX:When opening or autoloading wallets there should be clear messages about rescanning in progress and wallets' names."
(https://github.com/bitcoin-core/gui/pull/786)
(https://github.com/bitcoin-core/gui/pull/786)
🚀 hebasto merged a pull request: "GUIUtil::brintToFront workaround for Wayland"
(https://github.com/bitcoin-core/gui/pull/831)
(https://github.com/bitcoin-core/gui/pull/831)
⚠️ 0xawaz opened an issue: "contrib: Automation for Bitcoin Full Node Deployment"
(https://github.com/bitcoin/bitcoin/issues/30638)
### Please describe the feature you'd like to see added.
I followed the [Bitcoin Full Node installation guide](https://bitcoin.org/en/full-node). While the guide is thorough, it lacks automation, which can lead to human errors and platform compatibility issues during manual deployment.
I propose adding an automation contrib for seamless deployment and maintenance of a Bitcoin full node. This will help eliminate manual errors, improve consistency across environments, simplify the overall proc
...
(https://github.com/bitcoin/bitcoin/issues/30638)
### Please describe the feature you'd like to see added.
I followed the [Bitcoin Full Node installation guide](https://bitcoin.org/en/full-node). While the guide is thorough, it lacks automation, which can lead to human errors and platform compatibility issues during manual deployment.
I propose adding an automation contrib for seamless deployment and maintenance of a Bitcoin full node. This will help eliminate manual errors, improve consistency across environments, simplify the overall proc
...
💬 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
(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)
(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.
(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)
(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
(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.
(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.
(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
...
(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
...
(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.
...
(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.
(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.