🤔 mzumsande reviewed a pull request: "Fix -netinfo backward compat with getpeerinfo pre-v26"
(https://github.com/bitcoin/bitcoin/pull/29212#pullrequestreview-1812115477)
Concept ACK

💬 mzumsande commented on pull request "Fix -netinfo backward compat with getpeerinfo pre-v26":
(https://github.com/bitcoin/bitcoin/pull/29212#discussion_r1446679605)
I think that if we don't write the "v", we can make the width of the added column 1 instead of 2.

💬 jonatack commented on pull request "Fix -netinfo backward compat with getpeerinfo pre-v26":
(https://github.com/bitcoin/bitcoin/pull/29212#discussion_r1446686612)
Yes, I began with that, but it looked better to my eye when I stayed with the width you set.

💬 kristapsk commented on pull request "Fix -netinfo backward compat with getpeerinfo pre-v26":
(https://github.com/bitcoin/bitcoin/pull/29212#discussion_r1446693131)
I like that extra space as is, IMHO it visually looks better.


⚠️ mzumsande opened an issue: "clang-format returns error"
(https://github.com/bitcoin/bitcoin/issues/29214)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

clang-format is curently broken for me (possibly related to #29056, fyi @maflcko )
I'm using Ubuntu clang-format version 14.0.0-1ubuntu1.1

### Expected behaviour

no error

### Steps to reproduce

Add a newline anywhere in e.g. `addrman.cpp`, commit, then `git diff -U0 HEAD~1.. | ./contrib/devtools/clang-format-diff.py -p1 -i -v` as described in `contrib/Readme.md`


### Relevant log o
...

💬 maflcko commented on issue "clang-format returns error":
(https://github.com/bitcoin/bitcoin/issues/29214#issuecomment-1883957090)
Not sure why that happens, maybe the setting was a boolean before? You could try upgrading your OS or clang-format: https://packages.ubuntu.com/jammy/clang-format-15

Also, it should be fine to remove this line in your config.

Finally, it can also be removed from the config in this repo, if needed.

💬 mzumsande commented on issue "clang-format returns error":
(https://github.com/bitcoin/bitcoin/issues/29214#issuecomment-1883966638)
> Not sure why that happens, maybe the setting was a boolean before?

Yes, that's the case, [14.0.0 doc](https://releases.llvm.org/14.0.0/tools/clang/docs/ClangFormatStyleOptions.html)

The other added option, `RequiresExpressionIndentation`, also leads to an error since it was only added in clang-format 16 (https://clang.llvm.org/docs/ClangFormatStyleOptions.html) so I would need to upgrade to that.
Happy to upgrade if this not a problem for anyone else, I usually use gcc anyway.

💬 ryanofsky commented on pull request "wallet, rpc: add BIP44 `account` in `createwallet`":
(https://github.com/bitcoin/bitcoin/pull/29129#issuecomment-1883982591)
re: https://github.com/bitcoin/bitcoin/pull/29129#issuecomment-1869859432

> I think this might be a better candidate for a `createwalletdescriptor` (#29130) equivalent for external signers rather than continuing to jam more arguments into `createwallet`.

This makes sense, and I added this idea to my list of potential createwalletdescriptor extensions: https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1435607572

But in general, as long as the createwallet `blank` option defaults
...

💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446759709)
Done

💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446759761)
Fixed

💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446759842)
Done

💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760427)
Done

💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760540)
Done

💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760584)
Done

💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760618)
Done

💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760656)
Done

💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760708)
Done

💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760770)
Done

💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446760887)
Done

💬 achow101 commented on pull request "wallet: Add `createwalletdescriptor` and `gethdkeys` RPCs for adding new automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/29130#discussion_r1446761034)
Updated the test to check this.