💬 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.
💬 MarcoFalke commented on pull request "ci: Remove deprecated container.greedy":
(https://github.com/bitcoin/bitcoin/pull/28024#issuecomment-1618569388)
> It is still mentioned
I asked by email.
(https://github.com/bitcoin/bitcoin/pull/28024#issuecomment-1618569388)
> It is still mentioned
I asked by email.
💬 MarcoFalke commented on pull request "ci: Remove deprecated container.greedy":
(https://github.com/bitcoin/bitcoin/pull/28024#issuecomment-1618572545)
Added an unrelated commit to adjust the CCACHE_SIZE. For some reason the jammy macOS taks is uncacheable? https://cirrus-ci.com/task/4744875699077120?logs=ci#L2486
(https://github.com/bitcoin/bitcoin/pull/28024#issuecomment-1618572545)
Added an unrelated commit to adjust the CCACHE_SIZE. For some reason the jammy macOS taks is uncacheable? https://cirrus-ci.com/task/4744875699077120?logs=ci#L2486
💬 hebasto commented on pull request "ci: Remove deprecated container.greedy":
(https://github.com/bitcoin/bitcoin/pull/28024#issuecomment-1618585164)
> For some reason the jammy macOS taks is uncacheable? [cirrus-ci.com/task/4744875699077120?logs=ci#L2486](https://cirrus-ci.com/task/4744875699077120?logs=ci#L2486)
See:
- https://github.com/bitcoin/bitcoin/issues/21552
- https://github.com/bitcoin/bitcoin/pull/24620
(https://github.com/bitcoin/bitcoin/pull/28024#issuecomment-1618585164)
> For some reason the jammy macOS taks is uncacheable? [cirrus-ci.com/task/4744875699077120?logs=ci#L2486](https://cirrus-ci.com/task/4744875699077120?logs=ci#L2486)
See:
- https://github.com/bitcoin/bitcoin/issues/21552
- https://github.com/bitcoin/bitcoin/pull/24620
💬 MarcoFalke commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1618628124)
Maybe remove the sub-test case?
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1618628124)
Maybe remove the sub-test case?
📝 theStack opened a pull request: "test: refactor: deduplicate legacy ECDSA signing for tx inputs"
(https://github.com/bitcoin/bitcoin/pull/28025)
There are several instances in functional tests and the framework (MiniWallet, feature_block.py, p2p_segwit.py) where we create a legacy ECDSA signature for a certain transaction's input by doing the following steps:
1. calculate the `LegacySignatureHash` with the desired sighash type
2. create the actual digital signature by calling `ECKey.sign_ecdsa` on the signature message hash calculated above
3. put the DER-encoded result as CScript data push into tx input's scriptSig
Create a new
...
(https://github.com/bitcoin/bitcoin/pull/28025)
There are several instances in functional tests and the framework (MiniWallet, feature_block.py, p2p_segwit.py) where we create a legacy ECDSA signature for a certain transaction's input by doing the following steps:
1. calculate the `LegacySignatureHash` with the desired sighash type
2. create the actual digital signature by calling `ECKey.sign_ecdsa` on the signature message hash calculated above
3. put the DER-encoded result as CScript data push into tx input's scriptSig
Create a new
...