💬 sipa commented on pull request "Fix -netinfo backward compat with getpeerinfo pre-v26":
(https://github.com/bitcoin/bitcoin/pull/29212#issuecomment-1883827869)
Concept ACK

📝 mzumsande opened a pull request: "doc, test: test and explain service flag handling"
(https://github.com/bitcoin/bitcoin/pull/29213)
Service flags received on the peer-to-peer network are handled differently, depending on how we receive them.
If received directly from an outbound peer the flags belong to, they replace existing flags.
If received via gossip relay (so that anyone could send them), new flags are added, but existing ones but cannot be overwritten.

Document that and add test coverage for it.

👍 kristapsk approved a pull request: "Fix -netinfo backward compat with getpeerinfo pre-v26"
(https://github.com/bitcoin/bitcoin/pull/29212#pullrequestreview-1812079109)
ACK 5fa74609b833643334dfb5519f2023119984267b

💬 jonatack commented on pull request "doc, test: test and explain service flag handling":
(https://github.com/bitcoin/bitcoin/pull/29213#issuecomment-1883866390)
Concept ACK

💬 sipa commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-1883870883)
@instagibbs See https://github.com/sipa/bitcoin/commits/pr28984 for a commit that works exactly for all `FeeFrac` objects. It uses C++20 three-way comparisons to also reduce the line count by 25%.

🤔 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