💬 furszy commented on pull request "Migrate legacy wallets that are not loaded":
(https://github.com/bitcoin-core/gui/pull/824#discussion_r1713845934)
Compilation error: `error: 'wallet_name' in capture list does not name a variable`.
It seems the lambda expression cannot capture the structured binding. A local reference of the variable fixes it.
Same happens on the `m_open_wallet_menu` action lambda with the "path" variable.
(https://github.com/bitcoin-core/gui/pull/824#discussion_r1713845934)
Compilation error: `error: 'wallet_name' in capture list does not name a variable`.
It seems the lambda expression cannot capture the structured binding. A local reference of the variable fixes it.
Same happens on the `m_open_wallet_menu` action lambda with the "path" variable.
💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#issuecomment-2284311578)
`<datadir>/node.sock` seems fine, although on macOS with the longest permitted username you would go over the limit: `/Users/willem-alexanderclausgeorgeferdinand-konder-der-nederlanden-jonkheer-van-amsberg-67/Library/Application\ Support/Bitcoin/testnet4/bitcoin-node.sock`
(https://github.com/bitcoin/bitcoin/pull/30509#issuecomment-2284311578)
`<datadir>/node.sock` seems fine, although on macOS with the longest permitted username you would go over the limit: `/Users/willem-alexanderclausgeorgeferdinand-konder-der-nederlanden-jonkheer-van-amsberg-67/Library/Application\ Support/Bitcoin/testnet4/bitcoin-node.sock`
📝 paplorinc opened a pull request: "refactor: Migrate EmplaceCoinInternalDANGER to try_emplace"
(https://github.com/bitcoin/bitcoin/pull/30637)
Leftover from https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1652853326:
[`try_emplace`](https://en.cppreference.com/w/cpp/container/map/try_emplace) doc states
> 2) Behaves like emplace except that the element is constructed as
value_type(std::piecewise_construct,
std::forward_as_tuple(std::move(k)),
std::forward_as_tuple(std::forward<Args>(args)...))
---
The [`DANGER` part](https://github.com/bitcoin/bitcoin/pull/19806/commits/f6e2da5fb7c6406c37612
...
(https://github.com/bitcoin/bitcoin/pull/30637)
Leftover from https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1652853326:
[`try_emplace`](https://en.cppreference.com/w/cpp/container/map/try_emplace) doc states
> 2) Behaves like emplace except that the element is constructed as
value_type(std::piecewise_construct,
std::forward_as_tuple(std::move(k)),
std::forward_as_tuple(std::forward<Args>(args)...))
---
The [`DANGER` part](https://github.com/bitcoin/bitcoin/pull/19806/commits/f6e2da5fb7c6406c37612
...
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1714018553)
Opened https://github.com/bitcoin/bitcoin/pull/30637
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1714018553)
Opened https://github.com/bitcoin/bitcoin/pull/30637
💬 agroce commented on issue "fuzz: `script`: Assertion `!extract_destination_ret' failed.":
(https://github.com/bitcoin/bitcoin/issues/30615#issuecomment-2284321835)
Actually, looks like the random_max_len (which could be small enough) strategy is enabled 15% of the time in the OSS-Fuzz config anyway.
(https://github.com/bitcoin/bitcoin/issues/30615#issuecomment-2284321835)
Actually, looks like the random_max_len (which could be small enough) strategy is enabled 15% of the time in the OSS-Fuzz config anyway.
💬 agroce commented on issue "fuzz: `script`: Assertion `!extract_destination_ret' failed.":
(https://github.com/bitcoin/bitcoin/issues/30615#issuecomment-2284323856)
https://github.com/google/clusterfuzz/blob/578b401b655e95211b6ba9b805986832622b213f/src/clusterfuzz/_internal/fuzzing/strategy.py#L35
(https://github.com/bitcoin/bitcoin/issues/30615#issuecomment-2284323856)
https://github.com/google/clusterfuzz/blob/578b401b655e95211b6ba9b805986832622b213f/src/clusterfuzz/_internal/fuzzing/strategy.py#L35
💬 agroce commented on issue "fuzz: `script`: Assertion `!extract_destination_ret' failed.":
(https://github.com/bitcoin/bitcoin/issues/30615#issuecomment-2284331725)
.... though that's a random uniform value between 1 and 10000, so at 15% (which is of runs?) I'm not sure how often you'll ever see any target hit with a particularly _small_ max_len. it'll happen, but perhaps quite infrequently if there are targets that benefit substantially from small string runs
(https://github.com/bitcoin/bitcoin/issues/30615#issuecomment-2284331725)
.... though that's a random uniform value between 1 and 10000, so at 15% (which is of runs?) I'm not sure how often you'll ever see any target hit with a particularly _small_ max_len. it'll happen, but perhaps quite infrequently if there are targets that benefit substantially from small string runs
🤔 ryanofsky reviewed a pull request: "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals"
(https://github.com/bitcoin/bitcoin/pull/30409#pullrequestreview-2233198217)
Code review dcf984b055a367facf7704eb8055619d6fe64a55. I think there might be an infinite loop but if waitfornewblock is called with timeout of 0, and it looks like there are some other quirks. Left a few suggestions below.
(https://github.com/bitcoin/bitcoin/pull/30409#pullrequestreview-2233198217)
Code review dcf984b055a367facf7704eb8055619d6fe64a55. I think there might be an infinite loop but if waitfornewblock is called with timeout of 0, and it looks like there are some other quirks. Left a few suggestions below.
💬 ryanofsky commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713960772)
In commit "Add waitTipChanged to Mining interface" (3435d7dd430559abffc964c163a2ae73febd99d1)
I think it would be slightly better for `current_tip` to just be a block hash, rather than hash and height, because the height value is unused, and a accepting height might create misleading impression that this method could be called to wait for a height to be reached.
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713960772)
In commit "Add waitTipChanged to Mining interface" (3435d7dd430559abffc964c163a2ae73febd99d1)
I think it would be slightly better for `current_tip` to just be a block hash, rather than hash and height, because the height value is unused, and a accepting height might create misleading impression that this method could be called to wait for a height to be reached.
💬 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
...