💬 ItIsOHM commented on issue "rpc: doc: RPCHelpMan missing request parameters for getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1618350216)
> MSVC is one option, WSL another. See also https://github.com/bitcoin/bitcoin/blob/master/doc/build-windows.md.
>
> Build questions are a bit out of scope for this issue though, there's a fair amount of documentation online. Just downloading the binaries isn't enough to work on this, you need to compile it yourself to verify the changes are correct.
Before using `Bitcoin Core` I actually used `build_msvc` which gave me a bunch folders with `*.vcxproj` files, however I was unsure where to
...
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1618350216)
> MSVC is one option, WSL another. See also https://github.com/bitcoin/bitcoin/blob/master/doc/build-windows.md.
>
> Build questions are a bit out of scope for this issue though, there's a fair amount of documentation online. Just downloading the binaries isn't enough to work on this, you need to compile it yourself to verify the changes are correct.
Before using `Bitcoin Core` I actually used `build_msvc` which gave me a bunch folders with `*.vcxproj` files, however I was unsure where to
...
🤔 furszy reviewed a pull request: "wallet: don't duplicate change output if already exist"
(https://github.com/bitcoin/bitcoin/pull/27601#pullrequestreview-1511239594)
> How would it work if coinselection produced result without change output?
There is a test exercising the behavior inside a8ac2ad8 and also an explanation there.
But essentially, the process must adhere to the outputs specified by the user. If the change destination option in 'fundrawtransaction' matches one of the output scripts, the wallet is instructed to 'increase the specified output only if there is any change' and must not discard any output predefined by the user under any circums
...
(https://github.com/bitcoin/bitcoin/pull/27601#pullrequestreview-1511239594)
> How would it work if coinselection produced result without change output?
There is a test exercising the behavior inside a8ac2ad8 and also an explanation there.
But essentially, the process must adhere to the outputs specified by the user. If the change destination option in 'fundrawtransaction' matches one of the output scripts, the wallet is instructed to 'increase the specified output only if there is any change' and must not discard any output predefined by the user under any circums
...
💬 furszy commented on pull request "wallet: don't duplicate change output if already exist":
(https://github.com/bitcoin/bitcoin/pull/27601#discussion_r1250923477)
> Isn't the fee for all outputs accounted for in not_input_fees? It seems to me this will always overpay and then will be corrected when we detect surplus fee.
>
> Maybe a clearer way to handle this would be to pass chnage_fee=0 to GetChange if existent_change_out_index=true. I'd prefer to minimise number of cases when we need to correct fees at the later stage of transaction building.
Good point.
Could go further and set the coin control `m_change_fee` and `min_viable_change` to 0 so coin
...
(https://github.com/bitcoin/bitcoin/pull/27601#discussion_r1250923477)
> Isn't the fee for all outputs accounted for in not_input_fees? It seems to me this will always overpay and then will be corrected when we detect surplus fee.
>
> Maybe a clearer way to handle this would be to pass chnage_fee=0 to GetChange if existent_change_out_index=true. I'd prefer to minimise number of cases when we need to correct fees at the later stage of transaction building.
Good point.
Could go further and set the coin control `m_change_fee` and `min_viable_change` to 0 so coin
...
👍 fanquake approved a pull request: "ci: Remove deprecated container.greedy"
(https://github.com/bitcoin/bitcoin/pull/28024#pullrequestreview-1511281401)
ACK fa248a4c616be31d231c671e8feb0dbb46ac54cd
(https://github.com/bitcoin/bitcoin/pull/28024#pullrequestreview-1511281401)
ACK fa248a4c616be31d231c671e8feb0dbb46ac54cd
💬 hebasto commented on pull request "Exit and show error if unrecognized command line args are present":
(https://github.com/bitcoin-core/gui/pull/742#issuecomment-1618418896)
Is it safer and simpler to just document that a payment URi must follow all command line options? In `bitcoin-qt -help`?
(https://github.com/bitcoin-core/gui/pull/742#issuecomment-1618418896)
Is it safer and simpler to just document that a payment URi must follow all command line options? In `bitcoin-qt -help`?
💬 fanquake commented on pull request "guix: use GCC 12.2.0 to build releases":
(https://github.com/bitcoin/bitcoin/pull/27897#issuecomment-1618420576)
@theuni I will address your comments soon. Currently working on some additional changes so we can push the time-machine commit further along, and potentially take advantage of the now merged: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=64100. I've opened one additional issue upstream: https://lists.gnu.org/archive/html/bug-guix/2023-07/msg00009.html.
(https://github.com/bitcoin/bitcoin/pull/27897#issuecomment-1618420576)
@theuni I will address your comments soon. Currently working on some additional changes so we can push the time-machine commit further along, and potentially take advantage of the now merged: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=64100. I've opened one additional issue upstream: https://lists.gnu.org/archive/html/bug-guix/2023-07/msg00009.html.
💬 fanquake commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1618430203)
https://cirrus-ci.com/task/6504094303518720?logs=ci#L3264
```bash
test 2023-07-03T14:16:38.121000Z TestFramework (INFO): Polling buffer...
node0 2023-07-03T14:16:38.121067Z (mocktime: 2023-07-17T14:16:35Z) [http] [httpserver.cpp:255] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:40312
node0 2023-07-03T14:16:38.121207Z (mocktime: 2023-07-17T14:16:35Z) [httpworker.2] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=getrawmempool user=__cookie__
test
...
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1618430203)
https://cirrus-ci.com/task/6504094303518720?logs=ci#L3264
```bash
test 2023-07-03T14:16:38.121000Z TestFramework (INFO): Polling buffer...
node0 2023-07-03T14:16:38.121067Z (mocktime: 2023-07-17T14:16:35Z) [http] [httpserver.cpp:255] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:40312
node0 2023-07-03T14:16:38.121207Z (mocktime: 2023-07-17T14:16:35Z) [httpworker.2] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=getrawmempool user=__cookie__
test
...
🤔 furszy reviewed a pull request: "Debug Console implementation of generate method"
(https://github.com/bitcoin-core/gui/pull/692#pullrequestreview-1491894680)
Left a quick review, not really blocking. Mostly performance and code improvements.
Will finish testing it later today and ACK it if all goes well.
(https://github.com/bitcoin-core/gui/pull/692#pullrequestreview-1491894680)
Left a quick review, not really blocking. Mostly performance and code improvements.
Will finish testing it later today and ACK it if all goes well.
💬 furszy commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1248843611)
nit: format:
```suggestion
Q_EMIT reply(RPCConsole::CMD_ERROR, QString("Error: could not generate blocks"));
```
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1248843611)
nit: format:
```suggestion
Q_EMIT reply(RPCConsole::CMD_ERROR, QString("Error: could not generate blocks"));
```
💬 furszy commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1248842906)
Not really for this PR but this will not work with console-only commands that contain descriptors as they use `()` internally.
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1248842906)
Not really for this PR but this will not work with console-only commands that contain descriptors as they use `()` internally.
💬 furszy commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1248843898)
nit: format:
```suggestion
std::string answer = "{\n \"address\": \"" + address + "\",\n \"blocks\": " + result + "\n}";
Q_EMIT reply(RPCConsole::CMD_REPLY, QString::fromStdString("\n" + answer + "\n\n"));
```
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1248843898)
nit: format:
```suggestion
std::string answer = "{\n \"address\": \"" + address + "\",\n \"blocks\": " + result + "\n}";
Q_EMIT reply(RPCConsole::CMD_REPLY, QString::fromStdString("\n" + answer + "\n\n"));
```
💬 furszy commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1248846569)
Would be good to lock this only for test networks so users don't end up generating addresses for no reason.
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1248846569)
Would be good to lock this only for test networks so users don't end up generating addresses for no reason.
💬 furszy commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1248845934)
nit:
```suggestion
if (!RPCConsole::RPCExecuteCommandLine(m_node, result, strprintf("generatetoaddress %s %s %s\n", nblocks, address, maxtries), /*pstrFilteredOut=*/nullptr, wallet_model)) {
```
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1248845934)
nit:
```suggestion
if (!RPCConsole::RPCExecuteCommandLine(m_node, result, strprintf("generatetoaddress %s %s %s\n", nblocks, address, maxtries), /*pstrFilteredOut=*/nullptr, wallet_model)) {
```
💬 furszy commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1237708053)
This loop over all entries breaks the purpose of using a map, could have been a set<pair<string, method>>.
Or.. instead of having the loop, could:
```c++
std::string method = parsed_command[0];
bool exec_help = false;
if (method == "help") {
exec_help = true;
method = parsed_command[1];
}
auto it_method = m_method_map.find(method);
if (it_method == m_method_map.end()) return false; // method not found
return (*it_method.second)(parsed_command, wallet_model, exec_help);
```
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1237708053)
This loop over all entries breaks the purpose of using a map, could have been a set<pair<string, method>>.
Or.. instead of having the loop, could:
```c++
std::string method = parsed_command[0];
bool exec_help = false;
if (method == "help") {
exec_help = true;
method = parsed_command[1];
}
auto it_method = m_method_map.find(method);
if (it_method == m_method_map.end()) return false; // method not found
return (*it_method.second)(parsed_command, wallet_model, exec_help);
```
💬 Sjors commented on pull request "Drop macOS ForceActivation workaround":
(https://github.com/bitcoin-core/gui/pull/744#issuecomment-1618472088)
Tested 362e98937356a74a5b70d65348957edf05b1138a but it seems the workaround is still needed.
This was introduced in https://github.com/bitcoin/bitcoin/pull/14123 as a fix to https://github.com/bitcoin/bitcoin/issues/13829.
With this PR, built on macOS 13.4.1 (Intel) with qt@5/5.15.10 via Homebrew, when I use `⌘ + H` to hide the window, and then e.g. click on "Receive" in the dock, the window doesn't appear. Without this PR the window _does_ appear.
(https://github.com/bitcoin-core/gui/pull/744#issuecomment-1618472088)
Tested 362e98937356a74a5b70d65348957edf05b1138a but it seems the workaround is still needed.
This was introduced in https://github.com/bitcoin/bitcoin/pull/14123 as a fix to https://github.com/bitcoin/bitcoin/issues/13829.
With this PR, built on macOS 13.4.1 (Intel) with qt@5/5.15.10 via Homebrew, when I use `⌘ + H` to hide the window, and then e.g. click on "Receive" in the dock, the window doesn't appear. Without this PR the window _does_ appear.
💬 john-moffett commented on pull request "Exit and show error if unrecognized command line args are present":
(https://github.com/bitcoin-core/gui/pull/742#issuecomment-1618487197)
I could modify this PR to enforce that behavior and add the documentation to `bitcoin-qt -help`. Right now, the PR only allows non-option arguments which must start with `bitcoin:` (case insensitive). It will still ignore any options after that.
I think leaving the current `master` behavior and just adding documentation to reflect it would be insufficient. My worry is that a user could be fooled by something like:
Scammer: *"Can you send me some testnet coins? Start your client with `bitco
...
(https://github.com/bitcoin-core/gui/pull/742#issuecomment-1618487197)
I could modify this PR to enforce that behavior and add the documentation to `bitcoin-qt -help`. Right now, the PR only allows non-option arguments which must start with `bitcoin:` (case insensitive). It will still ignore any options after that.
I think leaving the current `master` behavior and just adding documentation to reflect it would be insufficient. My worry is that a user could be fooled by something like:
Scammer: *"Can you send me some testnet coins? Start your client with `bitco
...
💬 hebasto commented on pull request "Fix transaction view/table":
(https://github.com/bitcoin-core/gui/pull/662#issuecomment-1618516641)
Tested 8e428cc1188568aaeeaeb8f1f4498ae5b738a5e2 on Ubuntu 23.04.
Application window resizing leads to:

when the content of the last column "Amount" goes out of the scope.
(https://github.com/bitcoin-core/gui/pull/662#issuecomment-1618516641)
Tested 8e428cc1188568aaeeaeb8f1f4498ae5b738a5e2 on Ubuntu 23.04.
Application window resizing leads to:

when the content of the last column "Amount" goes out of the scope.
🤔 furszy reviewed a pull request: "Debug Console implementation of generate method"
(https://github.com/bitcoin-core/gui/pull/692#pullrequestreview-1511346984)
Two small scenarios to fix:
1) Should guard `generate` from using negative numbers. Currently, can call `generate -20` and the process will return an empty blocks array instead of failing.
2) The `generatetoaddress` result has a bracket without the correct indentation. See
<img width="549" alt="screenshot" src="https://github.com/bitcoin-core/gui/assets/5377650/d1459746-0732-4dd2-90b4-d2403704677c">
(https://github.com/bitcoin-core/gui/pull/692#pullrequestreview-1511346984)
Two small scenarios to fix:
1) Should guard `generate` from using negative numbers. Currently, can call `generate -20` and the process will return an empty blocks array instead of failing.
2) The `generatetoaddress` result has a bracket without the correct indentation. See
<img width="549" alt="screenshot" src="https://github.com/bitcoin-core/gui/assets/5377650/d1459746-0732-4dd2-90b4-d2403704677c">
💬 hebasto commented on pull request "Switch RPCConsole wallet selection to the one most recently opened/restored/created":
(https://github.com/bitcoin-core/gui/pull/696#issuecomment-1618528575)
ping @john-moffett :)
(https://github.com/bitcoin-core/gui/pull/696#issuecomment-1618528575)
ping @john-moffett :)
💬 ItIsOHM commented on issue "rpc: doc: RPCHelpMan missing request parameters for getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1618551972)
Hello @stickies-v, I added these two entries after `rules` in `template_request`:

Is this correct? I'm sorry if it's wrong, I'm just a beginner with this.
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1618551972)
Hello @stickies-v, I added these two entries after `rules` in `template_request`:

Is this correct? I'm sorry if it's wrong, I'm just a beginner with this.