💬 ryanofsky commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713879417)
In commit "Add waitTipChanged to Mining interface" (3435d7dd430559abffc964c163a2ae73febd99d1)
> I changed this in the last push, previously it would always wait for one second each interrupt "round" even if the requested `timeout` was shorter.
This doesn't seem right. If timeout is 1500ms and the block never changes, it looks like this will wait 1000ms+1000ms instead of 1000ms+500ms. I'm also concerned about the `deadline = now() + timeout` assignment above because it seems like that will
...
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713879417)
In commit "Add waitTipChanged to Mining interface" (3435d7dd430559abffc964c163a2ae73febd99d1)
> I changed this in the last push, previously it would always wait for one second each interrupt "round" even if the requested `timeout` was shorter.
This doesn't seem right. If timeout is 1500ms and the block never changes, it looks like this will wait 1000ms+1000ms instead of 1000ms+500ms. I'm also concerned about the `deadline = now() + timeout` assignment above because it seems like that will
...
💬 ryanofsky commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713965000)
In commit "Replace RPCNotifyBlockChange with waitTipChanged()" (d919a230563bd2efee1fccb83194b43f436781ef)
It seems like the implementation of the this loop always waits for the timeout to be reached, and doesn't exit early if the tip did change. So if timeout is 0 it will just infinite loop.
I think if it can be simplified to just:
```c++
if (IsRPCRunning()) {
block = timeout
? miner.waitTipChanged(block, std::chrono::milliseconds(timeout))
: min
...
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713965000)
In commit "Replace RPCNotifyBlockChange with waitTipChanged()" (d919a230563bd2efee1fccb83194b43f436781ef)
It seems like the implementation of the this loop always waits for the timeout to be reached, and doesn't exit early if the tip did change. So if timeout is 0 it will just infinite loop.
I think if it can be simplified to just:
```c++
if (IsRPCRunning()) {
block = timeout
? miner.waitTipChanged(block, std::chrono::milliseconds(timeout))
: min
...
💬 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
...