💬 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.

💬 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"));
```

💬 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.

💬 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)) {
```

💬 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);
```

💬 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.

💬 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
...

💬 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.

🤔 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">

💬 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 :)

💬 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.

💬 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.

💬 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

💬 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

💬 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?

📝 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
...

💬 john-moffett 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-1618803949)
Thanks for pings, @hebasto and @jarolrod. Apologies for the delay. I've renamed the method and moved the declaration to the `Q_SLOTS` section.

💬 theStack commented on pull request "Remove confusing "Dust" label from coincontrol / sendcoins dialog":
(https://github.com/bitcoin-core/gui/pull/719#issuecomment-1618813452)
> The commit message and the PR description should mention changes in the `SendCoinsDialog` as well.

Sorry for the reply there, missed that. Force-pushed with an adapted commit message (and changed PR title / description as well):
```
$ git range-diff 394300c18...ef4185d2b
1: 944263a93 ! 1: 210ef1e98 qt: remove confusing "Dust" label from coincontrol dialog
@@ Metadata
Author: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>

## Commit message ##
- qt: rem
...

💬 theStack commented on pull request "Remove confusing "Dust" label from coincontrol / sendcoins dialog":
(https://github.com/bitcoin-core/gui/pull/719#discussion_r1251087089)
I'm not too experienced with Qt, can anyone elaborate on that question (ping @furszy as author of the second commit)? It seems at least that we already have a couple of instances in master where both Qt::AlignLeading and Qt::AlignLeft are used together:
```
$ git grep AlignLeading
src/qt/forms/debugwindow.ui: <set>Qt::AlignBottom|Qt::AlignLeading|Qt::AlignLeft</set>
src/qt/forms/helpmessagedialog.ui: <set>Qt::AlignLeading|Qt::AlignLeft|Qt::AlignTop</set>
src/qt/forms/hel
...

💬 pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1618849607)
Updates:
- Rebased
- Implemented refactoring [suggestion](https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1245038873) from @stickies-v, moving other http reject validations into the constructor and regrouping them under a `Validate()` function to make the entire validation more consistent.