💬 ryanofsky commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1137754658)
In commit "wallet: Replace use of purpose strings with an enum" (db521d3e89b23046f73410a875bb9660178e3842)

This change should be moved to previous commit "wallet: add AddressPurpose enum to replace string values" (2932d7be3cdf3de79b2767735a91180192d4efce), instead of being here. Alternately, could also keep the fixed width type, though IMO it is not helpful.

💬 ryanofsky commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1137745598)
In commit "wallet: Replace use of purpose strings with an enum" (db521d3e89b23046f73410a875bb9660178e3842)

I think it might be a little better to drop the IsValidPurposeString function and just write:

```
purpose = PurposeFromString(purpose_str);
if (!purpose) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid purpose argument, must a known purpose string, typically 'send' or 'receive'.");
}
```

since underlying code should work for any past purpose string like "refund", or f
...

💬 ryanofsky commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1137760371)
In commit "wallet: Replace use of purpose strings with an enum" (db521d3e89b23046f73410a875bb9660178e3842)

s/spend/spent/

💬 ryanofsky commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1137756043)
In commit "wallet: Replace use of purpose strings with an enum" (db521d3e89b23046f73410a875bb9660178e3842)

Could add the same comment suggested previously here above NotifyAddressBookChanged ("In very old wallets, address purpose may not recorded, so derive it from IsMine value")

👍 ryanofsky approved a pull request: "refactor / kernel: Move non-gArgs chainparams functionality to kernel"
(https://github.com/bitcoin/bitcoin/pull/26177)
Code review ACK b3e78dc91d01e364b77aacd9fb9a2f88688ab8a6. Only changes since last review were recent review suggestions.

💬 Xekyo commented on pull request "wallet: 25806 follow-up":
(https://github.com/bitcoin/bitcoin/pull/27227#issuecomment-1470869150)
utACK 475c20aa568d597c7850c784058596ae26f37496

💬 ishaanam commented on pull request "test: fix race condition in encrypted wallet rescan tests":
(https://github.com/bitcoin/bitcoin/pull/27199#discussion_r1137792608)
I have now added test coverage for the lock as well.

💬 LarryRuane commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1137821241)
```suggestion
if (RPCExecutor::executeConsoleOnlyCommand(executableCommand, wallet_model)) {
return;
}
```
(or could put `return` on the same line)

💬 LarryRuane commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1137822858)
these can be removed

💬 LarryRuane commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1137822991)
```suggestion
const std::vector<std::string> parsedCommand{parseHelper(command)};
```

💬 LarryRuane commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1137827055)
That's strange, when I run `clang-format-diff`, it moves this left, where they should be:

<details>
<summary>click to expand</summary>

```
$ git show -U0 | ./contrib/devtools/clang-format-diff.py -p1 -i -v
Formatting src/qt/rpcconsole.cpp
$ git diff
diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp
index 1408b289f..63820464b 100644
--- a/src/qt/rpcconsole.cpp
+++ b/src/qt/rpcconsole.cpp
@@ -419,12 +419,12 @@ void RPCExecutor::request(const QString &command, const WalletMo
...

💬 achow101 commented on pull request "Remove almost all blockstorage globals":
(https://github.com/bitcoin/bitcoin/pull/25781#issuecomment-1470936630)
ACK fadf8b818226dc60adf88e927160f9c9680473a4

🚀 achow101 merged a pull request: "Remove almost all blockstorage globals"
(https://github.com/bitcoin/bitcoin/pull/25781)

💬 achow101 commented on pull request "wallet: 25806 follow-up":
(https://github.com/bitcoin/bitcoin/pull/27227#issuecomment-1470952578)
ACK 475c20aa568d597c7850c784058596ae26f37496

🚀 achow101 merged a pull request: "wallet: 25806 follow-up"
(https://github.com/bitcoin/bitcoin/pull/27227)

💬 achow101 commented on pull request "rest: add verbose and mempool_sequence query params for mempool/contents":
(https://github.com/bitcoin/bitcoin/pull/26207#issuecomment-1470979803)
ACK 1ff5d61dfdaf8987e5619162662e4c760af76a43

🚀 achow101 merged a pull request: "rest: add verbose and mempool_sequence query params for mempool/contents"
(https://github.com/bitcoin/bitcoin/pull/26207)

💬 ishaanam commented on pull request "bumpfee: allow send coins back to yourself":
(https://github.com/bitcoin/bitcoin/pull/27195#discussion_r1137912817)
In 7cb4dc157a711210e18f3c9b492150b6eb984b30 " bumpfee: enable send coins back to yourself "

While this certainly looks correct, I think there is more concise way of doing this within the outputs loop before this one. I've implemented it here if you would like to take a look: https://github.com/ishaanam/bitcoin/commit/5b1017c1ea11c8fe2351a9110e4c6ddad6e77f8a. The advantage of this would be that the sanity check would not be required and imo it would be a bit more readable.

💬 pablomartin4btc commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1137927627)
I'll try from my side and give it a go later this week.

👍 hernanmarino approved a pull request: "cli: add validation to cli side commands besides when it's used with -rpcwallet"
(https://github.com/bitcoin/bitcoin/pull/26990)
ACK a870f5affcfb85f91afe989db0824313985158d8

⚠️ SkybuckFlying opened an issue: "Blocks remaining falls offscreen with dutch language setting."
(https://github.com/bitcoin/bitcoin/issues/27266)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

It's difficult to see how many blocks remain to be downloaded.

### Expected behaviour

It should be possible to see how many blocks are remaining.

Possible solution1:

1. Make message re-sizeable.
2. Make message screen larger in width.
3. Make message description shorter.
4. Make it possible to copy & paste the text so the user can paste it elsewhere to get a better idea.
5. Mak
...